From d805fb508446c8b9a6a0299cf5ac585b2597362d Mon Sep 17 00:00:00 2001 From: Justin Date: Sat, 25 May 2019 00:35:17 +0300 Subject: [PATCH 1/6] Simplify deposits --- specs/core/0_beacon-chain.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 60f774e9a..48a686735 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -471,8 +471,6 @@ The types are defined topologically to aid in facilitating an executable version { # Branch in the deposit tree 'proof': ['bytes32', DEPOSIT_CONTRACT_TREE_DEPTH], - # Index in the deposit tree - 'index': 'uint64', # Data 'data': DepositData, } @@ -1763,12 +1761,11 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None: leaf=hash_tree_root(deposit.data), proof=deposit.proof, depth=DEPOSIT_CONTRACT_TREE_DEPTH, - index=deposit.index, + index=state.deposit_index, root=state.latest_eth1_data.deposit_root, ) # Deposits must be processed in order - assert deposit.index == state.deposit_index state.deposit_index += 1 pubkey = deposit.data.pubkey From cdfb886c22473f8c145b97e726f6182d68495f57 Mon Sep 17 00:00:00 2001 From: Justin Date: Sun, 26 May 2019 18:42:37 +0300 Subject: [PATCH 2/6] Avoid divisions by zero Possible fix to avoid four cases of divisions by zero: * `return state.validator_registry[index].effective_balance // adjusted_quotient // BASE_REWARDS_PER_EPOCH` * `rewards[index] += get_base_reward(state, index) * attesting_balance // total_balance` * `validator.effective_balance * min(total_penalties * 3, total_balance) // total_balance` * `rewards[index] += base_reward * attesting_balance // committee_balance` See also #1107. --- specs/core/0_beacon-chain.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 60f774e9a..104f3eb92 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -963,9 +963,9 @@ def bytes_to_int(data: bytes) -> int: ```python def get_total_balance(state: BeaconState, indices: List[ValidatorIndex]) -> Gwei: """ - Return the combined effective balance of an array of ``validators``. + Return the combined effective balance of the ``indices``. (1 Gwei minimum to avoid divisions by zero.) """ - return sum([state.validator_registry[index].effective_balance for index in indices]) + return max(sum([state.validator_registry[index].effective_balance for index in indices]), 1) ``` ### `get_domain` @@ -1413,10 +1413,9 @@ def process_crosslinks(state: BeaconState) -> None: ```python def get_base_reward(state: BeaconState, index: ValidatorIndex) -> Gwei: - adjusted_quotient = integer_squareroot(get_total_active_balance(state)) // BASE_REWARD_QUOTIENT - if adjusted_quotient == 0: - return 0 - return state.validator_registry[index].effective_balance // adjusted_quotient // BASE_REWARDS_PER_EPOCH + total_balance = get_total_active_balance(state) + effective_balance = state.validator_registry[index].effective_balance + return effective_balance * BASE_REWARD_QUOTIENT // integer_squareroot(total_balance) // BASE_REWARDS_PER_EPOCH ``` ```python @@ -1531,10 +1530,9 @@ def process_registry_updates(state: BeaconState) -> None: ```python def process_slashings(state: BeaconState) -> None: current_epoch = get_current_epoch(state) - active_validator_indices = get_active_validator_indices(state, current_epoch) - total_balance = get_total_balance(state, active_validator_indices) + total_balance = get_total_active_balance(state) - # Compute `total_penalties` + # Compute slashed balances in the current epoch total_at_start = state.latest_slashed_balances[(current_epoch + 1) % LATEST_SLASHED_EXIT_LENGTH] total_at_end = state.latest_slashed_balances[current_epoch % LATEST_SLASHED_EXIT_LENGTH] total_penalties = total_at_end - total_at_start From 47f3df6d0a6296f950289057f3082c6c95d907be Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 27 May 2019 16:40:00 -0600 Subject: [PATCH 3/6] fix deposit test for new index handling --- .../block_processing/test_process_deposit.py | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/test_libs/pyspec/tests/block_processing/test_process_deposit.py b/test_libs/pyspec/tests/block_processing/test_process_deposit.py index bbfb390ef..62b058a70 100644 --- a/test_libs/pyspec/tests/block_processing/test_process_deposit.py +++ b/test_libs/pyspec/tests/block_processing/test_process_deposit.py @@ -83,33 +83,45 @@ def test_success_top_up(state): return pre_state, deposit, post_state -def test_wrong_index(state): +def test_wrong_deposit_for_deposit_count(state): pre_state = deepcopy(state) deposit_data_leaves = [ZERO_HASH] * len(pre_state.validator_registry) - index = len(deposit_data_leaves) - pubkey = pubkeys[index] - privkey = privkeys[index] - deposit, root, deposit_data_leaves = build_deposit( + # build root for deposit_1 + index_1 = len(deposit_data_leaves) + pubkey_1 = pubkeys[index_1] + privkey_1 = privkeys[index_1] + deposit_1, root_1, deposit_data_leaves = build_deposit( pre_state, deposit_data_leaves, - pubkey, - privkey, + pubkey_1, + privkey_1, + spec.MAX_EFFECTIVE_BALANCE, + ) + deposit_count_1 = len(deposit_data_leaves) + + # build root for deposit_2 + index_2 = len(deposit_data_leaves) + pubkey_2 = pubkeys[index_2] + privkey_2 = privkeys[index_2] + deposit_2, root_2, deposit_data_leaves = build_deposit( + pre_state, + deposit_data_leaves, + pubkey_2, + privkey_2, spec.MAX_EFFECTIVE_BALANCE, ) - # mess up deposit_index - deposit.index = pre_state.deposit_index + 1 - - pre_state.latest_eth1_data.deposit_root = root - pre_state.latest_eth1_data.deposit_count = len(deposit_data_leaves) + # state has root for deposit_2 but is at deposit_count for deposit_1 + pre_state.latest_eth1_data.deposit_root = root_2 + pre_state.latest_eth1_data.deposit_count = deposit_count_1 post_state = deepcopy(pre_state) with pytest.raises(AssertionError): - process_deposit(post_state, deposit) + process_deposit(post_state, deposit_2) - return pre_state, deposit, None + return pre_state, deposit_2, None def test_bad_merkle_proof(state): From 12a7e264532800670d12c31e08dc2d0d69f47b31 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Tue, 28 May 2019 20:57:18 -0700 Subject: [PATCH 4/6] Update README.md --- specs/test_formats/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/test_formats/README.md b/specs/test_formats/README.md index d245fcfa4..f0acb444e 100644 --- a/specs/test_formats/README.md +++ b/specs/test_formats/README.md @@ -25,7 +25,8 @@ Test formats: - [`bls`](./bls/README.md) - [`operations`](./operations/README.md) - [`shuffling`](./shuffling/README.md) -- [`ssz`](./ssz/README.md) +- [`ssz_generic`](./ssz_generic/README.md) +- [`ssz_static`](./ssz_static/README.md) - More formats are planned, see tracking issues for CI/testing ## Glossary From 1c416541e1478af60ee3695f263f6499275a374c Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 29 May 2019 23:40:46 +0300 Subject: [PATCH 5/6] Update 0_beacon-chain.md --- specs/core/0_beacon-chain.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 104f3eb92..b394d59e1 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -17,7 +17,7 @@ - [Initial values](#initial-values) - [Time parameters](#time-parameters) - [State list lengths](#state-list-lengths) - - [Reward and penalty quotients](#reward-and-penalty-quotients) + - [Rewards and penalties](#rewards-and-penalties) - [Max operations per block](#max-operations-per-block) - [Signature domains](#signature-domains) - [Data structures](#data-structures) @@ -103,7 +103,7 @@ - [Helper functions](#helper-functions-1) - [Justification and finalization](#justification-and-finalization) - [Crosslinks](#crosslinks) - - [Rewards and penalties](#rewards-and-penalties) + - [Rewards and penalties](#rewards-and-penalties-1) - [Registry updates](#registry-updates) - [Slashings](#slashings) - [Final updates](#final-updates) @@ -220,17 +220,17 @@ These configurations are updated for releases, but may be out of sync during `de | `LATEST_ACTIVE_INDEX_ROOTS_LENGTH` | `2**13` (= 8,192) | epochs | ~36 days | | `LATEST_SLASHED_EXIT_LENGTH` | `2**13` (= 8,192) | epochs | ~36 days | -### Reward and penalty quotients +### Rewards and penalties | Name | Value | | - | - | -| `BASE_REWARD_QUOTIENT` | `2**5` (= 32) | +| `BASE_REWARD_FACTOR` | `2**5` (= 32) | | `WHISTLEBLOWING_REWARD_QUOTIENT` | `2**9` (= 512) | | `PROPOSER_REWARD_QUOTIENT` | `2**3` (= 8) | | `INACTIVITY_PENALTY_QUOTIENT` | `2**25` (= 33,554,432) | | `MIN_SLASHING_PENALTY_QUOTIENT` | `2**5` (= 32) | -* **The `BASE_REWARD_QUOTIENT` is NOT final. Once all other protocol details are finalized, it will be adjusted to target a theoretical maximum total issuance of `2**21` ETH per year if `2**27` ETH is validating (and therefore `2**20` per year if `2**25` ETH is validating, etc.)** +* **The `BASE_REWARD_FACTOR` is NOT final. Once all other protocol details are finalized, it will be adjusted to target a theoretical maximum total issuance of `2**21` ETH per year if `2**27` ETH is validating (and therefore `2**20` per year if `2**25` ETH is validating, etc.)** * The `INACTIVITY_PENALTY_QUOTIENT` equals `INVERSE_SQRT_E_DROP_TIME**2` where `INVERSE_SQRT_E_DROP_TIME := 2**12 epochs` (~18 days) is the time it takes the inactivity penalty to reduce the balance of non-participating [validators](#dfn-validator) to about `1/sqrt(e) ~= 60.6%`. Indeed, the balance retained by offline [validators](#dfn-validator) after `n` epochs is about `(1 - 1/INACTIVITY_PENALTY_QUOTIENT)**(n**2/2)` so after `INVERSE_SQRT_E_DROP_TIME` epochs it is roughly `(1 - 1/INACTIVITY_PENALTY_QUOTIENT)**(INACTIVITY_PENALTY_QUOTIENT/2) ~= 1/sqrt(e)`. ### Max operations per block @@ -1415,7 +1415,7 @@ def process_crosslinks(state: BeaconState) -> None: def get_base_reward(state: BeaconState, index: ValidatorIndex) -> Gwei: total_balance = get_total_active_balance(state) effective_balance = state.validator_registry[index].effective_balance - return effective_balance * BASE_REWARD_QUOTIENT // integer_squareroot(total_balance) // BASE_REWARDS_PER_EPOCH + return effective_balance * BASE_REWARD_FACTOR // integer_squareroot(total_balance) // BASE_REWARDS_PER_EPOCH ``` ```python From cc5b172da3d981f974e999dc21018bba4f95bcac Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 30 May 2019 12:38:55 +1000 Subject: [PATCH 6/6] Test deposit top-up with inconsistent withdrawal credentials (#1133) * Simplify deposits * Avoid divisions by zero Possible fix to avoid four cases of divisions by zero: * `return state.validator_registry[index].effective_balance // adjusted_quotient // BASE_REWARDS_PER_EPOCH` * `rewards[index] += get_base_reward(state, index) * attesting_balance // total_balance` * `validator.effective_balance * min(total_penalties * 3, total_balance) // total_balance` * `rewards[index] += base_reward * attesting_balance // committee_balance` See also #1107. * fix deposit test for new index handling * tests: deposit with inconsistent withdrawal credentials * Update README.md * Update 0_beacon-chain.md * Fix linter errors * Update test_process_deposit.py * fix deposit test * fix lint --- .../block_processing/test_process_deposit.py | 18 ++++++++++++++++++ .../pyspec/eth2spec/test/helpers/deposits.py | 16 +++++++++++----- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/block_processing/test_process_deposit.py b/test_libs/pyspec/eth2spec/test/block_processing/test_process_deposit.py index 0ef2e509a..0430dd12f 100644 --- a/test_libs/pyspec/eth2spec/test/block_processing/test_process_deposit.py +++ b/test_libs/pyspec/eth2spec/test/block_processing/test_process_deposit.py @@ -94,6 +94,22 @@ def test_invalid_sig_top_up(state): yield from run_deposit_processing(state, deposit, validator_index, valid=True, effective=True) +@spec_state_test +def test_invalid_withdrawal_credentials_top_up(state): + validator_index = 0 + amount = spec.MAX_EFFECTIVE_BALANCE // 4 + withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX_BYTE + spec.hash(b"junk")[1:] + deposit = prepare_state_and_deposit( + state, + validator_index, + amount, + withdrawal_credentials=withdrawal_credentials + ) + + # inconsistent withdrawal credentials, in top-ups, are allowed! + yield from run_deposit_processing(state, deposit, validator_index, valid=True, effective=True) + + @spec_state_test def test_wrong_index(state): validator_index = len(state.validator_registry) @@ -122,6 +138,7 @@ def test_wrong_deposit_for_deposit_count(state): pubkey_1, privkey_1, spec.MAX_EFFECTIVE_BALANCE, + withdrawal_credentials=b'\x00'*32, signed=True, ) deposit_count_1 = len(deposit_data_leaves) @@ -136,6 +153,7 @@ def test_wrong_deposit_for_deposit_count(state): pubkey_2, privkey_2, spec.MAX_EFFECTIVE_BALANCE, + withdrawal_credentials=b'\x00'*32, signed=True, ) diff --git a/test_libs/pyspec/eth2spec/test/helpers/deposits.py b/test_libs/pyspec/eth2spec/test/helpers/deposits.py index c5deb124e..2db3ae03c 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/deposits.py +++ b/test_libs/pyspec/eth2spec/test/helpers/deposits.py @@ -8,11 +8,10 @@ from eth2spec.utils.merkle_minimal import calc_merkle_tree_from_leaves, get_merk from eth2spec.utils.minimal_ssz import signing_root -def build_deposit_data(state, pubkey, privkey, amount, signed=False): +def build_deposit_data(state, pubkey, privkey, amount, withdrawal_credentials, signed=False): deposit_data = DepositData( pubkey=pubkey, - # insecurely use pubkey as withdrawal key as well - withdrawal_credentials=spec.BLS_WITHDRAWAL_PREFIX_BYTE + spec.hash(pubkey)[1:], + withdrawal_credentials=withdrawal_credentials, amount=amount, ) if signed: @@ -37,8 +36,9 @@ def build_deposit(state, pubkey, privkey, amount, + withdrawal_credentials, signed): - deposit_data = build_deposit_data(state, pubkey, privkey, amount, signed) + deposit_data = build_deposit_data(state, pubkey, privkey, amount, withdrawal_credentials, signed) item = deposit_data.hash_tree_root() index = len(deposit_data_leaves) @@ -57,7 +57,7 @@ def build_deposit(state, return deposit, root, deposit_data_leaves -def prepare_state_and_deposit(state, validator_index, amount, signed=False): +def prepare_state_and_deposit(state, validator_index, amount, withdrawal_credentials=None, signed=False): """ Prepare the state for the deposit, and create a deposit for the given validator, depositing the given amount. """ @@ -67,12 +67,18 @@ def prepare_state_and_deposit(state, validator_index, amount, signed=False): pubkey = pubkeys[validator_index] privkey = privkeys[validator_index] + + # insecurely use pubkey as withdrawal key if no credentials provided + if withdrawal_credentials is None: + withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX_BYTE + spec.hash(pubkey)[1:] + deposit, root, deposit_data_leaves = build_deposit( state, deposit_data_leaves, pubkey, privkey, amount, + withdrawal_credentials, signed )