Skip to content

Commit

Permalink
Addressed sigp#4444
Browse files Browse the repository at this point in the history 
Attestation Verification Post-Deneb
Fix Gossip Attestation Verification Test
  • Loading branch information
@ethDreamer
ethDreamer committed Jul 27, 2023
1 parent 2b3adbe commit 12a2ee2
Show file tree
Hide file tree
Showing 10 changed files with 390 additions and 56 deletions.
22 changes: 17 additions & 5 deletions beacon_node/beacon_chain/src/attestation_verification.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Attestation, BeaconCommittee, CommitteeIndex, Epoch, EthSpec, Hash256, IndexedAttestation,
SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
Attestation, 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 @@ -454,7 +454,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
//
// We do not queue future attestations for later processing.
verify_propagation_slot_range(&chain.slot_clock, attestation)?;
verify_propagation_slot_range(&chain.slot_clock, attestation, &chain.spec)?;

// Check the attestation's epoch matches its target.
if attestation.data.slot.epoch(T::EthSpec::slots_per_epoch())
Expand Down Expand Up @@ -722,7 +722,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
//
// We do not queue future attestations for later processing.
verify_propagation_slot_range(&chain.slot_clock, attestation)?;
verify_propagation_slot_range(&chain.slot_clock, attestation, &chain.spec)?;

