fix(levm): skip nonce validation in eth_call simulations#6325
fix(levm): skip nonce validation in eth_call simulations#6325
Conversation
Add a `disable_nonce_check` flag to `Environment` (following the existing `disable_balance_check` pattern) and set it to `true` in `simulate_tx_from_generic`, which is the code path used by `eth_call` and `eth_estimateGas`. This prevents `NonceMismatch` errors when the caller does not provide a nonce (the normal case for read-only calls), matching Geth's behavior of skipping nonce checks for `eth_call`. Closes #6312
🤖 Kimi Code ReviewReview SummaryThe changes look correct and address the issue of nonce validation in Detailed Review1. Code Correctness ✅
2. Security Considerations ✅
3. Consistency ✅
4. Minor SuggestionsFile:
File:
// Instead of spreading them across the struct initialization
disable_balance_check: false,
disable_nonce_check: false,These are minor style preferences and don't affect correctness. ConclusionThe PR correctly implements nonce check bypassing for Automated review by Kimi (Moonshot AI) · custom prompt |
There was a problem hiding this comment.
Pull request overview
Skip nonce validation during LEVM eth_call/simulation paths to avoid NonceMismatch errors when callers omit a nonce (defaulting to 0).
Changes:
- Added
disable_nonce_checktoEnvironmentto control nonce validation behavior. - Enabled
disable_nonce_checkforsimulate_tx_from_generic(used byeth_call/eth_estimateGas). - Guarded nonce mismatch validation in both default and L2 hooks.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/vm/levm/src/environment.rs | Adds an environment flag to control nonce checking behavior during simulations. |
| crates/vm/backends/levm/mod.rs | Initializes the new flag and enables it on the generic simulation path. |
| crates/vm/levm/src/hooks/default_hook.rs | Skips nonce mismatch validation when the new flag is enabled. |
| crates/vm/levm/src/hooks/l2_hook.rs | Skips nonce mismatch validation when the new flag is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .map_err(|_| TxValidationError::NonceIsMax)?; | ||
|
|
||
| // check for nonce mismatch | ||
| if sender_info.nonce != vm.env.tx_nonce { | ||
| if !vm.env.disable_nonce_check && sender_info.nonce != vm.env.tx_nonce { | ||
| return Err(TxValidationError::NonceMismatch { | ||
| expected: sender_info.nonce, | ||
| actual: vm.env.tx_nonce, |
There was a problem hiding this comment.
disable_nonce_check currently only guards the NonceMismatch branch, but the NonceIsMax validation still runs even when nonce checks are disabled. If the intent is to “skip nonce validation entirely” (per PR description / Geth behavior), the NonceIsMax check should also be bypassed when vm.env.disable_nonce_check is true (e.g., wrap both validations under the same conditional).
| .map_err(|_| TxValidationError::NonceIsMax)?; | ||
|
|
||
| // check for nonce mismatch | ||
| if sender_info.nonce != vm.env.tx_nonce { | ||
| if !vm.env.disable_nonce_check && sender_info.nonce != vm.env.tx_nonce { | ||
| return Err(TxValidationError::NonceMismatch { | ||
| expected: sender_info.nonce, | ||
| actual: vm.env.tx_nonce, |
There was a problem hiding this comment.
Same issue as in DefaultHook: disable_nonce_check skips the mismatch check but not the preceding NonceIsMax validation. If simulations should bypass nonce validation entirely, consider gating the max-nonce validation with !vm.env.disable_nonce_check as well.
| /// When true, skip nonce validation. Used by `eth_call` simulations | ||
| /// where the caller typically doesn't provide a nonce. |
There was a problem hiding this comment.
The doc comment says the flag is used by eth_call simulations, but the PR description also states it’s enabled for eth_estimateGas (via simulate_tx_from_generic). Consider updating the comment to mention both call sites so future readers don’t assume it’s eth_call-only.
| /// When true, skip nonce validation. Used by `eth_call` simulations | |
| /// where the caller typically doesn't provide a nonce. | |
| /// When true, skip nonce validation. Used by `eth_call` and `eth_estimateGas` | |
| /// simulations where the caller typically doesn't provide a nonce. |
Lines of code reportTotal lines added: Detailed view |
🤖 Codex Code ReviewNo blocking findings. The change is scoped correctly: nonce validation remains enabled for consensus execution and is only disabled for generic simulation.
Residual risk / test gap:
I could not run Automated review by OpenAI Codex · custom prompt |
Greptile SummaryThis PR fixes Key findings:
Confidence Score: 1/5
|
| Filename | Overview |
|---|---|
| crates/vm/levm/src/environment.rs | Adds disable_nonce_check: bool field to Environment, following the existing disable_balance_check pattern. Clean, well-documented addition. |
| crates/vm/backends/levm/mod.rs | Correctly sets disable_nonce_check: false in the two explicit Environment constructors (setup_env, env_from_generic) and enables it with true only in simulate_tx_from_generic. The third env constructor in this file uses ..Default::default() so it is unaffected. |
| crates/vm/levm/src/hooks/default_hook.rs | Guards the nonce-mismatch check with !vm.env.disable_nonce_check, but the unconditional increment_account_nonce call means NonceIsMax is still enforced during simulations — a minor divergence from Geth's full skip. |
| crates/vm/levm/src/hooks/l2_hook.rs | Mirrors the same !vm.env.disable_nonce_check guard applied in default_hook.rs, consistent application of the fix for the L2 path. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[eth_call / eth_estimateGas] --> B[simulate_tx_from_generic]
B --> C[env_from_generic]
C --> D[Environment\ndisable_nonce_check: false]
D --> E[env.disable_nonce_check = true]
E --> F[VM::execute]
F --> G{Hook type?}
G -->|L1| H[DefaultHook::prepare_execution]
G -->|L2| I[L2Hook::prepare_execution_fee_token]
H --> J[increment_account_nonce\nunconditional]
I --> J
J --> K{disable_nonce_check?}
K -->|true| L[Skip nonce mismatch check ✓]
K -->|false| M{nonce matches?}
M -->|yes| L
M -->|no| N[NonceMismatch error ✗]
O[execute_tx\nnormal block execution] --> P[setup_env]
P --> Q[Environment\ndisable_nonce_check: false]
Q --> R[VM::execute → nonce validated normally]
Comments Outside Diff (2)
-
tooling/ef_tests/state_v2/src/modules/runner.rs, line 151-152 (link)Missing
disable_nonce_checkfield — compilation errorThe
Environmentstruct literal is exhaustive (no..Default::default()spread), but the newly addeddisable_nonce_checkfield is not present here. This will fail to compile.The same issue exists in:
test/tests/levm/eip7708_tests.rs:203tooling/ef_tests/state/runner/levm_runner.rs:231
All three sites need
disable_nonce_check: false,added to the struct literal. -
crates/vm/levm/src/hooks/default_hook.rs, line 74-85 (link)NonceIsMaxstill enforced even when nonce check is disabledThe
NonceIsMaxcheck (which increments the nonce and errors if it overflowsu64::MAX) runs unconditionally, even whendisable_nonce_checkistrue. Geth'sSkipNonceChecksflag skips nonce validation entirely foreth_call, including the max-nonce guard.For the stated goal of matching Geth's behaviour, any account whose nonce is exactly
u64::MAXwill still faileth_callwithNonceIsMax, while Geth would succeed. This is an edge case, but it is a semantic divergence from the referenced Geth implementation. Consider guarding the increment (or at least the error path) with!vm.env.disable_nonce_checkas well.
Last reviewed commit: b2e86f1
🤖 Claude Code ReviewHere is my review of PR #6325: Review:
|
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
| pub disable_balance_check: bool, | ||
| /// When true, skip nonce validation. Used by `eth_call` simulations | ||
| /// where the caller typically doesn't provide a nonce. | ||
| pub disable_nonce_check: bool, |
There was a problem hiding this comment.
Adding the field here is correct, but since Environment doesn't use ..Default::default() at all construction sites, this will fail to compile. At minimum these files need disable_nonce_check: false added:
tooling/ef_tests/state_v2/src/modules/runner.rs(~line 145)tooling/ef_tests/state/runner/levm_runner.rs(~line 225)test/tests/levm/eip7708_tests.rs(~line 203)
Motivation
eth_callvalidates the transaction nonce against the sender account nonce. When the caller does not provide a nonce (the normal case for read-only calls), it defaults to0. Any account with nonce > 0 gets aNonceMismatcherror.For reference, Geth skips nonce validation entirely for
eth_callvia aSkipNonceChecksflag incore/state_transition.go.Closes #6312
Description
Add a
disable_nonce_checkflag toEnvironment(following the existingdisable_balance_checkpattern) and set it totrueinsimulate_tx_from_generic, which is the code path used byeth_callandeth_estimateGas.Changes:
environment.rs: Addeddisable_nonce_check: boolfieldbackends/levm/mod.rs: Set flag totrueinsimulate_tx_from_generic; set tofalsein explicitEnvironmentconstructorshooks/default_hook.rs,hooks/l2_hook.rs: Guard nonce mismatch check with!vm.env.disable_nonce_check