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

Remove redundant method for v4 payload #461

Merged
merged 2 commits into from
Feb 27, 2025
Merged

Conversation

emhane
Copy link
Collaborator

@emhane emhane commented Feb 27, 2025

No need for method OpExecutionPayloadV4::withdrawals and the types it imports, since the inner payload is in a publicly accessible field anyways. This list of withdrawals should always be empty.

@emhane emhane added the C-debt Category: technical debt or removable label Feb 27, 2025
@emhane emhane added the A-rpc-types-engine Area: rpc-types-engine crate label Feb 27, 2025
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

why remove this?

having withdrawals as fn is a lot nicer

@emhane
Copy link
Collaborator Author

emhane commented Feb 27, 2025

why remove this?

having withdrawals as fn is a lot nicer

if it stays, docs need to be updated to clarify it means l1 withdrawals and that it should always be empty...thought that was a bit weird, so removed it. if someone wants to access the empty Shanghai withdrawals, they can, but it's discouraged because they should always be empty.

imo the api is misleading

@emhane emhane requested a review from mattsse February 27, 2025 15:48
@mattsse mattsse added this pull request to the merge queue Feb 27, 2025
Merged via the queue into main with commit acc95f2 Feb 27, 2025
21 checks passed
@mattsse mattsse deleted the emhane/rm-redundant-method-v4 branch February 27, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc-types-engine Area: rpc-types-engine crate C-debt Category: technical debt or removable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants