Fix off-by-one error in syncing

beacon_chain/block_pool.nim

@@ -81,6 +81,7 @@ proc init*(T: type BlockPool, db: BeaconChainDB): BlockPool =
         link(newRef, curRef)
         curRef = curRef.parent
       blocks[curRef.root] = curRef
+      trace "Populating block pool", key = curRef.root, val = curRef
 
       if latestStateRoot.isNone() and db.containsState(blck.state_root):
         latestStateRoot = some(blck.state_root)
@@ -168,6 +169,7 @@ proc addResolvedBlock(
   link(parent, blockRef)
 
   pool.blocks[blockRoot] = blockRef
+  debug "Populating block pool", key = blockRoot, val = blockRef
 
   pool.addSlotMapping(blockRef)
 
@@ -322,9 +324,9 @@ proc add*(
 
 func getRef*(pool: BlockPool, root: Eth2Digest): BlockRef =
   ## Retrieve a resolved block reference, if available
-  pool.blocks.getOrDefault(root)
+  pool.blocks.getOrDefault(root, nil)
 
-func getBlockRange*(pool: BlockPool, headBlock: Eth2Digest,
+proc getBlockRange*(pool: BlockPool, headBlock: Eth2Digest,
                     startSlot: Slot, skipStep: Natural,
                     output: var openarray[BlockRef]): Natural =
   ## This function populates an `output` buffer of blocks
@@ -362,7 +364,7 @@ func getBlockRange*(pool: BlockPool, headBlock: Eth2Digest,
   template skip(n: int) =
     for i in 0 ..< n:
       if b.parent == nil:
-        trace "stopping at parentless block", slot = b.slot
+        trace "stopping at parentless block", slot = b.slot, root = b.root
         return
       b = b.parent
 
@@ -647,7 +649,7 @@ proc updateHead*(pool: BlockPool, state: var StateData, blck: BlockRef) =
   ## of operations naturally becomes important here - after updating the head,
   ## blocks that were once considered potential candidates for a tree will
   ## now fall from grace, or no longer be considered resolved.
-  doAssert blck.parent != nil
+  doAssert blck.parent != nil or blck.slot == 0
   logScope: pcs = "fork_choice"
 
   if pool.head.blck == blck:

beacon_chain/sync_protocol.nim

@@ -192,7 +192,7 @@ proc handleInitialStatus(peer: Peer,
       var s = ourStatus.headSlot + 1
       var theirStatus = theirStatus
       while s <= theirStatus.headSlot:
-        let numBlocksToRequest = min(uint64(theirStatus.headSlot - s),
+        let numBlocksToRequest = min(uint64(theirStatus.headSlot - s) + 1,
                                      MAX_REQUESTED_BLOCKS)
 
         debug "Requesting blocks", peer, remoteHeadSlot = theirStatus.headSlot,

tests/test_block_pool.nim

@@ -9,6 +9,7 @@
 
 import
   options, sequtils, unittest,
+  chronicles,
   ./testutil,
   ../beacon_chain/spec/[beaconstate, datatypes, digest],
   ../beacon_chain/[beacon_node_types, block_pool, beacon_chain_db, extras, ssz]
@@ -20,10 +21,18 @@ suite "Block pool processing" & preset():
       makeInitialDeposits(flags = {skipValidation}), {skipValidation})
     genBlock = get_initial_beacon_block(genState)
 
+  setup:
+    var
+      db = makeTestDB(genState, genBlock)
+      pool = BlockPool.init(db)
+      state = pool.loadTailState()
+
+  test "getRef returns nil for missing blocks":
+    check:
+      pool.getRef(default Eth2Digest) == nil
+
   test "loadTailState gets genesis block on first load" & preset():
     var
-      pool = BlockPool.init(makeTestDB(genState, genBlock))
-      state = pool.loadTailState()
       b0 = pool.get(state.blck.root)
 
     check:
@@ -32,10 +41,6 @@ suite "Block pool processing" & preset():
       toSeq(pool.blockRootsForSlot(GENESIS_SLOT)) == @[state.blck.root]
 
   test "Simple block add&get" & preset():
-    var
-      pool = BlockPool.init(makeTestDB(genState, genBlock))
-      state = pool.loadTailState()
-
     let
       b1 = makeBlock(state.data.data, state.blck.root, BeaconBlockBody())
       b1Root = signing_root(b1)
@@ -51,11 +56,6 @@ suite "Block pool processing" & preset():
       hash_tree_root(state.data.data) == state.data.root
 
   test "Reverse order block add & get" & preset():
-    var
-      db = makeTestDB(genState, genBlock)
-      pool = BlockPool.init(db)
-      state = pool.loadTailState()
-
     let
       b1 = addBlock(state.data.data, state.blck.root, BeaconBlockBody(), {})
       b1Root = signing_root(b1)