From 3a309155aa51288103b6d4f09343b68389b9dca7 Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 6 May 2019 22:06:00 +0200 Subject: [PATCH 1/6] fix deposit domain: forks are ignored for deposit validity, deposits are always accepted, if coming from the correct contract(s). --- specs/core/0_beacon-chain.md | 6 ++++-- specs/validator/0_beacon-chain-validator.md | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index dd2d3d1a6..cee97f9f5 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -966,7 +966,8 @@ def get_domain(state: BeaconState, """ epoch = get_current_epoch(state) if message_epoch is None else message_epoch fork_version = state.fork.previous_version if epoch < state.fork.epoch else state.fork.current_version - return bytes_to_int(fork_version + int_to_bytes4(domain_type)) + # fork version is on the big-endian side: when signing using only the type (e.g. deposits), the type can be passed directly. + return bytes_to_int(int_to_bytes4(domain_type) + fork_version) ``` ### `get_bitfield_bit` @@ -1766,7 +1767,8 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None: validator_pubkeys = [v.pubkey for v in state.validator_registry] if pubkey not in validator_pubkeys: # Verify the deposit signature (proof of possession) - if not bls_verify(pubkey, signing_root(deposit.data), deposit.data.signature, get_domain(state, DOMAIN_DEPOSIT)): + # Note: deposits are valid regardless of fork version, hence the type is passed directly as domain. + if not bls_verify(pubkey, signing_root(deposit.data), deposit.data.signature, DOMAIN_DEPOSIT): return # Add validator and balance entries diff --git a/specs/validator/0_beacon-chain-validator.md b/specs/validator/0_beacon-chain-validator.md index 600ed0839..a74fbb80f 100644 --- a/specs/validator/0_beacon-chain-validator.md +++ b/specs/validator/0_beacon-chain-validator.md @@ -103,7 +103,7 @@ To submit a deposit: * Pack the validator's [initialization parameters](#initialization) into `deposit_data`, a [`DepositData`](../core/0_beacon-chain.md#depositdata) SSZ object. * Let `amount` be the amount in Gwei to be deposited by the validator where `MIN_DEPOSIT_AMOUNT <= amount <= MAX_EFFECTIVE_BALANCE`. * Set `deposit_data.amount = amount`. -* Let `signature` be the result of `bls_sign` of the `signing_root(deposit_data)` with `domain=DOMAIN_DEPOSIT`. +* Let `signature` be the result of `bls_sign` of the `signing_root(deposit_data)` with `domain=DOMAIN_DEPOSIT`. (Deposits are valid regardless of fork version, hence the type is passed directly as domain.) * Send a transaction on the Ethereum 1.0 chain to `DEPOSIT_CONTRACT_ADDRESS` executing `def deposit(pubkey: bytes[48], withdrawal_credentials: bytes[32], signature: bytes[96])` along with a deposit of `amount` Gwei. *Note*: Deposits made for the same `pubkey` are treated as for the same validator. A singular `Validator` will be added to `state.validator_registry` with each additional deposit amount added to the validator's balance. A validator can only be activated when total deposits for the validator pubkey meet or exceed `MAX_EFFECTIVE_BALANCE`. From 847fcf52cc5228cb32998feffb05511b2312d7b8 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 21 May 2019 11:30:38 -0600 Subject: [PATCH 2/6] utilize bls_domain directly for deposits --- specs/core/0_beacon-chain.md | 20 ++++++++++++++++---- specs/validator/0_beacon-chain-validator.md | 2 +- test_libs/pyspec/tests/helpers.py | 5 +++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index f1986989b..79038eea8 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -174,6 +174,7 @@ These configurations are updated for releases, but may be out of sync during `de | Name | Value | | - | - | | `DEPOSIT_CONTRACT_TREE_DEPTH` | `2**5` (= 32) | +| `DEPOSIT_FORK_VERSION` | `b'\x00' * 4` | ### Gwei values @@ -629,6 +630,16 @@ The `hash` function is SHA256. `def signing_root(object: SSZContainer) -> Bytes32` is a function defined in the [SimpleSerialize spec](../simple-serialize.md#self-signed-containers) to compute signing messages. +### `bls_domain` + +```python +def bls_domain(domain_type: int, fork_version: bytes) -> int: + """ + Return the bls domain given by the ``domain_type`` and 4 byte ``fork_version``.. + """ + return bytes_to_int(int_to_bytes(domain_type, length=4) + fork_version) +``` + ### `slot_to_epoch` ```python @@ -968,8 +979,7 @@ def get_domain(state: BeaconState, """ epoch = get_current_epoch(state) if message_epoch is None else message_epoch fork_version = state.fork.previous_version if epoch < state.fork.epoch else state.fork.current_version - # fork version is on the big-endian side: when signing using only the type (e.g. deposits), the type can be passed directly. - return bytes_to_int(int_to_bytes(domain_type, length=4) + fork_version) + return bls_domain(domain_type, fork_version) ``` ### `get_bitfield_bit` @@ -1766,8 +1776,10 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None: validator_pubkeys = [v.pubkey for v in state.validator_registry] if pubkey not in validator_pubkeys: # Verify the deposit signature (proof of possession) - # Note: deposits are valid regardless of fork version, hence the type is passed directly as domain. - if not bls_verify(pubkey, signing_root(deposit.data), deposit.data.signature, DOMAIN_DEPOSIT): + # Note: deposits are valid across forks, hence the deposit domain is retrieved directly from `bls_domain` + if not bls_verify( + pubkey, signing_root(deposit.data), deposit.data.signature, bls_domain(DOMAIN_DEPOSIT, DEPOSIT_FORK_VERSION) + ): return # Add validator and balance entries diff --git a/specs/validator/0_beacon-chain-validator.md b/specs/validator/0_beacon-chain-validator.md index c2fe927c0..49bb4fc3a 100644 --- a/specs/validator/0_beacon-chain-validator.md +++ b/specs/validator/0_beacon-chain-validator.md @@ -98,7 +98,7 @@ To submit a deposit: * Pack the validator's [initialization parameters](#initialization) into `deposit_data`, a [`DepositData`](../core/0_beacon-chain.md#depositdata) SSZ object. * Let `amount` be the amount in Gwei to be deposited by the validator where `MIN_DEPOSIT_AMOUNT <= amount <= MAX_EFFECTIVE_BALANCE`. * Set `deposit_data.amount = amount`. -* Let `signature` be the result of `bls_sign` of the `signing_root(deposit_data)` with `domain=DOMAIN_DEPOSIT`. (Deposits are valid regardless of fork version, hence the type is passed directly as domain.) +* Let `signature` be the result of `bls_sign` of the `signing_root(deposit_data)` with `domain=bls_domain(DOMAIN_DEPOSIT, DEPOSIT_FORK_VERSION)`. (Deposits are valid regardless of fork version, hence the static fork version being directly passed into `bls_domain`). * Send a transaction on the Ethereum 1.0 chain to `DEPOSIT_CONTRACT_ADDRESS` executing `def deposit(pubkey: bytes[48], withdrawal_credentials: bytes[32], signature: bytes[96])` along with a deposit of `amount` Gwei. *Note*: Deposits made for the same `pubkey` are treated as for the same validator. A singular `Validator` will be added to `state.validator_registry` with each additional deposit amount added to the validator's balance. A validator can only be activated when total deposits for the validator pubkey meet or exceed `MAX_EFFECTIVE_BALANCE`. diff --git a/test_libs/pyspec/tests/helpers.py b/test_libs/pyspec/tests/helpers.py index 7af210f85..eebbd2daf 100644 --- a/test_libs/pyspec/tests/helpers.py +++ b/test_libs/pyspec/tests/helpers.py @@ -24,6 +24,7 @@ from eth2spec.phase0.spec import ( VoluntaryExit, # functions convert_to_indexed, + bls_domain, get_active_validator_indices, get_attesting_indices, get_block_root, @@ -144,9 +145,9 @@ def build_deposit_data(state, pubkey, privkey, amount): signature = bls.sign( message_hash=signing_root(deposit_data), privkey=privkey, - domain=get_domain( - state, + domain=bls_domain( spec.DOMAIN_DEPOSIT, + spec.DEPOSIT_FORK_VERSION, ) ) deposit_data.signature = signature From b075a7a0ab3e85b40716c3bf23cbaae76753fbb3 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 21 May 2019 11:33:52 -0600 Subject: [PATCH 3/6] add bls_domain to toc --- specs/core/0_beacon-chain.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 79038eea8..9f965c1d7 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -51,6 +51,7 @@ - [`hash`](#hash) - [`hash_tree_root`](#hash_tree_root) - [`signing_root`](#signing_root) + - [`bls_domain`](#bls_domain) - [`slot_to_epoch`](#slot_to_epoch) - [`get_previous_epoch`](#get_previous_epoch) - [`get_current_epoch`](#get_current_epoch) From 6b5f4b44eae5d3b1ba7b403eed8954aff5c96097 Mon Sep 17 00:00:00 2001 From: protolambda Date: Wed, 22 May 2019 01:35:24 +0200 Subject: [PATCH 4/6] avoid zero constant for deposits fork-version, just default to it --- specs/core/0_beacon-chain.md | 7 +++---- specs/validator/0_beacon-chain-validator.md | 2 +- test_libs/pyspec/tests/helpers.py | 1 - 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 9f965c1d7..c14228dc1 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -175,7 +175,6 @@ These configurations are updated for releases, but may be out of sync during `de | Name | Value | | - | - | | `DEPOSIT_CONTRACT_TREE_DEPTH` | `2**5` (= 32) | -| `DEPOSIT_FORK_VERSION` | `b'\x00' * 4` | ### Gwei values @@ -634,9 +633,9 @@ The `hash` function is SHA256. ### `bls_domain` ```python -def bls_domain(domain_type: int, fork_version: bytes) -> int: +def bls_domain(domain_type: int, fork_version=b'\x00\x00\x00\x00') -> int: """ - Return the bls domain given by the ``domain_type`` and 4 byte ``fork_version``.. + Return the bls domain given by the ``domain_type`` and optional 4 byte ``fork_version`` (defaults to zero). """ return bytes_to_int(int_to_bytes(domain_type, length=4) + fork_version) ``` @@ -1779,7 +1778,7 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None: # Verify the deposit signature (proof of possession) # Note: deposits are valid across forks, hence the deposit domain is retrieved directly from `bls_domain` if not bls_verify( - pubkey, signing_root(deposit.data), deposit.data.signature, bls_domain(DOMAIN_DEPOSIT, DEPOSIT_FORK_VERSION) + pubkey, signing_root(deposit.data), deposit.data.signature, bls_domain(DOMAIN_DEPOSIT) ): return diff --git a/specs/validator/0_beacon-chain-validator.md b/specs/validator/0_beacon-chain-validator.md index 49bb4fc3a..f8272d446 100644 --- a/specs/validator/0_beacon-chain-validator.md +++ b/specs/validator/0_beacon-chain-validator.md @@ -98,7 +98,7 @@ To submit a deposit: * Pack the validator's [initialization parameters](#initialization) into `deposit_data`, a [`DepositData`](../core/0_beacon-chain.md#depositdata) SSZ object. * Let `amount` be the amount in Gwei to be deposited by the validator where `MIN_DEPOSIT_AMOUNT <= amount <= MAX_EFFECTIVE_BALANCE`. * Set `deposit_data.amount = amount`. -* Let `signature` be the result of `bls_sign` of the `signing_root(deposit_data)` with `domain=bls_domain(DOMAIN_DEPOSIT, DEPOSIT_FORK_VERSION)`. (Deposits are valid regardless of fork version, hence the static fork version being directly passed into `bls_domain`). +* Let `signature` be the result of `bls_sign` of the `signing_root(deposit_data)` with `domain=bls_domain(DOMAIN_DEPOSIT)`. (Deposits are valid regardless of fork version, `bls_domain` will default to zeroes there). * Send a transaction on the Ethereum 1.0 chain to `DEPOSIT_CONTRACT_ADDRESS` executing `def deposit(pubkey: bytes[48], withdrawal_credentials: bytes[32], signature: bytes[96])` along with a deposit of `amount` Gwei. *Note*: Deposits made for the same `pubkey` are treated as for the same validator. A singular `Validator` will be added to `state.validator_registry` with each additional deposit amount added to the validator's balance. A validator can only be activated when total deposits for the validator pubkey meet or exceed `MAX_EFFECTIVE_BALANCE`. diff --git a/test_libs/pyspec/tests/helpers.py b/test_libs/pyspec/tests/helpers.py index eebbd2daf..06ca8a1d5 100644 --- a/test_libs/pyspec/tests/helpers.py +++ b/test_libs/pyspec/tests/helpers.py @@ -147,7 +147,6 @@ def build_deposit_data(state, pubkey, privkey, amount): privkey=privkey, domain=bls_domain( spec.DOMAIN_DEPOSIT, - spec.DEPOSIT_FORK_VERSION, ) ) deposit_data.signature = signature From 2c79e584c2b797efd4a19b1ed85c0f16dbc28d1a Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 21 May 2019 22:42:47 -0600 Subject: [PATCH 5/6] minor lint --- test_libs/pyspec/tests/helpers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test_libs/pyspec/tests/helpers.py b/test_libs/pyspec/tests/helpers.py index 06ca8a1d5..a4849bfbb 100644 --- a/test_libs/pyspec/tests/helpers.py +++ b/test_libs/pyspec/tests/helpers.py @@ -145,9 +145,7 @@ def build_deposit_data(state, pubkey, privkey, amount): signature = bls.sign( message_hash=signing_root(deposit_data), privkey=privkey, - domain=bls_domain( - spec.DOMAIN_DEPOSIT, - ) + domain=bls_domain(spec.DOMAIN_DEPOSIT), ) deposit_data.signature = signature return deposit_data From c13421a9a7d13585b0558e1c4a8a0221d97d3c2d Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 22 May 2019 16:52:44 -0400 Subject: [PATCH 6/6] type hinting for fork version Co-Authored-By: Hsiao-Wei Wang --- specs/core/0_beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index c14228dc1..60f774e9a 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -633,7 +633,7 @@ The `hash` function is SHA256. ### `bls_domain` ```python -def bls_domain(domain_type: int, fork_version=b'\x00\x00\x00\x00') -> int: +def bls_domain(domain_type: int, fork_version: bytes=b'\x00\x00\x00\x00') -> int: """ Return the bls domain given by the ``domain_type`` and optional 4 byte ``fork_version`` (defaults to zero). """