when adding duplicates to quarantine, schedule deepest missing parent (#6112)

During sync, sometimes the same block gets encountered and added to
quarantine multiple times. If its parent is already known, quarantine
incorrectly registers it as missing, leading to re-download. This can
be fixed by registering the parent's deepest missing parent recursively.

Also increase the stickiness of `missing`. We only perform 4 attempts
within ~16 seconds before giving up. Very frequently, this is not enough
and there is no progress until sync manager kicks in even on holesky.

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

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

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