move `BlockId` to `spec` (#3511)

The spec implicitly talks about the slot of a block in several places, and keeping it readily available is useful in a number of context - might as well put this implicitly refereneced helper in the spec code directly
2022-03-16 16:00:18 +01:00 · 2022-03-16 16:00:18 +01:00 · 8a63efc413
parent 88af3f2797
commit 8a63efc413
6 changed files with 90 additions and 65 deletions
--- a/beacon_chain/consensus_object_pools/block_dag.nim
+++ b/beacon_chain/consensus_object_pools/block_dag.nim
@ -14,18 +14,6 @@ import
 export chronicles, forks

 type
-  BlockId* = object
-    ## A BlockId is the root and the slot in which that block was
-    ## produced - there are no guarantees that this block is part of
-    ## the canonical chain, or that we have validated it
-    root*: Eth2Digest
-    slot*: Slot
-
-  BlockSlotId* = object
-    ## A BlockId at a slot equal to or higher than the slot of the block
-    bid*: BlockId
-    slot*: Slot
-
  BlockRef* = ref object
    ## Node in object graph guaranteed to lead back to tail block, and to have
    ## a corresponding entry in database.
@ -55,9 +43,6 @@ type
      ## Slot time for this BlockSlot which may differ from blck.slot when time
      ## has advanced without blocks

-func hash*(bid: BlockId): Hash =
-  hash(bid.root)
-
 template root*(blck: BlockRef): Eth2Digest = blck.bid.root
 template slot*(blck: BlockRef): Slot = blck.bid.slot

@ -70,12 +55,6 @@ func init*(T: type BlockRef, root: Eth2Digest, blck: SomeForkyBeaconBlock):
    BlockRef =
  BlockRef.init(root, blck.slot)

-func toBlockId*(blck: SomeForkySignedBeaconBlock): BlockId =
-  BlockId(root: blck.root, slot: blck.message.slot)
-
-func toBlockId*(blck: ForkedSignedBeaconBlock): BlockId =
-  withBlck(blck): BlockId(root: blck.root, slot: blck.message.slot)
-
 func parent*(bs: BlockSlot): BlockSlot =
  ## Return a blockslot representing the previous slot, using the parent block
  ## if the current slot had a block
@ -164,15 +143,6 @@ func atSlot*(blck: BlockRef, slot: Slot): BlockSlot =
 func atSlot*(blck: BlockRef): BlockSlot =
  blck.atSlot(blck.slot)

-func init*(T: type BlockSlotId, bid: BlockId, slot: Slot): T =
-  doAssert slot >= bid.slot
-  BlockSlotId(bid: bid, slot: slot)
-
-func atSlot*(bid: BlockId): BlockSlotId =
-  # BlockSlotId doesn't not have an atSlot function taking slot because it does
-  # not share the parent-traversing features of `atSlot(BlockRef)`
-  BlockSlotId.init(bid, bid.slot)
-
 func atEpochStart*(blck: BlockRef, epoch: Epoch): BlockSlot =
  ## Return the BlockSlot corresponding to the first slot in the given epoch
  atSlot(blck, epoch.start_slot())
@ -199,10 +169,6 @@ func toBlockSlotId*(bs: BlockSlot): Opt[BlockSlotId] =
  else:
    ok BlockSlotId.init(bs.blck.bid, bs.slot)

-func isProposed*(bid: BlockId, slot: Slot): bool =
-  ## Return true if `bid` was proposed in the given slot
-  bid.slot == slot and not bid.root.isZero
-
 func isProposed*(blck: BlockRef, slot: Slot): bool =
  ## Return true if `blck` was proposed in the given slot
  not isNil(blck) and blck.bid.isProposed(slot)
@ -212,22 +178,6 @@ func isProposed*(bs: BlockSlot): bool =
  ## slot)
  bs.blck.isProposed(bs.slot)

-func isProposed*(bsi: BlockSlotId): bool =
-  ## Return true if `bs` represents the proposed block (as opposed to an empty
-  ## slot)
-  bsi.bid.isProposed(bsi.slot)
-
-func shortLog*(v: BlockId): string =
-  # epoch:root when logging epoch, root:slot when logging slot!
-  shortLog(v.root) & ":" & $v.slot
-
-func shortLog*(v: BlockSlotId): string =
-  # epoch:root when logging epoch, root:slot when logging slot!
-  if v.bid.slot == v.slot:
-    shortLog(v.bid)
-  else: # There was a gap - log it
-    shortLog(v.bid) & "@" & $v.slot
-
 func shortLog*(v: BlockRef): string =
  # epoch:root when logging epoch, root:slot when logging slot!
  if v.isNil():
@ -244,7 +194,5 @@ func shortLog*(v: BlockSlot): string =
  else: # There was a gap - log it
    shortLog(v.blck) & "@" & $v.slot

-chronicles.formatIt BlockId: shortLog(it)
-chronicles.formatIt BlockSlotId: shortLog(it)
 chronicles.formatIt BlockSlot: shortLog(it)
 chronicles.formatIt BlockRef: shortLog(it)
--- a/beacon_chain/consensus_object_pools/blockchain_dag.nim
+++ b/beacon_chain/consensus_object_pools/blockchain_dag.nim
@ -1092,11 +1092,7 @@ proc updateState*(
  let
    startTick = Moment.now()
    current {.used.} = withState(state):
-      BlockSlotId.init(
-        BlockId(
-          root: state.latest_block_root,
-          slot: state.data.latest_block_header.slot),
-        state.data.slot)
+      BlockSlotId.init(state.latest_block_id, state.data.slot)

  var
    ancestors: seq[BlockRef]
@ -1216,11 +1212,7 @@ proc updateState*(
  let
    assignTick = Moment.now()
    ancestor {.used.} = withState(state):
-      BlockSlotId.init(
-        BlockId(
-          root: state.latest_block_root,
-          slot: state.data.latest_block_header.slot),
-        state.data.slot)
+      BlockSlotId.init(state.latest_block_id, state.data.slot)
    ancestorRoot {.used.} = getStateRoot(state)

  var info: ForkedEpochInfo
--- a/beacon_chain/spec/beaconstate.nim
+++ b/beacon_chain/spec/beaconstate.nim
@ -1005,11 +1005,22 @@ func attester_dependent_root*(state: ForkyHashedBeaconState): Eth2Digest =
  let epoch = state.data.slot.epoch
  state.dependent_root(if epoch == Epoch(0): epoch else: epoch - 1)

+func latest_block_id*(state: ForkyHashedBeaconState): BlockId =
+  ## Block id of the latest block applied to this state
+  BlockId(
+    root: state.latest_block_root,
+    slot: state.data.latest_block_header.slot)
+
+func latest_block_id*(state: ForkedHashedBeaconState): BlockId =
+  ## Block id of the latest block applied to this state
+  withState(state): state.latest_block_id()
+
 func matches_block*(
    state: ForkyHashedBeaconState, block_root: Eth2Digest): bool =
  ## Return true iff the latest block applied to this state matches the given
  ## `block_root`
  block_root == state.latest_block_root
+
 func matches_block*(
    state: ForkedHashedBeaconState, block_root: Eth2Digest): bool =
  withState(state): state.matches_block(block_root)
--- a/beacon_chain/spec/block_id.nim
+++ b/beacon_chain/spec/block_id.nim
@ -0,0 +1,66 @@
+# beacon_chain
+# Copyright (c) 2018-2022 Status Research & Development GmbH
+# Licensed and distributed under either of
+#   * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
+#   * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).
+# at your option. This file may not be copied, modified, or distributed except according to those terms.
+
+{.push raises: [Defect].}
+
+import
+  chronicles,
+  "."/[beacon_time, digest]
+
+export beacon_time, digest
+
+type
+  BlockId* = object
+    ## A BlockId is the root and the slot in which that block was
+    ## produced - there are no guarantees that this block is part of
+    ## the canonical chain, or that we have validated it - the type exists to
+    ## tie a slot to the root which helps find the block in various indices and
+    ## contexts
+    slot*: Slot # slot first for nicer sorting / comparisons :)
+    root*: Eth2Digest
+
+  BlockSlotId* = object
+    ## A BlockId at a slot equal to or higher than the slot of the block - when
+    ## a slot is missing its block, we still need a way to communicate that the
+    ## slot has changed - this type provides the necessary infrastructure
+    bid*: BlockId
+    slot*: Slot
+
+func hash*(bid: BlockId): Hash =
+  hash(bid.root)
+
+func init*(T: type BlockSlotId, bid: BlockId, slot: Slot): T =
+  doAssert slot >= bid.slot
+  BlockSlotId(bid: bid, slot: slot)
+
+func atSlot*(bid: BlockId): BlockSlotId =
+  # BlockSlotId doesn't not have an atSlot function taking slot because it does
+  # not share the parent-traversing features of `atSlot(BlockRef)`
+  BlockSlotId.init(bid, bid.slot)
+
+func isProposed*(bid: BlockId, slot: Slot): bool =
+  ## Return true if `bid` was proposed in the given slot
+  bid.slot == slot and not bid.root.isZero
+
+func isProposed*(bsi: BlockSlotId): bool =
+  ## Return true if `bs` represents the proposed block (as opposed to an empty
+  ## slot)
+  bsi.bid.isProposed(bsi.slot)
+
+func shortLog*(v: BlockId): string =
+  # epoch:root when logging epoch, root:slot when logging slot!
+  shortLog(v.root) & ":" & $v.slot
+
+func shortLog*(v: BlockSlotId): string =
+  # epoch:root when logging epoch, root:slot when logging slot!
+  if v.bid.slot == v.slot:
+    shortLog(v.bid)
+  else: # There was a gap - log it
+    shortLog(v.bid) & "@" & $v.slot
+
+chronicles.formatIt BlockId: shortLog(it)
+chronicles.formatIt BlockSlotId: shortLog(it)
--- a/beacon_chain/spec/forks.nim
+++ b/beacon_chain/spec/forks.nim
@ -11,12 +11,12 @@ import
  stew/assign2,
  chronicles,
  ../extras,
-  "."/[eth2_merkleization, eth2_ssz_serialization, presets],
+  "."/[block_id, eth2_merkleization, eth2_ssz_serialization, presets],
  ./datatypes/[phase0, altair, bellatrix]

 export
-  extras, phase0, altair, bellatrix, eth2_merkleization, eth2_ssz_serialization,
-  presets
+  extras, block_id, phase0, altair, bellatrix, eth2_merkleization,
+  eth2_ssz_serialization, presets

 # This file contains helpers for dealing with forks - we have two ways we can
 # deal with forks:
@ -577,3 +577,9 @@ func init*(T: type ForkDigests,
    sharding:
      compute_fork_digest(cfg.SHARDING_FORK_VERSION, genesisValidatorsRoot),
  )
+
+func toBlockId*(blck: SomeForkySignedBeaconBlock): BlockId =
+  BlockId(root: blck.root, slot: blck.message.slot)
+
+func toBlockId*(blck: ForkedSignedBeaconBlock | ForkedTrustedSignedBeaconBlock): BlockId =
+  withBlck(blck): BlockId(root: blck.root, slot: blck.message.slot)
--- a/tests/test_spec.nim
+++ b/tests/test_spec.nim
@ -54,6 +54,8 @@ suite "Beacon state" & preset():

    check: # Works for genesis block
      state[].phase0Data.latest_block_root == genBlock.root
+      state[].phase0Data.latest_block_id == genBlock.toBlockId()
+
      process_slots(cfg, state[], Slot 1, cache, info, {}).isOk()
      state[].phase0Data.latest_block_root == genBlock.root