Skip to content

Commit d7c1196

Browse files
Address review comment to optimize checks when selecting the peer
1 parent 546adc4 commit d7c1196

File tree

1 file changed

+13
-14
lines changed
  • ouroboros-network/src/Ouroboros/Network/BlockFetch/Decision

1 file changed

+13
-14
lines changed

ouroboros-network/src/Ouroboros/Network/BlockFetch/Decision/BulkSync.hs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,7 @@ selectThePeer
480480
(otherPeersB, (thePeerCandidate, _) : otherPeersA) -> do
481481
tell (List (map (first (const (FetchDeclineConcurrencyLimit FetchModeBulkSync 1))) otherPeersB))
482482
tell (List (map (first (const (FetchDeclineConcurrencyLimit FetchModeBulkSync 1))) otherPeersA))
483-
-- REVIEW: This is maybe overkill to check that the whole gross request
484-
-- fits in the peer's candidate. Maybe just checking that there is one
485-
-- block is sufficient.
486-
case checkRequestInCandidate thePeerCandidate =<< grossRequest of
483+
case checkRequestHeadInCandidate thePeerCandidate =<< grossRequest of
487484
Left reason -> tell (List [(reason, thePeerInfo)]) >> return Nothing
488485
Right () -> return $ Just (thePeerCandidate, thePeerInfo)
489486

@@ -494,7 +491,7 @@ selectThePeer
494491
peers <-
495492
filterM
496493
( \(candidate, peer) ->
497-
case checkRequestInCandidate candidate =<< grossRequest of
494+
case checkRequestHeadInCandidate candidate =<< grossRequest of
498495
Left reason -> tell (List [(reason, peer)]) >> pure False
499496
Right () -> pure True
500497
)
@@ -518,16 +515,18 @@ selectThePeer
518515
tell $ List $ map (first (const (FetchDeclineConcurrencyLimit FetchModeBulkSync 1))) otherPeers
519516
return $ Just (thePeerCandidate, thePeer)
520517
where
521-
checkRequestInCandidate ::
518+
checkRequestHeadInCandidate ::
522519
ChainSuffix header -> FetchRequest header -> FetchDecision ()
523-
checkRequestInCandidate candidate request =
524-
if all isSubfragmentOfCandidate $ fetchRequestFragments request
525-
then pure ()
526-
else Left $ FetchDeclineAlreadyFetched -- FIXME: A custom decline reason for this?
527-
where
528-
isSubfragmentOfCandidate fragment =
529-
AF.withinFragmentBounds (AF.anchorPoint fragment) (getChainSuffix candidate)
530-
&& AF.withinFragmentBounds (AF.headPoint fragment) (getChainSuffix candidate)
520+
checkRequestHeadInCandidate candidate request =
521+
case fetchRequestFragments request of
522+
fragments@(_:_)
523+
| AF.withinFragmentBounds
524+
(AF.headPoint $ last fragments)
525+
(getChainSuffix candidate)
526+
->
527+
Right ()
528+
_ ->
529+
Left FetchDeclineAlreadyFetched
531530

532531
-- | Given a candidate and a peer to sync from, create a request for that
533532
-- specific peer. We might take the 'FetchDecision' to decline the request, but

0 commit comments

Comments
 (0)