diff --git a/beacon_node/operation_pool/src/bls_to_execution_changes.rs b/beacon_node/operation_pool/src/bls_to_execution_changes.rs index cc8809c43e..c381780805 100644 --- a/beacon_node/operation_pool/src/bls_to_execution_changes.rs +++ b/beacon_node/operation_pool/src/bls_to_execution_changes.rs @@ -113,16 +113,18 @@ impl BlsToExecutionChanges { .validators() .get(validator_index as usize) .is_none_or(|validator| { - let prune = validator.has_execution_withdrawal_credential(spec) - && head_block - .message() - .body() - .bls_to_execution_changes() - .map_or(true, |recent_changes| { - !recent_changes - .iter() - .any(|c| c.message.validator_index == validator_index) - }); + let prune = validator.has_execution_withdrawal_credential( + spec, + head_state.fork_name_unchecked(), + ) && head_block + .message() + .body() + .bls_to_execution_changes() + .map_or(true, |recent_changes| { + !recent_changes + .iter() + .any(|c| c.message.validator_index == validator_index) + }); if prune { validator_indices_pruned.push(validator_index); } diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 24e2cfbbb5..cdcb66f91a 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -582,7 +582,12 @@ impl OperationPool { address_change.signature_is_still_valid(&state.fork()) && state .get_validator(address_change.as_inner().message.validator_index as usize) - .is_ok_and(|validator| !validator.has_execution_withdrawal_credential(spec)) + .is_ok_and(|validator| { + !validator.has_execution_withdrawal_credential( + spec, + state.fork_name_unchecked(), + ) + }) }, |address_change| address_change.as_inner().clone(), E::MaxBlsToExecutionChanges::to_usize(), diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 7b16dbe804..189e055d71 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -1,8 +1,12 @@ +use self::errors::ExecutionPayloadBidInvalid; use crate::consensus_context::ConsensusContext; use errors::{BlockOperationError, BlockProcessingError, HeaderInvalid}; use rayon::prelude::*; use safe_arith::{ArithError, SafeArith, SafeArithIter}; -use signature_sets::{block_proposal_signature_set, get_pubkey_from_state, randao_signature_set}; +use signature_sets::{ + block_proposal_signature_set, execution_payload_bid_signature_set, get_pubkey_from_state, + randao_signature_set, +}; use std::borrow::Cow; use tree_hash::TreeHash; use types::*; @@ -173,9 +177,7 @@ pub fn per_block_processing>( let body = block.body(); if state.fork_name_unchecked().gloas_enabled() { process_withdrawals::gloas::process_withdrawals::(state, spec)?; - - // TODO(EIP-7732): build out process_execution_bid - // process_execution_bid(state, block, verify_signatures, spec)?; + process_execution_payload_bid(state, block, verify_signatures, spec)?; } else { process_withdrawals::capella::process_withdrawals::( state, @@ -625,7 +627,7 @@ pub fn get_expected_withdrawals( index: withdrawal_index, validator_index: withdrawal.validator_index, address: validator - .get_execution_withdrawal_address(spec) + .get_execution_withdrawal_address(spec, state.fork_name_unchecked()) .ok_or(BeaconStateError::NonExecutionAddressWithdrawalCredential)?, amount: withdrawable_balance, }); @@ -662,7 +664,7 @@ pub fn get_expected_withdrawals( index: withdrawal_index, validator_index, address: validator - .get_execution_withdrawal_address(spec) + .get_execution_withdrawal_address(spec, state.fork_name_unchecked()) .ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?, amount: balance, }); @@ -672,7 +674,7 @@ pub fn get_expected_withdrawals( index: withdrawal_index, validator_index, address: validator - .get_execution_withdrawal_address(spec) + .get_execution_withdrawal_address(spec, state.fork_name_unchecked()) .ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?, amount: balance.safe_sub(validator.get_max_effective_balance(spec, fork_name))?, }); @@ -692,3 +694,159 @@ pub fn get_expected_withdrawals( processed_partial_withdrawals_count, )) } + +pub fn process_execution_payload_bid>( + state: &mut BeaconState, + block: BeaconBlockRef<'_, E, Payload>, + verify_signatures: VerifySignatures, + spec: &ChainSpec, +) -> Result<(), BlockProcessingError> { + // Verify the bid signature + let signed_bid = block.body().signed_execution_payload_bid()?; + + let bid = &signed_bid.message; + let amount = bid.value; + let builder_index = bid.builder_index; + let builder = state.get_validator(builder_index as usize)?; + + // For self-builds, amount must be zero regardless of withdrawal credential prefix + if builder_index == block.proposer_index() { + block_verify!(amount == 0, ExecutionPayloadBidInvalid::BadAmount.into()); + // TODO(EIP-7732): check with team if we should use ExecutionPayloadBidInvalid::BadSignature or a new error variant for this, like BadSelfBuildSignature + block_verify!( + signed_bid.signature.is_infinity(), + ExecutionPayloadBidInvalid::BadSignature.into() + ); + } else { + // Non-self builds require builder withdrawal credential + block_verify!( + builder.has_builder_withdrawal_credential(spec), + ExecutionPayloadBidInvalid::BadWithdrawalCredentials.into() + ); + if verify_signatures.is_true() { + block_verify!( + execution_payload_bid_signature_set( + state, + |i| get_pubkey_from_state(state, i), + signed_bid, + spec + )? + .verify(), + ExecutionPayloadBidInvalid::BadSignature.into() + ); + } + } + + // Verify builder is active and not slashed + block_verify!( + builder.is_active_at(state.current_epoch()), + ExecutionPayloadBidInvalid::BuilderNotActive(builder_index).into() + ); + block_verify!( + !builder.slashed, + ExecutionPayloadBidInvalid::BuilderSlashed(builder_index).into() + ); + + // Only perform payment related checks if amount > 0 + if amount > 0 { + // Check that the builder has funds to cover the bid + let pending_payments = state + .builder_pending_payments()? + .iter() + .filter_map(|payment| { + if payment.withdrawal.builder_index == builder_index { + Some(payment.withdrawal.amount) + } else { + None + } + }) + .safe_sum()?; + + let pending_withdrawals = state + .builder_pending_withdrawals()? + .iter() + .filter_map(|withdrawal| { + if withdrawal.builder_index == builder_index { + Some(withdrawal.amount) + } else { + None + } + }) + .safe_sum()?; + + let builder_balance = state.get_balance(builder_index as usize)?; + + block_verify!( + builder_balance + >= amount + .safe_add(pending_payments)? + .safe_add(pending_withdrawals)? + .safe_add(spec.min_activation_balance)?, + ExecutionPayloadBidInvalid::InsufficientBalance { + builder_index, + builder_balance, + bid_value: amount, + } + .into() + ); + } + + // Verify that the bid is for the current slot + block_verify!( + bid.slot == block.slot(), + ExecutionPayloadBidInvalid::SlotMismatch { + state_slot: block.slot(), + bid_slot: bid.slot, + } + .into() + ); + + // Verify that the bid is for the right parent block + let latest_block_hash = state.latest_block_hash()?; + block_verify!( + bid.parent_block_hash == *latest_block_hash, + ExecutionPayloadBidInvalid::ParentBlockHashMismatch { + state_block_hash: *latest_block_hash, + bid_parent_hash: bid.parent_block_hash, + } + .into() + ); + + block_verify!( + bid.parent_block_root == block.parent_root(), + ExecutionPayloadBidInvalid::ParentBlockRootMismatch { + block_parent_root: block.parent_root(), + bid_parent_root: bid.parent_block_root, + } + .into() + ); + + // Record the pending payment if there is some payment + if amount > 0 { + let pending_payment = BuilderPendingPayment { + weight: 0, + withdrawal: BuilderPendingWithdrawal { + fee_recipient: bid.fee_recipient, + amount, + builder_index, + withdrawable_epoch: spec.far_future_epoch, + }, + }; + + let payment_index = (E::slots_per_epoch() + .safe_add(bid.slot.as_u64().safe_rem(E::slots_per_epoch())?)?) + as usize; + + *state + .builder_pending_payments_mut()? + .get_mut(payment_index) + .ok_or(BlockProcessingError::BeaconStateError( + BeaconStateError::BuilderPendingPaymentsIndexNotSupported(payment_index), + ))? = pending_payment; + } + + // Cache the execution bid + *state.latest_execution_payload_bid_mut()? = bid.clone(); + + Ok(()) +} diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index ff7c0204e2..0374547a20 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -91,6 +91,9 @@ pub enum BlockProcessingError { }, WithdrawalCredentialsInvalid, PendingAttestationInElectra, + ExecutionPayloadBidInvalid { + reason: ExecutionPayloadBidInvalid, + }, } impl From for BlockProcessingError { @@ -147,6 +150,12 @@ impl From for BlockProcessingError { } } +impl From for BlockProcessingError { + fn from(reason: ExecutionPayloadBidInvalid) -> Self { + Self::ExecutionPayloadBidInvalid { reason } + } +} + impl From> for BlockProcessingError { fn from(e: BlockOperationError) -> BlockProcessingError { match e { @@ -440,6 +449,38 @@ pub enum ExitInvalid { PendingWithdrawalInQueue(u64), } +#[derive(Debug, PartialEq, Clone)] +pub enum ExecutionPayloadBidInvalid { + /// The builder sent a 0 amount + BadAmount, + /// The signature is invalid. + BadSignature, + /// The builder's withdrawal credential is invalid + BadWithdrawalCredentials, + /// The builder is not an active validator. + BuilderNotActive(u64), + /// The builder is slashed + BuilderSlashed(u64), + /// The builder has insufficient balance to cover the bid + InsufficientBalance { + builder_index: u64, + builder_balance: u64, + bid_value: u64, + }, + /// Bid slot doesn't match state slot + SlotMismatch { state_slot: Slot, bid_slot: Slot }, + /// The bid's parent block hash doesn't match the state's latest block hash + ParentBlockHashMismatch { + state_block_hash: ExecutionBlockHash, + bid_parent_hash: ExecutionBlockHash, + }, + /// The bid's parent block root doesn't match the block's parent root + ParentBlockRootMismatch { + block_parent_root: Hash256, + bid_parent_root: Hash256, + }, +} + #[derive(Debug, PartialEq, Clone)] pub enum BlsExecutionChangeInvalid { /// The specified validator is not in the state's validator registry. 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 9a1c6c2f6a..9f5b916743 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -513,9 +513,10 @@ pub fn process_withdrawal_requests( let validator = state.get_validator(validator_index)?; // Verify withdrawal credentials - let has_correct_credential = validator.has_execution_withdrawal_credential(spec); + let has_correct_credential = + validator.has_execution_withdrawal_credential(spec, state.fork_name_unchecked()); let is_correct_source_address = validator - .get_execution_withdrawal_address(spec) + .get_execution_withdrawal_address(spec, state.fork_name_unchecked()) .map(|addr| addr == request.source_address) .unwrap_or(false); @@ -560,7 +561,7 @@ pub fn process_withdrawal_requests( .safe_add(pending_balance_to_withdraw)?; // Only allow partial withdrawals with compounding withdrawal credentials - if validator.has_compounding_withdrawal_credential(spec) + if validator.has_compounding_withdrawal_credential(spec, state.fork_name_unchecked()) && has_sufficient_effective_balance && has_excess_balance { @@ -729,7 +730,9 @@ pub fn process_consolidation_request( 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 let Some(withdrawal_address) = + source_validator.get_execution_withdrawal_address(spec, state.fork_name_unchecked()) + { if withdrawal_address != consolidation_request.source_address { return Ok(()); } @@ -740,7 +743,7 @@ pub fn process_consolidation_request( let target_validator = state.get_validator(target_index)?; // Verify the target has compounding withdrawal credentials - if !target_validator.has_compounding_withdrawal_credential(spec) { + if !target_validator.has_compounding_withdrawal_credential(spec, state.fork_name_unchecked()) { return Ok(()); } diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index 1dc53ce1a8..c18cbbe4b5 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -11,9 +11,9 @@ use types::{ BeaconStateError, ChainSpec, DepositData, Domain, Epoch, EthSpec, Fork, Hash256, InconsistentFork, IndexedAttestation, IndexedAttestationRef, ProposerSlashing, PublicKey, PublicKeyBytes, Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedBeaconBlockHeader, - SignedBlsToExecutionChange, SignedContributionAndProof, SignedExecutionPayloadEnvelope, - SignedRoot, SignedVoluntaryExit, SigningData, Slot, SyncAggregate, SyncAggregatorSelectionData, - Unsigned, + SignedBlsToExecutionChange, SignedContributionAndProof, SignedExecutionPayloadBid, + SignedExecutionPayloadEnvelope, SignedRoot, SignedVoluntaryExit, SigningData, Slot, + SyncAggregate, SyncAggregatorSelectionData, Unsigned, }; pub type Result = std::result::Result; @@ -360,6 +360,34 @@ where )) } +pub fn execution_payload_bid_signature_set<'a, E, F>( + state: &'a BeaconState, + get_pubkey: F, + signed_execution_payload_bid: &'a SignedExecutionPayloadBid, + spec: &'a ChainSpec, +) -> Result> +where + E: EthSpec, + F: Fn(usize) -> Option>, +{ + let domain = spec.get_domain( + state.current_epoch(), + Domain::BeaconBuilder, + &state.fork(), + state.genesis_validators_root(), + ); + let execution_payload_bid = &signed_execution_payload_bid.message; + let pubkey = get_pubkey(execution_payload_bid.builder_index as usize) + .ok_or(Error::ValidatorUnknown(execution_payload_bid.builder_index))?; + let message = execution_payload_bid.signing_root(domain); + + Ok(SignatureSet::single_pubkey( + &signed_execution_payload_bid.signature, + pubkey, + message, + )) +} + /// Returns the signature set for the given `attester_slashing` and corresponding `pubkeys`. pub fn attester_slashing_signature_sets<'a, E, F>( state: &'a BeaconState, diff --git a/consensus/state_processing/src/upgrade/electra.rs b/consensus/state_processing/src/upgrade/electra.rs index 258b28a45b..a84b81d85c 100644 --- a/consensus/state_processing/src/upgrade/electra.rs +++ b/consensus/state_processing/src/upgrade/electra.rs @@ -82,7 +82,7 @@ pub fn upgrade_to_electra( // Ensure early adopters of compounding credentials go through the activation churn let validators = post.validators().clone(); for (index, validator) in validators.iter().enumerate() { - if validator.has_compounding_withdrawal_credential(spec) { + if validator.has_compounding_withdrawal_credential(spec, post.fork_name_unchecked()) { post.queue_excess_active_balance(index, spec)?; } } diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 4aaf899844..af31f5f1c5 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -161,6 +161,7 @@ pub enum Error { TotalActiveBalanceDiffUninitialized, GeneralizedIndexNotSupported(usize), IndexNotSupported(usize), + BuilderPendingPaymentsIndexNotSupported(usize), InvalidFlagIndex(usize), MerkleTreeError(merkle_proof::MerkleTreeError), PartialWithdrawalCountInvalid(usize), diff --git a/consensus/types/src/validator.rs b/consensus/types/src/validator.rs index dec8bba627..6d48f92a83 100644 --- a/consensus/types/src/validator.rs +++ b/consensus/types/src/validator.rs @@ -159,13 +159,41 @@ impl Validator { } /// Check if ``validator`` has an 0x02 prefixed "compounding" withdrawal credential. - pub fn has_compounding_withdrawal_credential(&self, spec: &ChainSpec) -> bool { + pub fn has_compounding_withdrawal_credential( + &self, + spec: &ChainSpec, + current_fork: ForkName, + ) -> bool { + if current_fork.gloas_enabled() { + self.has_compounding_withdrawal_credential_gloas(spec) + } else { + self.has_compounding_withdrawal_credential_electra(spec) + } + } + + /// Check if ``validator`` has an 0x02 prefixed "compounding" withdrawal credential + pub fn has_compounding_withdrawal_credential_electra(&self, spec: &ChainSpec) -> bool { is_compounding_withdrawal_credential(self.withdrawal_credentials, spec) } + /// Check if ``validator`` has an 0x02 prefixed "compounding" withdrawal credential or an 0x03 prefixed "builder" withdrawal credential + pub fn has_compounding_withdrawal_credential_gloas(&self, spec: &ChainSpec) -> bool { + is_compounding_withdrawal_credential(self.withdrawal_credentials, spec) + || is_builder_withdrawal_credential(self.withdrawal_credentials, spec) + } + + /// Check if ``validator`` has an 0x03 prefixed "builder" withdrawal credential. + pub fn has_builder_withdrawal_credential(&self, spec: &ChainSpec) -> bool { + is_builder_withdrawal_credential(self.withdrawal_credentials, spec) + } + /// Get the execution withdrawal address if this validator has one initialized. - pub fn get_execution_withdrawal_address(&self, spec: &ChainSpec) -> Option
{ - self.has_execution_withdrawal_credential(spec) + pub fn get_execution_withdrawal_address( + &self, + spec: &ChainSpec, + current_fork: ForkName, + ) -> Option
{ + self.has_execution_withdrawal_credential(spec, current_fork) .then(|| { self.withdrawal_credentials .as_slice() @@ -196,7 +224,7 @@ impl Validator { current_fork: ForkName, ) -> bool { if current_fork.electra_enabled() { - self.is_fully_withdrawable_validator_electra(balance, epoch, spec) + self.is_fully_withdrawable_validator_electra(balance, epoch, spec, current_fork) } else { self.is_fully_withdrawable_validator_capella(balance, epoch, spec) } @@ -220,8 +248,9 @@ impl Validator { balance: u64, epoch: Epoch, spec: &ChainSpec, + current_fork: ForkName, ) -> bool { - self.has_execution_withdrawal_credential(spec) + self.has_execution_withdrawal_credential(spec, current_fork) && self.withdrawable_epoch <= epoch && balance > 0 } @@ -261,21 +290,25 @@ impl Validator { let max_effective_balance = self.get_max_effective_balance(spec, current_fork); let has_max_effective_balance = self.effective_balance == max_effective_balance; let has_excess_balance = balance > max_effective_balance; - self.has_execution_withdrawal_credential(spec) + self.has_execution_withdrawal_credential(spec, current_fork) && has_max_effective_balance && has_excess_balance } /// Returns `true` if the validator has a 0x01 or 0x02 prefixed withdrawal credential. - pub fn has_execution_withdrawal_credential(&self, spec: &ChainSpec) -> bool { - self.has_compounding_withdrawal_credential(spec) + pub fn has_execution_withdrawal_credential( + &self, + spec: &ChainSpec, + current_fork: ForkName, + ) -> bool { + self.has_compounding_withdrawal_credential(spec, current_fork) || self.has_eth1_withdrawal_credential(spec) } /// Returns the max effective balance for a validator in gwei. pub fn get_max_effective_balance(&self, spec: &ChainSpec, current_fork: ForkName) -> u64 { if current_fork >= ForkName::Electra { - if self.has_compounding_withdrawal_credential(spec) { + if self.has_compounding_withdrawal_credential(spec, current_fork) { spec.max_effective_balance_electra } else { spec.min_activation_balance @@ -313,6 +346,15 @@ pub fn is_compounding_withdrawal_credential( .unwrap_or(false) } +/// Check if the withdrawal credential has the builder withdrawal prefix (0x03). +pub fn is_builder_withdrawal_credential(withdrawal_credentials: Hash256, spec: &ChainSpec) -> bool { + withdrawal_credentials + .as_slice() + .first() + .map(|prefix_byte| *prefix_byte == spec.builder_withdrawal_prefix_byte) + .unwrap_or(false) +} + #[cfg(test)] mod tests { use super::*;