cerc-io/lighthouse
10
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Ignore attestations to finalized blocks (don't reject) (#3052)

Browse Source 
## Issue Addressed

Addresses spec changes from v1.1.0:

- https://github.com/ethereum/consensus-specs/pull/2830
- https://github.com/ethereum/consensus-specs/pull/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.
This commit is contained in:
Michael Sproul 2022-03-04 00:41:22 +00:00
parent 09d2187198
commit 1829250ee4
2 changed files with 4 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
beacon_node/beacon_chain/src/attestation_verification.rs
View File

@ -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. /// 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: /// The block root may not be known for two reasons:
/// ///

10
beacon_node/network/src/beacon_processor/worker/gossip_methods.rs
View File

@ -1557,7 +1557,7 @@ impl<T: BeaconChainTypes> Worker<T> {
/* /*
* The block indicated by the target root is not known to us. * 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: * error, so this means we can get this error if:
* *
* 1. The target root does not represent a valid block. * 1. The target root does not represent a valid block.
@ -1566,7 +1566,7 @@ impl<T: BeaconChainTypes> Worker<T> {
* For (2), we should only be processing attestations when we should have * For (2), we should only be processing attestations when we should have
* all the available information. Note: if we do a weak-subjectivity sync * 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 * 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. * The peer has published an invalid consensus message.
*/ */
@ -1713,14 +1713,12 @@ impl<T: BeaconChainTypes> Worker<T> {
AttnError::HeadBlockFinalized { beacon_block_root } => { AttnError::HeadBlockFinalized { beacon_block_root } => {
debug!( debug!(
self.log, self.log,
"Rejected attestation to finalized block"; "Ignored attestation to finalized block";
"block_root" => ?beacon_block_root, "block_root" => ?beacon_block_root,
"attestation_slot" => failed_att.attestation().data.slot, "attestation_slot" => failed_att.attestation().data.slot,
); );
// We have to reject the message as it isn't a descendant of the finalized self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
// checkpoint.
self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);
// The peer that sent us this could be a lagger, or a spammer, or this failure could // 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. // be due to us processing attestations extremely slowly. Don't be too harsh.