fix(eth_simulateV1): return spec-compliant message for out-of-order block number (-38020)

[divi2806]

Fixes #11219

Changes

• eth_simulateV1 now returns the spec-mandated fixed message "Block number in sequence did not increase" for error code -38020 instead of the verbose internal message "Block number out of order {n} is <= than previous block number of {m}!"
• Added SimulateErrorMessages.BlocksOutOfOrder constant to keep the spec string as a single-point definition shared between the executor and tests
• Added regression test eth_simulateV1_blocks_out_of_order_returns_spec_error_code_and_message asserting the correct code and message when block numbers in the simulation sequence do not increase

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Notes on testing

• Added eth_simulateV1_blocks_out_of_order_returns_spec_error_code_and_message in EthSimulateTestsBlocksAndTransactions.cs — sends two blocks where the second has a lower number than the first and asserts error code -38020 and message "Block number in sequence did not increase"
• All 92 existing simulate tests continue to pass

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

[svlachakis]

@divi2806 please in general pick issues that are unassigned - #11219 is assigned to @deffrian

[svlachakis]

@claude review

[claude]

Claude finished @svlachakis's task in 1m 51s —— View job

---

Review: fix(eth_simulateV1) — out-of-order block number message (-38020)

• Read repo rules (coding-style, robustness, test-infrastructure)
• Read changed files
• Analyze correctness, edge cases, and regressions
• Check security and performance implications
• Post findings

---

Summary

A clean, minimal fix. The changed files are correct and well-structured. No Critical or High findings.

---

Findings

Low — multi-line XML doc comment on test method

EthSimulateTestsBlocksAndTransactions.cs:705-710 — The new test has a 3-line <summary> block. coding-style.md says "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." That said, this is consistent with the existing test at ~line 670 in the same file and the pattern is harmless in a test. Not blocking.

Note (pre-existing, out of scope) — -38021 has the same original bug

SimulateTxExecutor.cs:161 — BlockTimestampNotIncreased still returns the verbose dynamic message "Block timestamp out of order {time} is <= than given base timestamp of {lastBlockTime}!" rather than the spec-mandated "Block timestamp did not increase". This is the same class of issue as #11219, but pre-dates this PR. Worth opening a follow-up issue rather than blocking this fix. Fix this →

---

What's correct

• The spec-mandated constant "Block number in sequence did not increase" is placed in SimulateErrorMessages following the exact same pattern as IntrinsicGas, FeeCapBelowBaseFee, and InsufficientFunds — good single-point-of-definition.
• The <= guard (catching both equal and decreasing numbers) correctly maps to the spec message "did not increase".
• The regression test validates both ErrorCode == -38020 and the exact message string, covering the precise failure mode from the original issue.
• No performance or security implications — this is a pre-processing validation path, not a hot path.
• All 92 existing simulate tests pass per the PR description.

---

Verdict: Mergeable — no Critical, High, or Medium findings introduced by this PR.

[divi2806]

I am sorry @svlachakis, will keep in mind from next time to tackle unassigned issues

[svlachakis]

@divi2806 please cross check with go-ethereum the second point from claude review

Note (pre-existing, out of scope) — -38021 has the same original bug

[divi2806]

@svlachakis from https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/errors.go and https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/simulate.go,
The go eth still uses the verbose for error -38021
The fmt.Sprintf confirms in simulate.go return nil, &invalidBlockTimestampError{fmt.Sprintf("block timestamps must be in order: %d <= %d", t, prevTimestamp)}

Should i implement a fixed spec for -38021 in this branch as well?

[svlachakis]

@divi2806 hold on a sec, let's not merge yet because I see some discrepancy on the official docs

[divi2806]

Sure, waiting for your clarification on what the correct spec message should be, will update accordingly.
Thankyou

[divi2806]

@svlachakis,
Also is there anything that i can fix/help with? to resolve any blocking issue?

[divi2806]

@svlachakis quick cross-check from my side:

• In geth, this path is still mapped to error code -38021, but the message is verbose/dynamic (block timestamps must be in order: ... <= ...).
• In the execution-apis docs, -38021 message wording currently looks inconsistent (Block timestamp in sequence did not increase or stay the same). execution-api-docs

Given that, I can proceed either way you prefer:
1.) keep this PR strictly scoped to -38020 and open a separate follow-up for -38021, or
2.) include a -38021 message normalization + regression test in this PR as well.

[svlachakis]

@divi2806 we can't proceed I need to clarify with geth team and change either the spec or the test. Feel free to pick up something else for now.

[divi2806]

got it, Thankyou for the update :D