Fix ForkedChainRef

nimbus/beacon/beacon_engine.nim

@@ -216,7 +216,7 @@ proc generatePayload*(ben: BeaconEngineRef,
     let
       xp  = ben.txPool
       pos = xp.com.pos
-      headBlock = ?ben.chain.latestHeader()
+      headBlock = ben.chain.latestHeader
 
     pos.prevRandao   = ethHash attrs.prevRandao
     pos.timestamp    = ethTime attrs.timestamp

nimbus/core/chain/forked_chain.nim

@@ -311,11 +311,16 @@ func trimCanonicalChain(c: ForkedChainRef, head: CanonicalDesc) =
         break
       prevHash = header.parentHash
 
-proc setHead(c: ForkedChainRef, headHash: Hash256): Result[void, string] =
+proc setHead(c: ForkedChainRef, 
+             headHash: Hash256, 
+             number: BlockNumber): Result[void, string] =
   try:
     discard c.db.setHead(headHash)
   except RlpError as exc:
     return err(exc.msg)
+
+  # update global syncHighest
+  c.com.syncHighest = number
   ok()
 
 # ------------------------------------------------------------------------------
@@ -324,7 +329,7 @@ proc setHead(c: ForkedChainRef, headHash: Hash256): Result[void, string] =
 
 proc newForkedChain*(com: CommonRef,
                      baseHeader: BlockHeader,
-                     extraValidation: bool = false): ForkedChainRef =
+                     extraValidation: bool = true): ForkedChainRef =
   new(result)
   result.com = com
   result.db  = com.db
@@ -333,6 +338,9 @@ proc newForkedChain*(com: CommonRef,
   result.baseHash     = result.cursorHash
   result.cursorHeader = result.baseHeader
   result.extraValidation = extraValidation
+
+  # update global syncStart
+  com.syncStart = baseHeader.number
 
 proc importBlock*(c: ForkedChainRef, blk: EthBlock): Result[void, string] =
   # Try to import block to canonical or side chain.
@@ -390,7 +398,12 @@ proc forkChoice*(c: ForkedChainRef,
     if c.cursorHash != headHash:
       c.cursorHeader = head.header
       c.cursorHash = headHash
-    return c.setHead(headHash)
+
+    if c.stagingTx.isNil:
+      # setHead below don't go straight to db
+      c.stagingTx = c.db.newTransaction()
+
+    return c.setHead(headHash, head.header.number)
 
   # At this point cursorHeader.number > baseHeader.number
   if newBase.hash == c.cursorHash:
@@ -399,7 +412,7 @@ proc forkChoice*(c: ForkedChainRef,
 
     # Current segment is canonical chain
     c.writeBaggage(newBase.hash)
-    ?c.setHead(headHash)
+    ?c.setHead(headHash, head.header.number)
 
     # Paranoid check, guaranteed by `newBase.hash == c.cursorHash`
     doAssert(not c.stagingTx.isNil)
@@ -427,20 +440,23 @@ proc forkChoice*(c: ForkedChainRef,
   if newBase.header.number > c.baseHeader.number:
     c.replaySegment(newBase.hash)
     c.writeBaggage(newBase.hash)
-    ?c.setHead(headHash)
     c.stagingTx.commit()
     c.stagingTx = nil
     # Update base forward to newBase
     c.updateBase(newBase.hash, newBase.header, head.cursorHash)
     c.db.persistent(newBase.header.number).isOkOr:
       return err("Failed to save state: " & $$error)
 
+  if c.stagingTx.isNil:
+    # replaySegment or setHead below don't
+    # go straight to db
+    c.stagingTx = c.db.newTransaction()
+
   # Move chain state forward to current head
   if newBase.header.number < head.header.number:
-    if c.stagingTx.isNil:
-      c.stagingTx = c.db.newTransaction()
     c.replaySegment(headHash)
-    ?c.setHead(headHash)
+
+  ?c.setHead(headHash, head.header.number)
 
   # Move cursor to current head
   c.trimCanonicalChain(head)
@@ -457,14 +473,45 @@ func haveBlockAndState*(c: ForkedChainRef, hash: Hash256): bool =
     return true
   false
 
+func stateReady*(c: ForkedChainRef, header: BlockHeader): bool =
+  let blockHash = header.blockHash
+  blockHash == c.cursorHash
+
 func com*(c: ForkedChainRef): CommonRef =
   c.com
 
 func db*(c: ForkedChainRef): CoreDbRef =
   c.db
 
-func latestHeader*(c: ForkedChainRef): Result[BlockHeader, string] =
-  ok(c.cursorHeader)
+func latestHeader*(c: ForkedChainRef): BlockHeader =
+  c.cursorHeader
+
+func latestHash*(c: ForkedChainRef): Hash256 =
+  c.cursorHash
+
+proc headerByNumber*(c: ForkedChainRef, number: BlockNumber): Result[BlockHeader, string] =
+  if number == c.cursorHeader.number:
+    return err("Requested block number not exists: " & $number)
+
+  if number == c.cursorHeader.number:
+    return ok(c.cursorHeader)
+
+  if number == c.baseHeader.number:
+    return ok(c.baseHeader)
+
+  if number < c.baseHeader.number:
+    var header: BlockHeader
+    if c.db.getBlockHeader(number, header):
+      return ok(header)
+    else:
+      return err("Failed to get block with number: " & $number)
+
+  shouldNotKeyError:
+    var prevHash = c.cursorHeader.parentHash
+    while prevHash != c.baseHash:
+      let header = c.blocks[prevHash].blk.header
+      if header.number == number:
+        return ok(header)
+      prevHash = header.parentHash
 
-func headerByNumber*(c: ForkedChainRef, number: BlockNumber): Result[BlockHeader, string] =
-  discard
+  doAssert(false, "headerByNumber: Unreachable code")

nimbus/rpc/server_api.nim

@@ -34,11 +34,11 @@ func newServerAPI*(c: ForkedChainRef): ServerAPIRef =
   result.com = c.com
   result.chain = c
 
-func headerFromTag(api: ServerAPIRef, blockTag: BlockTag): Result[common.BlockHeader, string] =
+proc headerFromTag(api: ServerAPIRef, blockTag: BlockTag): Result[common.BlockHeader, string] =
   if blockTag.kind == bidAlias:
     let tag = blockTag.alias.toLowerAscii
     case tag
-    of "latest": return api.chain.latestHeader()
+    of "latest": return ok(api.chain.latestHeader)
     else:
       return err("Unsupported block tag " & tag)
   else:
@@ -47,9 +47,11 @@ func headerFromTag(api: ServerAPIRef, blockTag: BlockTag): Result[common.BlockHe
 
 proc ledgerFromTag(api: ServerAPIRef, blockTag: BlockTag): Result[LedgerRef, string] =
   let header = ?api.headerFromTag(blockTag)
-  # When we support more block tag,
-  # perhaps we also need to replay blocks
-  ok(LedgerRef.init(api.com.db, header.stateRoot))
+  if api.chain.stateReady(header):
+    ok(LedgerRef.init(api.com.db, header.stateRoot))
+  else:
+    # TODO: Replay state?
+    err("Block state not ready")
 
 proc setupServerAPI*(api: ServerAPIRef, server: RpcServer) =
 

nimbus/sync/beacon/skeleton_algo.nim

@@ -280,8 +280,7 @@ proc fillCanonicalChain*(sk: SkeletonRef): Result[void, string] =
   let subchain = sk.last
   if sk.progress.canonicalHeadReset:
     # Grab previous head block in case of resettng canonical head
-    let oldHead = sk.canonicalHead().valueOr:
-      return err(error)
+    let oldHead = sk.canonicalHead()
     maybeOldHead = Opt.some oldHead
 
     if subchain.tail > canonicalHead + 1:

nimbus/sync/beacon/skeleton_db.nim

@@ -184,9 +184,8 @@ proc deleteHeaderAndBody*(sk: SkeletonRef, header: BlockHeader) =
   sk.del(skeletonBlockHashToNumberKey(header.blockHash))
   sk.del(skeletonBodyKey(header.sumHash))
 
-proc canonicalHead*(sk: SkeletonRef): Result[BlockHeader, string] =
-  ## Returns Opt.some or error, never returns Opt.none
-  sk.chain.latestHeader()
+proc canonicalHead*(sk: SkeletonRef): BlockHeader =
+  sk.chain.latestHeader
 
 proc resetCanonicalHead*(sk: SkeletonRef, newHead, oldHead: uint64) =
   debug "RESET CANONICAL", newHead, oldHead

tests/test_beacon/test_6_abort_filling.nim

@@ -75,11 +75,7 @@ proc test6*() =
       check skel.blockHeight == 4
 
     test "canonical height should now be at head with correct chain":
-      let res = env.chain.latestHeader()
-      check res.isOk
-      let header = res.get
-      let latestHash = header.blockHash
-      check latestHash == block4PoS.blockHash
+      check env.chain.latestHash == block4PoS.blockHash
 
     test "should update to new height":
       skel.setHeadT(block5, true, false)

tests/test_beacon/test_8_pos_too_early.nim

@@ -61,8 +61,4 @@ proc test8*() =
       skel.initSyncT(block3, true)
       skel.putBlocksT([block2], 1, {FillCanonical})
       check skel.blockHeight == 3
-      let res = env.chain.latestHeader()
-      check res.isOk
-      let header = res.get
-      let latestHash = header.blockHash
-      check latestHash == block3.blockHash
+      check env.chain.latestHash == block3.blockHash