Implement EIP-7251 #5507
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,32 +8,85 @@ pub fn initiate_validator_exit<T: EthSpec>( | |
| index: usize, | ||
| spec: &ChainSpec, | ||
| ) -> Result<(), Error> { | ||
| // TODO: try to minimize lookups of the validator while satisfying the borrow checker | ||
| // Return if the validator already initiated exit | ||
| let validator = state.get_validator(index)?; | ||
| if validator.exit_epoch != spec.far_future_epoch { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| match &state { | ||
| &BeaconState::Base(_) | ||
| | &BeaconState::Altair(_) | ||
| | &BeaconState::Merge(_) | ||
| | &BeaconState::Capella(_) | ||
| | &BeaconState::Deneb(_) => { | ||
|
There was a problem hiding this comment. Why not do There was a problem hiding this comment. In general I try to make fork-related patterns enumerate the possibilities so as to draw our attention to things that have changed from one fork to the next. When we implement new forks, the compiler will draw your attention to places where things change. Match statements of this kind also turn the code into a kind of living documentation of where things change in which fork. Consider the activation churn limit for example. There was a problem hiding this comment. Before joining LH, I hated this. There's an insane amount of boilerplate that you force everyone to deal with when they may not have to. A new fork developer will know where and when to change code, and it is almost always covered by spec tests. There was a problem hiding this comment. I think we should put in effort to reduce this boiler plate, this is an example external contrib EIP implementation where it's really hard to even find the changes related to the EIP: https://github.com/kevinbogner/lighthouse/pull/3/files . Trying to merge this in post-electra boiler plate would probably be more painful than re-implementing it. From our POV it's a once-per fork pain but it can be a per-POC pain for any LH contributors. Maybe adding a generic and permanent "NextFork" would help here too. Kind of ugly though. There was a problem hiding this comment. leaning on fork order does make sense, we're attempting to embedding fork order into superstructs: sigp/superstruct#33 the places we've been burned on not doing complete matches are usually in networking code, not consensus code. We can evaluate on a case-by-case basis but I think so far it's been easier to default to complete matches only. There was a problem hiding this comment. state transition / networking is a good place to draw the line, since state transition is covered by spec tests. In Lodestar we use fork comparisons and have never had related bugs to forgetting this for forks. |
||
| // Ensure the exit cache is built. | ||
| state.build_exit_cache(spec)?; | ||
|
|
||
| // Compute exit queue epoch | ||
| let delayed_epoch = state.compute_activation_exit_epoch(state.current_epoch(), spec)?; | ||
| let mut exit_queue_epoch = state | ||
| .exit_cache() | ||
| .max_epoch()? | ||
| .map_or(delayed_epoch, |epoch| max(epoch, delayed_epoch)); | ||
| let exit_queue_churn = state.exit_cache().get_churn_at(exit_queue_epoch)?; | ||
|
|
||
| if exit_queue_churn >= state.get_churn_limit(spec)? { | ||
| exit_queue_epoch.safe_add_assign(1)?; | ||
| } | ||
| state | ||
| .exit_cache_mut() | ||
| .record_validator_exit(exit_queue_epoch)?; | ||
|
|
||
| let validator = state.get_validator_mut(index)?; | ||
| validator.exit_epoch = exit_queue_epoch; | ||
| validator.withdrawable_epoch = | ||
| exit_queue_epoch.safe_add(spec.min_validator_withdrawability_delay)?; | ||
| } | ||
| &BeaconState::Electra(_) => { | ||
| // Compute exit queue epoch [Modified in Electra:EIP7251] | ||
| let exit_queue_epoch = | ||
| compute_exit_epoch_and_update_churn(state, validator.effective_balance, spec)?; | ||
| let validator = state.get_validator_mut(index)?; | ||
| // Set validator exit epoch and withdrawable epoch | ||
| validator.exit_epoch = exit_queue_epoch; | ||
| validator.withdrawable_epoch = validator | ||
| .exit_epoch | ||
| .safe_add(spec.min_validator_withdrawability_delay)?; | ||
| // TODO: consider impact on exit cache | ||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| // TODO: should this function be moved to its own file? | ||
| pub fn compute_exit_epoch_and_update_churn<E: EthSpec>( | ||
| state: &mut BeaconState<E>, | ||
| exit_balance: u64, | ||
| spec: &ChainSpec, | ||
| ) -> Result<Epoch, Error> { | ||
| let earliest_exit_epoch = state.compute_activation_exit_epoch(state.current_epoch(), spec)?; | ||
| let per_epoch_churn = state.get_activation_exit_churn_limit(spec)?; | ||
|
|
||
| if state.earliest_exit_epoch()? < earliest_exit_epoch { | ||
| *state.earliest_exit_epoch_mut()? = earliest_exit_epoch; | ||
| *state.exit_balance_to_consume_mut()? = per_epoch_churn; | ||
| } | ||
| if exit_balance <= state.exit_balance_to_consume()? { | ||
| // Exit fits in the current earliest epoch | ||
| state | ||
| .exit_balance_to_consume_mut()? | ||
| .safe_sub_assign(exit_balance)?; | ||
| } else { | ||
| // Exit does not fit in the current earliest epoch | ||
| let balance_to_process = exit_balance.safe_sub(state.exit_balance_to_consume()?)?; | ||
| let additional_epochs = balance_to_process.safe_div(per_epoch_churn)?; | ||
| let remainder = balance_to_process.safe_rem(per_epoch_churn)?; | ||
| *state.earliest_exit_epoch_mut()? = Epoch::new(additional_epochs.safe_add(1)?); | ||
| *state.exit_balance_to_consume_mut()? = per_epoch_churn.safe_sub(remainder)?; | ||
| } | ||
|
|
||
| state.earliest_exit_epoch() | ||
| } | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ pub use verify_exit::verify_exit; | |
| pub mod altair; | ||
| pub mod block_signature_verifier; | ||
| pub mod deneb; | ||
| pub mod electra; | ||
| pub mod errors; | ||
| mod is_valid_indexed_attestation; | ||
| pub mod process_operations; | ||
|
|
@@ -500,12 +501,50 @@ pub fn compute_timestamp_at_slot<T: EthSpec>( | |
| pub fn get_expected_withdrawals<T: EthSpec>( | ||
| state: &BeaconState<T>, | ||
| spec: &ChainSpec, | ||
| ) -> Result<(Withdrawals<T>, usize), BlockProcessingError> { | ||
| let epoch = state.current_epoch(); | ||
| let mut withdrawal_index = state.next_withdrawal_index()?; | ||
| let mut validator_index = state.next_withdrawal_validator_index()?; | ||
| let mut withdrawals = vec![]; | ||
|
|
||
| let partial_withdrawals_count = match state { | ||
| BeaconState::Base(_) | BeaconState::Altair(_) | BeaconState::Merge(_) => { | ||
| return Err(BlockProcessingError::IncorrectStateType); | ||
| } | ||
| BeaconState::Capella(_) | BeaconState::Deneb(_) => 0, | ||
| BeaconState::Electra(_) => { | ||
| for withdrawal in state.pending_partial_withdrawals()? { | ||
| if withdrawal.withdrawable_epoch > epoch | ||
| || withdrawals.len() == spec.max_partial_withdrawals_per_payload as usize | ||
| { | ||
| break; | ||
| } | ||
| let validator = state.get_validator(withdrawal.index as usize)?; | ||
| let balance = *state.balances().get(withdrawal.index as usize).ok_or( | ||
| BeaconStateError::BalancesOutOfBounds(withdrawal.index as usize), | ||
| )?; | ||
| if validator.exit_epoch == spec.far_future_epoch | ||
| && balance > spec.min_activation_balance | ||
| { | ||
| let withdrawable_balance = std::cmp::min( | ||
| balance.safe_sub(spec.min_activation_balance)?, | ||
| withdrawal.amount, | ||
| ); | ||
| withdrawals.push(Withdrawal { | ||
| index: withdrawal_index, | ||
| validator_index: withdrawal.index, | ||
| address: validator | ||
| .get_eth1_withdrawal_address(spec) | ||
| .ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?, | ||
| amount: withdrawable_balance, | ||
| }); | ||
| withdrawal_index.safe_add_assign(1)?; | ||
| } | ||
| } | ||
| withdrawals.len() | ||
| } | ||
| }; | ||
|
|
||
| let bound = std::cmp::min( | ||
| state.validators().len() as u64, | ||
| spec.max_validators_per_withdrawals_sweep, | ||
|
|
@@ -532,7 +571,7 @@ pub fn get_expected_withdrawals<T: EthSpec>( | |
| address: validator | ||
| .get_eth1_withdrawal_address(spec) | ||
| .ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?, | ||
| amount: balance.safe_sub(validator.get_max_effective_balance(spec))?, | ||
| }); | ||
| withdrawal_index.safe_add_assign(1)?; | ||
| } | ||
|
|
@@ -544,7 +583,7 @@ pub fn get_expected_withdrawals<T: EthSpec>( | |
| .safe_rem(state.validators().len() as u64)?; | ||
| } | ||
|
|
||
| Ok((withdrawals.into(), partial_withdrawals_count)) | ||
| } | ||
|
|
||
| /// Apply withdrawals to the state. | ||
|
|
@@ -556,7 +595,8 @@ pub fn process_withdrawals<T: EthSpec, Payload: AbstractExecPayload<T>>( | |
| match state { | ||
| BeaconState::Merge(_) => Ok(()), | ||
| BeaconState::Capella(_) | BeaconState::Deneb(_) | BeaconState::Electra(_) => { | ||
| let (expected_withdrawals, partial_withdrawals_count) = | ||
| get_expected_withdrawals(state, spec)?; | ||
| let expected_root = expected_withdrawals.tree_hash_root(); | ||
| let withdrawals_root = payload.withdrawals_root()?; | ||
|
|
||
|
|
@@ -575,6 +615,14 @@ pub fn process_withdrawals<T: EthSpec, Payload: AbstractExecPayload<T>>( | |
| )?; | ||
| } | ||
|
|
||
| if let Ok(mut pending_partial_withdrawals) = state | ||
| .pending_partial_withdrawals_mut() | ||
| .map(|ppw| Vec::from(std::mem::replace(ppw, Vec::new().into()))) | ||
| { | ||
| pending_partial_withdrawals.drain(0..partial_withdrawals_count); | ||
|
There was a problem hiding this comment. This could be quite expensive with tree states done naively. It's possible to do a specialized operation on milhouse that splits the tree at some index while potentially preserving part of the hash cache @michaelsproul @ethDreamer can you add a |
||
| *state.pending_partial_withdrawals_mut()? = pending_partial_withdrawals.into(); | ||
| } | ||
|
|
||
| // Update the next withdrawal index if this block contained withdrawals | ||
| if let Some(latest_withdrawal) = expected_withdrawals.last() { | ||
| *state.next_withdrawal_index_mut()? = latest_withdrawal.index.safe_add(1)?; | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| use safe_arith::SafeArith; | ||
| use types::BeaconStateError as Error; | ||
| use types::{BeaconState, ChainSpec, Epoch, EthSpec}; | ||
|
|
||
| // Thus function will return an error if not called on a post-electra state | ||
| // | ||
| // TODO: finish commenting | ||
| pub fn compute_consolidation_epoch_and_update_churn<E: EthSpec>( | ||
| state: &mut BeaconState<E>, | ||
| consolidation_balance: u64, | ||
| spec: &ChainSpec, | ||
| ) -> Result<Epoch, Error> { | ||
| let earliest_consolidation_epoch = | ||
| state.compute_activation_exit_epoch(state.current_epoch(), spec)?; | ||
| let per_epoch_consolidation_churn = state.get_consolidation_churn_limit(spec)?; | ||
| // New epoch for consolidations | ||
| if state.earliest_consolidation_epoch()? < earliest_consolidation_epoch { | ||
| *state.earliest_consolidation_epoch_mut()? = earliest_consolidation_epoch; | ||
| *state.consolidation_balance_to_consume_mut()? = per_epoch_consolidation_churn; | ||
| } | ||
|
|
||
| if consolidation_balance <= state.consolidation_balance_to_consume()? { | ||
| // Consolidation fits in the current earliest consolidation epoch | ||
| state | ||
| .consolidation_balance_to_consume_mut()? | ||
| .safe_sub_assign(consolidation_balance)?; | ||
| } else { | ||
| // Consolidation doesn't fit in the current earliest consolidation epoch | ||
| let balance_to_process = | ||
| consolidation_balance.safe_sub(state.consolidation_balance_to_consume()?)?; | ||
| let additional_epochs = balance_to_process.safe_div(per_epoch_consolidation_churn)?; | ||
| let remainder = balance_to_process.safe_rem(per_epoch_consolidation_churn)?; | ||
|
|
||
| state | ||
| .earliest_consolidation_epoch_mut()? | ||
| .safe_add_assign(additional_epochs.safe_add(1)?)?; | ||
| *state.consolidation_balance_to_consume_mut()? = | ||
| per_epoch_consolidation_churn.safe_sub(remainder)?; | ||
| } | ||
|
|
||
| state.earliest_consolidation_epoch() | ||
| } |
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Updated to
PendingPartialWithdrawal