From 0a4e6be223f9ba1472a069930115e74f70c81663 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Thu, 10 Oct 2024 16:31:36 -0700 Subject: [PATCH] Switch to compounding when consolidating with source==target --- .../process_operations.rs | 83 +++++++++++++++---- .../src/per_epoch_processing/single_pass.rs | 3 - consensus/types/src/beacon_state.rs | 8 +- 3 files changed, 69 insertions(+), 25 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index fb1c5c7eee..97a283300a 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -7,7 +7,6 @@ use crate::per_block_processing::errors::{BlockProcessingError, IntoWithIndex}; use crate::VerifySignatures; use types::consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_DENOMINATOR}; use types::typenum::U33; -use types::validator::is_compounding_withdrawal_credential; pub fn process_operations>( state: &mut BeaconState, @@ -452,18 +451,6 @@ pub fn apply_deposit( // [Modified in Electra:EIP7251] if let Ok(pending_balance_deposits) = state.pending_balance_deposits_mut() { pending_balance_deposits.push(PendingBalanceDeposit { index, amount })?; - - let validator = state - .validators() - .get(index as usize) - .ok_or(BeaconStateError::UnknownValidator(index as usize))?; - - if is_compounding_withdrawal_credential(deposit_data.withdrawal_credentials, spec) - && validator.has_eth1_withdrawal_credential(spec) - && is_valid_deposit_signature(&deposit_data, spec).is_ok() - { - state.switch_to_compounding_validator(index as usize, spec)?; - } } else { // Update the existing validator balance. increase_balance(state, index as usize, amount)?; @@ -658,11 +645,72 @@ pub fn process_consolidation_requests( Ok(()) } +fn is_valid_switch_to_compounding_request( + state: &BeaconState, + consolidation_request: &ConsolidationRequest, + spec: &ChainSpec, +) -> Result { + // Switch to compounding requires source and target be equal + if consolidation_request.source_pubkey != consolidation_request.target_pubkey { + return Ok(false); + } + + // Verify pubkey exists + let Some(source_index) = state + .pubkey_cache() + .get(&consolidation_request.source_pubkey) + else { + // source validator doesn't exist + return Ok(false); + }; + + let source_validator = state.get_validator(source_index)?; + // Verify the source withdrawal credentials + if let Some(withdrawal_address) = source_validator.get_execution_withdrawal_address(spec) { + if withdrawal_address != consolidation_request.source_address { + return Ok(false); + } + } else { + // Source doen't have execution withdrawal credentials + return Ok(false); + } + + // Verify the source is active + let current_epoch = state.current_epoch(); + if !source_validator.is_active_at(current_epoch) { + return Ok(false); + } + // Verify exits for source has not been initiated + if source_validator.exit_epoch != spec.far_future_epoch { + return Ok(false); + } + + Ok(true) +} + pub fn process_consolidation_request( state: &mut BeaconState, consolidation_request: &ConsolidationRequest, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { + if is_valid_switch_to_compounding_request(state, consolidation_request, spec)? { + let Some(source_index) = state + .pubkey_cache() + .get(&consolidation_request.source_pubkey) + else { + // source validator doesn't exist. This is unreachable as `is_valid_switch_to_compounding_request` + // will return false in that case. + return Ok(()); + }; + state.switch_to_compounding_validator(source_index, spec)?; + return Ok(()); + } + + // Verify that source != target, so a consolidation cannot be used as an exit. + if consolidation_request.source_pubkey == consolidation_request.target_pubkey { + return Ok(()); + } + // If the pending consolidations queue is full, consolidation requests are ignored if state.pending_consolidations()?.len() == E::PendingConsolidationsLimit::to_usize() { return Ok(()); @@ -686,10 +734,6 @@ pub fn process_consolidation_request( // target validator doesn't exist return Ok(()); }; - // Verify that source != target, so a consolidation cannot be used as an exit. - if source_index == target_index { - return Ok(()); - } let source_validator = state.get_validator(source_index)?; // Verify the source withdrawal credentials @@ -736,5 +780,10 @@ pub fn process_consolidation_request( target_index: target_index as u64, })?; + let target_validator = state.get_validator(target_index)?; + // Churn any target excess active balance of target and raise its max + if target_validator.has_eth1_withdrawal_credential(spec) { + state.switch_to_compounding_validator(target_index, spec)?; + } Ok(()) } diff --git a/consensus/state_processing/src/per_epoch_processing/single_pass.rs b/consensus/state_processing/src/per_epoch_processing/single_pass.rs index 1d37562a95..2bf7f9feb9 100644 --- a/consensus/state_processing/src/per_epoch_processing/single_pass.rs +++ b/consensus/state_processing/src/per_epoch_processing/single_pass.rs @@ -952,9 +952,6 @@ fn process_pending_consolidations( max_effective_balance, ); - // Churn any target excess active balance of target and raise its max. - state.switch_to_compounding_validator(target_index, spec)?; - // Move active balance to target. Excess balance is withdrawable. decrease_balance(state, source_index, source_effective_balance)?; increase_balance(state, target_index, source_effective_balance)?; diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 0119c6a554..2eaf95e02a 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -2166,12 +2166,10 @@ impl BeaconState { .validators_mut() .get_mut(validator_index) .ok_or(Error::UnknownValidator(validator_index))?; - if validator.has_eth1_withdrawal_credential(spec) { - AsMut::<[u8; 32]>::as_mut(&mut validator.withdrawal_credentials)[0] = - spec.compounding_withdrawal_prefix_byte; + AsMut::<[u8; 32]>::as_mut(&mut validator.withdrawal_credentials)[0] = + spec.compounding_withdrawal_prefix_byte; - self.queue_excess_active_balance(validator_index, spec)?; - } + self.queue_excess_active_balance(validator_index, spec)?; Ok(()) }