From 456bdc87cd8bd1c1b95dff55214f4fbd56d66e6f Mon Sep 17 00:00:00 2001
From: tersec <tersec@users.noreply.github.com>
Date: Fri, 4 Sep 2020 06:39:46 +0000
Subject: [PATCH] address issue #1552 (#1601)

---
 .../block_pools/block_pools_types.nim         |  2 +-
 beacon_chain/block_pools/clearance.nim        |  4 +--
 beacon_chain/block_pools/quarantine.nim       | 27 ++++++++++++++++---
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/beacon_chain/block_pools/block_pools_types.nim b/beacon_chain/block_pools/block_pools_types.nim
index 859a9f3b0..95c971d01 100644
--- a/beacon_chain/block_pools/block_pools_types.nim
+++ b/beacon_chain/block_pools/block_pools_types.nim
@@ -51,7 +51,7 @@ type
     ##
     ## Invalid blocks are dropped immediately.
 
-    orphans*: Table[Eth2Digest, SignedBeaconBlock] ##\
+    orphans*: Table[(Eth2Digest, ValidatorSig), SignedBeaconBlock] ##\
     ## Blocks that have passed validation but that we lack a link back to tail
     ## for - when we receive a "missing link", we can use this data to build
     ## an entire branch
diff --git a/beacon_chain/block_pools/clearance.nim b/beacon_chain/block_pools/clearance.nim
index ae9570e67..781d5ab6f 100644
--- a/beacon_chain/block_pools/clearance.nim
+++ b/beacon_chain/block_pools/clearance.nim
@@ -187,7 +187,7 @@ proc addRawBlock*(
 
     # The block might have been in either of `orphans` or `missing` - we don't
     # want any more work done on its behalf
-    quarantine.orphans.del(blockRoot)
+    quarantine.removeOrphan(signedBlock)
 
     # 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
@@ -234,7 +234,7 @@ proc addRawBlock*(
   #      junk that's not part of the block graph
 
   if blck.parent_root in quarantine.missing or
-      blck.parent_root in quarantine.orphans:
+      containsOrphan(quarantine, signedBlock):
     debug "Unresolved block (parent missing or orphaned)",
       orphans = quarantine.orphans.len,
       missing = quarantine.missing.len
diff --git a/beacon_chain/block_pools/quarantine.nim b/beacon_chain/block_pools/quarantine.nim
index 9ee7170fe..f8e7eba0b 100644
--- a/beacon_chain/block_pools/quarantine.nim
+++ b/beacon_chain/block_pools/quarantine.nim
@@ -9,7 +9,7 @@ import
   chronicles, tables, options,
   stew/bitops2,
   metrics,
-  ../spec/[datatypes, digest],
+  ../spec/[crypto, datatypes, digest],
   block_pools_types
 
 export options, block_pools_types
@@ -40,14 +40,33 @@ func checkMissing*(quarantine: var QuarantineRef): seq[FetchRecord] =
     if countOnes(v.tries.uint64) == 1:
       result.add(FetchRecord(root: k))
 
+template anyIt(s, pred: untyped): bool =
+  # https://github.com/nim-lang/Nim/blob/version-1-2/lib/pure/collections/sequtils.nim#L682-L704
+  # without the items(...)
+  var result = false
+  for it {.inject.} in s:
+    if pred:
+      result = true
+      break
+  result
+
+func containsOrphan*(
+    quarantine: QuarantineRef, signedBlock: SignedBeaconBlock): bool =
+  (signedBlock.root, signedBlock.signature) in quarantine.orphans
+
 func addMissing*(quarantine: var QuarantineRef, root: Eth2Digest) =
   ## Schedule the download a the given block
-  if root notin quarantine.orphans:
+  # Can only request by root, not by signature, so partial match suffices
+  if not anyIt(quarantine.orphans.keys, it[0] == root):
     # If the block is in orphans, we no longer need it
     discard quarantine.missing.hasKeyOrPut(root, MissingBlock())
 
+func removeOrphan*(
+    quarantine: var QuarantineRef, signedBlock: SignedBeaconBlock) =
+  quarantine.orphans.del((signedBlock.root, signedBlock.signature))
+
 func removeOldBlocks(quarantine: var QuarantineRef, dag: ChainDAGRef) =
-  var oldBlocks: seq[Eth2Digest]
+  var oldBlocks: seq[(Eth2Digest, ValidatorSig)]
 
   for k, v in quarantine.orphans.pairs():
     if v.message.slot <= dag.finalizedHead.slot:
@@ -84,7 +103,7 @@ func add*(quarantine: var QuarantineRef, dag: ChainDAGRef,
   if quarantine.orphans.len >= MAX_QUARANTINE_ORPHANS:
     return false
 
-  quarantine.orphans[signedBlock.root] = signedBlock
+  quarantine.orphans[(signedBlock.root, signedBlock.signature)] = signedBlock
   quarantine.missing.del(signedBlock.root)
 
   quarantine.addMissing(signedBlock.message.parent_root)