relax BlockRef database assumptions

beacon_chain/consensus_object_pools/attestation_pool.nim

@@ -138,8 +138,9 @@ proc init*(T: type AttestationPool, dag: ChainDAGRef,
         else:
           epochRef = dag.getEpochRef(blckRef, blckRef.slot.epoch, false).expect(
             "Getting an EpochRef should always work for non-finalized blocks")
-
-          withBlck(dag.getForkedBlock(blckRef)):
+          let blck = dag.getForkedBlock(blckRef.bid).expect(
+              "Should be able to load initial fork choice blocks")
+          withBlck(blck):
             forkChoice.process_block(
               dag, epochRef, blckRef, blck.message,
               blckRef.slot.start_beacon_time)

beacon_chain/consensus_object_pools/block_dag.nim

@@ -198,7 +198,7 @@ func toBlockSlotId*(bs: BlockSlot): BlockSlotId =
 
 func isProposed*(bid: BlockId, slot: Slot): bool =
   ## Return true if `bid` was proposed in the given slot
-  bid.slot == 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

beacon_chain/consensus_object_pools/blockchain_dag.nim

@@ -68,7 +68,6 @@ template withStateVars*(
   template stateRoot(): Eth2Digest {.inject, used.} =
     getStateRoot(stateDataInternal.data)
   template blck(): BlockRef {.inject, used.} = stateDataInternal.blck
-  template root(): Eth2Digest {.inject, used.} = stateDataInternal.data.root
 
   body
 
@@ -429,11 +428,6 @@ proc getForkedBlock*(
       result.err()
       return
 
-proc getForkedBlock*(
-    dag: ChainDAGRef, blck: BlockRef): ForkedTrustedSignedBeaconBlock =
-  dag.getForkedBlock(blck.bid).expect(
-    "BlockRef block should always load, database corrupt?")
-
 proc getForkedBlock*(
     dag: ChainDAGRef, root: Eth2Digest): Opt[ForkedTrustedSignedBeaconBlock] =
   let bid = dag.getBlockId(root)
@@ -1220,7 +1214,6 @@ proc updateStateData*(
       found,
       assignDur,
       replayDur
-
   elif ancestors.len > 0:
     debug "State replayed",
       blocks = ancestors.len,

beacon_chain/gossip_processing/gossip_validation.nim

@@ -205,19 +205,24 @@ template validateBeaconBlockBellatrix(
       # mostly relevant around merge transition epochs. It's possible that
       # the previous block is phase 0 or Altair, if this is the transition
       # block itself.
-      let blockData = dag.getForkedBlock(parent)
-      case blockData.kind:
-      of BeaconBlockFork.Phase0:
+      let blockData = dag.getForkedBlock(parent.bid)
+      if blockData.isOk():
+        case blockData.get().kind:
+        of BeaconBlockFork.Phase0:
+          false
+        of BeaconBlockFork.Altair:
+          false
+        of BeaconBlockFork.Bellatrix:
+          # https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/bellatrix/beacon-chain.md#process_execution_payload
+          # shows how this gets folded into the state each block; checking this
+          # is equivalent, without ever requiring state replay or any similarly
+          # expensive computation.
+          blockData.get().bellatrixData.message.body.execution_payload !=
+            default(ExecutionPayload)
+      else:
+        warn "Cannot load block parent, assuming execution is disabled",
+          parent = shortLog(parent)
         false
-      of BeaconBlockFork.Altair:
-        false
-      of BeaconBlockFork.Bellatrix:
-        # https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/bellatrix/beacon-chain.md#process_execution_payload
-        # shows how this gets folded into the state each block; checking this
-        # is equivalent, without ever requiring state replay or any similarly
-        # expensive computation.
-        blockData.bellatrixData.message.body.execution_payload !=
-          default(ExecutionPayload)
 
   if executionEnabled:
     # [REJECT] The block's execution payload timestamp is correct with respect
@@ -294,11 +299,13 @@ proc validateBeaconBlock*(
 
   if slotBlock.isProposed() and
       slotBlock.blck.slot == signed_beacon_block.message.slot:
-    let data = dag.getForkedBlock(slotBlock.blck)
-    if getForkedBlockField(data, proposer_index) ==
-          signed_beacon_block.message.proposer_index and
-        data.signature.toRaw() != signed_beacon_block.signature.toRaw():
-      return errIgnore("BeaconBlock: already proposed in the same slot")
+    let curBlock = dag.getForkedBlock(slotBlock.blck.bid)
+    if curBlock.isOk():
+      let data = curBlock.get()
+      if getForkedBlockField(data, proposer_index) ==
+            signed_beacon_block.message.proposer_index and
+          data.signature.toRaw() != signed_beacon_block.signature.toRaw():
+        return errIgnore("BeaconBlock: already proposed in the same slot")
 
   # [IGNORE] The block's parent (defined by block.parent_root) has been seen
   # (via both gossip and non-gossip sources) (a client MAY queue blocks for

beacon_chain/rpc/rest_beacon_api.nim

@@ -682,15 +682,9 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
                                          $rroot.error())
       return RestApiResponse.jsonError(Http500, NoImplementationError)
 
-    let blck =
-      block:
-        let res = node.getCurrentBlock(qslot)
-        if res.isErr():
-          return RestApiResponse.jsonError(Http404, BlockNotFoundError,
-                                            $res.error())
-        res.get()
+    let bdata = node.getForkedBlock(BlockIdent.init(qslot)).valueOr:
+      return RestApiResponse.jsonError(Http404, BlockNotFoundError)
 
-    let bdata = node.dag.getForkedBlock(blck)
     return
       withBlck(bdata):
         RestApiResponse.jsonResponse(
@@ -765,42 +759,55 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
   router.api(MethodGet, "/eth/v1/beacon/blocks/{block_id}") do (
     block_id: BlockIdent) -> RestApiResponse:
     let
-      bid = block_id.valueOr:
+      blockIdent = block_id.valueOr:
         return RestApiResponse.jsonError(Http400, InvalidBlockIdValueError,
                                          $error)
-
-      bdata = node.getForkedBlock(bid).valueOr:
+      bid = node.getBlockId(blockIdent).valueOr:
         return RestApiResponse.jsonError(Http404, BlockNotFoundError)
 
+    if node.dag.cfg.blockForkAtEpoch(bid.slot.epoch) != BeaconBlockFork.Phase0:
+      return RestApiResponse.jsonError(
+        Http404, BlockNotFoundError, "v1 API supports only phase 0 blocks")
+
     let contentType =
       block:
         let res = preferredContentType(jsonMediaType,
                                        sszMediaType)
         if res.isErr():
           return RestApiResponse.jsonError(Http406, ContentNotAcceptableError)
         res.get()
+
     return
-      case bdata.kind
-      of BeaconBlockFork.Phase0:
-        if contentType == sszMediaType:
-          RestApiResponse.sszResponse(bdata.phase0Data)
-        elif contentType == jsonMediaType:
-          RestApiResponse.jsonResponse(bdata.phase0Data)
+      if contentType == sszMediaType:
+        var data: seq[byte]
+        if not node.dag.getBlockSSZ(bid, data):
+          return RestApiResponse.jsonError(Http404, BlockNotFoundError)
+
+        RestApiResponse.response(data, Http200, $sszMediaType)
+      elif contentType == jsonMediaType:
+        let bdata = node.dag.getForkedBlock(bid).valueOr:
+          return RestApiResponse.jsonError(Http404, BlockNotFoundError)
+
+        if bdata.kind == BeaconBlockFork.Phase0:
+          RestApiResponse.jsonResponse(bdata.phase0Data.asSigned())
         else:
-          RestApiResponse.jsonError(Http500, InvalidAcceptError)
-      of BeaconBlockFork.Altair, BeaconBlockFork.Bellatrix:
-        RestApiResponse.jsonError(Http404, BlockNotFoundError)
+          # Shouldn't happen, but in case there's some weird block database
+          # issue..
+          RestApiResponse.jsonError(
+            Http404, BlockNotFoundError, "v1 API supports only phase 0 blocks")
+      else:
+        RestApiResponse.jsonError(Http500, InvalidAcceptError)
 
   # https://ethereum.github.io/beacon-APIs/#/Beacon/getBlockV2
   router.api(MethodGet, "/eth/v2/beacon/blocks/{block_id}") do (
     block_id: BlockIdent) -> RestApiResponse:
     let
-      bid = block_id.valueOr:
+      blockIdent = block_id.valueOr:
         return RestApiResponse.jsonError(Http400, InvalidBlockIdValueError,
                                          $error)
-
-      bdata = node.getForkedBlock(bid).valueOr:
+      bid = node.getBlockId(blockIdent).valueOr:
         return RestApiResponse.jsonError(Http404, BlockNotFoundError)
+
     let contentType =
       block:
         let res = preferredContentType(jsonMediaType,
@@ -810,9 +817,15 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
         res.get()
     return
       if contentType == sszMediaType:
-        withBlck(bdata):
-          RestApiResponse.sszResponse(blck)
+        var data: seq[byte]
+        if not node.dag.getBlockSSZ(bid, data):
+          return RestApiResponse.jsonError(Http404, BlockNotFoundError)
+
+        RestApiResponse.response(data, Http200, $sszMediaType)
       elif contentType == jsonMediaType:
+        let bdata = node.dag.getForkedBlock(bid).valueOr:
+          return RestApiResponse.jsonError(Http404, BlockNotFoundError)
+
         RestApiResponse.jsonResponsePlain(bdata.asSigned())
       else:
         RestApiResponse.jsonError(Http500, InvalidAcceptError)
@@ -821,14 +834,14 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) =
   router.api(MethodGet, "/eth/v1/beacon/blocks/{block_id}/root") do (
     block_id: BlockIdent) -> RestApiResponse:
     let
-      bid = block_id.valueOr:
+      blockIdent = block_id.valueOr:
         return RestApiResponse.jsonError(Http400, InvalidBlockIdValueError,
                                          $error)
 
-      blck = node.getBlockId(bid).valueOr:
+      bid = node.getBlockId(blockIdent).valueOr:
         return RestApiResponse.jsonError(Http404, BlockNotFoundError)
 
-    return RestApiResponse.jsonResponse((root: blck.root))
+    return RestApiResponse.jsonResponse((root: bid.root))
 
   # https://ethereum.github.io/beacon-APIs/#/Beacon/getBlockAttestations
   router.api(MethodGet,

beacon_chain/rpc/rest_utils.nim

@@ -46,14 +46,6 @@ func getCurrentSlot*(node: BeaconNode, slot: Slot):
   else:
     err("Requesting slot too far ahead of the current head")
 
-func getCurrentBlock*(node: BeaconNode, slot: Slot):
-    Result[BlockRef, cstring] =
-  let bs = node.dag.getBlockAtSlot(? node.getCurrentSlot(slot))
-  if bs.isProposed():
-    ok(bs.blck)
-  else:
-    err("Block not found")
-
 proc getCurrentHead*(node: BeaconNode, slot: Slot): Result[BlockRef, cstring] =
   let res = node.dag.head
   # if not(node.isSynced(res)):
@@ -95,7 +87,7 @@ proc getBlockSlot*(node: BeaconNode,
       ok(node.dag.head.atEpochStart(getStateField(
         node.dag.headState.data, current_justified_checkpoint).epoch))
 
-proc getBlockId*(node: BeaconNode, id: BlockIdent): Result[BlockId, cstring] =
+proc getBlockId*(node: BeaconNode, id: BlockIdent): Opt[BlockId] =
   case id.kind
   of BlockQueryKind.Named:
     case id.value
@@ -106,33 +98,19 @@ proc getBlockId*(node: BeaconNode, id: BlockIdent): Result[BlockId, cstring] =
     of BlockIdentType.Finalized:
       ok(node.dag.finalizedHead.blck.bid)
   of BlockQueryKind.Root:
-    node.dag.getBlockId(id.root).orErr(cstring("Block not found"))
+    node.dag.getBlockId(id.root)
   of BlockQueryKind.Slot:
     let bsid = node.dag.getBlockIdAtSlot(id.slot)
     if bsid.isProposed():
       ok bsid.bid
     else:
-      err("Block not found")
+      err()
 
 proc getForkedBlock*(node: BeaconNode, id: BlockIdent):
-    Result[ForkedTrustedSignedBeaconBlock, cstring] =
-  case id.kind
-  of BlockQueryKind.Named:
-    case id.value
-    of BlockIdentType.Head:
-      ok(node.dag.getForkedBlock(node.dag.head))
-    of BlockIdentType.Genesis:
-      ok(node.dag.getForkedBlock(node.dag.genesis))
-    of BlockIdentType.Finalized:
-      ok(node.dag.getForkedBlock(node.dag.finalizedHead.blck))
-  of BlockQueryKind.Root:
-    node.dag.getForkedBlock(id.root).orErr(cstring("Block not found"))
-  of BlockQueryKind.Slot:
-    let bsid = node.dag.getBlockIdAtSlot(id.slot)
-    if bsid.isProposed():
-      node.dag.getForkedBlock(bsid.bid).orErr(cstring("Block not found"))
-    else:
-      err("Block not found")
+    Opt[ForkedTrustedSignedBeaconBlock] =
+  let bid = ? node.getBlockId(id)
+
+  node.dag.getForkedBlock(bid)
 
 proc disallowInterruptionsAux(body: NimNode) =
   for n in body:

beacon_chain/rpc/rpc_beacon_api.nim

@@ -159,11 +159,14 @@ proc getForkedBlockFromBlockId(
     raises: [Defect, CatchableError].} =
   case blockId:
     of "head":
-      node.dag.getForkedBlock(node.dag.head)
+      node.dag.getForkedBlock(node.dag.head.bid).valueOr:
+        raise newException(CatchableError, "Block not found")
     of "genesis":
-      node.dag.getForkedBlock(node.dag.genesis)
+      node.dag.getForkedBlock(node.dag.genesis.bid).valueOr:
+        raise newException(CatchableError, "Block not found")
     of "finalized":
-      node.dag.getForkedBlock(node.dag.finalizedHead.blck)
+      node.dag.getForkedBlock(node.dag.finalizedHead.blck.bid).valueOr:
+        raise newException(CatchableError, "Block not found")
     else:
       if blockId.startsWith("0x"):
         let

ncli/ncli_db.nim

@@ -956,7 +956,7 @@ proc cmdValidatorDb(conf: DbConf, cfg: RuntimeConfig) =
         clear cache
 
   for bi in 0 ..< blockRefs.len:
-    let forkedBlock = dag.getForkedBlock(blockRefs[blockRefs.len - bi - 1])
+    let forkedBlock = dag.getForkedBlock(blockRefs[blockRefs.len - bi - 1].bid).get()
     withBlck(forkedBlock):
       processSlots(blck.message.slot, {skipLastStateRootCalculation})
 

tests/test_block_dag.nim

@@ -83,10 +83,14 @@ suite "BlockSlot and helpers":
 
   test "parent sanity":
     let
+      root = block:
+        var d: Eth2Digest
+        d.data[0] = 1
+        d
       s0 = BlockRef(bid: BlockId(slot: Slot(0)))
       s00 = BlockSlot(blck: s0, slot: Slot(0))
       s01 = BlockSlot(blck: s0, slot: Slot(1))
-      s2 = BlockRef(bid: BlockId(slot: Slot(2)), parent: s0)
+      s2 = BlockRef(bid: BlockId(slot: Slot(2), root: root), parent: s0)
       s22 = BlockSlot(blck: s2, slot: Slot(2))
       s24 = BlockSlot(blck: s2, slot: Slot(4))
 

tests/test_blockchain_dag.nim

@@ -541,7 +541,7 @@ suite "chain DAG finalization tests" & preset():
         assign(tmpStateData[], dag.headState)
         check:
           dag.updateStateData(tmpStateData[], cur.atSlot(cur.slot), false, cache)
-          dag.getForkedBlock(cur).phase0Data.message.state_root ==
+          dag.getForkedBlock(cur.bid).get().phase0Data.message.state_root ==
             getStateRoot(tmpStateData[].data)
           getStateRoot(tmpStateData[].data) == hash_tree_root(
             tmpStateData[].data.phase0Data.data)