diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 539f750061..6002dc6524 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5726,45 +5726,7 @@ impl BeaconChain { execution_payload_value, ) } - // Below is my attempt at handling the Gloas variant - // Note that Mark's implementation had this as: - // BeaconState::EIP7732(_) => todo!("EIP-7732 block production"), - BeaconState::Gloas(_) => { - // Gloas blocks contain execution bids, not execution payloads - let block_proposal_contents = - block_contents.ok_or(BlockProductionError::MissingExecutionBid)?; - let (signed_execution_bid, payload_attestations) = block_proposal_contents - .into_execution_bid() - .map_err(|_| BlockProductionError::InvalidPayloadFork)?; - - ( - BeaconBlock::Gloas(BeaconBlockGloas { - slot, - proposer_index, - parent_root, - state_root: Hash256::zero(), - body: BeaconBlockBodyGloas { - randao_reveal, - eth1_data, - graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings_electra.into(), - attestations: attestations_electra.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), - sync_aggregate: sync_aggregate - .ok_or(BlockProductionError::MissingSyncAggregate)?, - bls_to_execution_changes: bls_to_execution_changes.into(), - // Gloas: Use actual execution bid data - signed_execution_bid: signed_execution_bid.clone(), - payload_attestations, - _phantom: PhantomData, - }, - }), - None, // blob commitments moved to `ExecutionPayloadEnvelope` - Uint256::ZERO, // No execution payload value for Gloas blocks, just bids value - ) - } + BeaconState::Gloas(_) => todo!("Gloas block production"), }; let block = SignedBeaconBlock::from_block( diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 28b190a6e7..e67da468d2 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -51,7 +51,7 @@ use types::non_zero_usize::new_non_zero_usize; use types::payload::BlockProductionVersion; use types::{ AbstractExecPayload, BlobsList, ExecutionPayloadDeneb, ExecutionRequests, KzgProofs, - PayloadAttestation, SignedBlindedBeaconBlock, SignedExecutionBid, + SignedBlindedBeaconBlock, }; use types::{ BeaconStateError, BlindedPayload, ChainSpec, Epoch, ExecPayload, ExecutionPayloadBellatrix, @@ -212,11 +212,6 @@ pub enum BlockProposalContents> { // See: https://github.com/sigp/lighthouse/issues/6981 requests: Option>, }, - /// Gloas: Execution bid and payload attestations - BidAndPayloadAttestations { - signed_execution_bid: SignedExecutionBid, - payload_attestations: VariableList, E::MaxPayloadAttestations>, - }, } impl From>> @@ -244,13 +239,6 @@ impl From>> blobs_and_proofs: None, requests, }, - BlockProposalContents::BidAndPayloadAttestations { - signed_execution_bid, - payload_attestations, - } => BlockProposalContents::BidAndPayloadAttestations { - signed_execution_bid, - payload_attestations, - }, } } } @@ -318,28 +306,6 @@ impl> BlockProposalContents { - panic!("Cannot deconstruct BidAndPayloadAttestations variant into execution payload components") - } - } - } - - /// Extract execution bid data for EIP-7732 Gloas blocks - pub fn into_execution_bid( - self, - ) -> Result< - ( - SignedExecutionBid, - VariableList, E::MaxPayloadAttestations>, - ), - &'static str, - > { - match self { - Self::BidAndPayloadAttestations { - signed_execution_bid, - payload_attestations, - } => Ok((signed_execution_bid, payload_attestations)), - _ => Err("Cannot extract execution bid from non-BidAndPayloadAttestations variant"), } } @@ -347,27 +313,18 @@ impl> BlockProposalContents payload, Self::PayloadAndBlobs { payload, .. } => payload, - Self::BidAndPayloadAttestations { .. } => { - panic!("BidAndPayloadAttestations variant does not contain execution payload") - } } } pub fn to_payload(self) -> Payload { match self { Self::Payload { payload, .. } => payload, Self::PayloadAndBlobs { payload, .. } => payload, - Self::BidAndPayloadAttestations { .. } => { - panic!("BidAndPayloadAttestations variant does not contain execution payload") - } } } pub fn block_value(&self) -> &Uint256 { match self { Self::Payload { block_value, .. } => block_value, Self::PayloadAndBlobs { block_value, .. } => block_value, - Self::BidAndPayloadAttestations { .. } => { - panic!("BidAndPayloadAttestations variant does not have block_value") - } } } } diff --git a/beacon_node/store/src/partial_beacon_state.rs b/beacon_node/store/src/partial_beacon_state.rs index fe5fd2b039..2d8226bf4c 100644 --- a/beacon_node/store/src/partial_beacon_state.rs +++ b/beacon_node/store/src/partial_beacon_state.rs @@ -153,7 +153,6 @@ where #[superstruct(only(Fulu, Gloas))] pub proposer_lookahead: Vector, - // Gloas // Gloas #[superstruct(only(Gloas))] pub execution_payload_availability: BitVector, diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index e8473fa6a0..5335c917cb 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -30,7 +30,6 @@ pub mod deneb; pub mod errors; mod is_valid_indexed_attestation; pub mod process_operations; -pub mod process_withdrawals; pub mod signature_sets; pub mod tests; mod verify_attestation; @@ -40,6 +39,7 @@ mod verify_deposit; mod verify_exit; mod verify_proposer_slashing; +use crate::common::decrease_balance; use crate::common::update_progressive_balances_cache::{ initialize_progressive_balances_cache, update_progressive_balances_metrics, }; @@ -171,16 +171,9 @@ pub fn per_block_processing>( // previous block. if is_execution_enabled(state, block.body()) { let body = block.body(); - if state.fork_name_unchecked().gloas_enabled() { - process_withdrawals::gloas::process_withdrawals::(state, spec)?; - } else { - process_withdrawals::capella::process_withdrawals::( - state, - body.execution_payload()?, - spec, - )?; - process_execution_payload::(state, body, spec)?; - } + // TODO(EIP-7732): build out process_withdrawals variant for gloas + process_withdrawals::(state, body.execution_payload()?, spec)?; + process_execution_payload::(state, body, spec)?; } // TODO(EIP-7732): build out process_execution_bid @@ -635,3 +628,69 @@ pub fn get_expected_withdrawals( Ok((withdrawals.into(), processed_partial_withdrawals_count)) } + +/// Apply withdrawals to the state. +/// TODO(EIP-7732): abstract this out and create gloas variant +pub fn process_withdrawals>( + state: &mut BeaconState, + payload: Payload::Ref<'_>, + spec: &ChainSpec, +) -> Result<(), BlockProcessingError> { + if state.fork_name_unchecked().capella_enabled() { + let (expected_withdrawals, processed_partial_withdrawals_count) = + get_expected_withdrawals(state, spec)?; + let expected_root = expected_withdrawals.tree_hash_root(); + let withdrawals_root = payload.withdrawals_root()?; + + if expected_root != withdrawals_root { + return Err(BlockProcessingError::WithdrawalsRootMismatch { + expected: expected_root, + found: withdrawals_root, + }); + } + + for withdrawal in expected_withdrawals.iter() { + decrease_balance( + state, + withdrawal.validator_index as usize, + withdrawal.amount, + )?; + } + + // Update pending partial withdrawals [New in Electra:EIP7251] + if let Some(processed_partial_withdrawals_count) = processed_partial_withdrawals_count { + state + .pending_partial_withdrawals_mut()? + .pop_front(processed_partial_withdrawals_count)?; + } + + // 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)?; + + // Update the next validator index to start the next withdrawal sweep + if expected_withdrawals.len() == E::max_withdrawals_per_payload() { + // Next sweep starts after the latest withdrawal's validator index + let next_validator_index = latest_withdrawal + .validator_index + .safe_add(1)? + .safe_rem(state.validators().len() as u64)?; + *state.next_withdrawal_validator_index_mut()? = next_validator_index; + } + } + + // Advance sweep by the max length of the sweep if there was not a full set of withdrawals + if expected_withdrawals.len() != E::max_withdrawals_per_payload() { + let next_validator_index = state + .next_withdrawal_validator_index()? + .safe_add(spec.max_validators_per_withdrawals_sweep)? + .safe_rem(state.validators().len() as u64)?; + *state.next_withdrawal_validator_index_mut()? = next_validator_index; + } + + Ok(()) + } else { + // these shouldn't even be encountered but they're here for completeness + Ok(()) + } +} diff --git a/consensus/state_processing/src/per_block_processing/process_withdrawals.rs b/consensus/state_processing/src/per_block_processing/process_withdrawals.rs deleted file mode 100644 index 562fcb0385..0000000000 --- a/consensus/state_processing/src/per_block_processing/process_withdrawals.rs +++ /dev/null @@ -1,105 +0,0 @@ -use super::errors::BlockProcessingError; -use super::get_expected_withdrawals; -use crate::common::decrease_balance; -use safe_arith::SafeArith; -use tree_hash::TreeHash; -use types::{AbstractExecPayload, BeaconState, ChainSpec, EthSpec, ExecPayload, Withdrawals}; - -fn process_withdrawals_common( - state: &mut BeaconState, - expected_withdrawals: Withdrawals, - partial_withdrawals_count: Option, - spec: &ChainSpec, -) -> Result<(), BlockProcessingError> { - match state { - BeaconState::Capella(_) - | BeaconState::Deneb(_) - | BeaconState::Electra(_) - | BeaconState::Fulu(_) - | BeaconState::Gloas(_) => { - for withdrawal in expected_withdrawals.iter() { - decrease_balance( - state, - withdrawal.validator_index as usize, - withdrawal.amount, - )?; - } - - // Update pending partial withdrawals [New in Electra:EIP7251] - if let Some(partial_withdrawals_count) = partial_withdrawals_count { - state - .pending_partial_withdrawals_mut()? - .pop_front(partial_withdrawals_count)?; - } - - // 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)?; - - // Update the next validator index to start the next withdrawal sweep - if expected_withdrawals.len() == E::max_withdrawals_per_payload() { - // Next sweep starts after the latest withdrawal's validator index - let next_validator_index = latest_withdrawal - .validator_index - .safe_add(1)? - .safe_rem(state.validators().len() as u64)?; - *state.next_withdrawal_validator_index_mut()? = next_validator_index; - } - } - - // Advance sweep by the max length of the sweep if there was not a full set of withdrawals - if expected_withdrawals.len() != E::max_withdrawals_per_payload() { - let next_validator_index = state - .next_withdrawal_validator_index()? - .safe_add(spec.max_validators_per_withdrawals_sweep)? - .safe_rem(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(_) | BeaconState::Bellatrix(_) => Ok(()), - } -} - -pub mod capella { - use super::*; - /// Apply withdrawals to the state. - pub fn process_withdrawals>( - state: &mut BeaconState, - payload: Payload::Ref<'_>, - spec: &ChainSpec, - ) -> Result<(), BlockProcessingError> { - 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()?; - if expected_root != withdrawals_root { - return Err(BlockProcessingError::WithdrawalsRootMismatch { - expected: expected_root, - found: withdrawals_root, - }); - } - - process_withdrawals_common(state, expected_withdrawals, partial_withdrawals_count, spec) - } -} - -pub mod gloas { - use super::*; - /// Apply withdrawals to the state. - pub fn process_withdrawals( - state: &mut BeaconState, - spec: &ChainSpec, - ) -> Result<(), BlockProcessingError> { - if !state.is_parent_block_full() { - return Ok(()); - } - - let (expected_withdrawals, partial_withdrawals_count) = - get_expected_withdrawals(state, spec)?; - process_withdrawals_common(state, expected_withdrawals, partial_withdrawals_count, spec) - } -} diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 981d5750f3..279f2eff18 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -2004,18 +2004,6 @@ impl BeaconState { } } - /// Get the PTC - /// Requires the committee cache to be initialized. - /// TODO(EIP-7732): definitely gonna have to cache this.. - /// TODO(EIP-7732): check this implementation against the latest spec - // https://ethereum.github.io/consensus-specs/specs/_features/eip7732/beacon-chain/#new-get_ptc - pub fn get_ptc(&self, slot: Slot) -> Result, Error> { - let committee_cache = self.committee_cache_at_slot(slot)?; - let committees = committee_cache.get_beacon_committees_at_slot(slot)?; - - PTC::from_committees(&committees) - } - /// Build all caches (except the tree hash cache), if they need to be built. #[instrument(skip_all, level = "debug")] pub fn build_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> { diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 68651eb869..e4cb84e291 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -69,7 +69,6 @@ pub mod pending_deposit; pub mod pending_partial_withdrawal; pub mod proposer_preparation_data; pub mod proposer_slashing; -pub mod ptc; pub mod relative_epoch; pub mod selection_proof; pub mod shuffling_id; @@ -250,7 +249,6 @@ pub use crate::preset::{ }; pub use crate::proposer_preparation_data::ProposerPreparationData; pub use crate::proposer_slashing::ProposerSlashing; -pub use crate::ptc::PTC; pub use crate::relative_epoch::{Error as RelativeEpochError, RelativeEpoch}; pub use crate::runtime_fixed_vector::RuntimeFixedVector; pub use crate::runtime_var_list::RuntimeVariableList; diff --git a/consensus/types/src/ptc.rs b/consensus/types/src/ptc.rs deleted file mode 100644 index 26cc1c2ca1..0000000000 --- a/consensus/types/src/ptc.rs +++ /dev/null @@ -1,57 +0,0 @@ -use crate::*; -use safe_arith::SafeArith; - -/// TODO(EIP-7732): is it easier to return u64 or usize? -#[derive(Clone, Debug, PartialEq)] -pub struct PTC(FixedVector); - -// TODO(EIP-7732): check this implementation against the latest spec -// https://ethereum.github.io/consensus-specs/specs/_features/eip7732/beacon-chain/#new-get_ptc -impl PTC { - pub fn from_committees(committees: &[BeaconCommittee]) -> Result { - // this function is only used here and - // I have no idea where else to put it - fn bit_floor(n: u64) -> u64 { - if n == 0 { - 0 - } else { - 1 << (n.leading_zeros() as u64 ^ 63) - } - } - - let committee_count_per_slot = committees.len() as u64; - let committees_per_slot = bit_floor(std::cmp::min( - committee_count_per_slot, - E::PTCSize::to_u64(), - )) as usize; - let members_per_committee = E::PTCSize::to_usize().safe_div(committees_per_slot)?; - - let mut ptc = Vec::with_capacity(E::PTCSize::to_usize()); - for idx in 0..committees_per_slot { - let beacon_committee = committees - .get(idx as usize) - .ok_or_else(|| Error::InvalidCommitteeIndex(idx as u64))?; - ptc.extend_from_slice(&beacon_committee.committee[..members_per_committee]); - } - - Ok(Self(FixedVector::from(ptc))) - } -} - -impl<'a, E: EthSpec> IntoIterator for &'a PTC { - type Item = &'a usize; - type IntoIter = std::slice::Iter<'a, usize>; - - fn into_iter(self) -> Self::IntoIter { - self.0.iter() - } -} - -impl IntoIterator for PTC { - type Item = usize; - type IntoIter = std::vec::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} diff --git a/consensus/types/src/signed_execution_payload_envelope.rs b/consensus/types/src/signed_execution_payload_envelope.rs index 6cab667d14..bc2c4e994f 100644 --- a/consensus/types/src/signed_execution_payload_envelope.rs +++ b/consensus/types/src/signed_execution_payload_envelope.rs @@ -54,37 +54,6 @@ impl SignedExecutionPayloadEnvelope { Self::NextFork(ref signed) => ExecutionPayloadEnvelopeRef::NextFork(&signed.message), } } - - /// Verify `self.signature`. - /// - /// The `parent_state` is the post-state of the beacon block with - /// block_root = self.message.beacon_block_root - pub fn verify_signature( - &self, - parent_state: &BeaconState, - genesis_validators_root: Hash256, - spec: &ChainSpec, - ) -> Result { - let domain = spec.get_domain( - parent_state.current_epoch(), - Domain::BeaconBuilder, - &parent_state.fork(), - genesis_validators_root, - ); - let pubkey = parent_state - .validators() - .get(self.message().builder_index() as usize) - .and_then(|v| { - let pk: Option = v.pubkey.decompress().ok(); - pk - }) - .ok_or_else(|| { - BeaconStateError::UnknownValidator(self.message().builder_index() as usize) - })?; - let message = self.message().signing_root(domain); - - Ok(self.signature().verify(&pubkey, message)) - } } impl<'de, E: EthSpec> ContextDeserialize<'de, ForkName> for SignedExecutionPayloadEnvelope { diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index 63b46945c2..a53bce927c 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -419,15 +419,8 @@ impl Operation for WithdrawalsPayload { spec: &ChainSpec, _: &Operations, ) -> Result<(), BlockProcessingError> { - if state.fork_name_unchecked().gloas_enabled() { - process_withdrawals::gloas::process_withdrawals(state, spec) - } else { - process_withdrawals::capella::process_withdrawals::<_, FullPayload<_>>( - state, - self.payload.to_ref(), - spec, - ) - } + // TODO(EIP-7732): implement separate gloas and non-gloas variants of process_withdrawals + process_withdrawals::<_, FullPayload<_>>(state, self.payload.to_ref(), spec) } }