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
23 changes: 22 additions & 1 deletion beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ use serde_utils::quoted_u64::Quoted;
use slasher::Slasher;
use slot_clock::SlotClock;
use ssz::Encode;
use state_processing::per_block_processing::errors::{ExitInvalid, ExitValidationError};
use state_processing::{
BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, SigVerifiedOp,
VerifyBlockRoot, VerifyOperation,
Expand Down Expand Up @@ -2827,7 +2828,27 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
) -> Result<ObservationOutcome<SignedVoluntaryExit, T::EthSpec>, Error> {
let head_snapshot = self.head().snapshot;
let head_state = &head_snapshot.beacon_state;
let wall_clock_epoch = self.epoch()?;
let wall_clock_epoch = self
.slot_clock
.now_with_future_tolerance(self.spec.maximum_gossip_clock_disparity())
.ok_or(Error::UnableToReadSlot)?
.epoch(T::EthSpec::slots_per_epoch());

let validator_index = exit.message.validator_index;
if exit.message.epoch > wall_clock_epoch {
return Err(ExitValidationError::invalid(ExitInvalid::FutureEpoch {
state: wall_clock_epoch,
exit: exit.message.epoch,
})
.into());
}
if let Some(validator) = head_state.validators().get(validator_index as usize)
&& validator.exit_epoch != self.spec.far_future_epoch
{
return Err(
ExitValidationError::invalid(ExitInvalid::AlreadyExited(validator_index)).into(),
);
}
Comment on lines +2837 to +2851

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this added/duplicated here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I guess it's for the condition ordering. That's OK I guess.


Ok(self
.observed_voluntary_exits
Expand Down
22 changes: 21 additions & 1 deletion beacon_node/beacon_chain/src/block_production/gloas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ use sensitive_url::SensitiveUrl;
use crate::block_production::bid_selection::{self, BidCandidate, BidSource, ExecutionPayloadData};
use crate::payload_bid_verification::PayloadBidError;
use crate::payload_bid_verification::direct_verified_bid::verify_direct_bid;
use crate::payload_bid_verification::gossip_verified_bid::verify_bid_state_conditions;
use crate::payload_bid_verification::gossip_verified_bid::{
builder_exit_requested, verify_bid_state_conditions,
};
use crate::payload_bid_verification::payload_bid_cache::BidParent;
use crate::pending_payload_envelopes::PendingEnvelopeData;
use crate::{
Expand Down Expand Up @@ -292,6 +294,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
&builder_config,
proposer_preferences.as_deref(),
&state,
&parent_execution_requests,
);
let local_fut = self.clone().produce_execution_payload_bid(
&state,
Expand Down Expand Up @@ -1019,6 +1022,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
builder_config: &BuilderConfig,
proposer_preferences: Option<&SignedProposerPreferences>,
state: &BeaconState<T::EthSpec>,
parent_execution_requests: &ExecutionRequestsGloas<T::EthSpec>,
) -> Vec<BidCandidate<T::EthSpec>> {
let mut externals = Vec::new();

Expand Down Expand Up @@ -1073,6 +1077,22 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
}
}

// The parent's exit requests apply to the state before this block's bid is processed, so a
// bid from a builder the parent payload exits fails `process_execution_payload_bid`.
externals.retain(|candidate| {
let builder_index = candidate.signed_bid.message.builder_index;
let exit_requested = state
.get_builder(builder_index)
.is_ok_and(|builder| builder_exit_requested(builder, parent_execution_requests));
if exit_requested {
warn!(
builder_index,
"Skipping bid from a builder the parent payload exits"
);
}
!exit_requested
});

externals
}

Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub use parking_lot;
pub use slot_clock;
pub use state_processing::per_block_processing::errors::{
AttestationValidationError, AttesterSlashingValidationError, DepositValidationError,
ExitValidationError, ProposerSlashingValidationError,
ExitInvalid, ExitValidationError, ProposerSlashingValidationError,
};
pub use store;
pub use types;
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ use crate::{
};
use educe::Educe;
use eth2::types::{EventKind, ForkVersionedResponse};
use proto_array::Block as ProtoBlock;
use slot_clock::SlotClock;
use state_processing::signature_sets::{
execution_payload_bid_signature_set, get_builder_pubkey_from_state,
};
use tracing::debug;
use types::{
BeaconState, ChainSpec, EthSpec, ExecutionPayloadBid, SignedExecutionPayloadBid,
SignedProposerPreferences, Slot, consts::gloas::PAYLOAD_BUILDER_VERSION,
BeaconState, Builder, ChainSpec, EthSpec, ExecutionPayloadBid, ExecutionRequestsGloas,
SignedExecutionPayloadBid, SignedProposerPreferences, Slot,
consts::gloas::PAYLOAD_BUILDER_VERSION,
};

