Skip to content

Commit

Permalink
Add reason of NoRequestNeeded to lookup sync (sigp#6317)
Browse files Browse the repository at this point in the history
* Add reason of NoRequestNeeded to lookup sync

* Merge branch 'unstable' into peerdas-reason

# Conflicts:
#	beacon_node/network/src/sync/network_context.rs
  • Loading branch information
dapplion authored Aug 30, 2024
1 parent e3d2d38 commit 60157ad
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 18 deletions.
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

0 comments on commit 60157ad

Please sign in to comment.