fix: correct hive errors

[DarkLord017]

Fixes failing hive tests
✖ ForkchoiceUpdatedV2 To Request Shanghai Payload, Non-Null Beacon Root (Cancun) (nethermind)
✖ NewPayloadV3 Versioned Hashes, Nil Hashes, Syncing=False (Cancun) (nethermind)
✖ NewPayloadV3 Versioned Hashes, Nil Hashes, Syncing=True (Cancun) (nethermind)

Changes

• In Payload Attributes return InvalidPayloadAttributes when actualVersion > timestampVersion
• When blob versioned hashes is nil return proper json rpc error

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

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

[claude]

Claude finished @DarkLord017's task in 4m 57s —— View job

---

PR Review

• Gathering context
• Reading changed files
• Analyzing correctness and edge cases
• Posting review findings

---

Summary

Two distinct bugs fixed, both targeting Engine API spec compliance. The logic is correct in both cases. No blocking issues found.

---

PayloadAttributes.cs — ValidateVersion logic fix

Correct. The old condition:

bool unsupportedFork = timestampVersion >= PayloadAttributesVersions.V2 &&
    (actualVersion > timestampVersion || fcuVersion != timestampVersion);

incorrectly returned UnsupportedFork when actualVersion > timestampVersion (e.g. V3 attrs carrying ParentBeaconBlockRoot sent to FCUv2 at a Shanghai timestamp). The Engine API spec is unambiguous: sending attributes that contain fields beyond the active fork is INVALID_PAYLOAD_ATTRIBUTES, not UNSUPPORTED_FORK. The new condition:

bool unsupportedFork = timestampVersion >= PayloadAttributesVersions.V2 &&
    actualVersion < timestampVersion &&
    fcuVersion != timestampVersion;

correctly restricts UnsupportedFork to the case where the FCU version doesn't match the fork (attrs missing required fields AND FCU is behind the timestamp-implied fork). All other version mismatches fall through to InvalidPayloadAttributes.

Verified against the key hive scenario (FCUv2, V3 attrs at Shanghai timestamp): timestampVersion=2 >= 2 && actualVersion=3 < 2 → false → InvalidPayloadAttributes. ✓

---

IExecutionPayloadParams.cs — null guard for blobVersionedHashes

Correct. Without this check, a null blobVersionedHashes was silently converted to an empty ReadOnlySpan<Hash256?> by the implicit array-to-span conversion, causing either a false-positive match (no-blob transaction payloads) or a wrong error message ("do not match" instead of "must not be null"). The fix returns ValidationResult.Fail → ErrorCodes.InvalidParams (-32602), which is the correct protocol-level error per the Engine API spec.

---

Test changes

Both updated test cases ("ForkchoiceUpdatedV2/V3 To Request Shanghai Payload, Zero Beacon Root") correctly track the behavior change from UnsupportedFork to InvalidPayloadAttributes. The scenario — FCUv2/v3 with non-null ParentBeaconBlockRoot at a Shanghai timestamp — is precisely the case where actualVersion (3) > timestampVersion (2).

---

Findings

Severity	File	Note
Low	PayloadAttributes.cs:166	Comment wording is a sentence fragment — suggestion left inline
Low	IExecutionPayloadParams.cs:91	ValidationResult.Fail choice is correct but non-obvious; a one-line comment would help future readers — suggestion left inline

No critical, high, or medium findings. Mergeable.

[claude]

Claude finished @DarkLord017's task in 4m 10s —— View job

---

PR Review

• Gathering context
• Reading changed files and agent rules
• Analyzing correctness, edge cases, and spec compliance
• Posting review findings

---

Summary

Two focused spec-compliance fixes with no critical, high, or medium issues. Both behavioral changes are correct and tested. The two Low findings from the previous review remain unaddressed (inline suggestions still pending).

---

PayloadAttributes.cs — ValidateVersion boolean fix

The logic change is correct. I verified all relevant combinations:

actualVersion	timestampVersion	fcuVersion	Old result	New result	Correct?
3 (V3 attrs)	2 (Shanghai)	2 (FCUv2)	UnsupportedFork	InvalidPayloadAttributes	✓ (spec: attrs have future fields → INVALID_PAYLOAD_ATTRIBUTES)
2 (V2 attrs)	3 (Cancun)	2 (FCUv2)	UnsupportedFork	UnsupportedFork	✓ (attrs are behind; FCU doesn't support Cancun)
3 (V3 attrs)	2 (Shanghai)	3 (FCUv3)	UnsupportedFork	InvalidPayloadAttributes	✓ (attrs carry future fields regardless of FCU version)
2 (V2 attrs)	3 (Cancun)	3 (FCUv3)	UnsupportedFork	InvalidPayloadAttributes	✓ (FCU matches fork, attrs missing required field)

The pre-Paris (V1 timestamp) case also holds: timestampVersion >= V2 is false, so the unsupportedFork guard always yields false → InvalidPayloadAttributes, which is correct since Paris has no UNSUPPORTED_FORK concept.

IExecutionPayloadParams.cs — null guard for blobVersionedHashes

Correct fix. Without the guard, null was implicitly converted to default(ReadOnlySpan<Hash256?>) (empty) in FlattenedHashesEqual, which would pass if no blob transactions were present. ValidationResult.Fail → ErrorCodes.InvalidParams (-32602) is the right protocol-level error per the Engine API spec (null is not a valid JSON value for this array parameter).

Tests

All new and updated test expectations are consistent with the spec:

• PayloadAttributesValidateTests: the new case (isAmsterdam=false, withSlot=true, fcu=V3) → InvalidPayloadAttributes traces correctly with the new logic (actualVersion(4) < timestampVersion(3) is false, so unsupportedFork = false).
• EngineModuleTests.V3.cs: "Zero Beacon Root" cases reclassified from UnsupportedFork → InvalidPayloadAttributes. The FCUv2/v3 with non-null ParentBeaconBlockRoot at a Shanghai timestamp is unambiguously INVALID_PAYLOAD_ATTRIBUTES per spec.
• NewPayloadV3_should_decline_null_blobversionedhashes already existed and now correctly expects ErrorCodes.InvalidParams thanks to the null guard.

Findings

Severity	Location	Note
Low	PayloadAttributes.cs:166–167	Comment is a sentence fragment: "from a more advanced fork than the" — incomplete. Inline suggestion was left in the previous review.
Low	IExecutionPayloadParams.cs:93	ValidationResult.Fail is correct but non-obvious vs Invalid; a brief inline comment would help. Inline suggestion was left in the previous review.

No critical, high, or medium findings. Mergeable.