From 276e1845fd9ff467bf0b3dfb473f8c51fd1c5268 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Sun, 13 Nov 2022 18:20:27 -0600 Subject: [PATCH] Added process_withdrawals --- .../src/per_block_processing.rs | 88 ++++++++++++++++++ .../src/per_block_processing/errors.rs | 4 + consensus/types/src/payload.rs | 93 ++++++++++++------- 3 files changed, 150 insertions(+), 35 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index c39f46b95b..2d8a01ff48 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -42,6 +42,7 @@ mod verify_deposit; mod verify_exit; mod verify_proposer_slashing; +use crate::common::decrease_balance; #[cfg(feature = "arbitrary-fuzz")] use arbitrary::Arbitrary; @@ -165,6 +166,8 @@ pub fn per_block_processing>( // previous block. if is_execution_enabled(state, block.body()) { let payload = block.body().execution_payload()?; + #[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] + process_withdrawals::(state, payload, spec)?; process_execution_payload::(state, payload, spec)?; } @@ -455,3 +458,88 @@ pub fn compute_timestamp_at_slot( .safe_mul(spec.seconds_per_slot) .and_then(|since_genesis| state.genesis_time().safe_add(since_genesis)) } + +/// FIXME: add link to this function once the spec is stable +#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] +pub fn get_expected_withdrawals( + state: &BeaconState, + spec: &ChainSpec, +) -> Result, 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![]; + + for _ in 0..state.validators().len() { + let validator = state.get_validator(validator_index as usize)?; + let balance = *state + .balances() + .get(validator_index as usize) + .ok_or_else(|| BeaconStateError::BalancesOutOfBounds(validator_index as usize))?; + if validator.is_fully_withdrawable_at(balance, epoch, spec) { + withdrawals.push(Withdrawal { + index: withdrawal_index, + validator_index, + address: Address::from_slice(&validator.withdrawal_credentials[12..]), + amount: balance, + }); + withdrawal_index.safe_add_assign(1)?; + } else if validator.is_partially_withdrawable_validator(balance, spec) { + withdrawals.push(Withdrawal { + index: withdrawal_index, + validator_index, + address: Address::from_slice(&validator.withdrawal_credentials[12..]), + amount: balance.safe_sub(spec.max_effective_balance)?, + }); + withdrawal_index.safe_add_assign(1)?; + } + if withdrawals.len() == T::max_withdrawals_per_payload() { + break; + } + validator_index = validator_index.safe_add(1)? % state.validators().len() as u64; + } + + Ok(withdrawals.into()) +} + +/// FIXME: add link to this function once the spec is stable +#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] +pub fn process_withdrawals<'payload, T: EthSpec, Payload: AbstractExecPayload>( + state: &mut BeaconState, + payload: Payload::Ref<'payload>, + spec: &ChainSpec, +) -> Result<(), BlockProcessingError> { + match state { + BeaconState::Merge(_) => Ok(()), + BeaconState::Capella(_) | BeaconState::Eip4844(_) => { + let expected_withdrawals = get_expected_withdrawals(state, spec)?; + let withdrawals_root = payload.withdrawals_root()?; + + if expected_withdrawals.tree_hash_root() != payload.withdrawals_root()? { + return Err(BlockProcessingError::WithdrawalsRootMismatch { + expected: expected_withdrawals.tree_hash_root(), + found: payload.withdrawals_root()?, + }); + } + + for withdrawal in expected_withdrawals.iter() { + decrease_balance( + state, + withdrawal.validator_index as usize, + withdrawal.amount, + )?; + } + + if let Some(latest_withdrawal) = expected_withdrawals.last() { + *state.next_withdrawal_index_mut()? = latest_withdrawal.index + 1; + let next_validator_index = + (latest_withdrawal.validator_index + 1) % state.validators().len() as u64; + *state.next_withdrawal_validator_index_mut()? = next_validator_index; + } + + Ok(()) + } + // these shouldn't even be encountered but they're here for completeness + BeaconState::Base(_) | BeaconState::Altair(_) => Ok(()), + } +} diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index 39c740480d..8de0fd337a 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -78,6 +78,10 @@ pub enum BlockProcessingError { }, ExecutionInvalid, ConsensusContext(ContextError), + WithdrawalsRootMismatch { + expected: Hash256, + found: Hash256, + }, BlobVersionHashMismatch, /// The number of commitments in blob transactions in the payload does not match the number /// of commitments in the block. diff --git a/consensus/types/src/payload.rs b/consensus/types/src/payload.rs index ae992002e9..84cc70ed8f 100644 --- a/consensus/types/src/payload.rs +++ b/consensus/types/src/payload.rs @@ -38,7 +38,7 @@ pub trait ExecPayload: Debug + Clone + PartialEq + Hash + TreeHash + fn transactions(&self) -> Option<&Transactions>; /// fork-specific fields #[cfg(feature = "withdrawals")] - fn withdrawals(&self) -> Option, Error>>; + fn withdrawals_root(&self) -> Result; /// Is this a default payload? (pre-merge) fn is_default(&self) -> bool; @@ -229,11 +229,15 @@ impl ExecPayload for FullPayload { } #[cfg(feature = "withdrawals")] - fn withdrawals(&self) -> Option, Error>> { + fn withdrawals_root(&self) -> Result { match self { - FullPayload::Merge(_) => Some(Err(Error::IncorrectStateVariant)), - FullPayload::Capella(ref inner) => Some(Ok(&inner.execution_payload.withdrawals)), - FullPayload::Eip4844(ref inner) => Some(Ok(&inner.execution_payload.withdrawals)), + FullPayload::Merge(_) => Err(Error::IncorrectStateVariant), + FullPayload::Capella(ref inner) => { + Ok(inner.execution_payload.withdrawals.tree_hash_root()) + } + FullPayload::Eip4844(ref inner) => { + Ok(inner.execution_payload.withdrawals.tree_hash_root()) + } } } @@ -322,11 +326,15 @@ impl<'b, T: EthSpec> ExecPayload for FullPayloadRef<'b, T> { } #[cfg(feature = "withdrawals")] - fn withdrawals(&self) -> Option, Error>> { + fn withdrawals_root(&self) -> Result { match self { - FullPayloadRef::Merge(_inner) => Some(Err(Error::IncorrectStateVariant)), - FullPayloadRef::Capella(inner) => Some(Ok(&inner.execution_payload.withdrawals)), - FullPayloadRef::Eip4844(inner) => Some(Ok(&inner.execution_payload.withdrawals)), + FullPayloadRef::Merge(_) => Err(Error::IncorrectStateVariant), + FullPayloadRef::Capella(inner) => { + Ok(inner.execution_payload.withdrawals.tree_hash_root()) + } + FullPayloadRef::Eip4844(inner) => { + Ok(inner.execution_payload.withdrawals.tree_hash_root()) + } } } @@ -485,8 +493,16 @@ impl ExecPayload for BlindedPayload { } #[cfg(feature = "withdrawals")] - fn withdrawals(&self) -> Option, Error>> { - None + fn withdrawals_root(&self) -> Result { + match self { + BlindedPayload::Merge(_) => Err(Error::IncorrectStateVariant), + BlindedPayload::Capella(ref inner) => { + Ok(inner.execution_payload_header.withdrawals_root) + } + BlindedPayload::Eip4844(ref inner) => { + Ok(inner.execution_payload_header.withdrawals_root) + } + } } fn is_default<'a>(&'a self) -> bool { @@ -563,8 +579,16 @@ impl<'b, T: EthSpec> ExecPayload for BlindedPayloadRef<'b, T> { } #[cfg(feature = "withdrawals")] - fn withdrawals<'a>(&'a self) -> Option, Error>> { - None + fn withdrawals_root(&self) -> Result { + match self { + BlindedPayloadRef::Merge(_) => Err(Error::IncorrectStateVariant), + BlindedPayloadRef::Capella(inner) => { + Ok(inner.execution_payload_header.withdrawals_root) + } + BlindedPayloadRef::Eip4844(inner) => { + Ok(inner.execution_payload_header.withdrawals_root) + } + } } // TODO: can this function be optimized? @@ -627,7 +651,7 @@ macro_rules! impl_exec_payload_common { } #[cfg(feature = "withdrawals")] - fn withdrawals(&self) -> Option, Error>> { + fn withdrawals_root(&self) -> Result { let g = $g; g(self) } @@ -642,7 +666,7 @@ macro_rules! impl_exec_payload_common { } macro_rules! impl_exec_payload_for_fork { - ($wrapper_type_header:ident, $wrapper_type_full:ident, $wrapped_type_header:ident, $wrapped_type_full:ident, $fork_variant:ident, $withdrawal_fn:block) => { + ($wrapper_type_header:ident, $wrapper_type_full:ident, $wrapped_type_header:ident, $wrapped_type_full:ident, $fork_variant:ident) => { //*************** Blinded payload implementations ******************// impl_exec_payload_common!( @@ -653,7 +677,14 @@ macro_rules! impl_exec_payload_for_fork { $fork_variant, Blinded, { |_| { None } }, - { |_| { None } } + { + let c: for<'a> fn(&'a $wrapper_type_header) -> Result = + |payload: &$wrapper_type_header| { + let wrapper_ref_type = BlindedPayloadRef::$fork_variant(&payload); + wrapper_ref_type.withdrawals_root() + }; + c + } ); impl TryInto<$wrapper_type_header> for BlindedPayload { @@ -719,7 +750,14 @@ macro_rules! impl_exec_payload_for_fork { |payload: &$wrapper_type_full| Some(&payload.execution_payload.transactions); c }, - $withdrawal_fn + { + let c: for<'a> fn(&'a $wrapper_type_full) -> Result = + |payload: &$wrapper_type_full| { + let wrapper_ref_type = FullPayloadRef::$fork_variant(&payload); + wrapper_ref_type.withdrawals_root() + }; + c + } ); impl Default for $wrapper_type_full { @@ -762,36 +800,21 @@ impl_exec_payload_for_fork!( FullPayloadMerge, ExecutionPayloadHeaderMerge, ExecutionPayloadMerge, - Merge, - { - let c: for<'a> fn(&'a FullPayloadMerge) -> Option, Error>> = - |_| Some(Err(Error::IncorrectStateVariant)); - c - } + Merge ); impl_exec_payload_for_fork!( BlindedPayloadCapella, FullPayloadCapella, ExecutionPayloadHeaderCapella, ExecutionPayloadCapella, - Capella, - { - let c: for<'a> fn(&'a FullPayloadCapella) -> Option, Error>> = - |payload: &FullPayloadCapella| Some(Ok(&payload.execution_payload.withdrawals)); - c - } + Capella ); impl_exec_payload_for_fork!( BlindedPayloadEip4844, FullPayloadEip4844, ExecutionPayloadHeaderEip4844, ExecutionPayloadEip4844, - Eip4844, - { - let c: for<'a> fn(&'a FullPayloadEip4844) -> Option, Error>> = - |payload: &FullPayloadEip4844| Some(Ok(&payload.execution_payload.withdrawals)); - c - } + Eip4844 ); impl AbstractExecPayload for BlindedPayload {