gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-05-07 00:42:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't use failed_peers for download errors, rely on randomness to skip potentially faulty peers

Browse Source
This commit is contained in:
dapplion
2025-06-11 12:38:55 +02:00
parent 4e13b3be0f
commit e426e45455
3 changed files with 5 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
beacon_node/network/src/sync/backfill_sync/mod.rs
View File

@@ -330,9 +330,7 @@ impl<T: BeaconChainTypes> BackFillSync<T> {
return Ok(()); return Ok(());
} }
debug!(batch_epoch = %batch_id, error = ?err, "Batch download failed"); debug!(batch_epoch = %batch_id, error = ?err, "Batch download failed");
// TODO(das): Is it necessary for the batch to track failed peers? Can we make this match batch.download_failed() {
// mechanism compatible with PeerDAS and before PeerDAS?
match batch.download_failed(None) {
Err(e) => self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0)), Err(e) => self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0)),
Ok(BatchOperationOutcome::Failed { blacklist: _ }) => self.fail_sync(match err { Ok(BatchOperationOutcome::Failed { blacklist: _ }) => self.fail_sync(match err {
RpcResponseError::RpcError(_) RpcResponseError::RpcError(_)
@@ -956,7 +954,7 @@ impl<T: BeaconChainTypes> BackFillSync<T> {
return self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0)); return self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0));
} }
match batch.download_failed(None) { match batch.download_failed() {
Err(e) => { Err(e) => {
self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0))? self.fail_sync(BackFillError::BatchInvalidState(batch_id, e.0))?
} }

10
beacon_node/network/src/sync/range_sync/batch.rs
View File

@@ -305,17 +305,9 @@ impl<E: EthSpec, B: BatchConfig> BatchInfo<E, B> {
/// The `peer` parameter, when set to None, does not increment the failed attempts of /// The `peer` parameter, when set to None, does not increment the failed attempts of
/// this batch and register the peer, rather attempts a re-download. /// this batch and register the peer, rather attempts a re-download.
#[must_use = "Batch may have failed"] #[must_use = "Batch may have failed"]
pub fn download_failed( pub fn download_failed(&mut self) -> Result<BatchOperationOutcome, WrongState> {
&mut self,
peer: Option<PeerId>,
) -> Result<BatchOperationOutcome, WrongState> {
match self.state.poison() { match self.state.poison() {
BatchState::Downloading(_request_id) => { BatchState::Downloading(_request_id) => {
// register the attempt and check if the batch can be tried again
if let Some(peer) = peer {
self.failed_peers.insert(peer);
}
self.failed_download_attempts += 1; self.failed_download_attempts += 1;
self.state = self.state =

8
beacon_node/network/src/sync/range_sync/chain.rs
View File

@@ -902,11 +902,7 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
%request_id, %request_id,
"Batch download error" "Batch download error"
); );
if let BatchOperationOutcome::Failed { blacklist } = if let BatchOperationOutcome::Failed { blacklist } = batch.download_failed()? {
// TODO(das): Is it necessary for the batch to track failed peers? Can we make this
// mechanism compatible with PeerDAS and before PeerDAS?
batch.download_failed(None)?
{
return Err(RemoveChain::ChainFailed { return Err(RemoveChain::ChainFailed {
blacklist, blacklist,
failing_batch: batch_id, failing_batch: batch_id,
@@ -966,7 +962,7 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
warn!(%batch_id, error = ?e, "batch_id" = %batch_id, %batch, "Could not send batch request"); warn!(%batch_id, error = ?e, "batch_id" = %batch_id, %batch, "Could not send batch request");
// register the failed download and check if the batch can be retried // register the failed download and check if the batch can be retried
batch.start_downloading(1)?; // fake request_id = 1 is not relevant batch.start_downloading(1)?; // fake request_id = 1 is not relevant
match batch.download_failed(None)? { match batch.download_failed()? {
BatchOperationOutcome::Failed { blacklist } => { BatchOperationOutcome::Failed { blacklist } => {
return Err(RemoveChain::ChainFailed { return Err(RemoveChain::ChainFailed {
blacklist, blacklist,