Skip to content

[#32] Fix D2: expect revert on empty royalty claim#33

Merged
realproject7 merged 1 commit into
mainfrom
task/32-d2-royalty-revert
Mar 20, 2026
Merged

[#32] Fix D2: expect revert on empty royalty claim#33
realproject7 merged 1 commit into
mainfrom
task/32-d2-royalty-revert

Conversation

@realproject7

Copy link
Copy Markdown
Owner

Summary

  • Convert D2 test from zero-balance-check to try/catch revert expectation
  • MCV2_Bond.claimRoyalties() reverts with MCV2_Royalty__NothingToClaim() when there are no royalties, rather than returning silently

Test plan

  • All 28 unit tests pass
  • forge fmt --check clean
  • E2E mainnet run completes past Group D without revert

Fixes #32

🤖 Generated with Claude Code

MCV2_Bond.claimRoyalties() reverts with MCV2_Royalty__NothingToClaim()
when there are no royalties to claim, rather than returning silently.
Convert D2 from a zero-balance-check to a try/catch revert expectation.

Fixes #32

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T2b review: APPROVED. Clean fix — same try/catch revert pattern as F3. LGTM.

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

This change aligns Group D2 with the actual on-chain behavior by treating the second royalty claim as a revert expectation instead of a silent zero-return path. The scope is correct and the fix directly addresses the mainnet failure reported from issue #29.

Findings

  • None.

Decision

Approving because the test now matches deployed MCV2_Bond behavior and GitHub checks are passing.

@realproject7 realproject7 merged commit bd88dba into main Mar 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] E2E Mainnet Script Must Expect Revert on Empty Royalty Claim

2 participants