Skip to content
Closed
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
79 changes: 62 additions & 17 deletions beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ pub enum Error {
///
/// The peer has sent an invalid message.
CommitteeIndexNonZero(usize),
/// The committee index is too high (>1) for Gloas
///
/// ## Peer scoring
///
/// The peer has sent an invalid message.
CommitteeIndexTooHigh(usize),
/// The `attestation.data.beacon_block_root` block is unknown.
///
/// ## Peer scoring
Expand Down Expand Up @@ -550,9 +556,6 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
}
.tree_hash_root();

// [New in Electra:EIP7549]
verify_committee_index(attestation)?;

if chain
.observed_attestations
.write()
Expand Down Expand Up @@ -595,6 +598,9 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
// attestation and do not delay consideration for later.
let head_block = verify_head_block_is_known(chain, attestation.data(), None)?;

// [EIP-7732] Attestation data index validation for gloas fork and [EIP-7549] for Electra
verify_attestation_data_index(attestation, &head_block, chain)?;

// Check the attestation target root is consistent with the head root.
//
// This check is not in the specification, however we guard against it since it opens us up
Expand Down Expand Up @@ -868,10 +874,34 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
&chain.spec,
)?;

// Attestations must be for a known block. If the block is unknown, we simply drop the
// attestation and do not delay consideration for later.
//
// Enforce a maximum skip distance for unaggregated attestations.
let head_block = verify_head_block_is_known(
chain,
&attestation.data,
chain.config.import_max_skip_slots,
)?;

