set nonce to current, ignoring all nonce check on CallAndRestore#8897
Merged
Conversation
…/nethermind into fix/no-nonce-check
benaadams
approved these changes
Jul 1, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates eth_call and eth_estimateGas to ignore the supplied transaction nonce—always using the on‐chain nonce instead—to mirror Geth’s behavior and fix the "wrong transaction nonce" error (#8813).
- Added unit tests for eth_call and eth_estimateGas to verify invalid nonces are ignored.
- Changed
BlockchainBridge.CallAndRestoreto unconditionally override the transaction nonce with the state reader’s nonce.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.EthCall.cs | Added Eth_call_ignores_invalid_nonce test to verify eth_call ignores invalid nonce. |
| src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.EstimateGas.cs | Added Eth_estimateGas_ignores_invalid_nonce test to verify eth_estimateGas ignores invalid nonce. |
| src/Nethermind/Nethermind.Facade/BlockchainBridge.cs | Removed conditional nonce check in CallAndRestore, now always uses state reader’s nonce. |
Comments suppressed due to low confidence (2)
src/Nethermind/Nethermind.Facade/BlockchainBridge.cs:255
- [nitpick] This comment is vague; consider elaborating why nonce is ignored (e.g., reference issue #8813) and specify it applies only to eth_call and eth_estimateGas methods.
//Ignore nonce on all CallAndRestore calls
src/Nethermind/Nethermind.Facade/BlockchainBridge.cs:256
- [nitpick] Since the method is named
CallAndRestore, consider capturing the original nonce before overwriting and restoring it after execution to preserve the caller’s transaction state.
transaction.Nonce = components.StateReader.GetNonce(stateRoot, transaction.SenderAddress);
| await TestEthCallOutOfGas(ctx, 300000000, 50000000); | ||
| } | ||
|
|
||
| [Test] |
There was a problem hiding this comment.
[nitpick] The setup logic for ignoring invalid nonces in eth_call and eth_estimateGas tests is duplicated; consider extracting a shared helper to reduce repetition.
Contributor
|
Erigon also does not check the nonce for |
emlautarom1
approved these changes
Jul 1, 2025
8 tasks
stdevMac
pushed a commit
that referenced
this pull request
Jul 31, 2025
* set nonce to current, ignoring all nonce check on CallAndRestore * module tests for ignoring rpc calls * comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial fix #8813
Geth do not check nonce on eth_estimateGas or eth_call. This will effectively remove our nonce check, and fix the "wrong transaction nonce" issue as reported here https://quicknode.notion.site/Geth-vs-Nethermind-JSON-RPC-Resposnes-19615a82e84c804bbebfcdb71e3793fc
Alternatively we could merge
ProcessingOptionsandExecutionOptionssoDoNotVerifyNoncecan be passed toTransactionProcessor, but this solution is of course simpler.