Merge pull request #1664 from ethereum/div-zero-total-balance

avoid div by zero in extreme balance case

specs/phase0/beacon-chain.md

@@ -996,10 +996,11 @@ def get_beacon_proposer_index(state: BeaconState) -> ValidatorIndex:
 ```python
 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.)
+    Return the combined effective balance of the ``indices``.
+    ``EFFECTIVE_BALANCE_INCREMENT`` 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])))
+    return Gwei(max(EFFECTIVE_BALANCE_INCREMENT, sum([state.validators[index].effective_balance for index in indices])))
 ```
 
 #### `get_total_active_balance`
@@ -1008,6 +1009,7 @@ def get_total_balance(state: BeaconState, indices: Set[ValidatorIndex]) -> Gwei:
 def get_total_active_balance(state: BeaconState) -> Gwei:
     """
     Return the combined effective balance of the active validators.
+    Note: ``get_total_balance`` returns ``EFFECTIVE_BALANCE_INCREMENT`` Gwei minimum to avoid divisions by zero.
     """
     return get_total_balance(state, set(get_active_validator_indices(state, get_current_epoch(state))))
 ```
@@ -1289,6 +1291,10 @@ def get_unslashed_attesting_indices(state: BeaconState,
 
 ```python
 def get_attesting_balance(state: BeaconState, attestations: Sequence[PendingAttestation]) -> Gwei:
+    """
+    Return the combined effective balance of the set of unslashed validators participating in ``attestations``.
+    Note: ``get_total_balance`` returns ``EFFECTIVE_BALANCE_INCREMENT`` Gwei minimum to avoid divisions by zero.
+    """
     return get_total_balance(state, get_unslashed_attesting_indices(state, attestations))
 ```
 

tests/core/pyspec/eth2spec/test/context.py

@@ -71,6 +71,14 @@ def default_activation_threshold(spec):
     return spec.MAX_EFFECTIVE_BALANCE
 
 
+def zero_activation_threshold(spec):
+    """
+    Helper method to use 0 gwei as the activation threshold for state creation for tests.
+    Usage: `@with_custom_state(threshold_fn=zero_activation_threshold, ...)`
+    """
+    return 0
+
+
 def default_balances(spec):
     """
     Helper method to create a series of default balances.
@@ -104,6 +112,14 @@ def misc_balances(spec):
     return [spec.MAX_EFFECTIVE_BALANCE] * num_validators + [spec.MIN_DEPOSIT_AMOUNT] * num_misc_validators
 
 
+def low_single_balance(spec):
+    """
+    Helper method to create a single of balance of 1 Gwei.
+    Usage: `@with_custom_state(balances_fn=low_single_balance, ...)`
+    """
+    return [1]
+
+
 def single_phase(fn):
     """
     Decorator that filters out the phases data.

tests/core/pyspec/eth2spec/test/helpers/attestations.py

@@ -39,7 +39,7 @@ def build_attestation_data(spec, state, slot, index):
     )
 
 
-def get_valid_attestation(spec, state, slot=None, index=None, signed=False):
+def get_valid_attestation(spec, state, slot=None, index=None, empty=False, signed=False):
     if slot is None:
         slot = state.slot
     if index is None:
@@ -59,6 +59,7 @@ def get_valid_attestation(spec, state, slot=None, index=None, signed=False):
         aggregation_bits=aggregation_bits,
         data=attestation_data,
     )
+    if not empty:
         fill_aggregate_attestation(spec, state, attestation)
     if signed:
         sign_attestation(spec, state, attestation)

tests/core/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py

@@ -292,14 +292,12 @@ def test_bad_source_root(spec, state):
 @with_all_phases
 @spec_state_test
 def test_empty_aggregation_bits(spec, state):
-    attestation = get_valid_attestation(spec, state)
+    attestation = get_valid_attestation(spec, state, empty=True)
     state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY
 
-    attestation.aggregation_bits = Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE](
+    assert attestation.aggregation_bits == Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE](
         *([0b0] * len(attestation.aggregation_bits)))
 
-    sign_attestation(spec, state, attestation)
-
     yield from run_attestation_processing(spec, state, attestation)
 
 

tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py

@@ -2,7 +2,9 @@ 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,
+    misc_balances, low_single_balance,
+    with_custom_state,
+    default_activation_threshold, zero_activation_threshold,
     single_phase,
 )
 from eth2spec.test.helpers.state import (
@@ -60,12 +62,12 @@ def test_genesis_epoch_full_attestations_no_rewards(spec, state):
         assert state.balances[index] == pre_state.balances[index]
 
 
-def prepare_state_with_full_attestations(spec, state):
+def prepare_state_with_full_attestations(spec, state, empty=False):
     attestations = []
     for slot in range(spec.SLOTS_PER_EPOCH + spec.MIN_ATTESTATION_INCLUSION_DELAY):
         # create an attestation for each slot in epoch
         if slot < spec.SLOTS_PER_EPOCH:
-            attestation = get_valid_attestation(spec, state, signed=True)
+            attestation = get_valid_attestation(spec, state, empty=empty, signed=True)
             attestations.append(attestation)
         # fill each created slot in state after inclusion delay
         if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0:
@@ -143,6 +145,20 @@ def test_full_attestations_misc_balances(spec, state):
             assert state.balances[index] == pre_state.balances[index]
 
 
+@with_all_phases
+@spec_test
+@with_custom_state(balances_fn=low_single_balance, threshold_fn=zero_activation_threshold)
+@single_phase
+def test_full_attestations_one_validaor_one_gwei(spec, state):
+    attestations = prepare_state_with_full_attestations(spec, state)
+
+    yield from run_process_rewards_and_penalties(spec, state)
+
+    # Few assertions. Mainly to check that this extreme case can run without exception
+    attesting_indices = spec.get_unslashed_attesting_indices(state, attestations)
+    assert len(attesting_indices) == 1
+
+
 @with_all_phases
 @spec_state_test
 def test_no_attestations_all_penalties(spec, state):
@@ -157,6 +173,22 @@ def test_no_attestations_all_penalties(spec, state):
         assert state.balances[index] < pre_state.balances[index]
 
 
+@with_all_phases
+@spec_state_test
+def test_empty_attestations(spec, state):
+    attestations = prepare_state_with_full_attestations(spec, state, empty=True)
+
+    pre_state = deepcopy(state)
+
+    yield from run_process_rewards_and_penalties(spec, state)
+
+    attesting_indices = spec.get_unslashed_attesting_indices(state, attestations)
+    assert len(attesting_indices) == 0
+
+    for index in range(len(pre_state.validators)):
+        assert state.balances[index] < pre_state.balances[index]
+
+
 @with_all_phases
 @spec_state_test
 def test_duplicate_attestation(spec, state):