Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PayloadAttributesV1 in spec is consistent with either engine_forkchoiceUpdatedV1/V2 but not V3 #2280

Open
tersec opened this issue Jun 1, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@tersec
Copy link
Contributor

tersec commented Jun 1, 2024

https://github.com/ethereum/execution-apis/blob/v1.0.0-beta.4/src/engine/shanghai.md#request-1

  1. payloadAttributes: Object|null - instance of PayloadAttributesV1 | PayloadAttributesV2 or null, where:
    • PayloadAttributesV1 MUST be used to build a payload with the timestamp value lower than the Shanghai timestamp,
    • PayloadAttributesV2 MUST be used to build a payload with the timestamp value greater or equal to the Shanghai timestamp,
    • Client software MUST return -32602: Invalid params error if the wrong version of the structure is used in the method call.

i.e. a payload with the timestamp value lower than the Shanghai timestamp (which implies a headBlockHash corresponding similarly to a block with a timestamp strictly lower than the payload timestamp, i.e. also lower than the Shanghai timestamp), is compatible with either fcuV1 or fcuV2, but not fcuV3, per https://github.com/ethereum/execution-apis/blob/v1.0.0-beta.4/src/engine/cancun.md#request-1

  1. forkchoiceState: ForkchoiceStateV1.
  2. payloadAttributes: Object|null - Instance of PayloadAttributesV3 or null.

...

  1. payloadAttributes.timestamp falls within the time frame of the Cancun fork, return -38005: Unsupported fork on failure.

(with Prague not defining its own fcuV4, it doesn't work in Prague for the fcuV3 validation to be as strict as "within the time frame of the Cancun fork", but would have to be read as within the Cancun or Prague fork, but it does not appear the engine API specs as of v1.0.0-beta.4 acknowledge this; separate issue, in any case)

Indeed, the verbiage around allowing headBlockHash for fcuV2 corresponding to Paris prevents fcUV1 from being used for Shanghai but not fcuV2 being used for Paris:

  1. Consensus layer client MUST call this method instead of engine_forkchoiceUpdatedV1 under any of the following conditions:
    • headBlockHash references a block which timestamp is greater or equal to the Shanghai timestamp,
    • payloadAttributes is not null and payloadAttributes.timestamp is greater or equal to the Shanghai timestamp.

However,

else:
if apiVersion != Version.V1:
raise invalidAttr("forkChoiceUpdated" & $apiVersion &
" doesn't support head block with timestamp earlier than Shanghai")

rejects not only fcUV3/non-null PayloadAttributesV1/Paris head block (validly, as that's not a valid combination) but also fcUV2/PayloadAttributesV2/Paris head block (not validly, because that is an allowed combination).

One weird edge case here is, in the fcuV3/null PayloadAttributes/Paris or Shanghai head block, is that allowed. The spec isn't very clear here; null is allowed as one of the payloadAttributes parameters, but the fcU3 spec goes out of its way to disallow non-null PayloadAttributesV1/V2 exactly the same way fcUV2 goes out of its way to allow non-null PayloadAttributesV1 so there's some evident intent to close down backwards compatibility.

But at least fcuV2 should allow Paris payloads/head blocks, that much seems evident.

@jangko jangko added the bug Something isn't working label Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants