test(stateless): pin the fail-closed handling of incomplete input

[LukaszRozmej]

Changes

• Assert that an empty transaction entry in the payload is rejected, as the stateless spec requires.
• Assert that a schema id the decoder does not support yields the zero sentinel result rather than a partial one.
• Assert that a BLOCKHASH ancestor the witness does not carry fails the block instead of resolving to zero.

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: tests only, pinning behaviour the stateless spec states and nothing here asserted

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Notes on testing

Nethermind.Consensus.Test 390/390. All three properties already hold on master; these tests pin them.

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

Remarks

Worth knowing for anyone writing payload mutation tests: SszExecutionPayload.Transactions caches the wrapped array on first access, so mutating the inner byte[][] through AsExecutionPayload() does not survive the re-encode. The first version of the empty-transaction test did exactly that and passed while exercising nothing; it now mutates through the SSZ view, and a comment says why.

The BLOCKHASH case only applies before EIP-2935, since from Prague on the hashes come from state and are witness-bound; the spec reaches the same rejection by indexing its block-hash list out of range.

[claude]

Claude finished @LukaszRozmej's task in 1m 28s —— View job

---

Review: PR #13788 (tests only)

• Read the diff and the code it tests (StatelessExecutor, InputDecoder, SszExecutionPayload, BlockhashProvider, StatelessBlockTree)
• Check each test fails for the reason it claims
• Post inline comments

Verdict: not mergeable yet (1 Medium open). Counts: 0 Critical, 0 High, 1 Medium, 1 Low. I didn't run the tests; this is a read-through only.

Medium: the empty-transaction test doesn't pin what it says (inline)

• Replacing Transactions[0] changes the reconstructed tx root, but the declared BlockHash stays the same. If empty entries were ever silently skipped or accepted, execution would still fail, either on the tx-count vs public-key check or on HeaderValidator.ValidateHash.
• So the test currently repeats the existing "transactions" mutation case.
• Fix: assert the decode itself rejects the input, e.g. input.NewPayloadRequest.ToBlock(true) throws InvalidOperationException. Alternatively, recompute BlockHash so the hash check can't be what rejects it.
• Also, the re-encode lines are copied from Mutate<TPayload>; a small helper would remove that.
• Fix this →

Low: the BLOCKHASH test checks BlockhashProvider's default, not the stateless setup (inline)

• It builds the provider by hand, copying StatelessBlockProcessingEnv.cs:72, so a change to the stateless setup wouldn't be caught.
• Throws.Exception accepts any exception; Throws.TypeOf<InvalidDataException>() would be more exact.
• The throw only happens because the stateless path never calls Prefetch. After a Prefetch (the BranchProcessor path), StatelessBlockTree.Prefetch leaves missing ancestors as null. GetBlockhash would then return null (zero) without asking the throwing policy. That's outside this diff, but it's the exact scenario this test claims to guard.

What looks fine

• Unsupported schema test: the three values are real unsupported ids: Prague rev1, Amsterdam rev2, and Current rev0. They correctly reach the zero sentinel through the InputDecoder catch.
• Comments: the inline comment about the SSZ transaction cache and the XML doc on the BLOCKHASH test both explain why, and the cache comment is accurate. No comment-quality problems.
• Security and performance: no production code changes, so nothing to review.

[claude]

Medium: this test can pass even if empty entries stop being rejected. Swapping Transactions[0] changes the reconstructed tx root, but BlockHash stays the same. So if the decoder ever skipped or accepted empty entries, StatelessExecutor.Execute would still fail: either the transactions.Length == publicKeys.Length check or HeaderValidator.ValidateHash would catch it. IsSuccess == false holds either way. That makes this the same case as the existing "transactions" mutation in Encoded_execution_checks_reconstructed_header_hash, and the empty-entry rule isn't actually pinned.

The simplest way to pin it is to assert on the decode step, the same way the first test asserts ValidateHash. NewPayloadRequest.ToBlock throws InvalidOperationException when TryGetBlock returns an error:

Assert.That(() => input.NewPayloadRequest.ToBlock(requestsEnabled: true), Throws.InvalidOperationException);

Another option is to recompute BlockHash so the hash check can't be what rejects the block.

Minor: lines 101–104 copy the re-encode lines from Mutate<TPayload> (79–83). A small Reencode<TPayload>(byte[] encoded, StatelessInput<TPayload> input) helper would remove the duplication.

[claude]

Low: this tests BlockhashProvider's default, not the stateless wiring.

• The provider here is built by hand, copying StatelessBlockProcessingEnv.cs:72. If the env ever passes a permissive IUnresolvedBlockhashPolicy, this test still passes. The name says "fails the block", but the test only checks that a direct call throws.
• Throws.Exception accepts any exception, including an NRE from the substitute. Throws.TypeOf<InvalidDataException>() (from ThrowingUnresolvedBlockhashPolicy) would be more exact.
• The throw only happens because the stateless path calls BlockProcessor.ProcessOne directly and never calls Prefetch. Through BranchProcessor, StatelessBlockTree.Prefetch fills _hashes and leaves missing ancestors as null. GetBlockhash would then return hashes[depth - 1] (null, which BLOCKHASH treats as zero) without consulting the policy. That's outside this diff, but it's the scenario this test says it guards against, so it may be worth noting in the remarks or covering.