use cache in process_voluntary_exit() (#1507)

beacon_chain/merkle_minimal.nim

@@ -70,7 +70,11 @@ func getMerkleProof[Depth: static int](tree: SparseMerkleTree[Depth],
   #   - 0 --> go left
   #   - 1 --> go right
   let path = uint32(index)
+
+  # This is what the nnznodes[depth].len would be if `index` had been the last
+  # deposit on the Merkle tree
   var depthLen = index + 1
+
   for depth in 0 ..< Depth:
     let nodeIdx = int((path shr depth) xor 1)
 
@@ -82,17 +86,20 @@ func getMerkleProof[Depth: static int](tree: SparseMerkleTree[Depth],
     else:
       result[depth] = zeroHashes[depth]
 
+    # Round up, i.e. a half-pair of Merkle nodes/leaves still requires a node
+    # in the next Merkle tree layer calculated
     depthLen = (depthLen + 1) div 2
 
 func attachMerkleProofs*(deposits: var openarray[Deposit]) =
-  let deposit_data_roots = mapIt(deposits, it.data.hash_tree_root)
+  let
+    deposit_data_roots = mapIt(deposits, it.data.hash_tree_root)
+    merkle_tree = merkleTreeFromLeaves(deposit_data_roots)
   var
     deposit_data_sums: seq[Eth2Digest]
   for prefix_root in hash_tree_roots_prefix(
       deposit_data_roots, 1'i64 shl DEPOSIT_CONTRACT_TREE_DEPTH):
     deposit_data_sums.add prefix_root
 
-  let merkle_tree = merkleTreeFromLeaves(deposit_data_roots)
   for val_idx in 0 ..< deposits.len:
     deposits[val_idx].proof[0..31] = merkle_tree.getMerkleProof(val_idx, true)
     deposits[val_idx].proof[32].data[0..7] = uint_to_bytes8((val_idx + 1).uint64)

beacon_chain/spec/state_transition_block.nim

@@ -230,7 +230,8 @@ proc process_attester_slashing*(
 proc process_voluntary_exit*(
     state: var BeaconState,
     signed_voluntary_exit: SignedVoluntaryExit,
-    flags: UpdateFlags): Result[void, cstring] {.nbench.} =
+    flags: UpdateFlags,
+    cache: var StateCache): Result[void, cstring] {.nbench.} =
 
   let voluntary_exit = signed_voluntary_exit.message
 
@@ -275,7 +276,6 @@ proc process_voluntary_exit*(
     validator_withdrawable_epoch = validator.withdrawable_epoch,
     validator_exit_epoch = validator.exit_epoch,
     validator_effective_balance = validator.effective_balance
-  var cache = StateCache()
   initiate_validator_exit(
     state, voluntary_exit.validator_index.ValidatorIndex, cache)
 
@@ -286,7 +286,7 @@ proc process_operations(preset: RuntimePreset,
                         state: var BeaconState,
                         body: SomeBeaconBlockBody,
                         flags: UpdateFlags,
-                        stateCache: var StateCache): Result[void, cstring] {.nbench.} =
+                        cache: var StateCache): Result[void, cstring] {.nbench.} =
   # Verify that outstanding deposits are processed up to the maximum number of
   # deposits
   let
@@ -296,21 +296,15 @@ proc process_operations(preset: RuntimePreset,
   if not (num_deposits == req_deposits):
     return err("incorrect number of deposits")
 
-  template for_ops_cached(operations: auto, fn: auto) =
-    for operation in operations:
-      let res = fn(state, operation, flags, stateCache)
-      if res.isErr:
-        return res
-
   template for_ops(operations: auto, fn: auto) =
     for operation in operations:
-      let res = fn(state, operation, flags)
+      let res = fn(state, operation, flags, cache)
       if res.isErr:
         return res
 
-  for_ops_cached(body.proposer_slashings, process_proposer_slashing)
-  for_ops_cached(body.attester_slashings, process_attester_slashing)
-  for_ops_cached(body.attestations, process_attestation)
+  for_ops(body.proposer_slashings, process_proposer_slashing)
+  for_ops(body.attester_slashings, process_attester_slashing)
+  for_ops(body.attestations, process_attestation)
 
   for deposit in body.deposits:
     let res = process_deposit(preset, state, deposit, flags)

nbench/scenarios.nim

@@ -306,4 +306,4 @@ genProcessBlockScenario(runProcessVoluntaryExits,
                         deposit,
                         SignedVoluntaryExit,
                         needFlags = true,
-                        needCache = false)
+                        needCache = true)

nfuzz/libnfuzz.nim

@@ -138,7 +138,7 @@ proc nfuzz_proposer_slashing(input: openArray[byte], xoutput: ptr byte,
 proc nfuzz_voluntary_exit(input: openArray[byte], xoutput: ptr byte,
     xoutput_size: ptr uint, disable_bls: bool): bool {.exportc, raises: [FuzzCrashError, Defect].} =
   decodeAndProcess(VoluntaryExitInput):
-    process_voluntary_exit(data.state, data.exit, flags).isOk
+    process_voluntary_exit(data.state, data.exit, flags, cache).isOk
 
 # Note: Could also accept raw input pointer and access list_size + seed here.
 # However, list_size needs to be known also outside this proc to allocate xoutput.

tests/official/test_fixture_operations_voluntary_exit.nim

@@ -42,14 +42,18 @@ proc runTest(identifier: string) =
       let voluntaryExit = parseTest(testDir/"voluntary_exit.ssz", SSZ, SignedVoluntaryExit)
       var preState = newClone(parseTest(testDir/"pre.ssz", SSZ, BeaconState))
 
+      var cache = StateCache()
+
       if existsFile(testDir/"post.ssz"):
         let postState = newClone(parseTest(testDir/"post.ssz", SSZ, BeaconState))
-        let done = process_voluntary_exit(preState[], voluntaryExit, {}).isOk
+        let done =
+          process_voluntary_exit(preState[], voluntaryExit, {}, cache).isOk
         doAssert done, "Valid voluntary exit not processed"
         check: preState[].hash_tree_root() == postState[].hash_tree_root()
         reportDiff(preState, postState)
       else:
-        let done = process_voluntary_exit(preState[], voluntaryExit, {}).isOk
+        let done =
+          process_voluntary_exit(preState[], voluntaryExit, {}, cache).isOk
         doAssert done == false, "We didn't expect this invalid voluntary exit to be processed."
 
   `testImpl _ voluntary_exit _ identifier`()