eth/catalyst: return -32602 for FCU V2 wrong payloadAttributes version

[Muzry]

I observed the following failure while running hive:

https://hive.ethpandaops.io/#/test/generic/1772351960-ad3e3e460605c670efe1b4f4178eb422?testnumber=153

Relevant log excerpt:

>> (658f6320) {"jsonrpc":"2.0","id":1,"method":"engine_forkchoiceUpdatedV2","params":[{"headBlockHash":"0x8efd0af809ef64f159eed0717051a46a6a019783ba42657c16529ef18385dabb","safeBlockHash":"0x0000000000000000000000000000000000000000000000000000000000000000","finalizedBlockHash":"0x0000000000000000000000000000000000000000000000000000000000000000"},{"timestamp":"0x1235","prevRandao":"0x0000000000000000000000000000000000000000000000000000000000000000","suggestedFeeRecipient":"0x0000000000000000000000000000000000000000","withdrawals":null,"parentBeaconBlockRoot":null}]}
<< (658f6320) {"jsonrpc":"2.0","id":1,"error":{"code":-38003,"message":"Invalid payload attributes","data":{"err":"missing withdrawals"}}}
DEBUG (Corrupted Block Hash Payload (INVALID)): Sent shanghai fcu using PayloadAttributesV1, error is expected
FAIL (Corrupted Block Hash Payload (INVALID)): Expected error code on EngineForkchoiceUpdatedV2: want=-32602, got=-38003

In short: geth returns -38003 for this FCU V2 invalid-structure/version case, while hive expects -32602.

Why return -32602

Per Shanghai engine_forkchoiceUpdatedV2:

• PayloadAttributesV1 MUST be used when timestamp < Shanghai.
• PayloadAttributesV2 MUST be used when timestamp >= Shanghai.
• If the wrong version is used, client software MUST return -32602: Invalid params.

Reference:

https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md#engine_forkchoiceupdatedv2

In this case, when timestamp >= Shanghai and withdrawals is null, we treat that as a version mismatch (i.e. not a valid PayloadAttributesV2), so it should return -32602.

This PR updates FCU V2 handling to follow that requirement.

Result

This change fixes 9 hive test cases.

Note

If my interpretation of the spec here is incorrect, please let me know and I can adjust the implementation accordingly.

[MariusVanDerWijden]

You are correct, but I think the spec is bad in that case. It makes no sense to return 38003 for V3 and V4 and 32602 for V2 and V1.

Maybe @marioevz and @MysticRyuujin can weigh in here

[MysticRyuujin]

Hmmm

Code	Message
38003	"Invalid payload attributes"
32602	"Invalid params"

I would think that 38003 is the correct error code for this scenario, as it's an invalid attribute, not an invalid param?

I think I agree that the SPEC should change 🤔

[mkalinin]

What do you think about introducing engine_getSupportedProtocos with a specific semantics instead of introducing a new capabilities method?

[MariusVanDerWijden]

@mkalinin I think your comment is not related to this PR, right?

[MysticRyuujin]

The spec was updated

[Muzry]

Thanks everyone for the clarification, and for updating the spec.
I’ll align with the updated spec, try to fix the other clients accordingly, and close this PR.