Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove equivocating validators from fork choice #3241

Closed
michaelsproul opened this issue Jun 5, 2022 · 2 comments
Closed

Remove equivocating validators from fork choice #3241

michaelsproul opened this issue Jun 5, 2022 · 2 comments
Assignees
@michaelsproul
Labels
A0 bellatrix Required to support the Bellatrix Upgrade consensus An issue/PR that touches consensus code, such as state_processing or block verification. database v2.5.0 Required for Goerli merge release

Comments

@michaelsproul
Copy link
Member

Description

The latest consensus spec includes a fortification of fork choice against attacks involving equivocating validators.

We should implement the changes as described in ethereum/consensus-specs#2845 and the latest version of the fork choice spec: https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md

Steps to resolve

  • Update fork choice so that it tracks the set of equivocating indices
  • Add a new version to the on-disk fork choice store so that it stores the equivocating indices. This will require a database schema upgrade (see https://github.com/sigp/lighthouse/tree/stable/beacon_node/beacon_chain/src/schema_change). We should also implement a database schema downgrade (it can simply drop the equivocating indices).
  • Add the new on_attester_slashing handler to fork choice and wire it into the beacon_processor.
@michaelsproul michaelsproul added consensus An issue/PR that touches consensus code, such as state_processing or block verification. database A0 bellatrix Required to support the Bellatrix Upgrade labels Jun 5, 2022
@michaelsproul michaelsproul mentioned this issue Jun 5, 2022
Closed
@paulhauner paulhauner added the v2.5.0 Required for Goerli merge release label Jul 20, 2022
@michaelsproul michaelsproul self-assigned this Jul 21, 2022
@michaelsproul
Copy link
Member Author

I'll take this for v2.5.0

@michaelsproul michaelsproul mentioned this issue Jul 25, 2022
Closed
5 tasks
bors bot pushed a commit that referenced this issue Jul 28, 2022
@michaelsproul
Remove equivocating validators from fork choice (#3371)
dfe34ad 
## Issue Addressed

Closes #3241
Closes #3242

## Proposed Changes

* [x] Implement logic to remove equivocating validators from fork choice per ethereum/consensus-specs#2845
* [x] Update tests to v1.2.0-rc.1. The new test which exercises `equivocating_indices` is passing.
* [x] Pull in some SSZ abstractions from the `tree-states` branch that make implementing Vec-compatible encoding for types like `BTreeSet` and `BTreeMap`.
* [x] Implement schema upgrades and downgrades for the database (new schema version is V11).
* [x] Apply attester slashings from blocks to fork choice

## Additional Info

* This PR doesn't need the `BTreeMap` impl, but `tree-states` does, and I don't think there's any harm in keeping it. But I could also be convinced to drop it.

Blocked on #3322.
bors bot pushed a commit that referenced this issue Jul 28, 2022
@michaelsproul
Remove equivocating validators from fork choice (#3371)
a8382ae 
## Issue Addressed

Closes #3241
Closes #3242

## Proposed Changes

* [x] Implement logic to remove equivocating validators from fork choice per ethereum/consensus-specs#2845
* [x] Update tests to v1.2.0-rc.1. The new test which exercises `equivocating_indices` is passing.
* [x] Pull in some SSZ abstractions from the `tree-states` branch that make implementing Vec-compatible encoding for types like `BTreeSet` and `BTreeMap`.
* [x] Implement schema upgrades and downgrades for the database (new schema version is V11).
* [x] Apply attester slashings from blocks to fork choice

## Additional Info

* This PR doesn't need the `BTreeMap` impl, but `tree-states` does, and I don't think there's any harm in keeping it. But I could also be convinced to drop it.

Blocked on #3322.
bors bot pushed a commit that referenced this issue Jul 28, 2022
@michaelsproul
Remove equivocating validators from fork choice (#3371)
c598945 
## Issue Addressed

Closes #3241
Closes #3242

## Proposed Changes

* [x] Implement logic to remove equivocating validators from fork choice per ethereum/consensus-specs#2845
* [x] Update tests to v1.2.0-rc.1. The new test which exercises `equivocating_indices` is passing.
* [x] Pull in some SSZ abstractions from the `tree-states` branch that make implementing Vec-compatible encoding for types like `BTreeSet` and `BTreeMap`.
* [x] Implement schema upgrades and downgrades for the database (new schema version is V11).
* [x] Apply attester slashings from blocks to fork choice

## Additional Info

* This PR doesn't need the `BTreeMap` impl, but `tree-states` does, and I don't think there's any harm in keeping it. But I could also be convinced to drop it.

Blocked on #3322.
bors bot pushed a commit that referenced this issue Jul 28, 2022
@michaelsproul
Remove equivocating validators from fork choice (#3371)
1d52735 
## Issue Addressed

Closes #3241
Closes #3242

## Proposed Changes

* [x] Implement logic to remove equivocating validators from fork choice per ethereum/consensus-specs#2845
* [x] Update tests to v1.2.0-rc.1. The new test which exercises `equivocating_indices` is passing.
* [x] Pull in some SSZ abstractions from the `tree-states` branch that make implementing Vec-compatible encoding for types like `BTreeSet` and `BTreeMap`.
* [x] Implement schema upgrades and downgrades for the database (new schema version is V11).
* [x] Apply attester slashings from blocks to fork choice

## Additional Info

* This PR doesn't need the `BTreeMap` impl, but `tree-states` does, and I don't think there's any harm in keeping it. But I could also be convinced to drop it.

Blocked on #3322.
bors bot pushed a commit that referenced this issue Jul 28, 2022
@michaelsproul
Remove equivocating validators from fork choice (#3371)
64522e9 
## Issue Addressed

Closes #3241
Closes #3242

## Proposed Changes

* [x] Implement logic to remove equivocating validators from fork choice per ethereum/consensus-specs#2845
* [x] Update tests to v1.2.0-rc.1. The new test which exercises `equivocating_indices` is passing.
* [x] Pull in some SSZ abstractions from the `tree-states` branch that make implementing Vec-compatible encoding for types like `BTreeSet` and `BTreeMap`.
* [x] Implement schema upgrades and downgrades for the database (new schema version is V11).
* [x] Apply attester slashings from blocks to fork choice

## Additional Info

* This PR doesn't need the `BTreeMap` impl, but `tree-states` does, and I don't think there's any harm in keeping it. But I could also be convinced to drop it.

Blocked on #3322.
bors bot pushed a commit that referenced this issue Jul 28, 2022
@michaelsproul
Remove equivocating validators from fork choice (#3371)
d04fde3 
## Issue Addressed

Closes #3241
Closes #3242

## Proposed Changes

* [x] Implement logic to remove equivocating validators from fork choice per ethereum/consensus-specs#2845
* [x] Update tests to v1.2.0-rc.1. The new test which exercises `equivocating_indices` is passing.
* [x] Pull in some SSZ abstractions from the `tree-states` branch that make implementing Vec-compatible encoding for types like `BTreeSet` and `BTreeMap`.
* [x] Implement schema upgrades and downgrades for the database (new schema version is V11).
* [x] Apply attester slashings from blocks to fork choice

## Additional Info

* This PR doesn't need the `BTreeMap` impl, but `tree-states` does, and I don't think there's any harm in keeping it. But I could also be convinced to drop it.

Blocked on #3322.
@paulhauner
Copy link
Member

Resolved via #3371 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelsproul michaelsproul

Labels
A0 bellatrix Required to support the Bellatrix Upgrade consensus An issue/PR that touches consensus code, such as state_processing or block verification. database v2.5.0 Required for Goerli merge release
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@michaelsproul @paulhauner