Skip to content

Commit 57ec167

Browse files
ethDreamerrealbigsean
ethDreamer
authored and
realbigsean
committed
Superstruct AggregateAndProof (sigp#5715)
* Upgrade `superstruct` to `0.8.0` * superstruct `AggregateAndProof`
1 parent 74746e3 commit 57ec167

File tree

20 files changed

+193
-174
lines changed

20 files changed

+193
-174
lines changed

‎Cargo.lock

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ smallvec = "1.11.2"
162162
snap = "1"
163163
ssz_types = "0.6"
164164
strum = { version = "0.24", features = ["derive"] }
165-
superstruct = "0.8"
165+
superstruct = { git = "https://github.com/sigp/superstruct", rev = "45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" }
166166
syn = "1"
167167
sysinfo = "0.26"
168168
tempfile = "3"

‎beacon_node/beacon_chain/src/attestation_verification.rs

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,8 @@ use std::borrow::Cow;
6161
use strum::AsRefStr;
6262
use tree_hash::TreeHash;
6363
use types::{
64-
Attestation, AttestationRef, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec,
65-
CommitteeIndex, Epoch, EthSpec, Hash256, IndexedAttestation, SelectionProof,
66-
SignedAggregateAndProof, Slot, SubnetId,
64+
Attestation, AttestationRef, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec,
65+
ForkName, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
6766
};
6867

6968
pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
@@ -348,7 +347,7 @@ pub trait VerifiedAttestation<T: BeaconChainTypes>: Sized {
348347

349348
// Inefficient default implementation. This is overridden for gossip verified attestations.
350349
fn into_attestation_and_indices(self) -> (Attestation<T::EthSpec>, Vec<u64>) {
351-
let attestation = self.attestation().clone();
350+
let attestation = self.attestation().clone_as_attestation();
352351
let attesting_indices = self.indexed_attestation().attesting_indices_to_vec();
353352
(attestation, attesting_indices)
354353
}
@@ -496,7 +495,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
496495
if chain
497496
.observed_attestations
498497
.write()
499-
.is_known_subset(attestation.to_ref(), attestation_data_root)
498+
.is_known_subset(attestation, attestation_data_root)
500499
.map_err(|e| Error::BeaconChainError(e.into()))?
501500
{
502501
metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
@@ -558,8 +557,10 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
558557
chain: &BeaconChain<T>,
559558
) -> Result<Self, AttestationSlashInfo<'a, T, Error>> {
560559
use AttestationSlashInfo::*;
561-
let observed_attestation_key_root = match Self::verify_early_checks(signed_aggregate, chain)
562-
{
560+
561+
let attestation = signed_aggregate.message().aggregate();
562+
let aggregator_index = signed_aggregate.message().aggregator_index();
563+
let attestation_data_root = match Self::verify_early_checks(signed_aggregate, chain) {
563564
Ok(root) => root,
564565
Err(e) => {
565566
return Err(SignatureNotChecked(
@@ -571,28 +572,12 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
571572

572573
// Committees must be sorted by ascending index order 0..committees_per_slot
573574
let get_indexed_attestation_with_committee =
574-
|(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
575-
let (index, aggregator_index, selection_proof, data) = match signed_aggregate {
576-
SignedAggregateAndProof::Base(signed_aggregate) => (
577-
signed_aggregate.message.aggregate.data.index,
578-
signed_aggregate.message.aggregator_index,
579-
// Note: this clones the signature which is known to be a relatively slow operation.
580-
// Future optimizations should remove this clone.
581-
signed_aggregate.message.selection_proof.clone(),
582-
signed_aggregate.message.aggregate.data.clone(),
583-
),
584-
SignedAggregateAndProof::Electra(signed_aggregate) => (
585-
signed_aggregate
586-
.message
587-
.aggregate
588-
.committee_index()
589-
.ok_or(Error::NotExactlyOneCommitteeBitSet(0))?,
590-
signed_aggregate.message.aggregator_index,
591-
signed_aggregate.message.selection_proof.clone(),
592-
signed_aggregate.message.aggregate.data.clone(),
593-
),
594-
};
595-
let slot = data.slot;
575+
|(committee, _): (BeaconCommittee, CommitteesPerSlot)| {
576+
// Note: this clones the signature which is known to be a relatively slow operation.
577+
//
578+
// Future optimizations should remove this clone.
579+
let selection_proof =
580+
SelectionProof::from(signed_aggregate.message().selection_proof().clone());
596581

597582
let committee = committees
598583
.get(index as usize)
@@ -610,7 +595,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
610595
return Err(Error::AggregatorNotInCommittee { aggregator_index });
611596
}
612597

613-
get_indexed_attestation(committee.committee, attestation.to_ref())
598+
get_indexed_attestation(committee.committee, attestation)
614599
.map_err(|e| BeaconChainError::from(e).into())
615600
};
616601

@@ -653,7 +638,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
653638
if let ObserveOutcome::Subset = chain
654639
.observed_attestations
655640
.write()
656-
.observe_item(attestation.to_ref(), Some(attestation_data_root))
641+
.observe_item(attestation, Some(attestation_data_root))
657642
.map_err(|e| Error::BeaconChainError(e.into()))?
658643
{
659644
metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
@@ -1326,7 +1311,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
13261311
attestation: AttestationRef<T::EthSpec>,
13271312
) -> Result<(IndexedAttestation<T::EthSpec>, CommitteesPerSlot), Error> {
13281313
map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| {
1329-
get_indexed_attestation(committee.committee, attestation.to_ref())
1314+
get_indexed_attestation(committee.committee, attestation)
13301315
.map(|attestation| (attestation, committees_per_slot))
13311316
.map_err(Error::Invalid)
13321317
})

‎beacon_node/beacon_chain/src/naive_aggregation_pool.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
use crate::metrics;
22
use crate::observed_aggregates::AsReference;
3-
use itertools::Itertools;
4-
use smallvec::SmallVec;
53
use std::collections::HashMap;
64
use tree_hash::{MerkleHasher, TreeHash, TreeHashType};
75
use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT;
86
use types::slot_data::SlotData;
97
use types::sync_committee_contribution::SyncContributionData;
108
use types::{
11-
Attestation, AttestationData, AttestationRef, CommitteeIndex, EthSpec, Hash256, Slot,
12-
SyncCommitteeContribution,
9+
Attestation, AttestationData, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution,
1310
};
1411

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

244241
let set_bits = match a {
245-
Attestation::Base(att) => att
242+
AttestationRef::Base(att) => att
246243
.aggregation_bits
247244
.iter()
248245
.enumerate()
249246
.filter(|(_i, bit)| *bit)
250247
.map(|(i, _bit)| i)
251248
.collect::<Vec<_>>(),
252-
Attestation::Electra(att) => att
249+
AttestationRef::Electra(att) => att
253250
.aggregation_bits
254251
.iter()
255252
.enumerate()
@@ -290,10 +287,8 @@ impl<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
290287
}
291288

292289
self.map
293-
.insert(attestation_key_root, a.clone_as_attestation());
294-
Ok(InsertOutcome::NewItemInserted {
295-
committee_index: aggregation_bit,
296-
})
290+
.insert(attestation_data_root, a.clone_as_attestation());
291+
Ok(InsertOutcome::NewItemInserted { committee_index })
297292
}
298293
}
299294

‎beacon_node/beacon_chain/src/test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1382,7 +1382,7 @@ where
13821382
committee_attestations.iter().skip(1).fold(
13831383
attestation.clone(),
13841384
|mut agg, (att, _)| {
1385-
agg.aggregate(att);
1385+
agg.aggregate(att.to_ref());
13861386
agg
13871387
},
13881388
)

‎beacon_node/http_api/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3368,9 +3368,9 @@ pub fn serve<T: BeaconChainTypes>(
33683368
"Failure verifying aggregate and proofs";
33693369
"error" => format!("{:?}", e),
33703370
"request_index" => index,
3371-
"aggregator_index" => aggregate.message.aggregator_index,
3372-
"attestation_index" => aggregate.message.aggregate.data().index,
3373-
"attestation_slot" => aggregate.message.aggregate.data().slot,
3371+
"aggregator_index" => aggregate.message().aggregator_index(),
3372+
"attestation_index" => aggregate.message().aggregate().data().index,
3373+
"attestation_slot" => aggregate.message().aggregate().data().slot,
33743374
);
33753375
failures.push(api_types::Failure::new(index, format!("Verification: {:?}", e)));
33763376
}
@@ -3389,7 +3389,7 @@ pub fn serve<T: BeaconChainTypes>(
33893389
"Failure applying verified aggregate attestation to fork choice";
33903390
"error" => format!("{:?}", e),
33913391
"request_index" => index,
3392-
"aggregator_index" => verified_aggregate.aggregate().message.aggregator_index,
3392+
"aggregator_index" => verified_aggregate.aggregate().message().aggregator_index(),
33933393
"attestation_index" => verified_aggregate.attestation().data().index,
33943394
"attestation_slot" => verified_aggregate.attestation().data().slot,
33953395
);

‎beacon_node/http_api/tests/tests.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3334,8 +3334,14 @@ impl ApiTester {
33343334

33353335
pub async fn test_get_validator_aggregate_and_proofs_invalid(mut self) -> Self {
33363336
let mut aggregate = self.get_aggregate().await;
3337-
3338-
aggregate.message.aggregate.data_mut().slot += 1;
3337+
match &mut aggregate {
3338+
SignedAggregateAndProof::Base(ref mut aggregate) => {
3339+
aggregate.message.aggregate.data.slot += 1;
3340+
}
3341+
SignedAggregateAndProof::Electra(ref mut aggregate) => {
3342+
aggregate.message.aggregate.data.slot += 1;
3343+
}
3344+
}
33393345

33403346
self.client
33413347
.post_validator_aggregate_and_proof::<E>(&[aggregate])

‎beacon_node/lighthouse_network/src/types/pubsub.rs

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ use std::sync::Arc;
99
use types::{
1010
Attestation, AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, BlobSidecar,
1111
EthSpec, ForkContext, ForkName, LightClientFinalityUpdate, LightClientOptimisticUpdate,
12-
ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBeaconBlockAltair,
13-
SignedBeaconBlockBase, SignedBeaconBlockCapella, SignedBeaconBlockDeneb,
14-
SignedBeaconBlockElectra, SignedBeaconBlockMerge, SignedBlsToExecutionChange,
12+
ProposerSlashing, SignedAggregateAndProof, SignedAggregateAndProofBase,
13+
SignedAggregateAndProofElectra, SignedBeaconBlock, SignedBeaconBlockAltair,
14+
SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella,
15+
SignedBeaconBlockDeneb, SignedBeaconBlockElectra, SignedBlsToExecutionChange,
1516
SignedContributionAndProof, SignedVoluntaryExit, SubnetId, SyncCommitteeMessage, SyncSubnetId,
1617
};
1718

@@ -156,19 +157,18 @@ impl<E: EthSpec> PubsubMessage<E> {
156157
GossipKind::BeaconAggregateAndProof => {
157158
let signed_aggregate_and_proof =
158159
match fork_context.from_context_bytes(gossip_topic.fork_digest) {
159-
Some(&fork_name) => {
160-
if fork_name.electra_enabled() {
161-
SignedAggregateAndProof::Electra(
162-
SignedAggregateAndProofElectra::from_ssz_bytes(data)
163-
.map_err(|e| format!("{:?}", e))?,
164-
)
165-
} else {
166-
SignedAggregateAndProof::Base(
167-
SignedAggregateAndProofBase::from_ssz_bytes(data)
168-
.map_err(|e| format!("{:?}", e))?,
169-
)
170-
}
171-
}
160+
Some(ForkName::Base)
161+
| Some(ForkName::Altair)
162+
| Some(ForkName::Bellatrix)
163+
| Some(ForkName::Capella)
164+
| Some(ForkName::Deneb) => SignedAggregateAndProof::Base(
165+
SignedAggregateAndProofBase::from_ssz_bytes(data)
166+
.map_err(|e| format!("{:?}", e))?,
167+
),
168+
Some(ForkName::Electra) => SignedAggregateAndProof::Electra(
169+
SignedAggregateAndProofElectra::from_ssz_bytes(data)
170+
.map_err(|e| format!("{:?}", e))?,
171+
),
172172
None => {
173173
return Err(format!(
174174
"Unknown gossipsub fork digest: {:?}",
@@ -402,9 +402,9 @@ impl<E: EthSpec> std::fmt::Display for PubsubMessage<E> {
402402
PubsubMessage::AggregateAndProofAttestation(att) => write!(
403403
f,
404404
"Aggregate and Proof: slot: {}, index: {}, aggregator_index: {}",
405-
att.message.aggregate.data().slot,
406-
att.message.aggregate.data().index,
407-
att.message.aggregator_index,
405+
att.message().aggregate().data().slot,
406+
att.message().aggregate().data().index,
407+
att.message().aggregator_index(),
408408
),
409409
PubsubMessage::Attestation(data) => write!(
410410
f,

‎beacon_node/network/src/network_beacon_processor/gossip_methods.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
3131
use store::hot_cold_store::HotColdDBError;
3232
use tokio::sync::mpsc;
3333
use types::{
34-
beacon_block::BlockImportSource, Attestation, AttestationRef, AttesterSlashing, BlobSidecar,
35-
EthSpec, Hash256, IndexedAttestation, LightClientFinalityUpdate, LightClientOptimisticUpdate,
36-
ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange,
34+
Attestation, AttestationRef, AttesterSlashing, BlobSidecar, EthSpec, Hash256,
35+
IndexedAttestation, LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing,
36+
SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange,
3737
SignedContributionAndProof, SignedVoluntaryExit, Slot, SubnetId, SyncCommitteeMessage,
3838
SyncSubnetId,
3939
};
@@ -105,7 +105,12 @@ impl<T: BeaconChainTypes> VerifiedAttestation<T> for VerifiedAggregate<T> {
105105

106106
/// Efficient clone-free implementation that moves out of the `Box`.
107107
fn into_attestation_and_indices(self) -> (Attestation<T::EthSpec>, Vec<u64>) {
108-
let attestation = self.signed_aggregate.message.aggregate;
108+
// TODO(electra): technically we shouldn't have to clone..
109+
let attestation = self
110+
.signed_aggregate
111+
.message()
112+
.aggregate()
113+
.clone_as_attestation();
109114
let attesting_indices = self.indexed_attestation.attesting_indices_to_vec();
110115
(attestation, attesting_indices)
111116
}
@@ -412,7 +417,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
412417
reprocess_tx: Option<mpsc::Sender<ReprocessQueueMessage>>,
413418
seen_timestamp: Duration,
414419
) {
415-
let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root;
420+
let beacon_block_root = aggregate.message().aggregate().data().beacon_block_root;
416421

417422
let result = match self
418423
.chain

‎beacon_node/network/src/network_beacon_processor/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
142142
processor.process_gossip_aggregate_batch(aggregates, Some(reprocess_tx))
143143
};
144144

145-
let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root;
145+
let beacon_block_root = aggregate.message().aggregate().data().beacon_block_root;
146146
self.try_send(BeaconWorkEvent {
147147
drop_during_sync: true,
148148
work: Work::GossipAggregate {

0 commit comments

Comments
 (0)