diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index 0b467a8ef..ca45c9755 100644 --- a/AllTests-mainnet.md +++ b/AllTests-mainnet.md @@ -103,9 +103,10 @@ OK: 5/5 Fail: 0/5 Skip: 0/5 OK: 1/1 Fail: 0/1 Skip: 0/1 ## Block quarantine ```diff ++ Recursive missing parent OK + Unviable smoke test OK ``` -OK: 1/1 Fail: 0/1 Skip: 0/1 +OK: 2/2 Fail: 0/2 Skip: 0/2 ## BlockId and helpers ```diff + atSlot sanity OK @@ -1017,4 +1018,4 @@ OK: 2/2 Fail: 0/2 Skip: 0/2 OK: 9/9 Fail: 0/9 Skip: 0/9 ---TOTAL--- -OK: 682/687 Fail: 0/687 Skip: 5/687 +OK: 683/688 Fail: 0/688 Skip: 5/688 diff --git a/beacon_chain/consensus_object_pools/block_quarantine.nim b/beacon_chain/consensus_object_pools/block_quarantine.nim index cf7efb87e..6f04a8307 100644 --- a/beacon_chain/consensus_object_pools/block_quarantine.nim +++ b/beacon_chain/consensus_object_pools/block_quarantine.nim @@ -16,6 +16,8 @@ import export tables, forks const + MaxRetriesPerMissingItem = 7 + ## Exponential backoff, double interval between each attempt MaxMissingItems = 1024 ## Arbitrary MaxOrphans = SLOTS_PER_EPOCH * 3 @@ -83,7 +85,7 @@ func checkMissing*(quarantine: var Quarantine, max: int): seq[FetchRecord] = var done: seq[Eth2Digest] for k, v in quarantine.missing.mpairs(): - if v.tries > 8: + if v.tries > static(1 shl MaxRetriesPerMissingItem): done.add(k) for k in done: @@ -97,32 +99,30 @@ func checkMissing*(quarantine: var Quarantine, max: int): seq[FetchRecord] = if result.len >= max: break -# TODO stew/sequtils2 -template anyIt(s, pred: untyped): bool = - # https://github.com/nim-lang/Nim/blob/v1.6.10/lib/pure/collections/sequtils.nim#L753-L775 - # without the items(...) - var result = false - for it {.inject.} in s: - if pred: - result = true - break - result - func addMissing*(quarantine: var Quarantine, root: Eth2Digest) = ## Schedule the download a the given block if quarantine.missing.len >= MaxMissingItems: return - if root in quarantine.unviable: - # Won't get anywhere with this block - return + var r = root + for i in 0 .. MaxOrphans: # Blocks are not trusted, avoid endless loops + if r in quarantine.unviable: + # Won't get anywhere with this block + return - # It's not really missing if we're keeping it in the quarantine - if anyIt(quarantine.orphans.keys, it[0] == root): - return + # It's not really missing if we're keeping it in the quarantine. + # In that case, add the next missing parent root instead + var found = false + for k, blck in quarantine.orphans: + if k[0] == r: + r = getForkedBlockField(blck, parent_root) + found = true + break - # Add if it's not there, but don't update missing counter - discard quarantine.missing.hasKeyOrPut(root, MissingBlock()) + # Add if it's not there, but don't update missing counter + if not found: + discard quarantine.missing.hasKeyOrPut(r, MissingBlock()) + return func removeOrphan*( quarantine: var Quarantine, signedBlock: ForkySignedBeaconBlock) = diff --git a/tests/test_block_quarantine.nim b/tests/test_block_quarantine.nim index 63b85fa3d..64accd788 100644 --- a/tests/test_block_quarantine.nim +++ b/tests/test_block_quarantine.nim @@ -77,3 +77,44 @@ suite "Block quarantine": b5.root notin quarantine.blobless b6.root notin quarantine.blobless + + test "Recursive missing parent": + let + b0 = makeBlock(Slot 0, ZERO_HASH) + b1 = makeBlock(Slot 1, b0.root) + b2 = makeBlock(Slot 2, b1.root) + + var quarantine: Quarantine + check: + b0.root notin quarantine.missing + b1.root notin quarantine.missing + b2.root notin quarantine.missing + + # Add b2 + quarantine.addOrphan(Slot 0, b2).isOk + b0.root notin quarantine.missing + b1.root in quarantine.missing + b2.root notin quarantine.missing + + # Add b1 + quarantine.addOrphan(Slot 0, b1).isOk + b0.root in quarantine.missing + b1.root notin quarantine.missing + b2.root notin quarantine.missing + + # Re-add b2 + quarantine.addOrphan(Slot 0, b2).isOk + b0.root in quarantine.missing + b1.root notin quarantine.missing + b2.root notin quarantine.missing + + # Empty missing + while quarantine.missing.len > 0: + discard quarantine.checkMissing(max = 5) + + check: + # Re-add b2 + quarantine.addOrphan(Slot 0, b2).isOk + b0.root in quarantine.missing + b1.root notin quarantine.missing + b2.root notin quarantine.missing