Fix backfill stalling

[pawanjay176]

Issue Addressed

N/A

Proposed Changes

Hopefully fixes all backfill stalled issues we have been seeing.

The root cause of the issue is that the peer_disconnect function returns early if the call to retry_batch_download within it returns an error. This can happen when we have no synced peers to retry from.
This early return fails to set the BatchState for multiple batches from BatchState::Downloading to BatchState::AwaitingDownload
If the processing_target batch was also one of the batches that failed to change states, then backfill effectively stalls.
When we resume backfill after getting new peers, we do the following which could potentially re-request the target:

1. Call to resume_batches
   

   lighthouse/beacon_node/network/src/sync/backfill_sync/mod.rs

   Lines 1002 to 1016 in c7e5dd1

   	fn resume_batches(&mut self, network: &mut SyncNetworkContext<T>) -> Result<(), BackFillError> {
   	let batch_ids_to_retry = self
   	.batches
   	.iter()
   	.filter_map(|(batch_id, batch)| {
   	// In principle there should only ever be on of these, and we could terminate the
   	// loop early, however the processing is negligible and we continue the search
   	// for robustness to handle potential future modification
   	if matches!(batch.state(), BatchState::AwaitingDownload) {
   	Some(*batch_id)
   	} else {
   	None
   	}
   	})
   	.collect::<Vec<_>>();

   
   Since the processing_target is stuck in BatchState::Downloading, it is not requested in this call
2. Call to request_batches which does nothing because of this condition:
   

   lighthouse/beacon_node/network/src/sync/backfill_sync/mod.rs

   Lines 1083 to 1091 in c7e5dd1

   	if self
   	.batches
   	.iter()
   	.filter(|&(_epoch, batch)| in_buffer(batch))
   	.count()
   	> BACKFILL_BATCH_BUFFER_SIZE as usize
   	{
   	return None;
   	}

   
   In most of the logs that I have seen of this issue, batches.len() is usually > BACKFILL_BATCH_BUFFER_SIZE

Hence, the target can never be requested again and the target state can never change until a restart.
This PR basically handles the error in peer_disconnect instead of short circuiting.

[AgeManning]

Yeah nice find.

I also notice that in the case that we short-circuit we also don't remove the peer from the participating_peers() mapping, which also looks like it could have dormant issues.

[AgeManning]

@Mergifyio queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 0b59d10