Critical fix: introduce back total-value check (#1220)

This was dropped in a376b6607f, as improvement in dust checking.
Now that dust-checking is done, we still need to check if the sender has the minimum value, as decrease balance just clips to 0.
See be86f966f8 for older dust-creation problem work around, which was dropped in the above.

The bug enabled you to transfer your full balance to someone else, and pay the same amount in fee, possibly to a puppet proposer to collect back funds.
Effectively enabling printing of money. Silly bug, good to fix and introduce tests for.

specs/core/0_beacon-chain.md

@@ -1795,8 +1795,8 @@ def process_transfer(state: BeaconState, transfer: Transfer) -> None:
     """
     Process ``Transfer`` operation.
     """
-    # Verify the amount and fee are not individually too big (for anti-overflow purposes)
+    # Verify the balance the covers amount and fee (with overflow protection)
-    assert state.balances[transfer.sender] >= max(transfer.amount, transfer.fee)
+    assert state.balances[transfer.sender] >= max(transfer.amount + transfer.fee, transfer.amount, transfer.fee)
     # A transfer is valid in only one slot
     assert state.slot == transfer.slot
     # Sender must satisfy at least one of the following conditions in the parenthesis:

test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_transfer.py

@@ -114,7 +114,7 @@ def test_incorrect_slot(spec, state):
 
 @with_all_phases
 @spec_state_test
-def test_insufficient_balance_for_fee(spec, state):
+def test_insufficient_balance_for_fee_result_dust(spec, state):
     sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
     state.balances[sender_index] = spec.MAX_EFFECTIVE_BALANCE
     transfer = get_valid_transfer(spec, state, sender_index=sender_index, amount=0, fee=1, signed=True)
@@ -127,7 +127,20 @@ def test_insufficient_balance_for_fee(spec, state):
 
 @with_all_phases
 @spec_state_test
-def test_insufficient_balance(spec, state):
+def test_insufficient_balance_for_fee_result_full(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index,
+                                  amount=0, fee=state.balances[sender_index] + 1, signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_for_amount_result_dust(spec, state):
     sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
     state.balances[sender_index] = spec.MAX_EFFECTIVE_BALANCE
     transfer = get_valid_transfer(spec, state, sender_index=sender_index, amount=1, fee=0, signed=True)
@@ -138,6 +151,127 @@ def test_insufficient_balance(spec, state):
     yield from run_transfer_processing(spec, state, transfer, False)
 
 
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_for_amount_result_full(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index,
+                                  amount=state.balances[sender_index] + 1, fee=0, signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_for_combined_result_dust(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    # Enough to pay fee without dust, and amount without dust, but not both.
+    state.balances[sender_index] = spec.MIN_DEPOSIT_AMOUNT + 1
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index, amount=1, fee=1, signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_for_combined_result_full(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    # Enough to pay fee fully without dust left, and amount fully without dust left, but not both.
+    state.balances[sender_index] = spec.MIN_DEPOSIT_AMOUNT * 2 + 1
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index,
+                                  amount=spec.MIN_DEPOSIT_AMOUNT + 1,
+                                  fee=spec.MIN_DEPOSIT_AMOUNT + 1, signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_for_combined_big_amount(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    # Enough to pay fee fully without dust left, and amount fully without dust left, but not both.
+    # Try to create a dust balance (off by 1) with combination of fee and amount.
+    state.balances[sender_index] = spec.MIN_DEPOSIT_AMOUNT * 2 + 1
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index,
+                                  amount=spec.MIN_DEPOSIT_AMOUNT + 1, fee=1, signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_for_combined_big_fee(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    # Enough to pay fee fully without dust left, and amount fully without dust left, but not both.
+    # Try to create a dust balance (off by 1) with combination of fee and amount.
+    state.balances[sender_index] = spec.MIN_DEPOSIT_AMOUNT * 2 + 1
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index,
+                                  amount=1, fee=spec.MIN_DEPOSIT_AMOUNT + 1, signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_off_by_1_fee(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    # Enough to pay fee fully without dust left, and amount fully without dust left, but not both.
+    # Try to print money by using the full balance as amount, plus 1 for fee.
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index,
+                                  amount=state.balances[sender_index], fee=1, signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_off_by_1_amount(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    # Enough to pay fee fully without dust left, and amount fully without dust left, but not both.
+    # Try to print money by using the full balance as fee, plus 1 for amount.
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index, amount=1,
+                                  fee=state.balances[sender_index], signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
+@with_all_phases
+@spec_state_test
+def test_insufficient_balance_duplicate_as_fee_and_amount(spec, state):
+    sender_index = spec.get_active_validator_indices(state, spec.get_current_epoch(state))[-1]
+    # Enough to pay fee fully without dust left, and amount fully without dust left, but not both.
+    # Try to print money by using the full balance, twice.
+    transfer = get_valid_transfer(spec, state, sender_index=sender_index,
+                                  amount=state.balances[sender_index],
+                                  fee=state.balances[sender_index], signed=True)
+
+    # un-activate so validator can transfer
+    state.validators[transfer.sender].activation_eligibility_epoch = spec.FAR_FUTURE_EPOCH
+
+    yield from run_transfer_processing(spec, state, transfer, False)
+
+
 @with_all_phases
 @spec_state_test
 def test_no_dust_sender(spec, state):