-
Notifications
You must be signed in to change notification settings - Fork 973
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
Unify equivocation slashing (substantive changes) #1758
Conversation
**TLDR**: Unify all forms of slashing from equivocation into a single equivocation slashing condition. This is a simplification for phase 1. This PR focuses on substantive changes. **substantive changes**: * introduce `equivocation_root` in `SigningData` container * generalise `ProposerSlashing` and `process_proposer_slashing` to be handle any kind of equivocation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does look better than the previous PR, but I am still hesitant with introducing changes to phase0. We should discuss this more, and then schedule it with the BLS change, if we (and implementers!) can agree on this.
Yep, should definitely discuss more :) Some dev feedback so far on technical feasibility/effort:
|
For the sake of completeness, here's come context around that: Paul Hauner, [21.04.20 17:28] Paul Hauner, [21.04.20 17:29] Paul Hauner, [21.04.20 17:30] Paul Hauner, [21.04.20 17:30] My thoughts are still the same. We already have a consensus-breaking change coming (v0.12 BLS) and perhaps it will save headache come phase 1. That being said, it's "one more thing to do" so it's not without cost. |
Maybe there is a way to add this functionality in phase 1 in a way that the data structure changes only involve adding new fields to the end of containers. For |
I'm pretty against this entering Phase 0 at this point. Most client efforts now are in security, stability, networking, and resource consumption optimizations. There are also a number of planned client audits starting in the next 2 to 6 weeks. Anything like this that we introduce at this point complicates all of the above efforts. My opinion is that if it isn't related to security, the state transition should remain stable. Also, given that we have 1 additional expected equivocation type coming in Phase 1 ( |
Closing as the rough consensus is pretty clear :) At a meta level it's great to see phase 0 be so mature that even relatively minor consensus changes are inappropriate. |
TLDR: Unify all forms of slashing from equivocation into a single equivocation slashing condition. This is a simplification for phase 1. This PR is a simplification of #1737 focusing on substantive changes.
substantive changes
uniqueness_root
in theSigningData
container (with support incompute_signing_root
andverify_block_signature
)ProposerSlashing
andprocess_proposer_slashing
to be handle any kind of equivocationhow it works
Signatures sign over a new
uniqueness_root
field which by default is set toRoot()
(all zero bytes). If a signed message (e.g. a signed block) has a slashing condition to protect against equivocations then theuniqueness_root
is set to thehash_tree_root
of the data which is meant to be unique. For example, for block proposals, a proposer must not sign two blocks with the sameslot
therefore signed blocks haveuniqueness_root
set tohash_tree_root(Slot(signed_block.slot))
.