Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Superstruct AggregateAndProof #5715

Merged
merged 2 commits into from
May 6, 2024
Merged

Superstruct AggregateAndProof #5715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ec6d06
Upgrade `superstruct` to `0.8.0`
ethDreamer May 3, 2024
5214f7f
superstruct `AggregateAndProof`
ethDreamer May 3, 2024
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
5 changes: 2 additions & 3 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ smallvec = "1.11.2"
snap = "1"
ssz_types = "0.6"
strum = { version = "0.24", features = ["derive"] }
superstruct = "0.7"
superstruct = { git = "https://github.com/sigp/superstruct", rev = "45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" }
syn = "1"
sysinfo = "0.26"
tempfile = "3"
Expand Down
82 changes: 46 additions & 36 deletions beacon_node/beacon_chain/src/attestation_verification.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Attestation, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName, Hash256,
IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
Attestation, AttestationRef, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec,
ForkName, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
};

pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
Expand Down Expand Up @@ -274,7 +274,7 @@ struct IndexedAggregatedAttestation<'a, T: BeaconChainTypes> {
///
/// These attestations have *not* undergone signature verification.
struct IndexedUnaggregatedAttestation<'a, T: BeaconChainTypes> {
attestation: &'a Attestation<T::EthSpec>,
attestation: AttestationRef<'a, T::EthSpec>,
indexed_attestation: IndexedAttestation<T::EthSpec>,
subnet_id: SubnetId,
validator_index: u64,
Expand All @@ -295,7 +295,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {

/// Wraps an `Attestation` that has been fully verified for propagation on the gossip network.
pub struct VerifiedUnaggregatedAttestation<'a, T: BeaconChainTypes> {
attestation: &'a Attestation<T::EthSpec>,
attestation: AttestationRef<'a, T::EthSpec>,
indexed_attestation: IndexedAttestation<T::EthSpec>,
subnet_id: SubnetId,
}
Expand All @@ -322,20 +322,20 @@ impl<'a, T: BeaconChainTypes> Clone for IndexedUnaggregatedAttestation<'a, T> {
/// A helper trait implemented on wrapper types that can be progressed to a state where they can be
/// verified for application to fork choice.
pub trait VerifiedAttestation<T: BeaconChainTypes>: Sized {
fn attestation(&self) -> &Attestation<T::EthSpec>;
fn attestation(&self) -> AttestationRef<T::EthSpec>;

fn indexed_attestation(&self) -> &IndexedAttestation<T::EthSpec>;

// Inefficient default implementation. This is overridden for gossip verified attestations.
fn into_attestation_and_indices(self) -> (Attestation<T::EthSpec>, Vec<u64>) {
let attestation = self.attestation().clone();
let attestation = self.attestation().clone_as_attestation();
let attesting_indices = self.indexed_attestation().attesting_indices_to_vec();
(attestation, attesting_indices)
}
}

impl<'a, T: BeaconChainTypes> VerifiedAttestation<T> for VerifiedAggregatedAttestation<'a, T> {
fn attestation(&self) -> &Attestation<T::EthSpec> {
fn attestation(&self) -> AttestationRef<T::EthSpec> {
self.attestation()
}

Expand All @@ -345,7 +345,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAttestation<T> for VerifiedAggregatedAttes
}

impl<'a, T: BeaconChainTypes> VerifiedAttestation<T> for VerifiedUnaggregatedAttestation<'a, T> {
fn attestation(&self) -> &Attestation<T::EthSpec> {
fn attestation(&self) -> AttestationRef<T::EthSpec> {
self.attestation
}

Expand All @@ -357,7 +357,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAttestation<T> for VerifiedUnaggregatedAtt
/// Information about invalid attestations which might still be slashable despite being invalid.
pub enum AttestationSlashInfo<'a, T: BeaconChainTypes, TErr> {
/// The attestation is invalid, but its signature wasn't checked.
SignatureNotChecked(&'a Attestation<T::EthSpec>, TErr),
SignatureNotChecked(AttestationRef<'a, T::EthSpec>, TErr),
/// As for `SignatureNotChecked`, but we know the `IndexedAttestation`.
SignatureNotCheckedIndexed(IndexedAttestation<T::EthSpec>, TErr),
/// The attestation's signature is invalid, so it will never be slashable.
Expand Down Expand Up @@ -446,7 +446,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
signed_aggregate: &SignedAggregateAndProof<T::EthSpec>,
chain: &BeaconChain<T>,
) -> Result<Hash256, Error> {
let attestation = &signed_aggregate.message.aggregate;
let attestation = signed_aggregate.message().aggregate();

// Ensure attestation is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots (within a
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
Expand All @@ -471,14 +471,14 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
if chain
.observed_attestations
.write()
.is_known_subset(attestation.to_ref(), attestation_data_root)
.is_known_subset(attestation, attestation_data_root)
.map_err(|e| Error::BeaconChainError(e.into()))?
{
metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
return Err(Error::AttestationSupersetKnown(attestation_data_root));
}

let aggregator_index = signed_aggregate.message.aggregator_index;
let aggregator_index = signed_aggregate.message().aggregator_index();

// Ensure there has been no other observed aggregate for the given `aggregator_index`.
//
Expand Down Expand Up @@ -532,11 +532,16 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
) -> Result<Self, AttestationSlashInfo<'a, T, Error>> {
use AttestationSlashInfo::*;

let attestation = &signed_aggregate.message.aggregate;
let aggregator_index = signed_aggregate.message.aggregator_index;
let attestation = signed_aggregate.message().aggregate();
let aggregator_index = signed_aggregate.message().aggregator_index();
let attestation_data_root = match Self::verify_early_checks(signed_aggregate, chain) {
Ok(root) => root,
Err(e) => return Err(SignatureNotChecked(&signed_aggregate.message.aggregate, e)),
Err(e) => {
return Err(SignatureNotChecked(
signed_aggregate.message().aggregate(),
e,
))
}
};

let get_indexed_attestation_with_committee =
Expand All @@ -545,7 +550,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
//
// Future optimizations should remove this clone.
let selection_proof =
SelectionProof::from(signed_aggregate.message.selection_proof.clone());
SelectionProof::from(signed_aggregate.message().selection_proof().clone());

if !selection_proof
.is_aggregator(committee.committee.len(), &chain.spec)
Expand All @@ -559,7 +564,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
return Err(Error::AggregatorNotInCommittee { aggregator_index });
}

get_indexed_attestation(committee.committee, attestation.to_ref())
get_indexed_attestation(committee.committee, attestation)
.map_err(|e| BeaconChainError::from(e).into())
};

Expand All @@ -569,7 +574,12 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
get_indexed_attestation_with_committee,
) {
Ok(indexed_attestation) => indexed_attestation,
Err(e) => return Err(SignatureNotChecked(&signed_aggregate.message.aggregate, e)),
Err(e) => {
return Err(SignatureNotChecked(
signed_aggregate.message().aggregate(),
e,
))
}
};

Ok(IndexedAggregatedAttestation {
Expand All @@ -587,8 +597,8 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
attestation_data_root: Hash256,
chain: &BeaconChain<T>,
) -> Result<(), Error> {
let attestation = &signed_aggregate.message.aggregate;
let aggregator_index = signed_aggregate.message.aggregator_index;
let attestation = signed_aggregate.message().aggregate();
let aggregator_index = signed_aggregate.message().aggregator_index();

// Observe the valid attestation so we do not re-process it.
//
Expand All @@ -597,7 +607,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
if let ObserveOutcome::Subset = chain
.observed_attestations
.write()
.observe_item(attestation.to_ref(), Some(attestation_data_root))
.observe_item(attestation, Some(attestation_data_root))
.map_err(|e| Error::BeaconChainError(e.into()))?
{
metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
Expand Down Expand Up @@ -696,8 +706,8 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
}

/// Returns the underlying `attestation` for the `signed_aggregate`.
pub fn attestation(&self) -> &Attestation<T::EthSpec> {
&self.signed_aggregate.message.aggregate
pub fn attestation(&self) -> AttestationRef<'a, T::EthSpec> {
self.signed_aggregate.message().aggregate()
}

/// Returns the underlying `signed_aggregate`.
Expand All @@ -709,7 +719,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
/// Run the checks that happen before an indexed attestation is constructed.
pub fn verify_early_checks(
attestation: &Attestation<T::EthSpec>,
attestation: AttestationRef<T::EthSpec>,
chain: &BeaconChain<T>,
) -> Result<(), Error> {
let attestation_epoch = attestation.data().slot.epoch(T::EthSpec::slots_per_epoch());
Expand Down Expand Up @@ -750,7 +760,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {

/// Run the checks that apply to the indexed attestation before the signature is checked.
pub fn verify_middle_checks(
attestation: &Attestation<T::EthSpec>,
attestation: AttestationRef<T::EthSpec>,
indexed_attestation: &IndexedAttestation<T::EthSpec>,
committees_per_slot: u64,
subnet_id: Option<SubnetId>,
Expand Down Expand Up @@ -806,7 +816,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
subnet_id: Option<SubnetId>,
chain: &BeaconChain<T>,
) -> Result<Self, Error> {
Self::verify_slashable(attestation, subnet_id, chain)
Self::verify_slashable(attestation.to_ref(), subnet_id, chain)
.map(|verified_unaggregated| {
if let Some(slasher) = chain.slasher.as_ref() {
slasher.accept_attestation(verified_unaggregated.indexed_attestation.clone());
Expand All @@ -818,7 +828,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {

/// Verify the attestation, producing extra information about whether it might be slashable.
pub fn verify_slashable(
attestation: &'a Attestation<T::EthSpec>,
attestation: AttestationRef<'a, T::EthSpec>,
subnet_id: Option<SubnetId>,
chain: &BeaconChain<T>,
) -> Result<Self, AttestationSlashInfo<'a, T, Error>> {
Expand Down Expand Up @@ -867,7 +877,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> {
/// Run the checks that apply after the signature has been checked.
fn verify_late_checks(
attestation: &Attestation<T::EthSpec>,
attestation: AttestationRef<T::EthSpec>,
validator_index: u64,
chain: &BeaconChain<T>,
) -> Result<(), Error> {
Expand Down Expand Up @@ -961,7 +971,7 @@ impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> {
}

/// Returns the wrapped `attestation`.
pub fn attestation(&self) -> &Attestation<T::EthSpec> {
pub fn attestation(&self) -> AttestationRef<T::EthSpec> {
self.attestation
}

Expand Down Expand Up @@ -991,7 +1001,7 @@ impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> {
/// already finalized.
fn verify_head_block_is_known<T: BeaconChainTypes>(
chain: &BeaconChain<T>,
attestation: &Attestation<T::EthSpec>,
attestation: AttestationRef<T::EthSpec>,
max_skip_slots: Option<u64>,
) -> Result<ProtoBlock, Error> {
let block_opt = chain
Expand Down Expand Up @@ -1039,7 +1049,7 @@ fn verify_head_block_is_known<T: BeaconChainTypes>(
/// Accounts for `MAXIMUM_GOSSIP_CLOCK_DISPARITY`.
pub fn verify_propagation_slot_range<S: SlotClock, E: EthSpec>(
slot_clock: &S,
attestation: &Attestation<E>,
attestation: AttestationRef<E>,
spec: &ChainSpec,
) -> Result<(), Error> {
let attestation_slot = attestation.data().slot;
Expand Down Expand Up @@ -1124,7 +1134,7 @@ pub fn verify_attestation_signature<T: BeaconChainTypes>(
/// `attestation.data.beacon_block_root`.
pub fn verify_attestation_target_root<E: EthSpec>(
head_block: &ProtoBlock,
attestation: &Attestation<E>,
attestation: AttestationRef<E>,
) -> Result<(), Error> {
// Check the attestation target root.
let head_block_epoch = head_block.slot.epoch(E::slots_per_epoch());
Expand Down Expand Up @@ -1193,7 +1203,7 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
.try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT)
.ok_or(BeaconChainError::ValidatorPubkeyCacheLockTimeout)?;

let aggregator_index = signed_aggregate.message.aggregator_index;
let aggregator_index = signed_aggregate.message().aggregator_index();
if aggregator_index >= pubkey_cache.len() as u64 {
return Err(Error::AggregatorPubkeyUnknown(aggregator_index));
}
Expand Down Expand Up @@ -1240,10 +1250,10 @@ type CommitteesPerSlot = u64;
/// public keys cached in the `chain`.
pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
chain: &BeaconChain<T>,
attestation: &Attestation<T::EthSpec>,
attestation: AttestationRef<T::EthSpec>,
) -> Result<(IndexedAttestation<T::EthSpec>, CommitteesPerSlot), Error> {
map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| {
get_indexed_attestation(committee.committee, attestation.to_ref())
get_indexed_attestation(committee.committee, attestation)
.map(|attestation| (attestation, committees_per_slot))
.map_err(Error::Invalid)
})
Expand All @@ -1260,7 +1270,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
/// from disk and then update the `shuffling_cache`.
fn map_attestation_committee<T, F, R>(
chain: &BeaconChain<T>,
attestation: &Attestation<T::EthSpec>,
attestation: AttestationRef<T::EthSpec>,
map_fn: F,
) -> Result<R, Error>
where
Expand Down
10 changes: 6 additions & 4 deletions beacon_node/beacon_chain/src/beacon_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1966,8 +1966,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// This method is called for API and gossip attestations, so this covers all unaggregated attestation events
if let Some(event_handler) = self.event_handler.as_ref() {
if event_handler.has_attestation_subscribers() {
event_handler
.register(EventKind::Attestation(Box::new(v.attestation().clone())));
event_handler.register(EventKind::Attestation(Box::new(
v.attestation().clone_as_attestation(),
)));
}
}
metrics::inc_counter(&metrics::UNAGGREGATED_ATTESTATION_PROCESSING_SUCCESSES);
Expand Down Expand Up @@ -2003,8 +2004,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// This method is called for API and gossip attestations, so this covers all aggregated attestation events
if let Some(event_handler) = self.event_handler.as_ref() {
if event_handler.has_attestation_subscribers() {
event_handler
.register(EventKind::Attestation(Box::new(v.attestation().clone())));
event_handler.register(EventKind::Attestation(Box::new(
v.attestation().clone_as_attestation(),
)));
}
}
metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_PROCESSING_SUCCESSES);
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/beacon_chain/src/bellatrix_readiness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
});
}

if let Some(&expected) = expected_withdrawals_root {
if let Some(&got) = got_withdrawals_root {
if let Some(expected) = expected_withdrawals_root {
if let Some(got) = got_withdrawals_root {
if got != expected {
return Ok(GenesisExecutionPayloadStatus::WithdrawalsRootMismatch {
got,
Expand Down
Loading
Loading