consistent peer scoring for missing non-finalized parent (#3381)

When the sync queue processes results for a blocks by range request,
and the requested range contained some slots that are already finalized,
`BlockError.MissingParent` currently leads to `PeerScoreBadBlocks` even
when the error occurs on a non-finalized slot in the requested range.
This patch changes the scoring in that case to `PeerScoreMissingBlocks`
for consistency with range requests solely covering non-finalized slots,
and, likewise, rewinds the sync queue to the next `rewindSlot`.

beacon_chain/sync/sync_queue.nim

@@ -697,7 +697,7 @@ proc push*[T](sq: SyncQueue[T], sr: SyncRequest[T],
           safeSlot = sq.getSafeSlot()
         case sq.kind
         of SyncQueueKind.Forward:
-          if safeSlot < req.slot:
+          if safeSlot < failSlot:
             let rewindSlot = sq.getRewindPoint(failSlot, safeSlot)
             debug "Unexpected missing parent, rewind happens",
                  request = req, rewind_to_slot = rewindSlot,
@@ -713,7 +713,7 @@ proc push*[T](sq: SyncQueue[T], sr: SyncRequest[T],
                   blocks_map = getShortMap(req, item.data)
             req.item.updateScore(PeerScoreBadBlocks)
         of SyncQueueKind.Backward:
-          if safeSlot > req.slot:
+          if safeSlot > failSlot:
             let rewindSlot = sq.getRewindPoint(failSlot, safeSlot)
             # It's quite common peers give us fewer blocks than we ask for
             info "Gap in block range response, rewinding", request = req,

tests/test_sync_manager.nim

@@ -17,6 +17,7 @@ import ../beacon_chain/gossip_processing/block_processor,
 
 type
   SomeTPeer = ref object
+    score: int
 
 proc `$`(peer: SomeTPeer): string =
   "SomeTPeer"
@@ -25,7 +26,7 @@ template shortLog(peer: SomeTPeer): string =
   $peer
 
 proc updateScore(peer: SomeTPeer, score: int) =
-  discard
+  peer[].score += score
 
 func getStaticSlotCb(slot: Slot): GetSlotCallback =
   proc getSlot(): Slot =
@@ -484,6 +485,7 @@ suite "SyncManager test suite":
 
     let p1 = SomeTPeer()
 
+    var expectedScore = 0
     proc runTest() {.async.} =
       while true:
         var request = queue.pop(finish, p1)
@@ -499,6 +501,7 @@ suite "SyncManager test suite":
             response.delete(response.len - 2)
           of SyncQueueKind.Backward:
             response.delete(1)
+          expectedScore += PeerScoreMissingBlocks
         if response.len >= 1:
           # Ensure requested values are past `safeSlot`
           case kkind
@@ -515,6 +518,7 @@ suite "SyncManager test suite":
       check (counter - 1) == int(finish)
     of SyncQueueKind.Backward:
       check (counter + 1) == int(start)
+    check p1.score >= expectedScore
 
   template outOfBandAdvancementTest(kkind: SyncQueueKind, start, finish: Slot,
                                     chunkSize: uint64) =