-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Electra: EIP-7251 implement process_withdrawal updates #14181
Conversation
f4c98b6
to
6852f64
Compare
// # If partial withdrawal queue is full, only full exits are processed | ||
// if len(state.pending_partial_withdrawals) == PENDING_PARTIAL_WITHDRAWALS_LIMIT and not is_full_exit_request: | ||
// return | ||
// amount = withdrawal_request.amount |
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.
These changes are just updating the spec definition after #14091
if len(wds) != len(expectedWithdrawals) { | ||
return nil, fmt.Errorf("execution payload header has %d withdrawals when %d were expected", len(wds), len(expectedWithdrawals)) | ||
} |
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.
Discussed offline with Potuz. This check isn't technically required since the withdrawal roots won't match if the length of withdrawals are different. However, I found it very helpful in debugging and it is essentially free to access the length of the slices.
143d472
to
968ea8b
Compare
968ea8b
to
5757e0b
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.
went through it, all changes made sense to me, mostly focused on the partialWithdrawalsCount usage
What type of PR is this?
Feature
What does this PR do? Why is it needed?
Part of ethereum/consensus-specs#3668
Which issues(s) does this PR fix?
Other notes for review
Draft until unit tests are added for the DequeuePendingPartialWithdrawals call.