Skip to content

Update Empty requests list behaviour for Pectra-5#12985

Merged
somnergy merged 10 commits intomainfrom
som/eip7685
Jan 14, 2025
Merged

Update Empty requests list behaviour for Pectra-5#12985
somnergy merged 10 commits intomainfrom
som/eip7685

Conversation

@somnergy
Copy link
Copy Markdown
Member

@somnergy somnergy commented Dec 3, 2024

The updated EIP-7685 says requests with empty request_data should be dropped from executionRequests field in the API and ignored for hash calculation.
See ethereum/EIPs#8989, ethereum/execution-apis#599

Issue board: #12401

@somnergy somnergy added pectra do-not-merge PR that is in a merge-able state but is waiting for something else to take place before merging labels Dec 3, 2024
@somnergy somnergy marked this pull request as draft December 4, 2024 07:02
@somnergy somnergy changed the base branch from main to som/fixReq_Addr January 9, 2025 10:14
@somnergy somnergy marked this pull request as ready for review January 9, 2025 10:14
@somnergy somnergy changed the title [DO-NOT-MERGE] Update Empty requests list behaviour for Pectra-5 Update Empty requests list behaviour for Pectra-5 Jan 9, 2025
@somnergy
Copy link
Copy Markdown
Member Author

somnergy commented Jan 9, 2025

This is 4/6 in the chain
Next: #13106

@somnergy somnergy removed the do-not-merge PR that is in a merge-able state but is waiting for something else to take place before merging label Jan 9, 2025
Base automatically changed from som/fixReq_Addr to main January 11, 2025 15:57
}
requests[i] = types.FlatRequest{Type: r, RequestData: executionRequests[i]}
requests = make(types.FlatRequests, 0)
for _, r := range executionRequests {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please guard against the case when r is empty

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this bit of the spec implemented: "Elements of the list MUST be ordered by request_type in ascending order. Elements with empty request_data MUST be excluded from the list. If any element is out of order or has a length of 1-byte or shorter, client software MUST return -32602: Invalid params error."?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, here it doesn't throw the error. This was beneficial to run some early implementations of CLs, but I have made the change now to address the concern.

@somnergy somnergy merged commit ee0a2da into main Jan 14, 2025
@somnergy somnergy deleted the som/eip7685 branch January 14, 2025 14:14
somnergy added a commit that referenced this pull request Jan 15, 2025
The updated EIP-7685 says requests with empty `request_data` should be
dropped from `executionRequests` field in the API and ignored for hash
calculation.
See ethereum/EIPs#8989,
ethereum/execution-apis#599

Issue board: #12401
somnergy added a commit that referenced this pull request Jan 15, 2025
The updated EIP-7685 says requests with empty `request_data` should be
dropped from `executionRequests` field in the API and ignored for hash
calculation.
See ethereum/EIPs#8989,
ethereum/execution-apis#599

Issue board: #12401

Cherry pick #12985
@VBulikov VBulikov mentioned this pull request Feb 5, 2025
revitteth pushed a commit to 0xPolygon/cdk-erigon that referenced this pull request Mar 21, 2025
…rigontech#13439)

The updated EIP-7685 says requests with empty `request_data` should be
dropped from `executionRequests` field in the API and ignored for hash
calculation.
See ethereum/EIPs#8989,
ethereum/execution-apis#599

Issue board: erigontech#12401

Cherry pick erigontech#12985
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants