avoid recomputing base_reward_per_increment() across attestations (#2704)

* avoid recomputing base_reward_per_increment() across attestations

* add adapter template to fix nbench process_attestations build

* nfuzz also uses process_attestations()

beacon_chain/spec/beaconstate.nim

@@ -702,7 +702,8 @@ proc check_attestation*(
 
 proc process_attestation*(
     state: var SomeBeaconState, attestation: SomeAttestation, flags: UpdateFlags,
-    cache: var StateCache): Result[void, cstring] {.nbench.} =
+    base_reward_per_increment: Gwei, cache: var StateCache):
+    Result[void, cstring] {.nbench.} =
   # In the spec, attestation validation is mixed with state mutation, so here
   # we've split it into two functions so that the validation logic can be
   # reused when looking for suitable blocks to include in attestations.
@@ -739,7 +740,6 @@ proc process_attestation*(
       participation_flag_indices =
         get_attestation_participation_flag_indices(
           state, attestation.data, state.slot - attestation.data.slot)
-      base_reward_per_increment = get_base_reward_per_increment(state, cache)
 
     for index in get_attesting_indices(state, attestation.data, attestation.aggregation_bits, cache):
         for flag_index, weight in PARTICIPATION_FLAG_WEIGHTS:
@@ -756,11 +756,13 @@ proc process_attestation*(
     increase_balance(state, proposer_index.get, proposer_reward)
 
   when state is phase0.BeaconState:
+    doAssert base_reward_per_increment == 0.Gwei
     if attestation.data.target.epoch == get_current_epoch(state):
       addPendingAttestation(state.current_epoch_attestations)
     else:
       addPendingAttestation(state.previous_epoch_attestations)
   elif state is altair.BeaconState:
+    doAssert base_reward_per_increment > 0.Gwei
     if attestation.data.target.epoch == get_current_epoch(state):
       updateParticipationFlags(state.current_epoch_participation)
     else:

beacon_chain/spec/state_transition_block.nim

@@ -310,13 +310,24 @@ proc process_operations(preset: RuntimePreset,
                         cache: var StateCache): Result[void, cstring] {.nbench.} =
   # Verify that outstanding deposits are processed up to the maximum number of
   # deposits
+  template base_reward_per_increment(state: phase0.BeaconState): Gwei = 0.Gwei
+  template base_reward_per_increment(state: altair.BeaconState): Gwei =
+    get_base_reward_per_increment(state, cache)
+
   let
     req_deposits = min(MAX_DEPOSITS,
                        state.eth1_data.deposit_count - state.eth1_deposit_index)
+    generalized_base_reward_per_increment = base_reward_per_increment(state)
+
   if state.eth1_data.deposit_count < state.eth1_deposit_index or
       body.deposits.lenu64 != req_deposits:
     return err("incorrect number of deposits")
 
+  template do_process_attestation(state, operation, flags, cache: untyped):
+      untyped =
+    process_attestation(
+      state, operation, flags, generalized_base_reward_per_increment, cache)
+
   template for_ops(operations: auto, fn: auto) =
     for operation in operations:
       let res = fn(state, operation, flags, cache)
@@ -325,7 +336,7 @@ proc process_operations(preset: RuntimePreset,
 
   for_ops(body.proposer_slashings, process_proposer_slashing)
   for_ops(body.attester_slashings, process_attester_slashing)
-  for_ops(body.attestations, process_attestation)
+  for_ops(body.attestations, do_process_attestation)
 
   for deposit in body.deposits:
     let res = process_deposit(preset, state, deposit, flags)

nbench/scenarios.nim

@@ -286,8 +286,11 @@ genProcessBlockScenario(runProcessProposerSlashing,
                         needFlags = true,
                         needCache = true)
 
+template do_process_attestation(state, operation, flags, cache: untyped):
+    untyped =
+  process_attestation(state, operation, flags, 0.Gwei, cache)
 genProcessBlockScenario(runProcessAttestation,
-                        process_attestation,
+                        do_process_attestation,
                         attestation,
                         Attestation,
                         needFlags = true,

nfuzz/libnfuzz.nim

@@ -98,7 +98,7 @@ template decodeAndProcess(typ, process: untyped): bool =
 proc nfuzz_attestation(input: openArray[byte], xoutput: ptr byte,
     xoutput_size: ptr uint, disable_bls: bool): bool {.exportc, raises: [FuzzCrashError, Defect].} =
   decodeAndProcess(AttestationInput):
-    process_attestation(data.state, data.attestation, flags, cache).isOk
+    process_attestation(data.state, data.attestation, flags, 0.Gwei, cache).isOk
 
 proc nfuzz_attester_slashing(input: openArray[byte], xoutput: ptr byte,
     xoutput_size: ptr uint, disable_bls: bool): bool {.exportc, raises: [FuzzCrashError, Defect].} =

tests/official/altair/test_fixture_operations_attestations.nim

@@ -51,12 +51,16 @@ proc runTest(identifier: string) =
       if existsFile(testDir/"post.ssz_snappy"):
         let postState =
           newClone(parseTest(testDir/"post.ssz_snappy", SSZ, BeaconState))
-        let done = process_attestation(preState[], attestation, {}, cache).isOk
+        let done = process_attestation(
+          preState[], attestation, {},
+          get_base_reward_per_increment(preState[], cache), cache).isOk
         doAssert done, "Valid attestation not processed"
         check: preState[].hash_tree_root() == postState[].hash_tree_root()
         reportDiff(preState, postState)
       else:
-        let done = process_attestation(preState[], attestation, {}, cache).isOk
+        let done = process_attestation(
+          preState[], attestation, {},
+          get_base_reward_per_increment(preState[], cache), cache).isOk
         doAssert done == false, "We didn't expect this invalid attestation to be processed."
 
   `testImpl _ operations_attestations _ identifier`()

tests/official/phase0/test_fixture_operations_attestations.nim

@@ -51,12 +51,14 @@ proc runTest(identifier: string) =
       if existsFile(testDir/"post.ssz_snappy"):
         let postState =
           newClone(parseTest(testDir/"post.ssz_snappy", SSZ, BeaconState))
-        let done = process_attestation(preState[], attestation, {}, cache).isOk
+        let done = process_attestation(
+          preState[], attestation, {}, 0.Gwei, cache).isOk
         doAssert done, "Valid attestation not processed"
         check: preState[].hash_tree_root() == postState[].hash_tree_root()
         reportDiff(preState, postState)
       else:
-        let done = process_attestation(preState[], attestation, {}, cache).isOk
+        let done = process_attestation(
+          preState[], attestation, {}, 0.Gwei, cache).isOk
         doAssert done == false, "We didn't expect this invalid attestation to be processed."
 
   `testImpl _ operations_attestations _ identifier`()

tests/spec_block_processing/test_process_attestation.nim

@@ -53,7 +53,7 @@ suite "[Unit - Spec - Block processing] Attestations " & preset():
       # ----------------------------------------
       var cache = StateCache()
       check process_attestation(
-        state.hbsPhase0.data, attestation, flags = {}, cache
+        state.hbsPhase0.data, attestation, flags = {}, 0.Gwei, cache
       ).isOk
 
       # Check that the attestation was processed