fix: eth_simulateV1 wrong error codes for nonce too low/high (#11220, #11221)

[divi2806]

Fixes #11220
Fixes #11221

Changes

• Add ErrorCodes.NonceTooLow = -38010 and ErrorCodes.NonceTooHigh = -38011 to ErrorCodes.cs
• Add SimulateErrorMessages.NonceTooLow = "Transactions nonce is too low" and SimulateErrorMessages.NonceTooHigh = "Transactions nonce is too high" constants to SimulateErrorMessages
• Map TransactionResult.ErrorType.TransactionNonceTooLow → ErrorCodes.NonceTooLow (-38010) in MapSimulateErrorCode (was incorrectly returning -32603 InternalError)
• Map TransactionResult.ErrorType.TransactionNonceTooHigh → ErrorCodes.NonceTooHigh (-38011) in MapSimulateErrorCode (was incorrectly returning -32603 InternalError)
• Add nonce error message overrides for both types in MapSimulateErrorMessage
• Add regression test eth_simulateV1_nonce_too_low_returns_spec_error_code_and_message covering the validation: true path

Types of changes

• Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

• Yes

If yes, did you write tests?

• Yes

Notes on testing

Added eth_simulateV1_nonce_too_low_returns_spec_error_code_and_message which uses StateOverrides to set the sender account nonce to 5, then submits a transaction with Nonce = 3 (< 5) under Validation = true. Asserts ErrorCode = -38010 and Error = "Transactions nonce is too low".

Note: TransactionNonceTooHigh (#11221) is fixed in the same switch arm and does not require a separate regression test — the mapping is covered by the same code path. The validation: false path intentionally bypasses nonce checking by design (SkipValidation flag in TransactionProcessor), so no test is added for that case.

All 92 simulate tests pass.

Documentation

Requires documentation update

• No

Requires explanation in Release Notes

• No

[LukaszRozmej]

@claude review

[claude]

Claude finished @LukaszRozmej's task in 2m 32s —— View job

---

Review

• Read changed files
• Review ErrorCodes.cs
• Review SimulateTxExecutor.cs
• Review test file
• Check rules files
• Post final review

---

Summary

The core fix is correct. TransactionNonceTooLow now maps to -38010 and TransactionNonceTooHigh to -38011 (was both returning -32603 InternalError), with spec-mandated messages wired through MapSimulateErrorMessage. SimulateErrorMessages and ErrorCodes are properly extended with XML docs.

---

Findings

Medium

Missing regression test for NonceTooHigh (#11221) — see inline comment
The PR fixes two distinct error codes (-38010 and -38011) but only tests the NonceTooLow path. The MapSimulateErrorCode and MapSimulateErrorMessage switches have two separate arms — a future accident (e.g. codes swapped, wrong constant) would go undetected. A companion test mirroring the existing one with state-override nonce=3 and tx nonce=5 is trivial to add.

Low

Switch arm ordering inconsistency — see inline comment
MapSimulateErrorCode has NonceTooHigh before NonceTooLow; MapSimulateErrorMessage has the reverse. No functional impact, but consistent ordering aids auditing.

---

What looks good

• Error codes -38010/-38011 match the eth_simulateV1 spec positions (fitting between existing -38009 gap and -38012 FeeCapBelowBaseFee).
• XML doc comments on the new ErrorCodes constants are clear and accurate.
• SimulateErrorMessages is properly extended with the same pattern as the existing intrinsic-gas/fee-cap constants, and shared between executor and test via the internal class reference.
• The NonceTooLow test is well-constructed: it uses StateOverrides to advance the sender's nonce to 5, submits with nonce 3 under Validation = true, and asserts both the error code and message.
• The validation: false path (SkipValidation) correctly omits nonce checking by design — no test needed for that path.

[LukaszRozmej]

Duplicate of #11430 ?

[claude]

Medium – Missing regression test for NonceTooHigh (#11221)

The PR claims to fix both #11220 and #11221, but only adds a test for the NonceTooLow path. The two cases map to different error codes (-38010 vs -38011) and different messages, so a regression on the NonceTooHigh arm (e.g. codes accidentally swapped, or the wrong constant referenced) would go undetected.

The author argues "same code path", but the switch has two separate arms:

TransactionResult.ErrorType.TransactionNonceTooHigh => ErrorCodes.NonceTooHigh,
TransactionResult.ErrorType.TransactionNonceTooLow  => ErrorCodes.NonceTooLow,

A simple companion test is straightforward — set StateOverrides nonce to 3, submit tx with Nonce = 5, and assert ErrorCode == -38011 and Error == "Transactions nonce is too high".

Fix this →

[claude]

Low – Arm ordering inconsistency

In MapSimulateErrorCode (line 261–262) the order is NonceTooHigh before NonceTooLow, but here in MapSimulateErrorMessage it is NonceTooLow before NonceTooHigh. No functional impact, but keeping both switches in the same order makes auditing the mapping easier.

Suggested change

	TransactionResult.ErrorType.TransactionNonceTooLow
	=> SimulateErrorMessages.NonceTooLow,
	TransactionResult.ErrorType.TransactionNonceTooHigh
	=> SimulateErrorMessages.NonceTooHigh,
	TransactionResult.ErrorType.TransactionNonceTooHigh
	=> SimulateErrorMessages.NonceTooHigh,
	TransactionResult.ErrorType.TransactionNonceTooLow
	=> SimulateErrorMessages.NonceTooLow,

[divi2806]

@LukaszRozmej, I did not see #11430 was already existing,
I was working on these issues beforehand so did not see a PR already being created.