Skip to content

Commit

Permalink
Attempt to continue lookups after adding peers (#5993)
Browse files Browse the repository at this point in the history
* Attempt to continue lookups after adding peers
  • Loading branch information
dapplion authored Jun 26, 2024
1 parent bf4cbd3 commit b38019c
Showing 1 changed file with 14 additions and 8 deletions.
22 changes: 14 additions & 8 deletions beacon_node/network/src/sync/block_lookups/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
}
}

if let Err(e) = self.add_peers_to_lookup_and_ancestors(lookup_id, peers) {
if let Err(e) = self.add_peers_to_lookup_and_ancestors(lookup_id, peers, cx) {
warn!(self.log, "Error adding peers to ancestor lookup"; "error" => ?e);
}

Expand Down Expand Up @@ -844,37 +844,43 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
&mut self,
lookup_id: SingleLookupId,
peers: &[PeerId],
cx: &mut SyncNetworkContext<T>,
) -> Result<(), String> {
let lookup = self
.single_block_lookups
.get_mut(&lookup_id)
.ok_or(format!("Unknown lookup for id {lookup_id}"))?;

let mut added_some_peer = false;
for peer in peers {
if lookup.add_peer(*peer) {
added_some_peer = true;
debug!(self.log, "Adding peer to existing single block lookup";
"block_root" => ?lookup.block_root(),
"peer" => ?peer
);
}
}

// We may choose to attempt to continue a lookup here. It is possible that a lookup had zero
// peers and after adding this set of peers it can make progress again. Note that this
// recursive function iterates from child to parent, so continuing the child first is weird.
// However, we choose to not attempt to continue the lookup for simplicity. It's not
// strictly required and just and optimization for a rare corner case.

if let Some(parent_root) = lookup.awaiting_parent() {
if let Some((&child_id, _)) = self
.single_block_lookups
.iter()
.find(|(_, l)| l.block_root() == parent_root)
{
self.add_peers_to_lookup_and_ancestors(child_id, peers)
self.add_peers_to_lookup_and_ancestors(child_id, peers, cx)
} else {
Err(format!("Lookup references unknown parent {parent_root:?}"))
}
} else if added_some_peer {
// If this lookup is not awaiting a parent and we added at least one peer, attempt to
// make progress. It is possible that a lookup is created with zero peers, attempted to
// make progress, and then receives peers. After that time the lookup will never be
// pruned with `drop_lookups_without_peers` because it has peers. This is rare corner
// case, but it can result in stuck lookups.
let result = lookup.continue_requests(cx);
self.on_lookup_result(lookup_id, result, "add_peers", cx);
Ok(())
} else {
Ok(())
}
Expand Down

0 comments on commit b38019c

Please sign in to comment.