Skip to content

Commit

Permalink
Ignore attestations to finalized blocks (don't reject) (#3052)
Browse files Browse the repository at this point in the history
## Issue Addressed

Addresses spec changes from v1.1.0:

- ethereum/consensus-specs#2830
- ethereum/consensus-specs#2846

## Proposed Changes

* Downgrade the REJECT for `HeadBlockFinalized` to an IGNORE. This applies to both unaggregated and aggregated attestations.

## Additional Info

I thought about also changing the penalty for `UnknownTargetRoot` but I don't think it's reachable in practice.
  • Loading branch information
michaelsproul committed Mar 4, 2022
1 parent 09d2187 commit 1829250
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 7 deletions.
1 change: 0 additions & 1 deletion beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,6 @@ impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> {
}

/// Returns `Ok(())` if the `attestation.data.beacon_block_root` is known to this chain.
/// You can use this `shuffling_id` to read from the shuffling cache.
///
/// The block root may not be known for two reasons:
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,7 @@ impl<T: BeaconChainTypes> Worker<T> {
/*
* The block indicated by the target root is not known to us.
*
* We should always get `AttnError::UnknwonHeadBlock` before we get this
* We should always get `AttnError::UnknownHeadBlock` before we get this
* error, so this means we can get this error if:
*
* 1. The target root does not represent a valid block.
Expand All @@ -1566,7 +1566,7 @@ impl<T: BeaconChainTypes> Worker<T> {
* For (2), we should only be processing attestations when we should have
* all the available information. Note: if we do a weak-subjectivity sync
* it's possible that this situation could occur, but I think it's
* unlikely. For now, we will declare this to be an invalid message>
* unlikely. For now, we will declare this to be an invalid message.
*
* The peer has published an invalid consensus message.
*/
Expand Down Expand Up @@ -1713,14 +1713,12 @@ impl<T: BeaconChainTypes> Worker<T> {
AttnError::HeadBlockFinalized { beacon_block_root } => {
debug!(
self.log,
"Rejected attestation to finalized block";
"Ignored attestation to finalized block";
"block_root" => ?beacon_block_root,
"attestation_slot" => failed_att.attestation().data.slot,
);

// We have to reject the message as it isn't a descendant of the finalized
// checkpoint.
self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);
self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);

// The peer that sent us this could be a lagger, or a spammer, or this failure could
// be due to us processing attestations extremely slowly. Don't be too harsh.
Expand Down

0 comments on commit 1829250

Please sign in to comment.