Skip to content

[Merged by Bors] - Implement slasher #1567

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.

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

Closed
wants to merge 40 commits into from
Closed

[Merged by Bors] - Implement slasher #1567

wants to merge 40 commits into from

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Aug 25, 2020

This is an implementation of a slasher that lives inside the BN and can be enabled via lighthouse bn --slasher.

Features included in this PR:

  • Detection of attester slashing conditions (double votes, surrounds existing, surrounded by existing)
  • Integration into Lighthouse's attestation verification flow
  • Detection of proposer slashing conditions
  • Extraction of attestations from blocks as they are verified
  • Compression of chunks
  • Configurable history length
  • Pruning of old attestations and blocks
  • More tests

Future work:

  • Focus on a slice of history separate from the most recent N epochs (e.g. epochs current - K to current - M)
  • Run out-of-process
  • Ingest attestations from the chain without a resync

Design notes are here https://hackmd.io/@sproul/HJSEklmPL

@michaelsproul michaelsproul added the work-in-progress PR is a work-in-progress label Aug 25, 2020
@michaelsproul michaelsproul force-pushed the slasher branch 2 times, most recently from 2b3c1a6 to 039b066 Compare August 26, 2020 09:50
@paulhauner paulhauner added the A1 label Sep 23, 2020
@michaelsproul michaelsproul changed the base branch from master to v0.3.0-staging October 8, 2020 00:00
Base automatically changed from v0.3.0-staging to master October 9, 2020 06:08
@paulhauner paulhauner added A0 and removed A0 labels Nov 8, 2020
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

This is an impressive research and engineering piece! It was a pleasure to read and a real contribution to Lighthouse and Eth2 🥇

I must admit I'm not 100% across the theory of the schema, but I don't think that should prevent a merge. The slasher is a best-effort component; as long as it does not crash then it is better than nothing. That being said, I see no reason to believe this implementation is lacking.

Although I'm not fully across the slasher theory, I did go through the whole implementation and have left several comments and suggestions. I would be happy to merge

🚀

@paulhauner paulhauner added work-in-progress PR is a work-in-progress and removed ready-for-review The code is ready for review labels Nov 18, 2020
@michaelsproul
Copy link
Member Author

This is ready for re-review and merge IMO. The only outstanding issue is #1890, which I'll try to get to before 1.0, but isn't a blocker for users running with the default configuration, or experienced users (who can drop their DB if they set a max DB size too low).

@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Nov 22, 2020
@michaelsproul
Copy link
Member Author

Will fix the merge conflict ASAP, just finishing off some slashing protection stuff

Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Awesome, LGTM!

Happy for you to bors whenever, assuming merge conflict change is trivial :)

@michaelsproul
Copy link
Member Author

Indeed the merge commit was trivial (97228b5)

I can't believe we're finally merging this 😮 😮 😮

🚀 🚀 🚀 🚀 🚀 🚀 🚀 🚀 🚀 🚀 🚀 🚀 🚀

Summoning Señor Bors...

bors r+

bors bot pushed a commit that referenced this pull request Nov 23, 2020
This is an implementation of a slasher that lives inside the BN and can be enabled via `lighthouse bn --slasher`.

Features included in this PR:

- [x] Detection of attester slashing conditions (double votes, surrounds existing, surrounded by existing)
- [x] Integration into Lighthouse's attestation verification flow
- [x] Detection of proposer slashing conditions
- [x] Extraction of attestations from blocks as they are verified
- [x] Compression of chunks
- [x] Configurable history length
- [x] Pruning of old attestations and blocks
- [x] More tests

Future work:

* Focus on a slice of history separate from the most recent N epochs (e.g. epochs `current - K` to `current - M`)
* Run out-of-process
* Ingest attestations from the chain without a resync

Design notes are here https://hackmd.io/@sproul/HJSEklmPL
@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Nov 23, 2020
@bors
Copy link

bors bot commented Nov 23, 2020

@bors bors bot changed the title Implement slasher [Merged by Bors] - Implement slasher Nov 23, 2020
@bors bors bot closed this Nov 23, 2020
@michaelsproul michaelsproul deleted the slasher branch November 23, 2020 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0 major-task A significant amount of work or conceptual task. ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants