Skip to content

[R-3] Change error for token amount invalid#834

Merged
Kukoomomo merged 3 commits intofeature/audit_fixfrom
err_change
Dec 17, 2025
Merged

[R-3] Change error for token amount invalid#834
Kukoomomo merged 3 commits intofeature/audit_fixfrom
err_change

Conversation

@Kukoomomo
Copy link
Copy Markdown
Contributor

@Kukoomomo Kukoomomo commented Dec 17, 2025 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error detection and reporting for token amount calculations to better handle edge cases during token transactions.

✏️ Tip: You can customize this high-level summary in your review settings.

@Kukoomomo Kukoomomo requested a review from a team as a code owner December 17, 2025 02:01
@Kukoomomo Kukoomomo requested review from panos-xyz and removed request for a team December 17, 2025 02:01
@Kukoomomo Kukoomomo changed the base branch from main to feature/audit_fix December 17, 2025 02:01
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 17, 2025 •
edited
Loading

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The L2TokenRegistry system refactors error handling by introducing a new ZeroTokenAmount error. The interface declaration is added, the calculateTokenAmount implementation updated to throw this error instead of InvalidPrice, and a test added to verify the error condition when ethAmount is zero.

Changes

Cohort / File(s) Summary
Error Declaration
contracts/contracts/l2/system/IL2TokenRegistry.sol		 Added ZeroTokenAmount() error declaration to interface
Error Implementation & Tests
contracts/contracts/l2/system/L2TokenRegistry.sol, contracts/contracts/test/L2TokenRegistry.t.sol		 Updated calculateTokenAmount to throw ZeroTokenAmount instead of InvalidPrice; added test function test_calculateTokenAmount_reverts_when_ethAmount_is_zero() to verify error condition

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward error renaming: single error type replaced consistently across interface, implementation, and tests
  • Changes are cohesive and aligned in purpose
  • Test addition is conventional and verifies the specific error condition

Poem

🐰 A token's zero amount—no good!
The error now speaks with clarity true,
ZeroTokenAmount declares what it should:
"This path brings no value through."
The registry hops with purpose renewed. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: renaming the error thrown in calculateTokenAmount from InvalidPrice to ZeroTokenAmount for better semantic clarity.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Dec 17, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
contracts/contracts/l2/system/L2TokenRegistry.sol (1)

403-416: Update documentation to mention the new error condition.

The function documentation states it will revert if token is not registered or priceRatio is not set, but doesn't mention the new ZeroTokenAmount error condition.

Consider updating line 415 to:

-     * - Will revert if token is not registered or priceRatio is not set
+     * - Will revert if token is not registered, priceRatio is not set, or calculated tokenAmount is zero
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27e80af and 5e3811c.

📒 Files selected for processing (3)
  • contracts/contracts/l2/system/IL2TokenRegistry.sol (1 hunks)
  • contracts/contracts/l2/system/L2TokenRegistry.sol (1 hunks)
  • contracts/contracts/test/L2TokenRegistry.t.sol (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: test
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (3)
contracts/contracts/l2/system/IL2TokenRegistry.sol (1)

68-68: LGTM! Clear and descriptive error declaration.

The new ZeroTokenAmount error improves error specificity for the token amount calculation edge case.

contracts/contracts/l2/system/L2TokenRegistry.sol (1)

435-435: LGTM! More specific error improves clarity.

The change from InvalidPrice to ZeroTokenAmount better describes the condition being checked. The error at line 427 correctly continues to use InvalidPrice for the unset ratio case.

contracts/contracts/test/L2TokenRegistry.t.sol (1)

614-624: LGTM! Test properly validates the new error condition.

The test correctly verifies that calculateTokenAmount reverts with ZeroTokenAmount() when called with ethAmount = 0. The test setup and assertion are appropriate.

@Kukoomomo Kukoomomo merged commit 5eece4f into feature/audit_fix Dec 17, 2025
3 checks passed
@Kukoomomo Kukoomomo deleted the err_change branch December 17, 2025 09:30
@coderabbitai coderabbitai bot mentioned this pull request Dec 17, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@FletcherMan FletcherMan FletcherMan approved these changes

@curryxbo curryxbo curryxbo approved these changes

@panos-xyz panos-xyz Awaiting requested review from panos-xyz panos-xyz was automatically assigned from morph-l2/security

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Kukoomomo @FletcherMan @curryxbo