-
Notifications
You must be signed in to change notification settings - Fork 376
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
Clarify Cancun payloads handling by earlier APIs; reorder checks #426
Conversation
flcl42
commented
Jun 29, 2023
- Remove uncertainty in handling Cancun payload by previous forks' APIs
- Remove redundant spec
- Clarify the order of overlapping payload verification rules
f0f5ba7
to
55c1b63
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 👍
c44d38e
to
8a0ef17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
1ce7366
to
806e44d
Compare
806e44d
to
292f5be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the change to first do input checks, then the timestamp check. LGTM!
@@ -96,14 +94,16 @@ Refer to the response for [`engine_newPayloadV2`](./shanghai.md#engine_newpayloa | |||
|
|||
This method follows the same specification as [`engine_newPayloadV2`](./shanghai.md#engine_newpayloadv2) with the addition of the following: | |||
|
|||
1. Given the expected array of blob versioned hashes client software **MUST** run its validation by taking the following steps: | |||
1. Client software **MUST** check that provided set of parameters and their fields strictly matches the expected one and return `-32602: Invalid params` error if this check fails. Any field having `null` value **MUST** be considered as not provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for first checking correct fields before timestamp
Co-authored-by: g11tech <[email protected]>
32259b5
to
ad2ccb0
Compare
…88) (#7969) Prerequisites: erigontech/interfaces#187 & ledgerwatch/erigon-lib#1069. Also implement ethereum/execution-apis#426.
…88) (#7969) Prerequisites: erigontech/interfaces#187 & ledgerwatch/erigon-lib#1069. Also implement ethereum/execution-apis#426.