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

Refactor signature handling #633

Merged
merged 4 commits into from
Feb 18, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 48 additions & 88 deletions specs/core/0_beacon-chain.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
- [Beacon chain blocks](#beacon-chain-blocks)
- [`BeaconBlock`](#beaconblock)
- [`BeaconBlockBody`](#beaconblockbody)
- [`ProposalSignedData`](#proposalsigneddata)
- [`Proposal`](#proposal)
- [Beacon chain state](#beacon-chain-state)
- [`BeaconState`](#beaconstate)
- [`Validator`](#validator)
Expand Down Expand Up @@ -94,7 +94,6 @@
- [`bls_verify`](#bls_verify)
- [`bls_verify_multiple`](#bls_verify_multiple)
- [`bls_aggregate_pubkeys`](#bls_aggregate_pubkeys)
- [`validate_proof_of_possession`](#validate_proof_of_possession)
- [`process_deposit`](#process_deposit)
- [Routines for updating validator status](#routines-for-updating-validator-status)
- [`activate_validator`](#activate_validator)
Expand All @@ -117,7 +116,7 @@
- [Block roots](#block-roots)
- [Per-block processing](#per-block-processing)
- [Slot](#slot-1)
- [Proposer signature](#proposer-signature)
- [Block signature](#block-signature)
- [RANDAO](#randao)
- [Eth1 data](#eth1-data)
- [Transactions](#transactions)
Expand Down Expand Up @@ -299,14 +298,10 @@ The following data structures are defined as [SimpleSerialize (SSZ)](https://git
{
# Proposer index
'proposer_index': 'uint64',
# First proposal data
'proposal_data_1': ProposalSignedData,
# First proposal signature
'proposal_signature_1': 'bytes96',
# Second proposal data
'proposal_data_2': ProposalSignedData,
# Second proposal signature
'proposal_signature_2': 'bytes96',
# First proposal
'proposal_1': Proposal,
# Second proposal
'proposal_2': Proposal,
}
```

Expand Down Expand Up @@ -363,9 +358,9 @@ The following data structures are defined as [SimpleSerialize (SSZ)](https://git
'slot': 'uint64',
# Shard number
'shard': 'uint64',
# Hash of root of the signed beacon block
# Root of the signed beacon block
'beacon_block_root': 'bytes32',
# Hash of root of the ancestor at the epoch boundary
# Root of the ancestor at the epoch boundary
'epoch_boundary_root': 'bytes32',
# Shard block's hash of root
'shard_block_root': 'bytes32',
Expand Down Expand Up @@ -474,16 +469,17 @@ The following data structures are defined as [SimpleSerialize (SSZ)](https://git

```python
{
## Header ##
# Header
'slot': 'uint64',
'parent_root': 'bytes32',
'state_root': 'bytes32',
'randao_reveal': 'bytes96',
'eth1_data': Eth1Data,
'signature': 'bytes96',

## Body ##
# Body
'body': BeaconBlockBody,
# Signature
'signature': 'bytes96',
}
```

Expand All @@ -500,16 +496,18 @@ The following data structures are defined as [SimpleSerialize (SSZ)](https://git
}
```

#### `ProposalSignedData`
#### `Proposal`

```python
{
# Slot number
'slot': 'uint64',
# Shard number (`BEACON_CHAIN_SHARD_NUMBER` for beacon chain)
'shard': 'uint64',
# Block's hash of root
# Block root
'block_root': 'bytes32',
# Signature
'signature': 'bytes96',
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we even need to do this? Why not be more radical and just remove the Proposal class entirely and directly sign the block? Or is that to reduce the number of slashing conditions down the line when we have shard blocks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do like the idea of having two separate slashing conditions for two reasons:

  1. It's somewhat cleaner (don't need Proposal and BEACON_CHAIN_SHARD_NUMBER)
  2. Beacon chain proposer slashing is arguably more important than shard proposer slashing. So with two separate transaction types we have guaranteed throughput for beacon chain proposer slashings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not completely trivial if we remove Proposal. The reason is that for the proposer slashing you would need a Merkle path to the block root to prove the slot number.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, why? Why not just have a ProposerSlashing object that contains two block headers? Sure it's maybe a hundred bytes longer, but that's nothing compared to attester slashings in any case...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could replace Proposal with a BeaconBlockHeader object but would that be more verbose (Proposal only have 4 fields, BeaconBlockHeader would have 7) and slightly less efficient?

Copy link
Contributor

Choose a reason for hiding this comment

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

Aah I see, BeaconBlock objects themselves are too big, and we technically don't have a BeaconBlockHeader (well, from any block you can create a block header with an equivalent SSZ root by replacing the body with a hash of the body, but if we're going to take advantage of this fact in the spec it would be good to document it well...

I see a few possibilities:

  1. Modify the BeaconBlock object so it has an explicit header that contains hash(body) and a separate body
  2. Define a BeaconBlockHeader as a header that can be derived from any BeaconBlock
  3. Keep the current approach

Note that in terms of hashes and bytes, (1) and (2) are exactly equivalent; object1 containing the SSZ root of object2 with object2 living "somewhere else", and obect1 literally containing object2 in its entirety, are in some sense exactly the same thing and will have exactly the same SSZ root. So it's a choice of presentation rather than content.

(3) would change things less relative to the status quo and it would not introduce this weirdness of the header being kinda part of and kinda separate from the rest of the block, though it would lead to a slightly longer spec. I guess my "don't change things at this stage of the game unless it's a really really good idea" bias does make me lean slightly toward (3)....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I'll keep as is and mull over it a bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you approve the PR?


Expand Down Expand Up @@ -670,6 +668,10 @@ Note: We aim to migrate to a S[T/N]ARK-friendly hash function in a future Ethere

`def hash_tree_root(object: SSZSerializable) -> Bytes32` is a function for hashing objects into a single root utilizing a hash tree structure. `hash_tree_root` is defined in the [SimpleSerialize spec](https://github.com/ethereum/eth2.0-specs/blob/master/specs/simple-serialize.md#tree-hash).

### `signed_root`

`def signed_root(object: SSZContainer) -> Bytes32` is a function defined in the [SimpleSerialize spec](https://github.com/ethereum/eth2.0-specs/blob/master/specs/simple-serialize.md#signed-roots) to compute signed messages.

### `slot_to_epoch`

```python
Expand Down Expand Up @@ -1241,60 +1243,33 @@ def get_entry_exit_effect_epoch(epoch: Epoch) -> Epoch:

`bls_aggregate_pubkeys` is a function for aggregating multiple BLS public keys into a single aggregate key, defined in the [BLS Signature spec](https://github.com/ethereum/eth2.0-specs/blob/master/specs/bls_signature.md#bls_aggregate_pubkeys).

### `validate_proof_of_possession`
### `process_deposit`

Used to add a [validator](#dfn-validator) or top up an existing [validator](#dfn-validator)'s balance by some `deposit` amount:

```python
def validate_proof_of_possession(state: BeaconState,
pubkey: BLSPubkey,
proof_of_possession: BLSSignature,
withdrawal_credentials: Bytes32) -> bool:
def process_deposit(state: BeaconState, deposit: Deposit) -> None:
"""
Verify the given ``proof_of_possession``.
Process a deposit from Ethereum 1.0.
Note that this function mutates ``state``.
"""
proof_of_possession_data = DepositInput(
pubkey=pubkey,
withdrawal_credentials=withdrawal_credentials,
proof_of_possession=EMPTY_SIGNATURE,
)
deposit_input = deposit.deposit_data.deposit_input

return bls_verify(
pubkey=pubkey,
message_hash=hash_tree_root(proof_of_possession_data),
signature=proof_of_possession,
assert bls_verify(
pubkey=deposit_input.pubkey,
message_hash=signed_root(deposit_input, "proof_of_possession"),
signature=deposit_input.proof_of_possession,
domain=get_domain(
state.fork,
get_current_epoch(state),
DOMAIN_DEPOSIT,
)
)
```

### `process_deposit`

Used to add a [validator](#dfn-validator) or top up an existing [validator](#dfn-validator)'s balance by some `deposit` amount:

```python
def process_deposit(state: BeaconState,
pubkey: BLSPubkey,
amount: Gwei,
proof_of_possession: BLSSignature,
withdrawal_credentials: Bytes32) -> None:
"""
Process a deposit from Ethereum 1.0.
Note that this function mutates ``state``.
"""
# Validate the given `proof_of_possession`
proof_is_valid = validate_proof_of_possession(
state,
pubkey,
proof_of_possession,
withdrawal_credentials,
)

if not proof_is_valid:
return

validator_pubkeys = [v.pubkey for v in state.validator_registry]
pubkey = deposit_input.pubkey
amount = deposit.deposit_data.amount
withdrawal_credentials = deposit_input.withdrawal_credentials

if pubkey not in validator_pubkeys:
# Add new validator
Expand Down Expand Up @@ -1521,13 +1496,7 @@ def get_genesis_beacon_state(genesis_validator_deposits: List[Deposit],

# Process genesis deposits
for deposit in genesis_validator_deposits:
process_deposit(
state=state,
pubkey=deposit.deposit_data.deposit_input.pubkey,
amount=deposit.deposit_data.amount,
proof_of_possession=deposit.deposit_data.deposit_input.proof_of_possession,
withdrawal_credentials=deposit.deposit_data.deposit_input.withdrawal_credentials,
)
process_deposit(state, deposit)

# Process genesis activations
for validator_index, _ in enumerate(state.validator_registry):
Expand Down Expand Up @@ -1653,16 +1622,15 @@ Below are the processing steps that happen at every `block`.

* Verify that `block.slot == state.slot`.

#### Proposer signature
#### Block signature

* Let `block_without_signature_root` be the `hash_tree_root` of `block` where `block.signature` is set to `EMPTY_SIGNATURE`.
* Let `proposal_root = hash_tree_root(ProposalSignedData(state.slot, BEACON_CHAIN_SHARD_NUMBER, block_without_signature_root))`.
* Verify that `bls_verify(pubkey=state.validator_registry[get_beacon_proposer_index(state, state.slot)].pubkey, message_hash=proposal_root, signature=block.signature, domain=get_domain(state.fork, get_current_epoch(state), DOMAIN_PROPOSAL))`.
* Let `proposer = state.validator_registry[get_beacon_proposer_index(state, state.slot)]`.
* Let `proposal = Proposal(block.slot, BEACON_CHAIN_SHARD_NUMBER, signed_root(block, "signature"), block.signature)`.
* Verify that `bls_verify(pubkey=proposer.pubkey, message_hash=signed_root(proposal, "signature"), signature=proposal.signature, domain=get_domain(state.fork, get_current_epoch(state), DOMAIN_PROPOSAL))`.

#### RANDAO

* Let `proposer = state.validator_registry[get_beacon_proposer_index(state, state.slot)]`.
* Verify that `bls_verify(pubkey=proposer.pubkey, message_hash=int_to_bytes32(get_current_epoch(state)), signature=block.randao_reveal, domain=get_domain(state.fork, get_current_epoch(state), DOMAIN_RANDAO))`.
* Verify that `bls_verify(pubkey=proposer.pubkey, message_hash=hash_tree_root(get_current_epoch(state)), signature=block.randao_reveal, domain=get_domain(state.fork, get_current_epoch(state), DOMAIN_RANDAO))`.
* Set `state.latest_randao_mixes[get_current_epoch(state) % LATEST_RANDAO_MIXES_LENGTH] = xor(get_randao_mix(state, get_current_epoch(state)), hash(block.randao_reveal))`.

#### Eth1 data
Expand All @@ -1679,12 +1647,12 @@ Verify that `len(block.body.proposer_slashings) <= MAX_PROPOSER_SLASHINGS`.
For each `proposer_slashing` in `block.body.proposer_slashings`:

* Let `proposer = state.validator_registry[proposer_slashing.proposer_index]`.
* Verify that `proposer_slashing.proposal_data_1.slot == proposer_slashing.proposal_data_2.slot`.
* Verify that `proposer_slashing.proposal_data_1.shard == proposer_slashing.proposal_data_2.shard`.
* Verify that `proposer_slashing.proposal_data_1.block_root != proposer_slashing.proposal_data_2.block_root`.
* Verify that `proposer_slashing.proposal_1.slot == proposer_slashing.proposal_2.slot`.
* Verify that `proposer_slashing.proposal_1.shard == proposer_slashing.proposal_2.shard`.
* Verify that `proposer_slashing.proposal_1.block_root != proposer_slashing.proposal_2.block_root`.
* Verify that `proposer.slashed_epoch > get_current_epoch(state)`.
* Verify that `bls_verify(pubkey=proposer.pubkey, message_hash=hash_tree_root(proposer_slashing.proposal_data_1), signature=proposer_slashing.proposal_signature_1, domain=get_domain(state.fork, slot_to_epoch(proposer_slashing.proposal_data_1.slot), DOMAIN_PROPOSAL))`.
* Verify that `bls_verify(pubkey=proposer.pubkey, message_hash=hash_tree_root(proposer_slashing.proposal_data_2), signature=proposer_slashing.proposal_signature_2, domain=get_domain(state.fork, slot_to_epoch(proposer_slashing.proposal_data_2.slot), DOMAIN_PROPOSAL))`.
* Verify that `bls_verify(pubkey=proposer.pubkey, message_hash=signed_root(proposer_slashing.proposal_1, "signature"), signature=proposer_slashing.proposal_1.signature, domain=get_domain(state.fork, slot_to_epoch(proposer_slashing.proposal_1.slot), DOMAIN_PROPOSAL))`.
* Verify that `bls_verify(pubkey=proposer.pubkey, message_hash=signed_root(proposer_slashing.proposal_2, "signature"), signature=proposer_slashing.proposal_2.signature, domain=get_domain(state.fork, slot_to_epoch(proposer_slashing.proposal_2.slot), DOMAIN_PROPOSAL))`.
* Run `slash_validator(state, proposer_slashing.proposer_index)`.

##### Attester slashings
Expand Down Expand Up @@ -1778,13 +1746,7 @@ def verify_merkle_branch(leaf: Bytes32, branch: List[Bytes32], depth: int, index
* Run the following:

```python
process_deposit(
state=state,
pubkey=deposit.deposit_data.deposit_input.pubkey,
amount=deposit.deposit_data.amount,
proof_of_possession=deposit.deposit_data.deposit_input.proof_of_possession,
withdrawal_credentials=deposit.deposit_data.deposit_input.withdrawal_credentials,
)
process_deposit(state, deposit)
```

* Set `state.deposit_index += 1`.
Expand All @@ -1798,8 +1760,7 @@ For each `exit` in `block.body.voluntary_exits`:
* Let `validator = state.validator_registry[exit.validator_index]`.
* Verify that `validator.exit_epoch > get_entry_exit_effect_epoch(get_current_epoch(state))`.
* Verify that `get_current_epoch(state) >= exit.epoch`.
* Let `exit_message = hash_tree_root(VoluntaryExit(epoch=exit.epoch, validator_index=exit.validator_index, signature=EMPTY_SIGNATURE))`.
* Verify that `bls_verify(pubkey=validator.pubkey, message_hash=exit_message, signature=exit.signature, domain=get_domain(state.fork, exit.epoch, DOMAIN_EXIT))`.
* Verify that `bls_verify(pubkey=validator.pubkey, message_hash=signed_root(exit, "signature"), signature=exit.signature, domain=get_domain(state.fork, exit.epoch, DOMAIN_EXIT))`.
* Run `initiate_validator_exit(state, exit.validator_index)`.

##### Transfers
Expand All @@ -1816,8 +1777,7 @@ For each `transfer` in `block.body.transfers`:
* Verify that `state.slot == transfer.slot`.
* Verify that `get_current_epoch(state) >= state.validator_registry[transfer.from].withdrawable_epoch`.
* Verify that `state.validator_registry[transfer.from].withdrawal_credentials == BLS_WITHDRAWAL_PREFIX_BYTE + hash(transfer.pubkey)[1:]`.
* Let `transfer_message = hash_tree_root(Transfer(from=transfer.from, to=transfer.to, amount=transfer.amount, fee=transfer.fee, slot=transfer.slot, signature=EMPTY_SIGNATURE))`.
* Verify that `bls_verify(pubkey=transfer.pubkey, message_hash=transfer_message, signature=transfer.signature, domain=get_domain(state.fork, slot_to_epoch(transfer.slot), DOMAIN_TRANSFER))`.
* Verify that `bls_verify(pubkey=transfer.pubkey, message_hash=signed_root(transfer, "signature"), signature=transfer.signature, domain=get_domain(state.fork, slot_to_epoch(transfer.slot), DOMAIN_TRANSFER))`.
* Set `state.validator_balances[transfer.from] -= transfer.amount + transfer.fee`.
* Set `state.validator_balances[transfer.to] += transfer.amount`.
* Set `state.validator_balances[get_beacon_proposer_index(state, state.slot)] += transfer.fee`.
Expand Down