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.

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

Unified slashing condition for equivocations #1387

Closed
JustinDrake opened this issue Aug 29, 2019 · 11 comments
Closed

Unified slashing condition for equivocations #1387

JustinDrake opened this issue Aug 29, 2019 · 11 comments
Labels
phase0 phase1 post-freeze (substantive) Substantive consensus change non-critical for long-lived cross-client testnets scope:BLS

Comments

@JustinDrake
Copy link
Collaborator

JustinDrake commented Aug 29, 2019

In phase 1 we want three equivocation slashing conditions for 1) beacon blocks, 2) shard blocks, and 3) shard attestations. To reduce complexity those can be rolled into a single slashing condition. The idea (from @vbuterin) is to introduce a "uniqueness root" at the signature level.

For example, given a message m, the signature could be over H(H(m), domain, uniqueness_root). Messages without equivocation protection would by default set uniqueness_root = Bytes(). Messages with equivocation protection would set uniqueness_root = hash_tree_root(uniqueness_data)).

For beacon blocks, shard blocks and shard attestations we can have uniqueness_data be of type Container(Domain: domain, Slot: slot). This substantive change to phase 0 can be part of the upcoming BLS signature revamp.

Beyond the complexity reductions, this also bring efficiency gains. Indeed, only the message hashes H(m) (as opposed to the full m) need to be included on-chain.

@JustinDrake JustinDrake added post-freeze (substantive) Substantive consensus change non-critical for long-lived cross-client testnets phase1 labels Aug 29, 2019
@djrtwo
Copy link
Contributor

djrtwo commented Aug 30, 2019

Not 100% sure I see the value here. Seems like it complicates the scheme.

For example, given a message m, the signature could be over H(H(m), domain_type, uniqueness_root)

Do you mean domain instead of domain_type here?

Beyond the complexity reductions, this also bring efficiency gains. Indeed, only the message hashes H(m) (as opposed to the full m) need to be included on-chain.

Do you mean an efficiency gain for the slashing proofs?


still chewing on it

@djrtwo
Copy link
Contributor

djrtwo commented Aug 30, 2019

^ unsure of my assessment. Thinking on it some more :)

@JustinDrake
Copy link
Collaborator Author

Do you mean domain instead of domain_type here?

Fixed, thank you :)

Do you mean an efficiency gain for the slashing proofs?

Yes. Smaller (less data) slashing proofs.

@vbuterin
Copy link
Contributor

vbuterin commented Aug 31, 2019

Seems like it complicates the scheme.

The simplifity benefit comes from the fact that we can use one slashing condition to cover (i) beacon block proposer slashings, (ii) shard block proposer slashings, (iii) shard block attester slashings, and that slashing condition doesn't need to understand any data structures, it can just see a hash and a signature and a validator index.

For example, given a message m, the signature could be over H(H(m), domain, uniqueness_root). Messages without equivocation protection would by default set uniqueness_root = Bytes(). Messages with equivocation protection would set uniqueness_root = hash_tree_root(uniqueness_data)). For beacon blocks, shard blocks and shard attestations we can have uniqueness_data be of type Container(Domain: domain, Slot: slot).

Why two different domains? It seems to me that in any case where we have different domains of the latter type we can make domains of the former type different too. If we go down to one domain, a lot of complexity goes down (specifically, no need for a container type).

@JustinDrake
Copy link
Collaborator Author

no need for a container type

We may want to introduce a container to handle domains as part of the BLS refactor. Right now we are mixing layers with hash(message_hash + domain at the BLS layer. IMO the signature API should only accept Bytes32 (no domain)—domains and uniqueness is logic specific to Eth2 consensus.

If we go down to one domain

What about

class Wrapper(Container):
    message_root: Hash
    domain: Domain
    slot: Slot

where the equivocation slashing condition checks

w_1.message_root != w_2.message_root and
w_1.domain == w_2.domain and
w_1.domain in (DOMAIN_BEACON_PROPOSER, DOMAIN_SHARD_PROPOSER, DOMAIN_SHARD_ATTESTER) and
w_1.slot == w_2.slot

@vbuterin
Copy link
Contributor

We are going to need domains for things that aren't subject to equivocation slashing and things that don't have slots attached to them (eg. even shard blocks would be ShardSlot and not Slot). Maybe this?

class Wrapper(Container):
    message_root: Hash
    domain: Domain
    unique: bool
    uniqueness_tag: Hash

@JustinDrake
Copy link
Collaborator Author

We can use domain in (DOMAIN_BEACON_PROPOSER, DOMAIN_SHARD_PROPOSER, DOMAIN_SHARD_ATTESTER) instead of unique (it's also safer for validators if consensus checks the domain rather than trusting unique). What about:

class Wrapper(Container):
    message_root: Hash
    domain: Domain
    uniqueness_root: Hash

@vbuterin
Copy link
Contributor

Sure, sounds good to me. Another possible approach to avoid hardcoding domain checking is to set uniqueness_root = message_root in cases where you don't care about uniqueness.

@hwwhww
Copy link
Contributor

hwwhww commented Apr 20, 2020

@JustinDrake has this issue been solved by #1532?

@JustinDrake
Copy link
Collaborator Author

Oh shoot! I think @djrtwo suggested that the phase 0 change be handled in a PR separate to #1532 to keep things simple, and then I forgot about this 🤦. Will create a draft PR tomorrow.

JustinDrake referenced this issue Apr 21, 2020
See https://github.com/ethereum/eth2.0-specs/issues/1387—thanks to @hwwhww for pinging me :)

**TLDR**: Significant simplification, especially for phase 1. The idea is that all forms of equivocation (beacon proposing, shard proposing, shard attesting) are unified in a single equivocation slashing condition using uniqueness roots.

**Substantive changes**:

* Introduce `uniqueness_root` in signing root
* Replace `process_proposer_slashing` by `process_equivocation`

**Cosmetic changes**:

* Move `BeaconBlockHeader` aside `BeaconBlockBody` and `BeaconBlock`
* Rename `SigningRoot` to `SigningData`
* Remove a couple of unnecessary new lines
@JustinDrake
Copy link
Collaborator Author

Closing in favour of #1758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
phase0 phase1 post-freeze (substantive) Substantive consensus change non-critical for long-lived cross-client testnets scope:BLS
Projects
None yet
Development

No branches or pull requests

4 participants