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

Update Empty requests list behaviour for Pectra-5 #12985

Merged
merged 10 commits into from
Jan 14, 2025
Merged

Update Empty requests list behaviour for Pectra-5 #12985

merged 10 commits into from
Jan 14, 2025

Conversation

somnathb1
Copy link
Contributor

@somnathb1 somnathb1 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

@somnathb1 somnathb1 added pectra The Prague/Electra protocol upgrade 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
@somnathb1 somnathb1 marked this pull request as draft December 4, 2024 07:02
@somnathb1 somnathb1 changed the base branch from main to som/fixReq_Addr January 9, 2025 10:14
@somnathb1 somnathb1 marked this pull request as ready for review January 9, 2025 10:14
@somnathb1 somnathb1 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
@somnathb1
Copy link
Contributor Author

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

@somnathb1 somnathb1 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
core/types/block.go Outdated Show resolved Hide resolved
}
requests[i] = types.FlatRequest{Type: r, RequestData: executionRequests[i]}
requests = make(types.FlatRequests, 0)
for _, r := range executionRequests {
Copy link
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
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
Contributor 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.

@somnathb1 somnathb1 merged commit ee0a2da into main Jan 14, 2025
13 checks passed
@somnathb1 somnathb1 deleted the som/eip7685 branch January 14, 2025 14:14
somnathb1 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
somnathb1 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pectra The Prague/Electra protocol upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants