Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions beacon_node/operation_pool/src/bls_to_execution_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,18 @@ impl<E: EthSpec> BlsToExecutionChanges<E> {
.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);
}
Expand Down
7 changes: 6 additions & 1 deletion beacon_node/operation_pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,12 @@ impl<E: EthSpec> OperationPool<E> {
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(),
Expand Down
172 changes: 165 additions & 7 deletions consensus/state_processing/src/per_block_processing.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -173,9 +177,7 @@ pub fn per_block_processing<E: EthSpec, Payload: AbstractExecPayload<E>>(
let body = block.body();
if state.fork_name_unchecked().gloas_enabled() {
process_withdrawals::gloas::process_withdrawals::<E>(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::<E, Payload>(
state,
Expand Down Expand Up @@ -625,7 +627,7 @@ pub fn get_expected_withdrawals<E: EthSpec>(
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,
});
Expand Down Expand Up @@ -662,7 +664,7 @@ pub fn get_expected_withdrawals<E: EthSpec>(
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,
});
Expand All @@ -672,7 +674,7 @@ pub fn get_expected_withdrawals<E: EthSpec>(
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))?,
});
Expand All @@ -692,3 +694,159 @@ pub fn get_expected_withdrawals<E: EthSpec>(
processed_partial_withdrawals_count,
))
}

pub fn process_execution_payload_bid<E: EthSpec, Payload: AbstractExecPayload<E>>(
state: &mut BeaconState<E>,
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()
);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmmm so the spec is written for clarity, not efficiency. Some of this work should be avoided if the amount is zero.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So true! def will keep that more top of mind when translating from spec to a performant code base. updated in commit 5ba3d54

// 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(())
}
41 changes: 41 additions & 0 deletions consensus/state_processing/src/per_block_processing/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ pub enum BlockProcessingError {
},
WithdrawalCredentialsInvalid,
PendingAttestationInElectra,
ExecutionPayloadBidInvalid {
reason: ExecutionPayloadBidInvalid,
},
}

impl From<BeaconStateError> for BlockProcessingError {
Expand Down Expand Up @@ -147,6 +150,12 @@ impl From<milhouse::Error> for BlockProcessingError {
}
}

impl From<ExecutionPayloadBidInvalid> for BlockProcessingError {
fn from(reason: ExecutionPayloadBidInvalid) -> Self {
Self::ExecutionPayloadBidInvalid { reason }
}
}

impl From<BlockOperationError<HeaderInvalid>> for BlockProcessingError {
fn from(e: BlockOperationError<HeaderInvalid>) -> BlockProcessingError {
match e {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,10 @@ pub fn process_withdrawal_requests<E: EthSpec>(

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);

Expand Down Expand Up @@ -560,7 +561,7 @@ pub fn process_withdrawal_requests<E: EthSpec>(
.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
{
Expand Down Expand Up @@ -729,7 +730,9 @@ pub fn process_consolidation_request<E: EthSpec>(

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(());
}
Expand All @@ -740,7 +743,7 @@ pub fn process_consolidation_request<E: EthSpec>(

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(());
}

Expand Down
Loading