eth/catalyst: add timestamp checks to fcu and new payload and improve param checks

[lightclient]

This PR introduces a few changes with respect to payload verification in fcu and new payload requests:

• First of all, it undoes the verifyPayloadAttributes(..) simplification I attempted in beacon/engine, eth/catalyst, miner: 4788 CL/EL protocol updates #27872. At the time I expected the pattern from forkchoiceUpdatedV2 to continue where the previous version's payload attributes would be accepted by the method if the timestamp allowed. It turns out this is only a special case in forkchoiceUpdatedV2. In forkchoiceUpdatedV3 is not possible to provide PayloadAttributesV2. Only V3 is accepted. This sort of defeats the purpose of the shared method.

  To see this in the spec, you may first view the parameters for forkchoiceUpdatedV2 which accepts both PayloadAttributesV1 and PayloadAttributesV2 versus the parameters for forkchoiceUpdatedV3 which accepts only PayloadAttributesV3.

  For this reason I have removed verifyPayloadAttributes(..) and its related functions in favor of a more straight forward approach of verifying the existence of optional values on engine.PayloadAttributes. I went ahead and updated this logic for forkchoiceUpdatedV2 even though it should technically still allow V1 payload attributes, because we have passed Paris on all networks and have no need to continue support of building V1 blocks via forkchoiceUpdatedV2. I think it also simplifies the API to not have these two conflicting approaches for V1 vs. V2 fcu.

• Next I have added timestamp validation to fcu payload attributes as required (section 2) by the Engine API spec. I back ported this from forkchoiceUpdatedV3 to forkchoiceUpdatedV2 so that it is only possible to call forkchoiceUpdatedV2 with attributes occurring in the Shanghai fork. This is done for the same reason a referenced above.

• For the new payload methods, I also update the verification of the executable data. For newPayloadV2, it does not currently ensure that cancun values are nil. Which could make it possible to submit cancun payloads through it. Additionally, it verifies the timestamp of the payload is exactly within the Shanghai fork. Both of these break the ability to use newPayloadV2 for pre-shanghai blocks, but as already stated, all networks are past this point and it is not necessary to continue support. Decided to retain support for v1 payloads in new payload v2.

  On newPayloadV3 the same types of checks are added. All shanghai and cancun related fields in the executable data must be non-nil, with the addition that the timestamp is only with cancun.

• Finally it updates a newly failing catalyst test to call the correct fcu and new payload methods depending on the fork.

--

The general flow of all engine API methods are:

• check the well-formedness of the parameters -- this makes sense because the spec is written in a way were object is versioned, whereas we use a single object with optional values. So it would expect an immediate rejection by the API in case an object does not meet the expected criteria.
• check the timing of the request -- although the fcu V2 and new payload V2 methods are special and also accept V1 requests, it is clear that going forward (as we see in V3) that the methods will be locked strictly to the fork they are related to and so we should check this after the well-formedness.
• the thing -- after the checks are complete we can continue with processing the request

[lightclient]

Agreed, the string comparison sucks. I am just hesitant to expose a new public interface for the fork order. I too another stab at it in the last commit though. LatestFork is a bit tricky to get right. We technically have three "markers" for determining which fork we're in. Block, total difficulty, and timestamp. I opted to only support time-based forks as that is how we will operate moving forward and the method doesn't need to be backwards compatible.

[MariusVanDerWijden]

I have never heard of frontierThawing before, very interesting

[holiman]

Me neither... Do you have a reference @lightclient ?

[MariusVanDerWijden]

it was the fork that increased the gaslimit so transactions could be executed. I don't know if it was a hardfork or a softfork though

[holiman]

Wait, what is paris, and why is it before shanghai?

[MariusVanDerWijden]

Paris was the official name for the merge

[MariusVanDerWijden]

@parithosh told me that the devnet-10 branch (which I think contains this) had some issues on a shadowfork.

WARN [11-03|09:50:03.922] Served engine_newPayloadV2               conn=172.18.0.4:56686 reqid=1330 duration=2.520127ms err="Invalid parameters"       errdata="{Error:non-nil withdrawals pre-shanghai}"

even though the node was post-shanghai already. The CL node send (imo correctly) and empty withdrawal slice post-shanghai via v2, which should be accepted

[lightclient]

Currently we're failing a handful of engine-cancun tests. This PR resolves most of them, still couple which will i) be fixed by #28246 (depends on this PR) or ii) a forthcoming PR to split up forkchoiceUpdate(..) into methods which update the actual head and then a method to kick off payload building.

Before:

After:

[holiman]

LGTM

[holiman]

Shouldn't something like this work?

if api.eth.BlockChain().Config.IsCancun(..) { 
    return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("NewPayloadV2 used post-shanghai"))
}
if api.eth.BlockChain().Config().LatestFork(params.Timestamp) == forks.Shanghai {

[MariusVanDerWijden]

LGTM