eip7251: Bugfix and more withdrawal tests #3979
Conversation
tests/core/pyspec/eth2spec/test/electra/block_processing/test_process_withdrawals.py
Show resolved
Hide resolved
tests/core/pyspec/eth2spec/test/electra/block_processing/test_process_withdrawals.py
Outdated
Show resolved
Hide resolved
tests/core/pyspec/eth2spec/test/electra/block_processing/test_process_withdrawals.py
Outdated
Show resolved
Hide resolved
| balance = state.balances[validator_index] | ||
| # [Modified in Electra:EIP7251] | ||
| partially_withdrawn_balance = sum( | ||
| withdrawal.amount for withdrawal in withdrawals if withdrawal.validator_index == validator_index) |
There was a problem hiding this comment.
Alternatively, it could iterate over withdrawals[:effective_partial_withdrawals_count] which contains partial withdrawals included during the current run. In cases when there are no withdrawal requests eligible for processing this computation will be a noop. But we can also leave the existing computation in the spec for simplicity and defer possible optimisation to the implementations
There was a problem hiding this comment.
What is effective_partial_withdrawals_count? I cannot find its definition.
There was a problem hiding this comment.
@ppopth I think he meant processed_partial_withdrawals_count, the newly renamed variable.
There was a problem hiding this comment.
effective_partial_withdrawals_count = len(withdrawals) computed before the sweep. processed_partial_withdrawals_count may be greater than that number due to skipped partial withdrawals
tests/core/pyspec/eth2spec/test/electra/block_processing/test_process_withdrawals.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Justin Traglia <[email protected]>
Follow up to #3943 with more withdrawal tests and yet another bugfix.
The PR does also proposes to rename
partial_withdrawals_counttoprocessed_partial_withdrawals_countand withdrawals test helper refactor.The fix
The balance withdrawn upon request, i.e. by processing a pending partial withdrawal, wasn’t taken into account for the sweep. Potential outcome is a validator being considered partially withdrawable while it has no excess balance and effectively is not partially withdrawable.
The proposed fix is simply to check if validator index is already presented in the withdrawals and deduct the already withdrawn balance from the balance considered by the sweep.