// Check to ensure that the attestation is "unaggregated". I.e., it has exactly one
// aggregation bit set.
Expand Down Expand Up @@ -1037,6 +1037,7 @@ fn verify_head_block_is_known<T: BeaconChainTypes>(
pub fn verify_propagation_slot_range<S: SlotClock, E: EthSpec>(
slot_clock: &S,
attestation: &Attestation<E>,
spec: &ChainSpec,
) -> Result<(), Error> {
let attestation_slot = attestation.data.slot;

Expand All @@ -1051,10 +1052,21 @@ pub fn verify_propagation_slot_range<S: SlotClock, E: EthSpec>(
}

// Taking advantage of saturating subtraction on `Slot`.
let earliest_permissible_slot = slot_clock
let one_epoch_prior = slot_clock
.now_with_past_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY)
.ok_or(BeaconChainError::UnableToReadSlot)?
- E::slots_per_epoch();

let current_fork =
spec.fork_name_at_slot::<E>(slot_clock.now().ok_or(BeaconChainError::UnableToReadSlot)?);
let earliest_permissible_slot = match current_fork {
ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => one_epoch_prior,
// EIP-7045
ForkName::Deneb => one_epoch_prior
.epoch(E::slots_per_epoch())
.start_slot(E::slots_per_epoch()),
};

if attestation_slot < earliest_permissible_slot {
return Err(Error::PastSlot {
attestation_slot,
Expand Down
102 changes: 88 additions & 14 deletions beacon_node/beacon_chain/src/beacon_block_reward.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use safe_arith::SafeArith;
use slog::error;
use state_processing::{
common::{
altair, get_attestation_participation_flag_indices, get_attesting_indices_from_state,
altair, get_attestation_participation_flag_indices_altair,
get_attestation_participation_flag_indices_deneb, get_attesting_indices_from_state,
},
per_block_processing::{
altair::sync_committee::compute_sync_aggregate_rewards, get_slashable_indices,
Expand Down Expand Up @@ -60,26 +61,37 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
BeaconChainError::BlockRewardError
})?;

let block_attestation_reward = if let BeaconState::Base(_) = state {
self.compute_beacon_block_attestation_reward_base(block, block_root, state)
let block_attestation_reward = match state {
BeaconState::Base(_) => self
.compute_beacon_block_attestation_reward_base(block, block_root, state)
.map_err(|e| {
error!(
self.log,
"Error calculating base block attestation reward";
"error" => ?e
self.log,
"Error calculating base block attestation reward";
"error" => ?e
);
BeaconChainError::BlockRewardAttestationError
})?
} else {
self.compute_beacon_block_attestation_reward_altair(block, state)
})?,
BeaconState::Altair(_) | BeaconState::Merge(_) | BeaconState::Capella(_) => self
.compute_beacon_block_attestation_reward_altair(block, state)
.map_err(|e| {
error!(
self.log,
"Error calculating altair block attestation reward";
"error" => ?e
);
BeaconChainError::BlockRewardAttestationError
})?,
BeaconState::Deneb(_) => self
.compute_beacon_block_attestation_reward_deneb(block, state)
.map_err(|e| {
error!(
self.log,
"Error calculating altair block attestation reward";
"error" => ?e
self.log,
"Error calculating deneb block attestation reward";
"error" => ?e
);
BeaconChainError::BlockRewardAttestationError
})?
})?,
};

let total_reward = sync_aggregate_reward
Expand Down Expand Up @@ -192,7 +204,69 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
for attestation in block.body().attestations() {
let data = &attestation.data;
let inclusion_delay = state.slot().safe_sub(data.slot)?.as_u64();
let participation_flag_indices = get_attestation_participation_flag_indices(
let participation_flag_indices = get_attestation_participation_flag_indices_altair(
state,
data,
inclusion_delay,
&self.spec,
)?;

let attesting_indices = get_attesting_indices_from_state(state, attestation)?;

let mut proposer_reward_numerator = 0;
for index in attesting_indices {
let index = index as usize;
for (flag_index, &weight) in PARTICIPATION_FLAG_WEIGHTS.iter().enumerate() {
let epoch_participation =
state.get_epoch_participation_mut(data.target.epoch)?;
let validator_participation = epoch_participation
.get_mut(index)
.ok_or(BeaconStateError::ParticipationOutOfBounds(index))?;

if participation_flag_indices.contains(&flag_index)
&& !validator_participation.has_flag(flag_index)?
{
validator_participation.add_flag(flag_index)?;
proposer_reward_numerator.safe_add_assign(
altair::get_base_reward(
state,
index,
base_reward_per_increment,
&self.spec,
)?
.safe_mul(weight)?,
)?;
}
}
}
total_proposer_reward.safe_add_assign(
proposer_reward_numerator.safe_div(proposer_reward_denominator)?,
)?;
}

Ok(total_proposer_reward)
}

fn compute_beacon_block_attestation_reward_deneb<Payload: AbstractExecPayload<T::EthSpec>>(
&self,
block: BeaconBlockRef<'_, T::EthSpec, Payload>,
state: &mut BeaconState<T::EthSpec>,
) -> Result<BeaconBlockSubRewardValue, BeaconChainError> {
let total_active_balance = state.get_total_active_balance()?;
let base_reward_per_increment =
altair::BaseRewardPerIncrement::new(total_active_balance, &self.spec)?;

let mut total_proposer_reward = 0;

let proposer_reward_denominator = WEIGHT_DENOMINATOR
.safe_sub(PROPOSER_WEIGHT)?
.safe_mul(WEIGHT_DENOMINATOR)?
.safe_div(PROPOSER_WEIGHT)?;

for attestation in block.body().attestations() {
let data = &attestation.data;
let inclusion_delay = state.slot().safe_sub(data.slot)?.as_u64();
let participation_flag_indices = get_attestation_participation_flag_indices_deneb(
state,
data,
inclusion_delay,
Expand Down
48 changes: 34 additions & 14 deletions beacon_node/beacon_chain/tests/attestation_verification.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,28 @@ impl GossipTester {
self.harness.chain.epoch().unwrap()
}

pub fn two_epochs_ago(&self) -> Slot {
pub fn earliest_valid_attestation_slot(&self) -> Slot {
let offset = match self.harness.spec.fork_name_at_epoch(self.epoch()) {
ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => {
// Subtract an additional slot since the harness will be exactly on the start of the
// slot and the propagation tolerance will allow an extra slot.
E::slots_per_epoch() + 1
}
// EIP-7045
ForkName::Deneb => {
let epoch_slot_offset = (self.slot() % E::slots_per_epoch()).as_u64();
if epoch_slot_offset != 0 {
E::slots_per_epoch() + epoch_slot_offset
} else {
// Here the propagation tolerance will cause the cutoff to be an entire epoch earlier
2 * E::slots_per_epoch()
}
}
};

self.slot()
.as_u64()
.checked_sub(E::slots_per_epoch() + 2)
.checked_sub(offset)
.expect("chain is not sufficiently deep for test")
.into()
}
Expand Down Expand Up @@ -476,18 +494,21 @@ async fn aggregated_gossip_verification() {
)
.inspect_aggregate_err(
"aggregate from past slot",
|tester, a| a.message.aggregate.data.slot = tester.two_epochs_ago(),
|tester, a| {
let too_early_slot = tester.earliest_valid_attestation_slot() - 1;
a.message.aggregate.data.slot = too_early_slot;
a.message.aggregate.data.target.epoch = too_early_slot.epoch(E::slots_per_epoch());
},
|tester, err| {
let valid_early_slot = tester.earliest_valid_attestation_slot();
assert!(matches!(
err,
AttnError::PastSlot {
attestation_slot,
// Subtract an additional slot since the harness will be exactly on the start of the
// slot and the propagation tolerance will allow an extra slot.
earliest_permissible_slot
}
if attestation_slot == tester.two_epochs_ago()
&& earliest_permissible_slot == tester.slot() - E::slots_per_epoch() - 1
if attestation_slot == valid_early_slot - 1
&& earliest_permissible_slot == valid_early_slot
))
},
)
Expand Down Expand Up @@ -792,21 +813,20 @@ async fn unaggregated_gossip_verification() {
.inspect_unaggregate_err(
"attestation from past slot",
|tester, a, _| {
let early_slot = tester.two_epochs_ago();
a.data.slot = early_slot;
a.data.target.epoch = early_slot.epoch(E::slots_per_epoch());
let too_early_slot = tester.earliest_valid_attestation_slot() - 1;
a.data.slot = too_early_slot;
a.data.target.epoch = too_early_slot.epoch(E::slots_per_epoch());
},
|tester, err| {
let valid_early_slot = tester.earliest_valid_attestation_slot();
assert!(matches!(
err,
AttnError::PastSlot {
attestation_slot,
// Subtract an additional slot since the harness will be exactly on the start of the
// slot and the propagation tolerance will allow an extra slot.
earliest_permissible_slot,
}
if attestation_slot == tester.two_epochs_ago()
&& earliest_permissible_slot == tester.slot() - E::slots_per_epoch() - 1
if attestation_slot == valid_early_slot - 1
&& earliest_permissible_slot == valid_early_slot
))
},
)
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/network/src/network_beacon_processor/gossip_methods.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1798,6 +1798,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
attestation_verification::verify_propagation_slot_range(
seen_clock,
failed_att.attestation(),
&self.chain.spec,
);

// Only penalize the peer if it would have been invalid at the moment we received
Expand Down Expand Up @@ -2661,6 +2662,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
let is_timely = attestation_verification::verify_propagation_slot_range(
&self.chain.slot_clock,
attestation,
&self.chain.spec,
)
.is_ok();

Expand Down
Loading

0 comments on commit 12a2ee2

Please sign in to comment.