From 2d4ec7d52f0759b5e1d2ea8337d369e300a8f301 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 3 Mar 2020 09:29:59 -0700 Subject: [PATCH 1/3] add REWARD_OVERFLOW_INCREMENT to avoid overflow in rewards calculation --- specs/phase0/beacon-chain.md | 5 ++++- .../epoch_processing/test_process_rewards_and_penalties.py | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/specs/phase0/beacon-chain.md b/specs/phase0/beacon-chain.md index acf098663..7c14550bb 100644 --- a/specs/phase0/beacon-chain.md +++ b/specs/phase0/beacon-chain.md @@ -194,6 +194,7 @@ The following values are (non-configurable) constants used throughout the specif | `MAX_EFFECTIVE_BALANCE` | `Gwei(2**5 * 10**9)` (= 32,000,000,000) | | `EJECTION_BALANCE` | `Gwei(2**4 * 10**9)` (= 16,000,000,000) | | `EFFECTIVE_BALANCE_INCREMENT` | `Gwei(2**0 * 10**9)` (= 1,000,000,000) | +| `REWARD_OVERFLOW_INCREMENT` | `Gwei(2**6)` (= 64) | ### Initial values @@ -1313,7 +1314,9 @@ def get_attestation_deltas(state: BeaconState) -> Tuple[Sequence[Gwei], Sequence attesting_balance = get_total_balance(state, unslashed_attesting_indices) for index in eligible_validator_indices: if index in unslashed_attesting_indices: - rewards[index] += get_base_reward(state, index) * attesting_balance // total_balance + increment = REWARD_OVERFLOW_INCREMENT # Factored out from reward numerator to avoid uint64 overflow + reward_numerator = get_base_reward(state, index) // increment * attesting_balance + rewards[index] = reward_numerator // total_balance * increment else: penalties[index] += get_base_reward(state, index) diff --git a/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py b/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py index fa394df56..b4f50179e 100644 --- a/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py +++ b/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py @@ -1,7 +1,10 @@ from copy import deepcopy -from eth2spec.test.context import spec_state_test, with_all_phases, spec_test, \ - misc_balances, with_custom_state, default_activation_threshold, single_phase +from eth2spec.test.context import ( + spec_state_test, with_all_phases, spec_test, + misc_balances, with_custom_state, default_activation_threshold, + single_phase, +) from eth2spec.test.helpers.state import ( next_epoch, next_slot, From f082aa6ca9dfac0b1e03fe91b0cdf1e8886f3199 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 3 Mar 2020 15:34:02 -0700 Subject: [PATCH 2/3] use EFFECTIVE_BALANCE_INCREMENT to normalize reward calculations --- specs/phase0/beacon-chain.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/specs/phase0/beacon-chain.md b/specs/phase0/beacon-chain.md index 7c14550bb..cd1d765fe 100644 --- a/specs/phase0/beacon-chain.md +++ b/specs/phase0/beacon-chain.md @@ -194,7 +194,6 @@ The following values are (non-configurable) constants used throughout the specif | `MAX_EFFECTIVE_BALANCE` | `Gwei(2**5 * 10**9)` (= 32,000,000,000) | | `EJECTION_BALANCE` | `Gwei(2**4 * 10**9)` (= 16,000,000,000) | | `EFFECTIVE_BALANCE_INCREMENT` | `Gwei(2**0 * 10**9)` (= 1,000,000,000) | -| `REWARD_OVERFLOW_INCREMENT` | `Gwei(2**6)` (= 64) | ### Initial values @@ -1314,9 +1313,9 @@ def get_attestation_deltas(state: BeaconState) -> Tuple[Sequence[Gwei], Sequence attesting_balance = get_total_balance(state, unslashed_attesting_indices) for index in eligible_validator_indices: if index in unslashed_attesting_indices: - increment = REWARD_OVERFLOW_INCREMENT # Factored out from reward numerator to avoid uint64 overflow - reward_numerator = get_base_reward(state, index) // increment * attesting_balance - rewards[index] = reward_numerator // total_balance * increment + increment = EFFECTIVE_BALANCE_INCREMENT # Factored out from balance totals to avoid uint64 overflow + reward_numerator = get_base_reward(state, index) * (attesting_balance // increment) + rewards[index] = reward_numerator // (total_balance // increment) else: penalties[index] += get_base_reward(state, index) From 1579072e15e564904e41252f7db4ed5d1a04c057 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 10 Mar 2020 13:12:17 -0600 Subject: [PATCH 3/3] add note about total balance overflowing --- specs/phase0/beacon-chain.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/phase0/beacon-chain.md b/specs/phase0/beacon-chain.md index a90a8aa1a..b2b96de9d 100644 --- a/specs/phase0/beacon-chain.md +++ b/specs/phase0/beacon-chain.md @@ -954,6 +954,7 @@ def get_beacon_proposer_index(state: BeaconState) -> ValidatorIndex: def get_total_balance(state: BeaconState, indices: Set[ValidatorIndex]) -> Gwei: """ Return the combined effective balance of the ``indices``. (1 Gwei minimum to avoid divisions by zero.) + Math safe up to ~10B ETH, afterwhich this overflows uint64. """ return Gwei(max(1, sum([state.validators[index].effective_balance for index in indices]))) ```