From 03cefd0065b9e8dcbf49f35b2134cb9c1f1f8744 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 29 Mar 2021 23:42:34 +0000 Subject: [PATCH] Expand observed attestations capacity (#2266) ## Issue Addressed NA ## Proposed Changes I noticed the following error on one of our nodes: ``` Mar 18 00:03:35 ip-xxxx lighthouse-bn[333503]: Mar 18 00:03:35.103 ERRO Unable to validate aggregate error: ObservedAttestersError(EpochTooLow { epoch: Epoch(23961), lowest_permissible_epoch: Epoch(23962) }), peer_id: 16Uiu2HAm5GL5KzPLhvfg9MBBFSpBqTVGRFSiTg285oezzWcZzwEv ``` The slot during this log was 766,815 (the last slot of the epoch). I believe this is due to an off-by-one error in `observed_attesters` where we were failing to provide enough capacity to store observations from the previous, current and next epochs. See code comments for further reasoning. Here's a link to the spec: https://github.com/ethereum/eth2.0-specs/blob/v1.0.1/specs/phase0/p2p-interface.md#beacon_aggregate_and_proof ## Additional Info NA --- .../beacon_chain/src/observed_attesters.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/beacon_node/beacon_chain/src/observed_attesters.rs b/beacon_node/beacon_chain/src/observed_attesters.rs index f93ba6fce4..c657c04933 100644 --- a/beacon_node/beacon_chain/src/observed_attesters.rs +++ b/beacon_node/beacon_chain/src/observed_attesters.rs @@ -252,11 +252,18 @@ impl AutoPruningContainer { /// The maximum number of epochs stored in `self`. fn max_capacity(&self) -> u64 { - // The current epoch and the previous epoch. This is sufficient whilst - // GOSSIP_CLOCK_DISPARITY is 1/2 a slot or less: + // The next, current and previous epochs. We require the next epoch due to the + // `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. We require the previous epoch since the + // specification delcares: // - // https://github.com/ethereum/eth2.0-specs/pull/1706#issuecomment-610151808 - 2 + // ``` + // aggregate.data.slot + ATTESTATION_PROPAGATION_SLOT_RANGE + // >= current_slot >= aggregate.data.slot + // ``` + // + // This means that during the current epoch we will always accept an attestation + // from at least one slot in the previous epoch. + 3 } /// Updates `self` with the current epoch, removing all attestations that become expired