let fork_name = chain
.spec
.fork_name_at_slot::<T::EthSpec>(attestation.data.slot);
if fork_name.electra_enabled() {
if fork_name.gloas_enabled() {
// [New in Gloas:EIP7732]
if attestation.data.index > 1 {
return Err(Error::CommitteeIndexTooHigh(
attestation.data.index as usize,
));
}

// [New in Gloas:EIP7732]
if head_block.slot == attestation.data.slot && attestation.data.index != 0 {
return Err(Error::CommitteeIndexNonZero(
attestation.data.index as usize,
));
}
} else if fork_name.electra_enabled() {
// [New in Electra:EIP7549]
if attestation.data.index != 0 {
return Err(Error::CommitteeIndexNonZero(
Expand All @@ -880,16 +910,6 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
}
}

// Attestations must be for a known block. If the block is unknown, we simply drop the
// attestation and do not delay consideration for later.
//
// Enforce a maximum skip distance for unaggregated attestations.
let head_block = verify_head_block_is_known(
chain,
&attestation.data,
chain.config.import_max_skip_slots,
)?;

// Check the attestation target root is consistent with the head root.
verify_attestation_target_root::<T::EthSpec>(&head_block, &attestation.data)?;

Expand Down Expand Up @@ -1402,18 +1422,43 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
Ok(verify_signature_sets(signature_sets.iter()))
}

/// Verify that the `attestation` committee index is properly set for the attestation's fork.
/// Verify that the `attestation` data index is properly set for the attestation's fork.
/// This function will only apply verification post-Electra.
pub fn verify_committee_index<E: EthSpec>(attestation: AttestationRef<E>) -> Result<(), Error> {
pub fn verify_attestation_data_index<T: BeaconChainTypes>(
attestation: AttestationRef<T::EthSpec>,
head_block: &ProtoBlock,
chain: &BeaconChain<T>,
) -> Result<(), Error> {
let fork_name = chain
.spec
.fork_name_at_slot::<T::EthSpec>(attestation.data().slot);

// Check Electra+ committee_bits validation
if let Ok(committee_bits) = attestation.committee_bits() {
// Check to ensure that the attestation is for a single committee.
let num_committee_bits = get_committee_indices::<E>(committee_bits);
let num_committee_bits = get_committee_indices::<T::EthSpec>(committee_bits);
if num_committee_bits.len() != 1 {
return Err(Error::NotExactlyOneCommitteeBitSet(
num_committee_bits.len(),
));
}
}

if fork_name.gloas_enabled() {
// [New in Gloas:EIP7732]
if attestation.data().index > 1 {
return Err(Error::CommitteeIndexTooHigh(
attestation.data().index as usize,
));
}

// [New in Gloas:EIP7732]
if head_block.slot == attestation.data().slot && attestation.data().index != 0 {
return Err(Error::CommitteeIndexNonZero(
attestation.data().index as usize,
));
}
} else if fork_name.electra_enabled() {
// Ensure the attestation index is set to zero post Electra.
if attestation.data().index != 0 {
return Err(Error::CommitteeIndexNonZero(
Expand Down
31 changes: 31 additions & 0 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,16 @@ pub enum BlockError {
max_blobs_at_epoch: usize,
block: usize,
},
/// The block's execution payload bid has a `parent_block_root` that doesn't match
/// the block's `parent_root`.
///
/// ## Peer scoring
///
/// This block is invalid and the peer should be penalised.
BidParentBlockRootMismatch {
block_parent_root: Hash256,
bid_parent_root: Hash256,
},
}

/// Which specific signature(s) are invalid in a SignedBeaconBlock
Expand Down Expand Up @@ -878,6 +888,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {

// Do not gossip blocks that claim to contain more blobs than the max allowed
// at the given block epoch.
// Note that `blob_kzg_commitments` were removed from the body in gloas fork, so this check will no longer apply
if let Ok(commitments) = block.message().body().blob_kzg_commitments() {
let max_blobs_at_epoch = chain
.spec
Expand Down Expand Up @@ -1030,8 +1041,28 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
}

// Validate the block's execution_payload (if any).
// Note that `execution_payload` was removed from the body in gloas fork, so this check will no longer apply
validate_execution_payload_for_gossip(&parent_block, block.message(), chain)?;

// TODO(EIP-7732): from the spec: https://ethereum.github.io/consensus-specs/specs/gloas/p2p-interface/#beacon_block
// we will need to satisfy this condition:
// If execution_payload verification of block's execution payload parent by an execution node is complete:
// [REJECT] The block's execution payload parent (defined by bid.parent_block_hash) passes all validation.
// discussed the options on eth r&d: https://discord.com/channels/595666850260713488/874767108809031740/1420935382706425867
// the idea is that during envelope processing, if we get back an `INVALID` status from the EL, then we add the `block_hash` to a list and if the next block's bid's `parent_block_hash` is in the list, we ignore the block. Something like this should be implemented as part of the envelope processing flow

// [REJECT] `bid.parent_block_root` == `block.parent_root`
// This check is only applicable post-Gloas when blocks contain execution payload bids
if let Ok(signed_bid) = block.message().body().signed_execution_payload_bid() {
let bid = &signed_bid.message;
if bid.parent_block_root != block.message().parent_root() {
return Err(BlockError::BidParentBlockRootMismatch {
block_parent_root: block.message().parent_root(),
bid_parent_root: bid.parent_block_root,
});
}
}

// Beacon API block_gossip events
if let Some(event_handler) = chain.event_handler.as_ref()
&& event_handler.has_block_gossip_subscribers()
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/execution_payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ pub fn validate_execution_payload_for_gossip<T: BeaconChainTypes>(
block: BeaconBlockRef<'_, T::EthSpec>,
chain: &BeaconChain<T>,
) -> Result<(), BlockError> {
// Only apply this validation if this is a Bellatrix beacon block.
// Only apply this validation if this is a post-Bellatrix or pre-Gloas beacon block.
if let Ok(execution_payload) = block.body().execution_payload() {
// This logic should match `is_execution_enabled`. We use only the execution block hash of
// the parent here in order to avoid loading the parent state during gossip verification.
Expand Down
27 changes: 25 additions & 2 deletions beacon_node/network/src/network_beacon_processor/gossip_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,8 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
| Err(e @ BlockError::ParentExecutionPayloadInvalid { .. })
| Err(e @ BlockError::KnownInvalidExecutionPayload(_))
| Err(e @ BlockError::GenesisBlock)
| Err(e @ BlockError::InvalidBlobCount { .. }) => {
| Err(e @ BlockError::InvalidBlobCount { .. })
| Err(e @ BlockError::BidParentBlockRootMismatch { .. }) => {
warn!(error = %e, "Could not verify block for gossip. Rejecting the block");
self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);
self.gossip_penalize_peer(
Expand Down Expand Up @@ -2389,7 +2390,9 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
}
AttnError::CommitteeIndexNonZero(index) => {
/*
* The validator index is not set to zero after Electra.
* The committee index is not set to zero when required by the fork rules.
* - Electra: always must be zero
* - Gloas: must be zero if attestation.slot == block.slot
*
* The peer has published an invalid consensus message.
*/
Expand All @@ -2407,6 +2410,26 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
"attn_comm_index_non_zero",
);
}
AttnError::CommitteeIndexTooHigh(index) => {
/*
* The committee index is too high for the gloas fork rules.
*
* The peer has published an invalid consensus message.
*/
debug!(
%peer_id,
block = ?beacon_block_root,
?attestation_type,
committee_index = index,
"Committee index too high"
);
self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);
self.gossip_penalize_peer(
peer_id,
PeerAction::LowToleranceError,
"attn_comm_index_too_high",
);
}
AttnError::UnknownHeadBlock { beacon_block_root } => {
trace!(
%peer_id,
Expand Down
Loading