fork choice cleanups (#3849)

Removes a few extra-ambitious templates to make `self` updates explicit,
and moves the `FinalityCheckpoints` type from `base` to `helpers` as it
is an additional Nimbus specific type not defined by spec.

beacon_chain/fork_choice/fork_choice.nim

@@ -159,15 +159,12 @@ proc update_checkpoints(
 proc on_tick(
     self: var ForkChoice, dag: ChainDAGRef, time: BeaconTime): FcResult[void] =
   ## Must be called at least once per slot.
-  template backend(): auto = self.backend
+  let previous_time = self.checkpoints.time
-  template checkpoints(): auto = self.checkpoints
-
-  let previous_time = checkpoints.time
 
   # update store time
   if time < previous_time:
     return err ForkChoiceError(kind: fcInconsistentTick)
-  checkpoints.time = time
+  self.checkpoints.time = time
 
   let
     current_slot = time.slotOrZero
@@ -175,7 +172,7 @@ proc on_tick(
 
   # Reset store.proposer_boost_root if this is a new slot
   if current_slot > previous_slot:
-    checkpoints.proposer_boost_root.reset()
+    self.checkpoints.proposer_boost_root = ZERO_HASH
 
   # Not a new epoch, return
   if not (current_slot > previous_slot and current_slot.is_epoch):
@@ -183,19 +180,22 @@ proc on_tick(
 
   # Update store.justified_checkpoint if a better checkpoint on the
   # store.finalized_checkpoint chain
-  if checkpoints.best_justified.epoch > checkpoints.justified.checkpoint.epoch:
   let
-      blck = dag.getBlockRef(checkpoints.best_justified.root).valueOr:
+    best_justified_epoch = self.checkpoints.best_justified.epoch
+    store_justified_epoch = self.checkpoints.justified.checkpoint.epoch
+  if best_justified_epoch > store_justified_epoch:
+    let
+      blck = dag.getBlockRef(self.checkpoints.best_justified.root).valueOr:
         return err ForkChoiceError(
           kind: fcJustifiedNodeUnknown,
-          blockRoot: checkpoints.best_justified.root)
+          blockRoot: self.checkpoints.best_justified.root)
-      finalized_ancestor = blck.atEpochStart(checkpoints.finalized.epoch)
+      finalized_ancestor = blck.atEpochStart(self.checkpoints.finalized.epoch)
-    if finalized_ancestor.blck.root == checkpoints.finalized.root:
+    if finalized_ancestor.blck.root == self.checkpoints.finalized.root:
-      checkpoints.update_justified(dag, blck, checkpoints.best_justified.epoch)
+      self.checkpoints.update_justified(dag, blck, best_justified_epoch)
 
   # Pull-up chain tips from previous epoch
-  for realized in backend.proto_array.realizePendingCheckpoints():
+  for realized in self.backend.proto_array.realizePendingCheckpoints():
-    ? checkpoints.update_checkpoints(dag, realized)
+    ? self.checkpoints.update_checkpoints(dag, realized)
 
   ok()
 

beacon_chain/fork_choice/fork_choice_types.nim

@@ -14,7 +14,8 @@ import
   stew/results,
   chronicles,
   # Internal
-  ../spec/datatypes/base
+  ../spec/datatypes/base,
+  ../spec/helpers
 
 # https://github.com/ethereum/consensus-specs/blob/v0.11.1/specs/phase0/fork-choice.md
 # This is a port of https://github.com/sigp/lighthouse/pull/804

beacon_chain/fork_choice/proto_array.nim

@@ -15,6 +15,7 @@ import
   stew/results,
   # Internal
   ../spec/datatypes/base,
+  ../spec/helpers,
   # Fork choice
   ./fork_choice_types
 

beacon_chain/spec/datatypes/base.nim

@@ -246,10 +246,6 @@ type
     epoch*: Epoch
     root*: Eth2Digest
 
-  FinalityCheckpoints* = object
-    justified*: Checkpoint
-    finalized*: Checkpoint
-
   # https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.1/specs/phase0/beacon-chain.md#AttestationData
   AttestationData* = object
     slot*: Slot
@@ -752,12 +748,6 @@ func shortLog*(v: Checkpoint): auto =
   # epoch:root when logging epoch, root:slot when logging slot!
   $shortLog(v.epoch) & ":" & shortLog(v.root)
 
-func shortLog*(v: FinalityCheckpoints): auto =
-  (
-    justified: shortLog(v.justified),
-    finalized: shortLog(v.finalized)
-  )
-
 func shortLog*(v: AttestationData): auto =
   (
     slot: shortLog(v.slot),
@@ -829,7 +819,6 @@ func shortLog*(v: SomeSignedVoluntaryExit): auto =
 chronicles.formatIt AttestationData: it.shortLog
 chronicles.formatIt Attestation: it.shortLog
 chronicles.formatIt Checkpoint: it.shortLog
-chronicles.formatIt FinalityCheckpoints: it.shortLog
 
 const
   # http://facweb.cs.depaul.edu/sjost/it212/documents/ascii-pr.htm

beacon_chain/spec/helpers.nim

@@ -18,6 +18,7 @@ import
   std/[algorithm, math, sets, tables],
   # Status libraries
   stew/[bitops2, byteutils, endians2, objects],
+  chronicles,
   # Internal
   ./datatypes/[phase0, altair, bellatrix],
   "."/[eth2_merkleization, forks, ssz_codec]
@@ -27,6 +28,18 @@ import
 export
   forks, eth2_merkleization, ssz_codec
 
+type FinalityCheckpoints* = object
+  justified*: Checkpoint
+  finalized*: Checkpoint
+
+func shortLog*(v: FinalityCheckpoints): auto =
+  (
+    justified: shortLog(v.justified),
+    finalized: shortLog(v.finalized)
+  )
+
+chronicles.formatIt FinalityCheckpoints: it.shortLog
+
 # https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.1/specs/phase0/beacon-chain.md#integer_squareroot
 func integer_squareroot*(n: SomeInteger): SomeInteger =
   ## Return the largest integer ``x`` such that ``x**2 <= n``.

tests/fork_choice/interpreter.nim

@@ -12,9 +12,10 @@ import
   stew/[results, endians2],
   # Internals
   ../../beacon_chain/spec/datatypes/base,
+  ../../beacon_chain/spec/helpers,
   ../../beacon_chain/fork_choice/[fork_choice, fork_choice_types]
 
-export results, base, fork_choice, fork_choice_types, tables, options
+export results, base, helpers, fork_choice, fork_choice_types, tables, options
 
 func fakeHash*(index: SomeInteger): Eth2Digest =
   ## Create fake hashes