EIP-7251: Handle top-ups to exiting/exited validators #3650
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, a top-up can be made to an exiting validator, so that the balance exits the active set soon after being activated. Even if the top-up goes through the activation churn, this may still have some weak subjectivity impact because exits have twice the weak subjectivity impact of activations (see https://notes.ethereum.org/SyjG0FomTxqKki1m2cmU9w#Consolidations-and-churn-limit). To handle this, we just postpone the deposits of exiting validators, by skipping the `pending_balance_deposit` and appending it at the end of `state.pending_balance_deposits`. This might happen multiple times, as the deposit is only processed once the validator is exited. We also modify the processing of pending balance deposits from exited validators. Since the deposited balance goes straight to an exited validator and thus never becomes active, we don't need to have it go through the churn, so we don't count it towards decreasing `state.deposit_balance_to_consume`
hwwhww
changed the title
mkalinin
reviewed
Apr 4, 2024
- Fixes infinite loop pointed out by Mikhail, when there are only deposits of exiting validators in the queue - Prevents accumulation of deposit_balance_to_consume when there are just postponed deposits in the queue
|
I think this generally makes sense but I haven't gone through the diff with a fine-toothed comb yet. We should be careful with the final set of 7251 changes to preserve weak subjectivity invariants as much as possible. |
As discussed with @mkalinin, we only want to process a top-up to an an exiting validator (strictly) after withdrawable epoch, to ensure that the topped-up balance never becomes active even when the exiting validator is actually consolidating. In that case, the consolidation gets processed on withdrawable epoch, after which any balance deposited to the validator can only ever be withdrawn.
mkalinin
reviewed
Apr 8, 2024
Co-authored-by: Mikhail Kalinin <[email protected]>
mkalinin
approved these changes
Apr 8, 2024
hwwhww
reviewed
Apr 11, 2024
Comment on lines
132
to
135
| pre_balances = state.balances | ||
| yield from run_epoch_processing_with(spec, state, 'process_pending_balance_deposits') | ||
| # All deposits are postponed, no balance changes | ||
| assert state.balances == pre_balances |
There was a problem hiding this comment.
Python mutable trap: pre_balances = state.balances makes pre_balances point to state.balances itself, therefore assert state.balances == pre_balances is always true.
Suggested change
| pre_balances = state.balances | |
| yield from run_epoch_processing_with(spec, state, 'process_pending_balance_deposits') | |
| # All deposits are postponed, no balance changes | |
| assert state.balances == pre_balances | |
| pre_balances = state.balances.copy() | |
| yield from run_epoch_processing_with(spec, state, 'process_pending_balance_deposits') | |
| # All deposits are postponed, no balance changes | |
| assert state.balances == pre_balances |
I'm going to push commit to fix this file!
hwwhww
approved these changes
Apr 11, 2024
Comment on lines
+165
to
+166
| amount01 = spec.EFFECTIVE_BALANCE_INCREMENT | ||
| amount2 = spec.MAX_EFFECTIVE_BALANCE_EIP7251 |
There was a problem hiding this comment.
At first, my OCD was triggered, but then I found out it's "amount for 0 and 1".
13 tasks
12 tasks
|
Closing, superseded by #3776 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, a top-up can be made to an exiting validator, so that the balance exits the active set soon after being activated. Even if the top-up goes through the activation churn, this may still have some weak subjectivity impact because exits have twice the weak subjectivity impact of activations (see https://notes.ethereum.org/SyjG0FomTxqKki1m2cmU9w#Consolidations-and-churn-limit). Similarly, a top-up can be made to a consolidating validator, where the consolidation target is itself exiting, so that the balance ends up moving to the target and then shortly after leaving the active set without going through the exit churn.
To handle this, we just postpone the deposits of exiting validators (consolidating validators are included, since they have a set exit epoch), by skipping the
pending_balance_depositand appending it at the end ofstate.pending_balance_deposits. The deposit is only processed (strictly) after the withdrawable epoch. In the case of a normal exit, even just processing the deposit after the exit epoch would suffice, because the balance would never become active and can only be withdrawn. In the case of a consolidating validator, the consolidation happens on withdrawable epoch, so any balance deposited to the source after that never becomes active and can only be withdrawn.Since the balance of a postponed deposit never become active, we don't require it to go through the churn , so we don't count it towards decreasing
state.deposit_balance_to_consume.Note that such a deposit might be postponed (skipped and appended at the end of the queue) multiple times, until it is processed at the right epoch.