Skip to content

Commit

Permalink
superstruct the AttesterSlashing (#5636)
Browse files Browse the repository at this point in the history
* `superstruct` Attester Fork Variants

* Push a little further

* Deal with Encode / Decode of AttesterSlashing

* not so sure about this..

* Stop Encode/Decode Bounds from Propagating Out

* Tons of Changes..

* More Conversions to AttestationRef

* Add AsReference trait (#15)

* Add AsReference trait

* Fix some snafus

* Got it Compiling! :D

* Got Tests Building

* Get beacon chain tests compiling

---------

Co-authored-by: Michael Sproul <micsproul@gmail.com>
  • Loading branch information
ethDreamer and michaelsproul authored May 2, 2024
1 parent 3b7132b commit e6c7f14
Show file tree
Hide file tree
Showing 53 changed files with 1,408 additions and 440 deletions.
8 changes: 4 additions & 4 deletions beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
if chain
.observed_attestations
.write()
.is_known_subset(attestation, attestation_data_root)
.is_known_subset(attestation.to_ref(), attestation_data_root)
.map_err(|e| Error::BeaconChainError(e.into()))?
{
metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
Expand Down Expand Up @@ -559,7 +559,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
return Err(Error::AggregatorNotInCommittee { aggregator_index });
}

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

Expand Down Expand Up @@ -597,7 +597,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
if let ObserveOutcome::Subset = chain
.observed_attestations
.write()
.observe_item(attestation, Some(attestation_data_root))
.observe_item(attestation.to_ref(), Some(attestation_data_root))
.map_err(|e| Error::BeaconChainError(e.into()))?
{
metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
Expand Down Expand Up @@ -1241,7 +1241,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
attestation: &Attestation<T::EthSpec>,
) -> Result<(IndexedAttestation<T::EthSpec>, CommitteesPerSlot), Error> {
map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| {
get_indexed_attestation(committee.committee, attestation)
get_indexed_attestation(committee.committee, attestation.to_ref())
.map(|attestation| (attestation, committees_per_slot))
.map_err(Error::Invalid)
})
Expand Down
107 changes: 88 additions & 19 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2113,7 +2113,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
.fork_choice_write_lock()
.on_attestation(
self.slot()?,
verified.indexed_attestation(),
verified.indexed_attestation().to_ref(),
AttestationFromBlock::False,
)
.map_err(Into::into)
Expand Down Expand Up @@ -2465,7 +2465,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// Add to fork choice.
self.canonical_head
.fork_choice_write_lock()
.on_attester_slashing(attester_slashing.as_inner());
.on_attester_slashing(attester_slashing.as_inner().to_ref());

// Add to the op pool (if we have the ability to propose blocks).
if self.eth1_chain.is_some() {
Expand Down Expand Up @@ -3820,7 +3820,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
continue;
}
};
slasher.accept_attestation(indexed_attestation.clone());
slasher.accept_attestation(indexed_attestation.clone_as_indexed_attestation());
}
}
}
Expand All @@ -3839,7 +3839,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
if block.slot() + 2 * T::EthSpec::slots_per_epoch() >= current_slot {
metrics::observe(
&metrics::OPERATIONS_PER_BLOCK_ATTESTATION,
block.body().attestations().len() as f64,
block.body().attestations().count() as f64,
);

if let Ok(sync_aggregate) = block.body().sync_aggregate() {
Expand Down Expand Up @@ -4859,7 +4859,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
metrics::start_timer(&metrics::BLOCK_PRODUCTION_UNAGGREGATED_TIMES);
for attestation in self.naive_aggregation_pool.read().iter() {
let import = |attestation: &Attestation<T::EthSpec>| {
let attesting_indices = get_attesting_indices_from_state(&state, attestation)?;
let attesting_indices =
get_attesting_indices_from_state(&state, attestation.to_ref())?;
self.op_pool
.insert_attestation(attestation.clone(), attesting_indices)
};
Expand Down Expand Up @@ -4909,7 +4910,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
attestations.retain(|att| {
verify_attestation_for_block_inclusion(
&state,
att,
att.to_ref(),
&mut tmp_ctxt,
VerifySignatures::True,
&self.spec,
Expand Down Expand Up @@ -5040,6 +5041,72 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
bls_to_execution_changes,
} = partial_beacon_block;

let (attester_slashings_base, attester_slashings_electra) =
attester_slashings.into_iter().fold(
(Vec::new(), Vec::new()),
|(mut base, mut electra), slashing| {
match slashing {
AttesterSlashing::Base(slashing) => base.push(slashing),
AttesterSlashing::Electra(slashing) => electra.push(slashing),
}
(base, electra)
},
);
let (attestations_base, attestations_electra) = attestations.into_iter().fold(
(Vec::new(), Vec::new()),
|(mut base, mut electra), attestation| {
match attestation {
Attestation::Base(attestation) => base.push(attestation),
Attestation::Electra(attestation) => electra.push(attestation),
}
(base, electra)
},
);

// TODO(electra): figure out what should *actually* be done here when we have attestations / attester_slashings of the wrong type
match &state {
BeaconState::Base(_)
| BeaconState::Altair(_)
| BeaconState::Merge(_)
| BeaconState::Capella(_)
| BeaconState::Deneb(_) => {
if !attestations_electra.is_empty() {
error!(
self.log,
"Tried to produce block with attestations of the wrong type";
"slot" => slot,
"attestations" => attestations_electra.len(),
);
}
if !attester_slashings_electra.is_empty() {
error!(
self.log,
"Tried to produce block with attester slashings of the wrong type";
"slot" => slot,
"attester_slashings" => attester_slashings_electra.len(),
);
}
}
BeaconState::Electra(_) => {
if !attestations_base.is_empty() {
error!(
self.log,
"Tried to produce block with attestations of the wrong type";
"slot" => slot,
"attestations" => attestations_base.len(),
);
}
if !attester_slashings_base.is_empty() {
error!(
self.log,
"Tried to produce block with attester slashings of the wrong type";
"slot" => slot,
"attester_slashings" => attester_slashings_base.len(),
);
}
}
};

let (inner_block, maybe_blobs_and_proofs, execution_payload_value) = match &state {
BeaconState::Base(_) => (
BeaconBlock::Base(BeaconBlockBase {
Expand All @@ -5052,8 +5119,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
eth1_data,
graffiti,
proposer_slashings: proposer_slashings.into(),
attester_slashings: attester_slashings.into(),
attestations: attestations.into(),
attester_slashings: attester_slashings_base.into(),
attestations: attestations_base.into(),
deposits: deposits.into(),
voluntary_exits: voluntary_exits.into(),
_phantom: PhantomData,
Expand All @@ -5073,8 +5140,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
eth1_data,
graffiti,
proposer_slashings: proposer_slashings.into(),
attester_slashings: attester_slashings.into(),
attestations: attestations.into(),
attester_slashings: attester_slashings_base.into(),
attestations: attestations_base.into(),
deposits: deposits.into(),
voluntary_exits: voluntary_exits.into(),
sync_aggregate: sync_aggregate
Expand All @@ -5100,8 +5167,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
eth1_data,
graffiti,
proposer_slashings: proposer_slashings.into(),
attester_slashings: attester_slashings.into(),
attestations: attestations.into(),
attester_slashings: attester_slashings_base.into(),
attestations: attestations_base.into(),
deposits: deposits.into(),
voluntary_exits: voluntary_exits.into(),
sync_aggregate: sync_aggregate
Expand Down Expand Up @@ -5132,8 +5199,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
eth1_data,
graffiti,
proposer_slashings: proposer_slashings.into(),
attester_slashings: attester_slashings.into(),
attestations: attestations.into(),
attester_slashings: attester_slashings_base.into(),
attestations: attestations_base.into(),
deposits: deposits.into(),
voluntary_exits: voluntary_exits.into(),
sync_aggregate: sync_aggregate
Expand Down Expand Up @@ -5166,8 +5233,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
eth1_data,
graffiti,
proposer_slashings: proposer_slashings.into(),
attester_slashings: attester_slashings.into(),
attestations: attestations.into(),
attester_slashings: attester_slashings_base.into(),
attestations: attestations_base.into(),
deposits: deposits.into(),
voluntary_exits: voluntary_exits.into(),
sync_aggregate: sync_aggregate
Expand Down Expand Up @@ -5204,8 +5271,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
eth1_data,
graffiti,
proposer_slashings: proposer_slashings.into(),
attester_slashings: attester_slashings.into(),
attestations: attestations.into(),
attester_slashings: attester_slashings_electra.into(),
attestations: attestations_electra.into(),
deposits: deposits.into(),
voluntary_exits: voluntary_exits.into(),
sync_aggregate: sync_aggregate
Expand All @@ -5216,6 +5283,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
bls_to_execution_changes: bls_to_execution_changes.into(),
blob_kzg_commitments: kzg_commitments
.ok_or(BlockProductionError::InvalidPayloadFork)?,
// TODO(electra): finish consolidations when they're more spec'd out
consolidations: Vec::new().into(),
},
}),
maybe_blobs_and_proofs,
Expand Down Expand Up @@ -5321,7 +5390,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
self.log,
"Produced beacon block";
"parent" => ?block.parent_root(),
"attestations" => block.body().attestations().len(),
"attestations" => block.body().attestations_len(),
"slot" => block.slot()
);

Expand Down
7 changes: 4 additions & 3 deletions beacon_node/beacon_chain/src/block_reward.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
let split_attestations = block
.body()
.attestations()
.iter()
.map(|att| {
let attesting_indices = get_attesting_indices_from_state(state, att)?;
Ok(SplitAttestation::new(att.clone(), attesting_indices))
Ok(SplitAttestation::new(
att.clone_as_attestation(),
attesting_indices,
))
})
.collect::<Result<Vec<_>, BeaconChainError>>()?;

Expand Down Expand Up @@ -86,7 +88,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
block
.body()
.attestations()
.iter()
.map(|a| a.data().clone())
.collect()
} else {
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1623,7 +1623,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
}

// Register each attestation in the block with fork choice.
for (i, attestation) in block.message().body().attestations().iter().enumerate() {
for (i, attestation) in block.message().body().attestations().enumerate() {
let _fork_choice_attestation_timer =
metrics::start_timer(&metrics::FORK_CHOICE_PROCESS_ATTESTATION_TIMES);

Expand Down
Loading

0 comments on commit e6c7f14

Please sign in to comment.