fix(revm): validate fee token registration in simulation paths - #182
fix(revm): validate fee token registration in simulation paths#182panos-xyz wants to merge 2 commits into
Conversation
Simulation paths (eth_call / eth_estimateGas) with is_fee_charge_disabled() previously short-circuited validate_and_deduct_token_fee before checking whether the specified fee_token_id was registered and active in the L2 Token Registry. This caused eth_estimateGas to incorrectly return 21000 for unregistered fee tokens like 65535 instead of rejecting the transaction. Move the token registration and active checks prior to the simulation short-circuit so that invalid tokens are rejected consistently across both execution and simulation paths.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughSimulation paths now validate that fee tokens are registered and active before skipping fee deduction. Unit and RPC integration tests verify rejection of unregistered and inactive tokens. ChangesFee token validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Simulation RPCs now reject unregistered or inactive fee-token IDs before skipping fee deduction, while preserving balance-free simulation behavior for valid tokens. The implementation and coverage support merge readiness. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
…ookup Decouple fee-token registry metadata loading into TokenRegistryEntry so simulation paths (eth_call / eth_estimateGas) only validate token existence and active status without reading the caller's token balance or invoking balanceOf. Move the caller balance lookup and fee deduction to non-simulation execution only, and verify that simulation paths never touch fee-token contract storage.
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
Closes #181
Summary
Fixes an issue where
eth_estimateGas(andeth_call) incorrectly returned0x5208(21000) for transactions specifying an unregistered or inactivefeeTokenID(e.g.65535) instead of rejecting them.Root Cause
In
crates/revm/src/handler.rs,validate_and_deduct_token_feehandles simulation paths wherecfg.is_fee_charge_disabled()istrue. It previously short-circuited at the entry of the function and returnedOk(()), skipping the L2 Token Registry checks (TokenFeeInfo::load_for_callerandtoken_fee_info.is_active). Upstream reth'sis_basic_transferoptimization then executed a trial withMIN_TRANSACTION_GAS(21000), which succeeded and returned0x5208.Changes
validate_and_deduct_token_feebefore theis_fee_charge_disabled()short-circuit.morph-revm:validate_and_deduct_token_fee_rejects_unregistered_token_in_simulationvalidate_and_deduct_token_fee_rejects_inactive_token_in_simulationcrates/node/tests/it/rpc.rs:estimate_gas_rejects_unregistered_fee_tokenSummary by CodeRabbit
Bug Fixes
Tests