Engine API: respond with error if payload attributes are invalid

[mkalinin]

Enforces the following validity condition before starting a build process: payloadAttributes.timestamp > headBlock.timestamp. If this condition isn't met responds with error but does not rolls back fork choice updated. Entire forkchoiceUpdated processing flow looks as follows after this change:

• Check headBlock is known and there is no missing data. If there are missing data respond with SYNCING
• Check headBlock is VALID. If it's not respond with INVALID
• Apply forkchoiceState
• Check payloadAttributes.timestamp. If it's invalid respond with error: -32002: Invalid payload attributes
• Start payload build process and respond with VALID

This might seem a bit ugly but this is a cost of double semantics of forkchoiceUpdated call. An alternative would be to check payloadAttributes at the beginning and stop processing a call if attributes are invalid, i.e. do not apply fork choice state if payloadAttributes are invalid. Though, this check may not always be performed at the very beginning (imagine the case when headBlock is missing and EL has to respond with SYNCING). Also, having forkchoice state applied disregarding a build process state is valuable.

cc @djrtwo @MariusVanDerWijden @MarekM25

[MariusVanDerWijden]

As I already wrote in the previous thread, I prefer this approach as it makes the code more modular and we don't interleave the two different semantics of fcu

[djrtwo]

I'm generally fine with this error code.

So there are other things that could make payloadAttributes invalid wrt CL (e.g. not being the correct 12s offset, having a bad prevrandao) but is the point here that the only thing that can be invalid wrt EL is the timestamp?

After looking at the current values there, I agree. just wanted to make sure.

[mkalinin]

I'm generally fine with this error code.

So there are other things that could make payloadAttributes invalid wrt CL (e.g. not being the correct 12s offset, having a bad prevrandao) but is the point here that the only thing that can be invalid wrt EL is the timestamp?

After looking at the current values there, I agree. just wanted to make sure.

Yes, it seems that EL can do the timestamp check only. Preventing EL from making a block that it won't accept (due to buggy CL) is the main reason to have this error and the check.