Add gossip conditions from spec v0.12.3 (#1667)

## Issue Addressed

NA

## Proposed Changes

There are four new conditions introduced in v0.12.3:

 1. _[REJECT]_ The attestation's epoch matches its target -- i.e. `attestation.data.target.epoch ==
  compute_epoch_at_slot(attestation.data.slot)`
1. _[REJECT]_ The attestation's target block is an ancestor of the block named in the LMD vote -- i.e.
  `get_ancestor(store, attestation.data.beacon_block_root, compute_start_slot_at_epoch(attestation.data.target.epoch)) == attestation.data.target.root`
1. _[REJECT]_ The committee index is within the expected range -- i.e. `data.index < get_committee_count_per_slot(state, data.target.epoch)`.
1. _[REJECT]_ The number of aggregation bits matches the committee size -- i.e.
  `len(attestation.aggregation_bits) == len(get_beacon_committee(state, data.slot, data.index))`.

This PR implements new logic to suit (1) and (2). Tests are added for (3) and (4), although they were already implicitly enforced.

## Additional Info

- There's a bit of edge-case with target root verification that I raised here: https://github.com/ethereum/eth2.0-specs/pull/2001#issuecomment-699246659
- I've had to add an `--ignore` to `cargo audit` to get CI to pass. See https://github.com/sigp/lighthouse/issues/1669
This commit is contained in:
Paul Hauner
2020-09-27 20:59:40 +00:00
parent f1180a8947
commit 1ef4f0ea12
5 changed files with 208 additions and 11 deletions

View File

@@ -37,6 +37,7 @@ use crate::{
BeaconChain, BeaconChainError, BeaconChainTypes,
};
use bls::verify_signature_sets;
use proto_array::Block as ProtoBlock;
use slog::debug;
use slot_clock::SlotClock;
use state_processing::{
@@ -226,6 +227,21 @@ pub enum Error {
head_block_slot: Slot,
attestation_slot: Slot,
},
/// The attestation has an invalid target epoch.
///
/// ## Peer scoring
///
/// The peer has sent an invalid message.
InvalidTargetEpoch { slot: Slot, epoch: Epoch },
/// The attestation references an invalid target block.
///
/// ## Peer scoring
///
/// The peer has sent an invalid message.
InvalidTargetRoot {
attestation: Hash256,
expected: Hash256,
},
/// There was an error whilst processing the attestation. It is not known if it is valid or invalid.
///
/// ## Peer scoring
@@ -425,6 +441,16 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
subnet_id: SubnetId,
chain: &BeaconChain<T>,
) -> Result<Self, Error> {
let attestation_epoch = attestation.data.slot.epoch(T::EthSpec::slots_per_epoch());
// Check the attestation's epoch matches its target.
if attestation_epoch != attestation.data.target.epoch {
return Err(Error::InvalidTargetEpoch {
slot: attestation.data.slot,
epoch: attestation.data.target.epoch,
});
}
// Ensure attestation is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots (within a
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
//
@@ -433,16 +459,49 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<T> {
// Check to ensure that the attestation is "unaggregated". I.e., it has exactly one
// aggregation bit set.
let num_aggreagtion_bits = attestation.aggregation_bits.num_set_bits();
if num_aggreagtion_bits != 1 {
return Err(Error::NotExactlyOneAggregationBitSet(num_aggreagtion_bits));
let num_aggregation_bits = attestation.aggregation_bits.num_set_bits();
if num_aggregation_bits != 1 {
return Err(Error::NotExactlyOneAggregationBitSet(num_aggregation_bits));
}
// Attestations must be for a known block. If the block is unknown, we simply drop the
// attestation and do not delay consideration for later.
//
// Enforce a maximum skip distance for unaggregated attestations.
verify_head_block_is_known(chain, &attestation, chain.config.import_max_skip_slots)?;
let head_block =
verify_head_block_is_known(chain, &attestation, chain.config.import_max_skip_slots)?;
// Check the attestation target root.
let head_block_epoch = head_block.slot.epoch(T::EthSpec::slots_per_epoch());
if head_block_epoch > attestation_epoch {
// The attestation points to a head block from an epoch later than the attestation.
//
// Whilst this seems clearly invalid in the "spirit of the protocol", there is nothing
// in the specification to prevent these messages from propagating.
//
// Reference:
// https://github.com/ethereum/eth2.0-specs/pull/2001#issuecomment-699246659
} else {
let target_root = if head_block_epoch == attestation_epoch {
// If the block is in the same epoch as the attestation, then use the target root
// from the block.
head_block.target_root
} else {
// If the head block is from a previous epoch then skip slots will cause the head block
// root to become the target block root.
//
// We know the head block is from a previous epoch due to a previous check.
head_block.root
};
// Reject any attestation with an invalid target root.
if target_root != attestation.data.target.root {
return Err(Error::InvalidTargetRoot {
attestation: attestation.data.target.root,
expected: target_root,
});
}
}
let (indexed_attestation, committees_per_slot) =
obtain_indexed_attestation_and_committees_per_slot(chain, &attestation)?;
@@ -541,7 +600,7 @@ fn verify_head_block_is_known<T: BeaconChainTypes>(
chain: &BeaconChain<T>,
attestation: &Attestation<T::EthSpec>,
max_skip_slots: Option<u64>,
) -> Result<(), Error> {
) -> Result<ProtoBlock, Error> {
if let Some(block) = chain
.fork_choice
.read()
@@ -556,7 +615,7 @@ fn verify_head_block_is_known<T: BeaconChainTypes>(
});
}
}
Ok(())
Ok(block)
} else {
Err(Error::UnknownHeadBlock {
beacon_block_root: attestation.data.beacon_block_root,
@@ -718,7 +777,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| {
get_indexed_attestation(committee.committee, &attestation)
.map(|attestation| (attestation, committees_per_slot))
.map_err(|e| BeaconChainError::from(e).into())
.map_err(Error::Invalid)
})
}