Skip to content

Add rate and scale check#273

Merged
Kukoomomo merged 2 commits intomainfrom
rate_check
Dec 22, 2025
Merged

Add rate and scale check#273
Kukoomomo merged 2 commits intomainfrom
rate_check

Conversation

@Kukoomomo
Copy link
Copy Markdown
Contributor

@Kukoomomo Kukoomomo commented Dec 22, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened validation for fee rate configurations to detect and reject missing or zero rate/scale values earlier, improving error reporting (includes token identification in logs) and preventing propagation of invalid parameters.

✏️ 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 22, 2025 04:26
@Kukoomomo Kukoomomo requested review from secmgt and removed request for a team December 22, 2025 04:26
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 22, 2025

Walkthrough

Added non-nil and non-zero validation checks in TokenRate (rollup/fees/rate.go) for the returned rate and for info.Scale; on validation failure the code logs an error including token ID and address and returns the existing error early.

Changes

Cohort / File(s) Summary
Rate validation
rollup/fees/rate.go
Added non-nil and non-zero validation for the computed rate and for info.Scale; logs tokenID and token address and returns early on validation failure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-file, localized validation additions
  • Attention: confirm callers expect original error propagation and that logged tokenID/address are safe to emit

Possibly related PRs

Suggested reviewers

  • tomatoishealthy
  • curryxbo

Poem

🐇 I nibble bytes beneath the moonlight,

Checking rates to keep things right,
No nils to hide, no zeros stay—
I guard the hops along the way,
A tiny rabbit, code-bright.

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 'Add rate and scale check' accurately summarizes the main change: adding validation checks for rate and scale in the TokenRate function.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch rate_check

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 768898d and 64e9dcd.

📒 Files selected for processing (1)
  • rollup/fees/rate.go
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Kukoomomo
Repo: morph-l2/go-ethereum PR: 205
File: core/token_gas.go:140-155
Timestamp: 2025-11-10T06:56:24.937Z
Learning: In the ERC20 fee implementation (core/token_gas.go), ERC20 transfer and balance check calls use math.MaxUint64 gas limit. This is intentional given that fee tokens are restricted to an allowlist (via IsTokenActive checks and AllowListSlot/AllowListEnabledSlot in rollup/rcfg/config.go), so only vetted contracts can be invoked. The team has chosen this approach understanding the tradeoffs versus adding fixed gas stipends.
⏰ 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). (1)
  • GitHub Check: Analyze (go)

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.

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: 2

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efcdc5e and 768898d.

📒 Files selected for processing (1)
  • rollup/fees/rate.go
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Kukoomomo
Repo: morph-l2/go-ethereum PR: 205
File: core/token_gas.go:140-155
Timestamp: 2025-11-10T06:56:24.937Z
Learning: In the ERC20 fee implementation (core/token_gas.go), ERC20 transfer and balance check calls use math.MaxUint64 gas limit. This is intentional given that fee tokens are restricted to an allowlist (via IsTokenActive checks and AllowListSlot/AllowListEnabledSlot in rollup/rcfg/config.go), so only vetted contracts can be invoked. The team has chosen this approach understanding the tradeoffs versus adding fixed gas stipends.
⏰ 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). (1)
  • GitHub Check: Analyze (go)

Comment thread rollup/fees/rate.go
Comment thread rollup/fees/rate.go
@Kukoomomo Kukoomomo merged commit d108c19 into main Dec 22, 2025
8 checks passed
@Kukoomomo Kukoomomo deleted the rate_check branch December 22, 2025 07:56
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.

2 participants