From 61ec6fa167e65b826e06770cd15195eb661c3877 Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Fri, 26 Apr 2019 19:38:56 +0300 Subject: [PATCH] Move incoming block from unresolved to pending if it can't be applied (#253) * Move resolved block from unresolved to pending if it can't be applied * Renamed unresolved to missing --- beacon_chain/beacon_node.nim | 2 +- beacon_chain/beacon_node_types.nim | 4 ++-- beacon_chain/block_pool.nim | 29 +++++++++++++++++------------ tests/test_block_pool.nim | 2 +- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/beacon_chain/beacon_node.nim b/beacon_chain/beacon_node.nim index e438ebd1b..48d98c920 100644 --- a/beacon_chain/beacon_node.nim +++ b/beacon_chain/beacon_node.nim @@ -652,7 +652,7 @@ proc onSlotStart(node: BeaconNode, lastSlot, scheduledSlot: Slot) {.gcsafe, asyn asyncCheck node.onSlotStart(slot, nextSlot) proc onSecond(node: BeaconNode, moment: Moment) {.async.} = - let missingBlocks = node.blockPool.checkUnresolved() + let missingBlocks = node.blockPool.checkMissing() if missingBlocks.len > 0: info "Requesting detected missing blocks", missingBlocks node.requestManager.fetchAncestorBlocks(missingBlocks) do (b: BeaconBlock): diff --git a/beacon_chain/beacon_node_types.nim b/beacon_chain/beacon_node_types.nim index bbe41e4ce..1c8809548 100644 --- a/beacon_chain/beacon_node_types.nim +++ b/beacon_chain/beacon_node_types.nim @@ -158,7 +158,7 @@ type ## for - when we receive a "missing link", we can use this data to build ## an entire branch - unresolved*: Table[Eth2Digest, UnresolvedBlock] ##\ + missing*: Table[Eth2Digest, MissingBlock] ##\ ## Roots of blocks that we would like to have (either parent_root of ## unresolved blocks or block roots of attestations) @@ -180,7 +180,7 @@ type db*: BeaconChainDB - UnresolvedBlock* = object + MissingBlock* = object slots*: uint64 # number of slots that are suspected missing tries*: int diff --git a/beacon_chain/block_pool.nim b/beacon_chain/block_pool.nim index c5bc6d0b5..27d13d243 100644 --- a/beacon_chain/block_pool.nim +++ b/beacon_chain/block_pool.nim @@ -122,7 +122,7 @@ proc init*(T: type BlockPool, db: BeaconChainDB): BlockPool = BlockPool( pending: initTable[Eth2Digest, BeaconBlock](), - unresolved: initTable[Eth2Digest, UnresolvedBlock](), + missing: initTable[Eth2Digest, MissingBlock](), blocks: blocks, blocksBySlot: blocksBySlot, tail: tailRef, @@ -157,6 +157,8 @@ proc add*( return pool.blocks[blockRoot] + pool.missing.del(blockRoot) + # If the block we get is older than what we finalized already, we drop it. # One way this can happen is that we start resolving a block and finalization # happens in the meantime - the block we requested will then be stale @@ -169,16 +171,17 @@ proc add*( return + let parent = pool.blocks.getOrDefault(blck.previous_block_root) if parent != nil: # The block might have been in either of these - we don't want any more # work done on its behalf - pool.unresolved.del(blockRoot) pool.pending.del(blockRoot) # The block is resolved, now it's time to validate it to ensure that the # blocks we add to the database are clean for the given state + # TODO if the block is from the future, we should not be resolving it (yet), # but maybe we should use it as a hint that our clock is wrong? updateState(pool, state, BlockSlot(blck: parent, slot: blck.slot - 1)) @@ -231,15 +234,18 @@ proc add*( return blockRef + pool.pending[blockRoot] = blck + # TODO possibly, it makes sense to check the database - that would allow sync # to simply fill up the database with random blocks the other clients # think are useful - but, it would also risk filling the database with # junk that's not part of the block graph - if blck.previous_block_root in pool.unresolved: + if blck.previous_block_root in pool.missing or + blck.previous_block_root in pool.pending: return - # This is an unresolved block - put it on the unresolved list for now... + # This is an unresolved block - put its parent on the missing list for now... # TODO if we receive spam blocks, one heurestic to implement might be to wait # for a couple of attestations to appear before fetching parents - this # would help prevent using up network resources for spam - this serves @@ -251,13 +257,13 @@ proc add*( # filter. # TODO when we receive the block, we don't know how many others we're missing # from that branch, so right now, we'll just do a blind guess - debug "Unresolved block", + debug "Unresolved block (parent missing)", blck = shortLog(blck), blockRoot = shortLog(blockRoot) let parentSlot = blck.slot - 1 - pool.unresolved[blck.previous_block_root] = UnresolvedBlock( + pool.missing[blck.previous_block_root] = MissingBlock( slots: # The block is at least two slots ahead - try to grab whole history if parentSlot > pool.head.slot: @@ -268,7 +274,6 @@ proc add*( max(1.uint64, SLOTS_PER_EPOCH.uint64 - (parentSlot.uint64 mod SLOTS_PER_EPOCH.uint64)) ) - pool.pending[blockRoot] = blck proc get*(pool: BlockPool, blck: BlockRef): BlockData = ## Retrieve the associated block body of a block reference @@ -294,18 +299,18 @@ proc getOrResolve*(pool: var BlockPool, root: Eth2Digest): BlockRef = result = pool.blocks.getOrDefault(root) if result.isNil: - pool.unresolved[root] = UnresolvedBlock(slots: 1) + pool.missing[root] = MissingBlock(slots: 1) iterator blockRootsForSlot*(pool: BlockPool, slot: uint64|Slot): Eth2Digest = for br in pool.blocksBySlot.getOrDefault(slot.uint64, @[]): yield br.root -proc checkUnresolved*(pool: var BlockPool): seq[FetchRecord] = +proc checkMissing*(pool: var BlockPool): seq[FetchRecord] = ## Return a list of blocks that we should try to resolve from other client - ## to be called periodically but not too often (once per slot?) var done: seq[Eth2Digest] - for k, v in pool.unresolved.mpairs(): + for k, v in pool.missing.mpairs(): if v.tries > 8: done.add(k) else: @@ -314,10 +319,10 @@ proc checkUnresolved*(pool: var BlockPool): seq[FetchRecord] = for k in done: # TODO Need to potentially remove from pool.pending - this is currently a # memory leak here! - pool.unresolved.del(k) + pool.missing.del(k) # simple (simplistic?) exponential backoff for retries.. - for k, v in pool.unresolved.pairs(): + for k, v in pool.missing.pairs(): if v.tries.popcount() == 1: result.add(FetchRecord(root: k, historySlots: v.slots)) diff --git a/tests/test_block_pool.nim b/tests/test_block_pool.nim index 0419d59e4..6f73f6bee 100644 --- a/tests/test_block_pool.nim +++ b/tests/test_block_pool.nim @@ -63,7 +63,7 @@ suite "Block pool processing": check: pool.get(b2Root).isNone() # Unresolved, shouldn't show up - FetchRecord(root: b1Root, historySlots: 1) in pool.checkUnresolved() + FetchRecord(root: b1Root, historySlots: 1) in pool.checkMissing() discard pool.add(state, b1Root, b1)