Skip to content

Allow for sync state where batch is unknown #7391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from
Open

Allow for sync state where batch is unknown #7391

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a000af
Handle unforseen batch state
AgeManning May 2, 2025
1abb1ed
Add a log
AgeManning May 8, 2025
2d58133
Merge latest unstable
AgeManning May 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions beacon_node/network/src/sync/range_sync/chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
BatchState::Poisoned => unreachable!("Poisoned batch"),
BatchState::Failed | BatchState::AwaitingDownload | BatchState::Processing(_) => {
// these are all inconsistent states:
// - Failed -> non recoverable batch. Chain should have beee removed
// - Failed -> non recoverable batch. Chain should have been removed
// - AwaitingDownload -> A recoverable failed batch should have been
// re-requested.
// - Processing -> `self.current_processing_batch` is None
Expand Down Expand Up @@ -406,11 +406,15 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
// return an error.
return Ok(KeepChain);
} else {
return Err(RemoveChain::WrongChainState(format!(
"Batch not found for current processing target {}",
self.processing_target
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be a rare event, should we add a debug log? If we ever wonder why the SyncingChain is not processing anything

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it will ever stop processing, something will progress it, but agree a log would be handy

)));
// NOTE: It is possible that the batch doesn't exist for the processing id. This can happen
// when we complete a batch and attempt to download a new batch but there are:
// 1. No idle peers to download from
// 2. No good peers on sampling subnets
//
// In these cases, a batch will not yet exist.
debug!(batch = %self.processing_target, "The processing batch has not been scheduled for download yet. Awaiting progress");
}

Ok(KeepChain)
}

Expand Down Expand Up @@ -523,7 +527,7 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
imported_blocks,
penalty,
} => {
// Penalize the peer appropiately.
// Penalize the peer appropriately.
network.report_peer(peer, *penalty, "faulty_batch");

// Check if this batch is allowed to continue
Expand Down Expand Up @@ -565,7 +569,8 @@ impl<T: BeaconChainTypes> SyncingChain<T> {
}
BatchProcessResult::NonFaultyFailure => {
batch.processing_completed(BatchProcessingResult::NonFaultyFailure)?;
// Simply redownload the batch.

// Simply re-download the batch.
self.send_batch(network, batch_id)
}
}
Expand Down
Loading