From 0df7be181436179c8a91df42b54810fd5956dc57 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 00:24:39 +0000 Subject: [PATCH] Add check for aggregate target (#2306) ## Issue Addressed NA ## Proposed Changes - Ensure that the [target consistency check](https://github.com/ethereum/eth2.0-specs/commit/b356f52c5c3c300b8881ccac8ef4cb30ce69af3c) is always performed on aggregates. - Add a regression test. ## Additional Info NA --- .../src/attestation_verification.rs | 10 ++++++++++ .../tests/attestation_verification.rs | 17 +++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 11eb4bbdb6..ddd7f93ca9 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -399,6 +399,16 @@ impl VerifiedAggregatedAttestation { // We do not queue future attestations for later processing. verify_propagation_slot_range(chain, attestation)?; + // Check the attestation's epoch matches its target. + if attestation.data.slot.epoch(T::EthSpec::slots_per_epoch()) + != attestation.data.target.epoch + { + return Err(Error::InvalidTargetEpoch { + slot: attestation.data.slot, + epoch: attestation.data.target.epoch, + }); + } + // Ensure the valid aggregated attestation has not already been seen locally. let attestation_root = attestation.tree_hash_root(); if chain diff --git a/beacon_node/beacon_chain/tests/attestation_verification.rs b/beacon_node/beacon_chain/tests/attestation_verification.rs index 580307ddc2..0b7e7ef8d1 100644 --- a/beacon_node/beacon_chain/tests/attestation_verification.rs +++ b/beacon_node/beacon_chain/tests/attestation_verification.rs @@ -276,6 +276,23 @@ fn aggregated_gossip_verification() { && earliest_permissible_slot == current_slot - E::slots_per_epoch() - 1 ); + /* + * The following test ensures: + * + * The aggregate attestation's epoch matches its target -- i.e. `aggregate.data.target.epoch == + * compute_epoch_at_slot(attestation.data.slot)` + * + */ + + assert_invalid!( + "attestation with invalid target epoch", + { + let mut a = valid_aggregate.clone(); + a.message.aggregate.data.target.epoch += 1; + a + }, + AttnError::InvalidTargetEpoch { .. } + ); /* * This is not in the specification for aggregate attestations (only unaggregates), but we * check it anyway to avoid weird edge cases.