diff --git a/beacon_chain/block_pool.nim b/beacon_chain/block_pool.nim index 8a18df0fc..a556d4178 100644 --- a/beacon_chain/block_pool.nim +++ b/beacon_chain/block_pool.nim @@ -450,33 +450,42 @@ func getRef*(pool: BlockPool, root: Eth2Digest): BlockRef = pool.blocks.getOrDefault(root, nil) proc getBlockRange*( - pool: BlockPool, startSlot: Slot, count, skipStep: Natural): seq[BlockRef] = + pool: BlockPool, startSlot: Slot, skipStep: Natural, + output: var openArray[BlockRef]): Natural = ## This function populates an `output` buffer of blocks - ## with a range starting from `startSlot` and skipping - ## every `skipTest` number of blocks. + ## with a slots ranging from `startSlot` up to, but not including, + ## `startSlot + skipStep * output.len`, skipping any slots that don't have + ## a block. ## + ## Blocks will be written to `output` from the end without gaps, even if + ## a block is missing in a particular slot. The return value shows how + ## many slots were missing blocks - to iterate over the result, start + ## at this index. + ## + ## If there were no blocks in the range, `output.len` will be returned. + let count = output.len trace "getBlockRange entered", head = shortLog(pool.head.blck.root), count, startSlot, skipStep - if startSlot > pool.head.blck.slot: - debug "Range request for future slot", - head = shortLog(pool.head.blck.slot), count, startSlot, skipStep - return - let + skipStep = max(1, skipStep) # Treat 0 step as 1 endSlot = startSlot + uint64(count * skipStep) var - ret = newSeq[BlockRef](count) b = pool.head.blck.atSlot(endSlot) - + o = count for i in 0.. 0'u64: + var blocks: array[MAX_REQUESTED_BLOCKS, BlockRef] let - count = if step != 0: min(count, MAX_REQUESTED_BLOCKS.uint64) else: 1 pool = peer.networkState.blockPool - for b in pool.getBlockRange(startSlot, count, step): - if not b.isNil: - trace "wrote response block", slot = b.slot, roor = shortLog(b.root) - await response.write(pool.get(b).data) + # Limit number of blocks in response + count = min(count.Natural, blocks.len) + + let startIndex = + pool.getBlockRange(startSlot, step, blocks.toOpenArray(0, count - 1)) + + for b in blocks[startIndex..^1]: + doAssert not b.isNil, "getBlockRange should return non-nil blocks only" + trace "wrote response block", slot = b.slot, roor = shortLog(b.root) + await response.write(pool.get(b).data) + + debug "Block range request done", + peer, startSlot, count, step, found = count - startIndex proc beaconBlocksByRoot( peer: Peer, @@ -149,10 +158,15 @@ p2pProtocol BeaconSync(version = 1, let pool = peer.networkState.blockPool + var found = 0 for root in blockRoots: let blockRef = pool.getRef(root) if not isNil(blockRef): await response.write(pool.get(blockRef).data) + inc found + + debug "Block root request done", + peer, roots = blockRoots.len, found proc beaconBlocks( peer: Peer, diff --git a/tests/test_block_pool.nim b/tests/test_block_pool.nim index ff2309e8b..f6d39f2c7 100644 --- a/tests/test_block_pool.nim +++ b/tests/test_block_pool.nim @@ -11,7 +11,7 @@ import unittest, chronicles, ./testutil, ../beacon_chain/spec/datatypes, - ../beacon_chain/[beacon_node_types, block_pool, ssz] + ../beacon_chain/[beacon_node_types, block_pool, ssz, state_transition] suiteReport "BlockRef and helpers" & preset(): timedTest "isAncestorOf sanity" & preset(): @@ -133,6 +133,47 @@ when const_preset == "minimal": # Too much stack space used on mainnet pool.heads.len == 1 pool.heads[0].blck == b2Add + # Skip one slot to get a gap + process_slots(state, state.slot + 1) + + let + b4 = addTestBlock(state, b2Root) + b4Root = hash_tree_root(b4.message) + b4Add = pool.add(b4Root, b4) + + check: + b4Add.parent == b2Add + + pool.updateHead(b4Add) + + var blocks: array[3, BlockRef] + + check: + pool.getBlockRange(Slot(0), 1, blocks.toOpenArray(0, 0)) == 0 + blocks[0..<1] == [pool.tail] + + pool.getBlockRange(Slot(0), 1, blocks.toOpenArray(0, 1)) == 0 + blocks[0..<2] == [pool.tail, b1Add] + + pool.getBlockRange(Slot(0), 2, blocks.toOpenArray(0, 1)) == 0 + blocks[0..<2] == [pool.tail, b2Add] + + pool.getBlockRange(Slot(0), 3, blocks.toOpenArray(0, 1)) == 1 + blocks[0..<2] == [nil, pool.tail] # block 3 is missing! + + pool.getBlockRange(Slot(2), 2, blocks.toOpenArray(0, 1)) == 0 + blocks[0..<2] == [b2Add, b4Add] # block 3 is missing! + + # empty length + pool.getBlockRange(Slot(2), 2, blocks.toOpenArray(0, -1)) == 0 + + # No blocks in sight + pool.getBlockRange(Slot(5), 1, blocks.toOpenArray(0, 1)) == 2 + + # No blocks in sight either due to gaps + pool.getBlockRange(Slot(3), 2, blocks.toOpenArray(0, 1)) == 2 + blocks[0..<2] == [BlockRef nil, nil] # block 3 is missing! + timedTest "Reverse order block add & get" & preset(): discard pool.add(b2Root, b2) @@ -274,10 +315,6 @@ when const_preset == "minimal": # Too much stack space used on mainnet pool.head.justified.slot.compute_epoch_at_slot() == 5 pool.tail.children.len == 1 - pool.getBlockRange(Slot(1), 1, 1).mapIt(it.slot) == [Slot(1)] - pool.getBlockRange(Slot(1), 2, 1).mapIt(it.slot) == [Slot(1), Slot(2)] - pool.getBlockRange(Slot(1), 2, 2).mapIt(it.slot) == [Slot(1), Slot(3)] - let pool2 = BlockPool.init(db)