cleanup `OnBlockAdded` usage (#5426)

Reduce repetitiveness when using forked `OnBlockAdded` callbacks by
introducing a template to obtain appropriate cb from `ConsensusFork`.

beacon_chain/consensus_object_pools/block_pools_types.nim

@@ -279,7 +279,7 @@ type
     # balances, as used in fork choice
     effective_balances_bytes*: seq[byte]
 
-  OnBlockAdded[T] = proc(
+  OnBlockAdded[T: ForkyTrustedSignedBeaconBlock] = proc(
     blckRef: BlockRef, blck: T, epochRef: EpochRef,
     unrealized: FinalityCheckpoints) {.gcsafe, raises: [].}
   OnPhase0BlockAdded* = OnBlockAdded[phase0.TrustedSignedBeaconBlock]
@@ -321,6 +321,20 @@ type
     block_root* {.serializedFieldName: "block".}: Eth2Digest
     optimistic* {.serializedFieldName: "execution_optimistic".}: Option[bool]
 
+template OnBlockAddedCallback*(kind: static ConsensusFork): auto =
+  when kind == ConsensusFork.Deneb:
+    typedesc[OnDenebBlockAdded]
+  elif kind == ConsensusFork.Capella:
+    typedesc[OnCapellaBlockAdded]
+  elif kind == ConsensusFork.Bellatrix:
+    typedesc[OnBellatrixBlockAdded]
+  elif kind == ConsensusFork.Altair:
+    typedesc[OnAltairBlockAdded]
+  elif kind == ConsensusFork.Phase0:
+    typedesc[OnPhase0BlockAdded]
+  else:
+    static: raiseAssert "Unreachable"
+
 func proposer_dependent_slot*(epochRef: EpochRef): Slot =
   epochRef.key.epoch.proposer_dependent_slot()
 

tests/test_blockchain_dag.nim

@@ -1212,9 +1212,7 @@ suite "Ancestry":
 
       let
         blck = dag.headState.addTestBlock(cache, nextSlot = false, cfg = cfg)
-        added = block:
-          const nilCallback = OnPhase0BlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.phase0Data, nilCallback)
+        added = dag.addHeadBlock(verifier, blck.phase0Data, nilPhase0Callback)
       check added.isOk()
       dag.updateHead(added[], quarantine[], [])
       (blck: dag.head, state: newClone(dag.headState.phase0Data))
@@ -1499,27 +1497,13 @@ template runShufflingTests(cfg: RuntimeConfig, numRandomTests: int) =
     graffiti: GraffitiBytes
   proc addBlocks(blocks: uint64, attested: bool, cache: var StateCache) =
     inc distinctBase(graffiti)[0]  # Avoid duplicate blocks across branches
-    for blck in makeTestBlocks(
+    for forkedBlck in makeTestBlocks(
         dag.headState, cache, blocks.int, eth1_data = eth1Data,
         attested = attested, allDeposits = deposits,
         graffiti = graffiti, cfg = cfg):
-      let added =
-        case blck.kind
-        of ConsensusFork.Phase0:
-          const nilCallback = OnPhase0BlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.phase0Data, nilCallback)
-        of ConsensusFork.Altair:
-          const nilCallback = OnAltairBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.altairData, nilCallback)
-        of ConsensusFork.Bellatrix:
-          const nilCallback = OnBellatrixBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.bellatrixData, nilCallback)
-        of ConsensusFork.Capella:
-          const nilCallback = OnCapellaBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.capellaData, nilCallback)
-        of ConsensusFork.Deneb:
-          const nilCallback = OnDenebBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.denebData, nilCallback)
+      let added = withBlck(forkedBlck):
+        const nilCallback = (consensusFork.OnBlockAddedCallback)(nil)
+        dag.addHeadBlock(verifier, blck, nilCallback)
       check added.isOk()
       dag.updateHead(added[], quarantine[], [])
 

tests/test_gossip_validation.nim