pub(crate) fn verify_bid_slot(bid_slot: Slot, current_slot: Slot) -> Result<(), PayloadBidError> {
Expand Down Expand Up @@ -92,8 +94,8 @@ pub(crate) fn verify_bid_consistency<E: EthSpec>(
verify_bid_state_conditions(bid, head_state, spec)
}

/// Verify the bid conditions that depend on the beacon `state`: the builder is active, is a payload
/// builder, and can cover the bid. These are exactly the state-dependent checks
/// Verify the bid conditions that depend on the beacon `state`: the builder is a payload builder,
/// is active, and can cover the bid. These are exactly the state-dependent checks
/// `process_execution_payload_bid` re-applies in `per_block_processing`, and the only bid conditions
/// that can go stale between gossip verification and block production (e.g. the builder's balance
/// dropping). Re-running them against the production state lets bid selection drop a gossip bid that
Expand All @@ -109,10 +111,10 @@ pub(crate) fn verify_bid_state_conditions<E: EthSpec>(
.map_err(|_| PayloadBidError::InvalidBuilder { builder_index })?
.version;

if !head_state.can_builder_cover_bid(builder_index, bid.value, spec)? {
return Err(PayloadBidError::BuilderCantCoverBid {
if builder_version != PAYLOAD_BUILDER_VERSION {
return Err(PayloadBidError::InvalidBuilderVersion {
builder_index,
builder_bid: bid.value,
version: builder_version,
});
}

Expand All @@ -123,16 +125,53 @@ pub(crate) fn verify_bid_state_conditions<E: EthSpec>(
return Err(PayloadBidError::InvalidBuilder { builder_index });
}

if builder_version != PAYLOAD_BUILDER_VERSION {
return Err(PayloadBidError::InvalidBuilderVersion {
if !head_state.can_builder_cover_bid(builder_index, bid.value, spec)? {
return Err(PayloadBidError::BuilderCantCoverBid {
builder_index,
version: builder_version,
builder_bid: bid.value,
});
}

Ok(())
}

/// Returns `true` if the bid builds on the parent's full payload and that payload carries an exit
/// request for the bid's builder.
pub(crate) fn parent_payload_exits_builder<T: BeaconChainTypes>(
bid: &ExecutionPayloadBid<T::EthSpec>,
parent_block: &ProtoBlock,
head_state: &BeaconState<T::EthSpec>,
store: &BeaconStore<T>,
) -> Result<bool, PayloadBidError> {
if parent_block.execution_payload_block_hash != Some(bid.parent_block_hash) {
return Ok(false);
}

let builder = head_state.get_builder(bid.builder_index)?;
let parent_envelope = store
.get_payload_envelope(&bid.parent_block_root)
.map_err(|e| {
PayloadBidError::InternalError(format!("failed to load parent payload envelope: {e:?}"))
})?
.ok_or(PayloadBidError::ParentExecutionPayloadUnknown {
parent_block_hash: bid.parent_block_hash,
})?;

Ok(builder_exit_requested(
builder,
&parent_envelope.message.execution_requests,
))
}

pub(crate) fn builder_exit_requested<E: EthSpec>(
builder: &Builder,
execution_requests: &ExecutionRequestsGloas<E>,
) -> bool {
execution_requests.builder_exits.iter().any(|request| {
request.pubkey == builder.pubkey && request.source_address == builder.execution_address
})
}

/// Checks if `bid` is compatible with the head branch
pub(crate) fn is_bid_compatible_with_head<T: BeaconChainTypes>(
cached_head: &CachedHead<T::EthSpec>,
Expand Down Expand Up @@ -280,7 +319,7 @@ impl<E: EthSpec> GossipVerifiedPayloadBid<E> {

verify_bid_payment_and_blobs(&signed_bid.message, ctx.spec)?;

parent_block.ok_or(PayloadBidError::ParentBlockRootUnknown {
let parent_block = parent_block.ok_or(PayloadBidError::ParentBlockRootUnknown {
parent_block_root: bid_parent_block_root,
})?;
drop(fork_choice);
Expand Down Expand Up @@ -379,6 +418,17 @@ impl<E: EthSpec> GossipVerifiedPayloadBid<E> {

verify_bid_state_conditions(&signed_bid.message, head_state, ctx.spec)?;

if parent_payload_exits_builder::<T>(
&signed_bid.message,
&parent_block,
head_state,
ctx.store,
)? {
return Err(PayloadBidError::BuilderExitPending {
builder_index: signed_bid.message.builder_index,
});
}

execution_payload_bid_signature_set(
head_state,
|i| get_builder_pubkey_from_state(head_state, i),
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/beacon_chain/src/payload_bid_verification/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ pub enum PayloadBidError {
builder_index: u64,
builder_bid: u64,
},
/// The parent's payload contains an exit request for this builder.
BuilderExitPending { builder_index: u64 },
/// The bids fee recipient doesn't match the proposer preferences fee recipient.
InvalidFeeRecipient,
/// The bid's gas limit is not compatible with the proposer's target gas limit.
Expand Down
Loading
Loading