Conversation
WalkthroughJSON marshaling tags for Alt fee transaction fields are renamed from snake_case to camelCase in both core transaction marshalling and test utilities. FeeTokenID and FeeLimit field tags are updated consistently across two files without modifying field types or logic. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/types/transaction_marshalling.go (1)
65-68: AltFeeTx JSON field rename changes external wire format; verify compatibility expectationsRenaming the tags from
"fee_token_id"/"fee_limit"to"feeTokenID"/"feeLimit"changes both marshaling and unmarshaling ofAltFeeTxJSON. Any existing clients (tests, tooling, RPC consumers) that still emit snake_case keys will now silently produceFeeTokenID == 0andFeeLimit == nilon decode.If this JSON has already been exposed outside the repo, consider either:
- explicitly treating this as a breaking change (and updating downstream callers), or
- adding a small backward‑compat shim in
Transaction.UnmarshalJSON(e.g., pre‑unmarshal intomap[string]json.RawMessageand mappingfee_token_id→feeTokenID,fee_limit→feeLimit).As long as all producers/fixtures are updated to camelCase, the current change is consistent and correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/types/transaction_marshalling.go(1 hunks)tests/state_test_util.go(1 hunks)
🧰 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.
🧬 Code graph analysis (2)
tests/state_test_util.go (1)
accounts/abi/unpack_test.go (1)
Int(547-547)
core/types/transaction_marshalling.go (1)
common/hexutil/json.go (2)
Uint16(276-276)Big(156-156)
⏰ 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)
🔇 Additional comments (1)
tests/state_test_util.go (1)
116-117: Test transaction tags correctly aligned with production AltFeeTx JSONUpdating
stTransactionto use"feeTokenID,omitempty"/"feeLimit,omitempty"keeps the state tests in sync with the coretxJSONrepresentation, so marshaled test transactions will match what the node expects.Just ensure any existing JSON test vectors or tooling that generate state tests have been updated to use the new camelCase field names.
1. Purpose or design rationale of this PR
...
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.