Skip to content

Ignore attestations voting for the wrong finalized checkpoint - #2830

Merged
djrtwo merged 1 commit into
ethereum:devfrom
status-im:ignore-atts
Feb 25, 2022
Merged

djrtwo merged 1 commit into
ethereum:devfrom
status-im:ignore-atts

Conversation

@arnetheduck

Copy link
Copy Markdown
Contributor

When nodes are syncing but have not yet reached the canonical head,
they cannot determine whether nodes they are connected to serve a valid
history or are making bogus claims in their Status advertisement.

Thus, the best course of action that a client can take is to vote for
its "current" best synced head, regardless of whether it's connected to
peers that claim to have other heads or not.

However, in the p2p spec, we penalize such peers with a REJECT - this
should be an IGNORE instead because this vote is correct per the spec,
albeit "late" according to the validating clients' view of the chain.

When nodes are syncing but have not yet reached the canonical `head`,
they cannot determine whether nodes they are connected to serve a valid
history or are making bogus claims in their `Status` advertisement.

Thus, the best course of action that a client can take is to vote for
its "current" best synced head, regardless of whether it's connected to
peers that claim to have other heads or not.

However, in the p2p spec, we penalize such peers with a `REJECT` - this
should be an `IGNORE` instead because this vote is correct per the spec,
albeit "late" according to the validating clients' view of the chain.

@djrtwo djrtwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed, this could help from network wide deadlocks or network fragmentation in the extreme

@djrtwo
djrtwo merged commit bbdb502 into ethereum:dev Feb 25, 2022
bors Bot pushed a commit to sigp/lighthouse that referenced this pull request Mar 4, 2022
## 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants