Skip to content

Commit

Permalink
Superstruct AggregateAndProof (sigp#5715)
Browse files Browse the repository at this point in the history
* Upgrade `superstruct` to `0.8.0`

* superstruct `AggregateAndProof`
  • Loading branch information
ethDreamer authored and realbigsean committed Jun 25, 2024
1 parent 68af35a commit 7f32da8
Show file tree
Hide file tree
Showing 20 changed files with 193 additions and 174 deletions.
3 changes: 1 addition & 2 deletions Cargo.lock

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
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.8"
superstruct = { git = "https://github.com/sigp/superstruct", rev = "45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" }
syn = "1"
sysinfo = "0.26"
tempfile = "3"
Expand Down
49 changes: 17 additions & 32 deletions beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Attestation, AttestationRef, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec,
CommitteeIndex, Epoch, EthSpec, 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 @@ -348,7 +347,7 @@ pub trait VerifiedAttestation<T: BeaconChainTypes>: Sized {

// 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)
}
Expand Down Expand Up @@ -496,7 +495,7 @@ 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);
Expand Down Expand Up @@ -558,8 +557,10 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
chain: &BeaconChain<T>,
) -> Result<Self, AttestationSlashInfo<'a, T, Error>> {
use AttestationSlashInfo::*;
let observed_attestation_key_root = match Self::verify_early_checks(signed_aggregate, chain)
{

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(
Expand All @@ -571,28 +572,12 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {

// Committees must be sorted by ascending index order 0..committees_per_slot
let get_indexed_attestation_with_committee =
|(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
let (index, aggregator_index, selection_proof, data) = match signed_aggregate {
SignedAggregateAndProof::Base(signed_aggregate) => (
signed_aggregate.message.aggregate.data.index,
signed_aggregate.message.aggregator_index,
// Note: this clones the signature which is known to be a relatively slow operation.
// Future optimizations should remove this clone.
signed_aggregate.message.selection_proof.clone(),
signed_aggregate.message.aggregate.data.clone(),
),
SignedAggregateAndProof::Electra(signed_aggregate) => (
signed_aggregate
.message
.aggregate
.committee_index()
.ok_or(Error::NotExactlyOneCommitteeBitSet(0))?,
signed_aggregate.message.aggregator_index,
signed_aggregate.message.selection_proof.clone(),
signed_aggregate.message.aggregate.data.clone(),
),
};
let slot = data.slot;
|(committee, _): (BeaconCommittee, CommitteesPerSlot)| {
// Note: this clones the signature which is known to be a relatively slow operation.
//
// Future optimizations should remove this clone.
let selection_proof =
SelectionProof::from(signed_aggregate.message().selection_proof().clone());

let committee = committees
.get(index as usize)
Expand All @@ -610,7 +595,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 Down Expand Up @@ -653,7 +638,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 @@ -1326,7 +1311,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
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 Down
15 changes: 5 additions & 10 deletions beacon_node/beacon_chain/src/naive_aggregation_pool.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
use crate::metrics;
use crate::observed_aggregates::AsReference;
use itertools::Itertools;
use smallvec::SmallVec;
use std::collections::HashMap;
use tree_hash::{MerkleHasher, TreeHash, TreeHashType};
use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT;
use types::slot_data::SlotData;
use types::sync_committee_contribution::SyncContributionData;
use types::{
Attestation, AttestationData, AttestationRef, CommitteeIndex, EthSpec, Hash256, Slot,
SyncCommitteeContribution,
Attestation, AttestationData, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution,
};

type AttestationKeyRoot = Hash256;
Expand Down Expand Up @@ -242,14 +239,14 @@ impl<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
let _timer = metrics::start_timer(&metrics::ATTESTATION_PROCESSING_AGG_POOL_CORE_INSERT);

let set_bits = match a {
Attestation::Base(att) => att
AttestationRef::Base(att) => att
.aggregation_bits
.iter()
.enumerate()
.filter(|(_i, bit)| *bit)
.map(|(i, _bit)| i)
.collect::<Vec<_>>(),
Attestation::Electra(att) => att
AttestationRef::Electra(att) => att
.aggregation_bits
.iter()
.enumerate()
Expand Down Expand Up @@ -290,10 +287,8 @@ impl<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
}

self.map
.insert(attestation_key_root, a.clone_as_attestation());
Ok(InsertOutcome::NewItemInserted {
committee_index: aggregation_bit,
})
.insert(attestation_data_root, a.clone_as_attestation());
Ok(InsertOutcome::NewItemInserted { committee_index })
}
}

Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1382,7 +1382,7 @@ where
committee_attestations.iter().skip(1).fold(
attestation.clone(),
|mut agg, (att, _)| {
agg.aggregate(att);
agg.aggregate(att.to_ref());
agg
},
)
Expand Down
8 changes: 4 additions & 4 deletions beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3368,9 +3368,9 @@ pub fn serve<T: BeaconChainTypes>(
"Failure verifying aggregate and proofs";
"error" => format!("{:?}", e),
"request_index" => index,
"aggregator_index" => aggregate.message.aggregator_index,
"attestation_index" => aggregate.message.aggregate.data().index,
"attestation_slot" => aggregate.message.aggregate.data().slot,
"aggregator_index" => aggregate.message().aggregator_index(),
"attestation_index" => aggregate.message().aggregate().data().index,
"attestation_slot" => aggregate.message().aggregate().data().slot,
);
failures.push(api_types::Failure::new(index, format!("Verification: {:?}", e)));
}
Expand All @@ -3389,7 +3389,7 @@ pub fn serve<T: BeaconChainTypes>(
"Failure applying verified aggregate attestation to fork choice";
"error" => format!("{:?}", e),
"request_index" => index,
"aggregator_index" => verified_aggregate.aggregate().message.aggregator_index,
"aggregator_index" => verified_aggregate.aggregate().message().aggregator_index(),
"attestation_index" => verified_aggregate.attestation().data().index,
"attestation_slot" => verified_aggregate.attestation().data().slot,
);
Expand Down
10 changes: 8 additions & 2 deletions beacon_node/http_api/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3334,8 +3334,14 @@ impl ApiTester {

pub async fn test_get_validator_aggregate_and_proofs_invalid(mut self) -> Self {
let mut aggregate = self.get_aggregate().await;

aggregate.message.aggregate.data_mut().slot += 1;
match &mut aggregate {
SignedAggregateAndProof::Base(ref mut aggregate) => {
aggregate.message.aggregate.data.slot += 1;
}
SignedAggregateAndProof::Electra(ref mut aggregate) => {
aggregate.message.aggregate.data.slot += 1;
}
}

self.client
.post_validator_aggregate_and_proof::<E>(&[aggregate])
Expand Down
38 changes: 19 additions & 19 deletions beacon_node/lighthouse_network/src/types/pubsub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ use std::sync::Arc;
use types::{
Attestation, AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, BlobSidecar,
EthSpec, ForkContext, ForkName, LightClientFinalityUpdate, LightClientOptimisticUpdate,
ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBeaconBlockAltair,
SignedBeaconBlockBase, SignedBeaconBlockCapella, SignedBeaconBlockDeneb,
SignedBeaconBlockElectra, SignedBeaconBlockMerge, SignedBlsToExecutionChange,
ProposerSlashing, SignedAggregateAndProof, SignedAggregateAndProofBase,
SignedAggregateAndProofElectra, SignedBeaconBlock, SignedBeaconBlockAltair,
SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella,
SignedBeaconBlockDeneb, SignedBeaconBlockElectra, SignedBlsToExecutionChange,
SignedContributionAndProof, SignedVoluntaryExit, SubnetId, SyncCommitteeMessage, SyncSubnetId,
};

Expand Down Expand Up @@ -156,19 +157,18 @@ impl<E: EthSpec> PubsubMessage<E> {
GossipKind::BeaconAggregateAndProof => {
let signed_aggregate_and_proof =
match fork_context.from_context_bytes(gossip_topic.fork_digest) {
Some(&fork_name) => {
if fork_name.electra_enabled() {
SignedAggregateAndProof::Electra(
SignedAggregateAndProofElectra::from_ssz_bytes(data)
.map_err(|e| format!("{:?}", e))?,
)
} else {
SignedAggregateAndProof::Base(
SignedAggregateAndProofBase::from_ssz_bytes(data)
.map_err(|e| format!("{:?}", e))?,
)
}
}
Some(ForkName::Base)
| Some(ForkName::Altair)
| Some(ForkName::Bellatrix)
| Some(ForkName::Capella)
| Some(ForkName::Deneb) => SignedAggregateAndProof::Base(
SignedAggregateAndProofBase::from_ssz_bytes(data)
.map_err(|e| format!("{:?}", e))?,
),
Some(ForkName::Electra) => SignedAggregateAndProof::Electra(
SignedAggregateAndProofElectra::from_ssz_bytes(data)
.map_err(|e| format!("{:?}", e))?,
),
None => {
return Err(format!(
"Unknown gossipsub fork digest: {:?}",
Expand Down Expand Up @@ -402,9 +402,9 @@ impl<E: EthSpec> std::fmt::Display for PubsubMessage<E> {
PubsubMessage::AggregateAndProofAttestation(att) => write!(
f,
"Aggregate and Proof: slot: {}, index: {}, aggregator_index: {}",
att.message.aggregate.data().slot,
att.message.aggregate.data().index,
att.message.aggregator_index,
att.message().aggregate().data().slot,
att.message().aggregate().data().index,
att.message().aggregator_index(),
),
PubsubMessage::Attestation(data) => write!(
f,
Expand Down
15 changes: 10 additions & 5 deletions beacon_node/network/src/network_beacon_processor/gossip_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
use store::hot_cold_store::HotColdDBError;
use tokio::sync::mpsc;
use types::{
beacon_block::BlockImportSource, Attestation, AttestationRef, AttesterSlashing, BlobSidecar,
EthSpec, Hash256, IndexedAttestation, LightClientFinalityUpdate, LightClientOptimisticUpdate,
ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange,
Attestation, AttestationRef, AttesterSlashing, BlobSidecar, EthSpec, Hash256,
IndexedAttestation, LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing,
SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange,
SignedContributionAndProof, SignedVoluntaryExit, Slot, SubnetId, SyncCommitteeMessage,
SyncSubnetId,
};
Expand Down Expand Up @@ -105,7 +105,12 @@ impl<T: BeaconChainTypes> VerifiedAttestation<T> for VerifiedAggregate<T> {

/// Efficient clone-free implementation that moves out of the `Box`.
fn into_attestation_and_indices(self) -> (Attestation<T::EthSpec>, Vec<u64>) {
let attestation = self.signed_aggregate.message.aggregate;
// TODO(electra): technically we shouldn't have to clone..
let attestation = self
.signed_aggregate
.message()
.aggregate()
.clone_as_attestation();
let attesting_indices = self.indexed_attestation.attesting_indices_to_vec();
(attestation, attesting_indices)
}
Expand Down Expand Up @@ -412,7 +417,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
reprocess_tx: Option<mpsc::Sender<ReprocessQueueMessage>>,
seen_timestamp: Duration,
) {
let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root;
let beacon_block_root = aggregate.message().aggregate().data().beacon_block_root;

let result = match self
.chain
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/network/src/network_beacon_processor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
processor.process_gossip_aggregate_batch(aggregates, Some(reprocess_tx))
};

let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root;
let beacon_block_root = aggregate.message().aggregate().data().beacon_block_root;
self.try_send(BeaconWorkEvent {
drop_during_sync: true,
work: Work::GossipAggregate {
Expand Down
11 changes: 5 additions & 6 deletions beacon_node/operation_pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,7 @@ mod release_tests {
.unwrap();

let att1_indices = get_attesting_indices_from_state(&state, att1.to_ref()).unwrap();
let att2_indices = get_attesting_indices_from_state(&state, att2.to_ref()).unwrap();
let att2_indices = get_attesting_indices_from_state(&state, att2).unwrap();
let att1_split = SplitAttestation::new(att1.clone(), att1_indices);
let att2_split = SplitAttestation::new(att2.clone_as_attestation(), att2_indices);

Expand Down Expand Up @@ -1064,14 +1064,13 @@ mod release_tests {
);

for (_, aggregate) in attestations {
let att = aggregate.unwrap().message.aggregate;
let attesting_indices = get_attesting_indices_from_state(&state, att.to_ref()).unwrap();
let agg = aggregate.unwrap();
let att = agg.message().aggregate();
let attesting_indices = get_attesting_indices_from_state(&state, att).unwrap();
op_pool
.insert_attestation(att.clone_as_attestation(), attesting_indices.clone())
.unwrap();
op_pool
.insert_attestation(att.clone_as_attestation(), attesting_indices)
.unwrap();
op_pool.insert_attestation(att.clone_as_attestation(), attesting_indices).unwrap();
}

assert_eq!(op_pool.num_attestations(), committees.len());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ where
E: EthSpec,
F: Fn(usize) -> Option<Cow<'a, PublicKey>>,
{
let slot = signed_aggregate_and_proof.message.aggregate.data().slot;
let slot = signed_aggregate_and_proof.message().aggregate().data().slot;

let domain = spec.get_domain(
slot.epoch(E::slots_per_epoch()),
Expand All @@ -436,6 +436,7 @@ where
let message = slot.signing_root(domain);
let signature = signed_aggregate_and_proof.message().selection_proof();
let validator_index = signed_aggregate_and_proof.message().aggregator_index();

Ok(SignatureSet::single_pubkey(
signature,
get_pubkey(validator_index as usize).ok_or(Error::ValidatorUnknown(validator_index))?,
Expand All @@ -455,8 +456,8 @@ where
F: Fn(usize) -> Option<Cow<'a, PublicKey>>,
{
let target_epoch = signed_aggregate_and_proof
.message
.aggregate
.message()
.aggregate()
.data()
.target
.epoch;
Expand Down
Loading

0 comments on commit 7f32da8

Please sign in to comment.