more block range request updates

* handle skip == 0 gracefully
* avoid memory allocation at expense of more complex API
* add more tests
* log block request results
This commit is contained in:
Jacek Sieka 2020-04-21 08:43:39 +02:00 committed by zah
parent fccce85f0d
commit da988e4e40
3 changed files with 82 additions and 22 deletions

View File

@ -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..<count:
for j in 0..<skipStep:
b = b.parent
if b.blck.slot == b.slot:
ret[ret.len - i - 1] = b.blck
output[o - 1] = b.blck
dec o
ret
# Make sure the given input is cleared, just in case
for i in 0..<o:
output[i] = nil
o # Return the index of the first non-nil item in the output
func getBlockBySlot*(pool: BlockPool, slot: Slot): BlockRef =
## Retrieves the first block in the current canonical chain

View File

@ -134,13 +134,22 @@ p2pProtocol BeaconSync(version = 1,
trace "got range request", peer, startSlot, count, step
if count > 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,

View File

@ -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)