@@ -193,23 +193,9 @@ suite "Gossip validation - Altair":
     for blck in makeTestBlocks(
         dag.headState, cache, blocks = 1,
         attested = false, cfg = cfg):
-      let added =
-        case blck.kind
-        of ConsensusFork.Phase0:
-          const nilCallback = OnPhase0BlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.phase0Data, nilCallback)
-        of ConsensusFork.Altair:
-          const nilCallback = OnAltairBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.altairData, nilCallback)
-        of ConsensusFork.Bellatrix:
-          const nilCallback = OnBellatrixBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.bellatrixData, nilCallback)
-        of ConsensusFork.Capella:
-          const nilCallback = OnCapellaBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.capellaData, nilCallback)
-        of ConsensusFork.Deneb:
-          const nilCallback = OnDenebBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.denebData, nilCallback)
+      let added = withBlck(blck):
+        const nilCallback = (consensusFork.OnBlockAddedCallback)(nil)
+        dag.addHeadBlock(verifier, blck, nilCallback)
       check: added.isOk()
       dag.updateHead(added[], quarantine, [])
 

tests/test_light_client.nim

@@ -63,24 +63,9 @@ suite "Light client" & preset():
       for blck in makeTestBlocks(
           dag.headState, cache, blocks.int, attested = attested,
           syncCommitteeRatio = syncCommitteeRatio, cfg = cfg):
-        let added =
-          case blck.kind
-          of ConsensusFork.Phase0:
-            const nilCallback = OnPhase0BlockAdded(nil)
-            dag.addHeadBlock(verifier, blck.phase0Data, nilCallback)
-          of ConsensusFork.Altair:
-            const nilCallback = OnAltairBlockAdded(nil)
-            dag.addHeadBlock(verifier, blck.altairData, nilCallback)
-          of ConsensusFork.Bellatrix:
-            const nilCallback = OnBellatrixBlockAdded(nil)
-            dag.addHeadBlock(verifier, blck.bellatrixData, nilCallback)
-          of ConsensusFork.Capella:
-            const nilCallback = OnCapellaBlockAdded(nil)
-            dag.addHeadBlock(verifier, blck.capellaData, nilCallback)
-          of ConsensusFork.Deneb:
-            const nilCallback = OnDenebBlockAdded(nil)
-            dag.addHeadBlock(verifier, blck.denebData, nilCallback)
-
+        let added = withBlck(blck):
+          const nilCallback = (consensusFork.OnBlockAddedCallback)(nil)
+          dag.addHeadBlock(verifier, blck, nilCallback)
         check: added.isOk()
         dag.updateHead(added[], quarantine, [])
 

tests/test_light_client_processor.nim

@@ -51,23 +51,9 @@ suite "Light client processor" & preset():
     for blck in makeTestBlocks(
         dag.headState, cache, blocks.int, attested = true,
         syncCommitteeRatio = syncCommitteeRatio, cfg = cfg):
-      let added =
-        case blck.kind
-        of ConsensusFork.Phase0:
-          const nilCallback = OnPhase0BlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.phase0Data, nilCallback)
-        of ConsensusFork.Altair:
-          const nilCallback = OnAltairBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.altairData, nilCallback)
-        of ConsensusFork.Bellatrix:
-          const nilCallback = OnBellatrixBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.bellatrixData, nilCallback)
-        of ConsensusFork.Capella:
-          const nilCallback = OnCapellaBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.capellaData, nilCallback)
-        of ConsensusFork.Deneb:
-          const nilCallback = OnDenebBlockAdded(nil)
-          dag.addHeadBlock(verifier, blck.denebData, nilCallback)
+      let added = withBlck(blck):
+        const nilCallback = (consensusFork.OnBlockAddedCallback)(nil)
+        dag.addHeadBlock(verifier, blck, nilCallback)
       doAssert added.isOk()
       dag.updateHead(added[], quarantine[], [])