Voluntary exit - split single + multiple exits proc & tests (#421)

* add test suite for voluntary exit

* update API to process_voluntary_exit

* Add range check of validator_index for voluntary exits

* Revert to dual single + multiple voluntary exits API + enable in test suite

* no cache or mocking needed

beacon_chain/spec/state_transition_block.nim

@@ -329,53 +329,68 @@ proc processDeposits(state: var BeaconState, blck: BeaconBlock): bool =
 
   true
 
-# https://github.com/ethereum/eth2.0-specs/blob/v0.6.3/specs/core/0_beacon-chain.md#voluntary-exits
-proc processVoluntaryExits(
-    state: var BeaconState, blck: BeaconBlock, flags: UpdateFlags): bool =
-  # Process ``VoluntaryExit`` transaction.
-  if len(blck.body.voluntary_exits) > MAX_VOLUNTARY_EXITS:
-    notice "Exit: too many!"
+# https://github.com/ethereum/eth2.0-specs/blob/v0.8.3/specs/core/0_beacon-chain.md#voluntary-exits
+proc process_voluntary_exit*(
+    state: var BeaconState,
+    exit: VoluntaryExit,
+    flags: UpdateFlags): bool =
+
+  # Not in spec. Check that validator_index is in range
+  if exit.validator_index.int >= state.validators.len:
+    notice "Exit: invalid validator index",
+      index = exit.validator_index,
+      num_validators = state.validators.len
     return false
 
-  for exit in blck.body.voluntary_exits:
-    let validator = state.validators[exit.validator_index.int]
+  let validator = state.validators[exit.validator_index.int]
 
-    # Verify the validator is active
-    if not is_active_validator(validator, get_current_epoch(state)):
-      notice "Exit: validator not active"
+  # Verify the validator is active
+  if not is_active_validator(validator, get_current_epoch(state)):
+    notice "Exit: validator not active"
+    return false
+
+  # Verify the validator has not yet exited
+  if validator.exit_epoch != FAR_FUTURE_EPOCH:
+    notice "Exit: validator has exited"
+    return false
+
+  ## Exits must specify an epoch when they become valid; they are not valid
+  ## before then
+  if not (get_current_epoch(state) >= exit.epoch):
+    notice "Exit: exit epoch not passed"
+    return false
+
+  # Verify the validator has been active long enough
+  if not (get_current_epoch(state) >= validator.activation_epoch +
+      PERSISTENT_COMMITTEE_PERIOD):
+    notice "Exit: not in validator set long enough"
+    return false
+
+  # Verify signature
+  if skipValidation notin flags:
+    let domain = get_domain(state, DOMAIN_VOLUNTARY_EXIT, exit.epoch)
+    if not bls_verify(
+        validator.pubkey,
+        signing_root(exit).data,
+        exit.signature,
+        domain):
+      notice "Exit: invalid signature"
       return false
 
-    # Verify the validator has not yet exited
-    if not (validator.exit_epoch == FAR_FUTURE_EPOCH):
-      notice "Exit: validator has exited"
-      return false
-
-    ## Exits must specify an epoch when they become valid; they are not valid
-    ## before then
-    if not (get_current_epoch(state) >= exit.epoch):
-      notice "Exit: exit epoch not passed"
-      return false
-
-    # Verify the validator has been active long enough
-    # TODO detect underflow
-    if not (get_current_epoch(state) - validator.activation_epoch >=
-        PERSISTENT_COMMITTEE_PERIOD):
-      notice "Exit: not in validator set long enough"
-      return false
-
-    # Verify signature
-    if skipValidation notin flags:
-      if not bls_verify(
-          validator.pubkey, signing_root(exit).data, exit.signature,
-          get_domain(state, DOMAIN_VOLUNTARY_EXIT, exit.epoch)):
-        notice "Exit: invalid signature"
-        return false
-
-    # Initiate exit
-    initiate_validator_exit(state, exit.validator_index.ValidatorIndex)
+  # Initiate exit
+  initiate_validator_exit(state, exit.validator_index.ValidatorIndex)
 
   true
 
+proc processVoluntaryExits(state: var BeaconState, blck: BeaconBlock, flags: UpdateFlags): bool =
+  if len(blck.body.voluntary_exits) > MAX_VOLUNTARY_EXITS:
+    notice "[Block processing - Voluntary Exit]: too many exits!"
+    return false
+  for exit in blck.body.voluntary_exits:
+    if not process_voluntary_exit(state, exit, flags):
+      return false
+  return true
+
 # https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#transfers
 proc processTransfers(state: var BeaconState, blck: BeaconBlock,
                       flags: UpdateFlags, stateCache: var StateCache): bool =
@@ -469,6 +484,9 @@ proc processBlock*(
 
   processEth1Data(state, blck.body)
 
+  # TODO, everything below is now in process_operations
+  # and implementation is per element instead of the whole seq
+
   if not processProposerSlashings(state, blck, flags, stateCache):
     debug "[Block processing] Proposer slashing failure", slot = shortLog(state.slot)
     return false
@@ -486,7 +504,7 @@ proc processBlock*(
     return false
 
   if not processVoluntaryExits(state, blck, flags):
-    debug "[Block processing] Exit processing failure", slot = shortLog(state.slot)
+    debug "[Block processing - Voluntary Exit] Exit processing failure", slot = shortLog(state.slot)
     return false
 
   if not processTransfers(state, blck, flags, stateCache):

tests/official/all_fixtures_require_ssz.nim

@@ -14,4 +14,5 @@ import
   ./test_fixture_sanity_blocks,
   ./test_fixture_state_transition_epoch,
   ./test_fixture_operations_attestations,
-  ./test_fixture_operations_block_header
+  ./test_fixture_operations_block_header,
+  ./test_fixture_operations_voluntary_exit

tests/official/test_fixture_operations_voluntary_exit.nim

@@ -0,0 +1,73 @@
+# beacon_chain
+# Copyright (c) 2018-Present Status Research & Development GmbH
+# Licensed and distributed under either of
+#   * MIT license (license terms in the root directory or at http://opensource.org/licenses/MIT).
+#   * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0).
+# at your option. This file may not be copied, modified, or distributed except according to those terms.
+
+import
+  # Standard library
+  os, unittest, strutils,
+  # Beacon chain internals
+  ../../beacon_chain/spec/[datatypes, state_transition_block],
+  ../../beacon_chain/[ssz, extras],
+  # Test utilities
+  ../testutil,
+  ./fixtures_utils,
+  ../helpers/debug_state
+
+const OpVoluntaryExitDir = SszTestsDir/const_preset/"phase0"/"operations"/"voluntary_exit"/"pyspec_tests"
+
+template runTest(identifier: untyped) =
+  # We wrap the tests in a proc to avoid running out of globals
+  # in the future: Nim supports up to 3500 globals
+  # but unittest with the macro/templates put everything as globals
+  # https://github.com/nim-lang/Nim/issues/12084#issue-486866402
+
+  const testDir = OpVoluntaryExitDir / astToStr(identifier)
+
+  proc `testImpl _ voluntary_exit _ identifier`() =
+
+    var flags: UpdateFlags
+    var prefix: string
+    if not existsFile(testDir/"meta.yaml"):
+      flags.incl skipValidation
+    if existsFile(testDir/"post.ssz"):
+      prefix = "[Valid]   "
+    else:
+      prefix = "[Invalid] "
+
+    test prefix & astToStr(identifier):
+      var stateRef, postRef: ref BeaconState
+      var voluntaryExit: ref VoluntaryExit
+      new voluntaryExit
+      new stateRef
+
+      voluntaryExit[] = parseTest(testDir/"voluntary_exit.ssz", SSZ, VoluntaryExit)
+      stateRef[] = parseTest(testDir/"pre.ssz", SSZ, BeaconState)
+
+      if existsFile(testDir/"post.ssz"):
+        new postRef
+        postRef[] = parseTest(testDir/"post.ssz", SSZ, BeaconState)
+
+      if postRef.isNil:
+        let done = process_voluntary_exit(stateRef[], voluntaryExit[], flags)
+        doAssert done == false, "We didn't expect this invalid voluntary exit to be processed."
+      else:
+        let done = process_voluntary_exit(stateRef[], voluntaryExit[], flags)
+        doAssert done, "Valid voluntary exit not processed"
+        check: stateRef.hash_tree_root() == postRef.hash_tree_root()
+        reportDiff(stateRef, postRef)
+
+  `testImpl _ voluntary_exit _ identifier`()
+
+suite "Official - Operations - Voluntary exit " & preset():
+  runTest(success)
+  runTest(invalid_signature)
+  runTest(success_exit_queue)
+  runTest(validator_exit_in_future)
+  runTest(validator_invalid_validator_index)
+  runTest(validator_not_active)
+  runTest(validator_already_exited)
+  runTest(validator_not_active_long_enough)
+