Skip to content
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.

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

Add reason of NoRequestNeeded to lookup sync #6317

Merged
merged 2 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,11 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
// with this `req_id`.
request.get_state_mut().on_download_start(req_id)?
}
LookupRequestResult::NoRequestNeeded => {
LookupRequestResult::NoRequestNeeded(reason) => {
// Lookup sync event safety: Advances this request to the terminal `Processed`
// state. If all requests reach this state, the request is marked as completed
// in `Self::continue_requests`.
request.get_state_mut().on_completed_request()?
request.get_state_mut().on_completed_request(reason)?
}
// Sync will receive a future event to make progress on the request, do nothing now
LookupRequestResult::Pending(reason) => {
Expand Down Expand Up @@ -357,13 +357,13 @@ pub struct DownloadResult<T: Clone> {

#[derive(IntoStaticStr)]
pub enum State<T: Clone> {
AwaitingDownload(&'static str),
AwaitingDownload(/* reason */ &'static str),
Downloading(ReqId),
AwaitingProcess(DownloadResult<T>),
/// Request is processing, sent by lookup sync
Processing(DownloadResult<T>),
/// Request is processed
Processed,
Processed(/* reason */ &'static str),
}

/// Object representing the state of a single block or blob lookup request.
Expand Down Expand Up @@ -554,7 +554,7 @@ impl<T: Clone> SingleLookupRequestState<T> {
pub fn on_processing_success(&mut self) -> Result<(), LookupRequestError> {
match &self.state {
State::Processing(_) => {
self.state = State::Processed;
self.state = State::Processed("processing success");
Ok(())
}
other => Err(LookupRequestError::BadState(format!(
Expand All @@ -564,10 +564,10 @@ impl<T: Clone> SingleLookupRequestState<T> {
}

/// Mark a request as complete without any download or processing
pub fn on_completed_request(&mut self) -> Result<(), LookupRequestError> {
pub fn on_completed_request(&mut self, reason: &'static str) -> Result<(), LookupRequestError> {
match &self.state {
State::AwaitingDownload { .. } => {
self.state = State::Processed;
self.state = State::Processed(reason);
Ok(())
}
other => Err(LookupRequestError::BadState(format!(
Expand Down Expand Up @@ -598,11 +598,11 @@ impl<T: Clone> std::fmt::Display for State<T> {
impl<T: Clone> std::fmt::Debug for State<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::AwaitingDownload(status) => write!(f, "AwaitingDownload({:?})", status),
Self::AwaitingDownload(reason) => write!(f, "AwaitingDownload({})", reason),
Self::Downloading(req_id) => write!(f, "Downloading({:?})", req_id),
Self::AwaitingProcess(d) => write!(f, "AwaitingProcess({:?})", d.peer_group),
Self::Processing(d) => write!(f, "Processing({:?})", d.peer_group),
Self::Processed { .. } => write!(f, "Processed"),
Self::Processed(reason) => write!(f, "Processed({})", reason),
}
}
}
Expand Down
24 changes: 16 additions & 8 deletions beacon_node/network/src/sync/network_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,9 @@ pub enum LookupRequestResult<I = ReqId> {
/// A request is sent. Sync MUST receive an event from the network in the future for either:
/// completed response or failed request
RequestSent(I),
/// No request is sent, and no further action is necessary to consider this request completed
NoRequestNeeded,
/// No request is sent, and no further action is necessary to consider this request completed.
/// Includes a reason why this request is not needed.
NoRequestNeeded(&'static str),
/// No request is sent, but the request is not completed. Sync MUST receive some future event
/// that makes progress on the request. For example: request is processing from a different
/// source (i.e. block received from gossip) and sync MUST receive an event with that processing
Expand Down Expand Up @@ -543,7 +544,9 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
// Block is fully validated. If it's not yet imported it's waiting for missing block
// components. Consider this request completed and do nothing.
BlockProcessStatus::ExecutionValidated { .. } => {
return Ok(LookupRequestResult::NoRequestNeeded)
return Ok(LookupRequestResult::NoRequestNeeded(
"block execution validated",
))
}
}

Expand Down Expand Up @@ -623,7 +626,12 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {

// Check if we are in deneb, before peerdas and inside da window
if !self.chain.should_fetch_blobs(block_epoch) {
return Ok(LookupRequestResult::NoRequestNeeded);
return Ok(LookupRequestResult::NoRequestNeeded("blobs not required"));
}

// No data required for this block
if expected_blobs == 0 {
return Ok(LookupRequestResult::NoRequestNeeded("no data"));
}

let imported_blob_indexes = self
Expand All @@ -638,7 +646,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {

if indices.is_empty() {
// No blobs required, do not issue any request
return Ok(LookupRequestResult::NoRequestNeeded);
return Ok(LookupRequestResult::NoRequestNeeded("no indices to fetch"));
}

let req_id = self.next_id();
Expand Down Expand Up @@ -736,12 +744,12 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {

// Check if we are into peerdas and inside da window
if !self.chain.should_fetch_custody_columns(block_epoch) {
return Ok(LookupRequestResult::NoRequestNeeded);
return Ok(LookupRequestResult::NoRequestNeeded("columns not required"));
}

// No data required for this block
if expected_blobs == 0 {
return Ok(LookupRequestResult::NoRequestNeeded);
return Ok(LookupRequestResult::NoRequestNeeded("no data"));
}

let custody_indexes_imported = self
Expand All @@ -760,7 +768,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {

if custody_indexes_to_fetch.is_empty() {
// No indexes required, do not issue any request
return Ok(LookupRequestResult::NoRequestNeeded);
return Ok(LookupRequestResult::NoRequestNeeded("no indices to fetch"));
}

let req_id = self.next_id();
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/network/src/sync/network_context/custody.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl<T: BeaconChainTypes> ActiveCustodyRequest<T> {
self.active_batch_columns_requests
.insert(req_id, ActiveBatchColumnsRequest { indices, peer_id });
}
LookupRequestResult::NoRequestNeeded => unreachable!(),
LookupRequestResult::NoRequestNeeded(_) => unreachable!(),
LookupRequestResult::Pending(_) => unreachable!(),
}
}
Expand Down
Loading