fix: honour engine fixture errorCode instead of accepting any RPC error

[Marchhill]

Changes

BlockchainTestBase.AssertExpectedRpcError accepted any JSON-RPC error from engine_newPayloadV* as satisfying a fixture's expected consensus rejection, as long as the fixture carried a validationError:

private static void AssertExpectedRpcError(int errorCode, string? errorMessage, string? validationError, int payloadVersion) =>
    Assert.That(validationError, Is.Not.Null, $"engine_newPayloadV{payloadVersion} RPC error: {errorCode} {errorMessage}");

A payload refused with -38005 (unsupported fork) or -32602 (invalid params) is rejected by the engine API before any consensus rule runs, so it cannot demonstrate the rejection an invalid-block fixture asserts. Every such payload scored green.

EEST fixtures already say which payloads are meant to come back as an engine API error — each engineNewPayloads entry carries an optional errorCode alongside validationError — and the harness ignored it entirely.

• Bind errorCode on TestEngineNewPayloadsJson, parsed by JsonToEthereumTest.ParseErrorCode (throws rather than silently returning null on a malformed value).
• DescribeUnexpectedRpcError accepts an RPC error only when the fixture asked for that exact code. No errorCode ⇒ any error fails, with a message naming the code and saying the payload was never validated.
• Assert the converse too: a fixture that expects an errorCode now fails if the payload was accepted for validation instead.
• EngineRpcErrorTests pins both directions plus errorCode parsing.

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

EngineRpcErrorTests (9 cases) covers the classifier in both directions and errorCode parsing.

Blast radius was measured over the full blockchain_tests_engine corpus, not sampled.

Corpus shape (tests-glamsterdam-devnet@v8.1.0) — 10,971 files, 79,564 test cases, 98,376 newPayload calls. Exactly 78 payloads carry an errorCode, all -32602 (osaka 20, prague 20, amsterdam 27, shanghai→cancun 6, bpo2→amsterdam 5). 10,313 payloads expect INVALID with a validationError; the rest expect VALID.

frames-devnet8-integration × tests-glamsterdam-devnet@v8.1.0 (the pairing that branch's CI uses), full corpus, before and after:

	tests	pass	fail
baseline	79,564	79,561	3
with this change	79,564	79,561	3

Identical, down to the same three pre-existing test_invalid_layout_with_swapped_decodable_offsets failures. All 78 RPC errors observed were the 78 fixtures asking for -32602, answered with -32602. Zero unexpected protocol-level errors.

master-base × tests-bal@v7.2.0 (this branch's pinned archive), full corpus: 72,378 tests, 3 fail. Zero unexpected protocol-level errors here too, so the half this PR is named for costs nothing. Of the three:

• test_invalid_post_fork_block_without_bal_hash_field[fork_BPO2ToAmsterdamAtTime15k] and test_invalid_pre_fork_block_with_bal_hash_field[fork_BPO2ToAmsterdamAtTime15k] — new, from the converse assertion. Both fixtures expect -32602; the client validates the payload and rejects it as an invalid block instead. ExecutionPayloadParams<T>.ValidateEngineApiVersionParams does implement both directions of the BAL/newPayloadV5 rule, so the check exists and simply did not fire for these two — most likely the spec resolved pre-fork for that payload. Needs a trace before attributing a cause. These two are the only reds; if they should not land, the converse assertion is the separable half.
• test_run_until_out_of_gas[fork_Cancun-...-tstore_wide_address_space] returning SYNCING — unrelated, a local 8-process contention artefact.

Reproducing the false green. On master-base × v8.1.0, test_invalid_pre_fork_block_with_bal_hash_field[fork_BPO2ToAmsterdamAtTime15k] passed while the client answered -32602 "Block access list must not be set before engine_newPayloadV5" at V4 — the fixture's consensus rule was never exercised. With this change:

engine_newPayloadV4 failed at the protocol level with -32602 Block access list must not be set before engine_newPayloadV5; the payload was never validated, so it cannot demonstrate the rejection this fixture expects.

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

Remarks

The premise that a strict check would flip a large number of engine tests red does not survive measurement — it flips 0 on both matched corpora. The false green only fires when the client and the fixture archive are out of step, which is why it went unnoticed: the reported 5939/24805 and 59/135 figures came from a stale corpus and from EIP-8141 fixtures, not from what CI runs.

For branches paired with a foreign or stale corpus the new reds are the signal, and triage by code: -38005 means the spec provider does not recognise the fork carried by the payload (repin the archive, or wire the fork); -32602 means a param-shape mismatch (endpoint version vs. fixture, or a field the endpoint does not accept yet). If such a branch must stay green while its pin catches up, skip the fixture tree at load time — a skipped test is honest, a green one is not.