gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-06-17 02:38:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Thread typed RPC errors through download response handlers

Browse Source 
Drop the log-and-strip pattern in the four download response wrappers:
on_{block,blob,custody,payload}_download_response now take their typed
*DownloadResponse aliases (Result<_, RpcResponseError>) directly, and
the inner state machine's on_download_response matches Err(_). This
removes three #[allow(clippy::type_complexity)] annotations and keeps
the option of branching on RPC error kind inside the state machine
open.

Remove the redundant "… download result" debug logs in the four
wrappers — the error is already logged upstream at
requests.rs "Sync RPC request error" (block/blob/payload envelope)
and network_context "Custody request failure, removing", and the
block_root → id association reappears at "Sending block for processing"
on the success path.

Fix has_no_peers callers to use the new !has_peers() API.
This commit is contained in:
dapplion
2026-05-19 13:18:04 -06:00
parent 6e1ee05ca5
commit 5c58f7e4b7
2 changed files with 21 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
beacon_node/network/src/sync/block_lookups/mod.rs
View File

@@ -450,17 +450,6 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
debug!(?id, "Block returned for single block lookup not present");
return;
};
let block_root = lookup.block_root();
// The downstream state machine only needs success / failure: details about RPC
// failures (peer info, error category) are logged here before being collapsed, so
// debugging still has the full context.
let response = match response {
Ok(ok) => Ok(ok),
Err(err) => {
debug!(?block_root, ?id, ?err, "Block download failed");
Err(())
}
};
let result = lookup.on_block_download_response(id.req_id, response, cx);
self.on_lookup_result(id.lookup_id, result, "block_download_response", cx);
}
@@ -475,14 +464,6 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
debug!(?id, "Blob returned for single block lookup not present");
return;
};
let block_root = lookup.block_root();
let response = match response {
Ok(ok) => Ok(ok),
Err(err) => {
debug!(?block_root, ?id, ?err, "Blob download failed");
Err(())
}
};
let result = lookup.on_blob_download_response(id.req_id, response, cx);
self.on_lookup_result(id.lookup_id, result, "blob_download_response", cx);
}
@@ -497,14 +478,6 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
debug!(?id, "Custody returned for single block lookup not present");
return;
};
let block_root = lookup.block_root();
let response = match response {
Ok(ok) => Ok(ok),
Err(err) => {
debug!(?block_root, ?id, ?err, "Custody download failed");
Err(())
}
};
let result = lookup.on_custody_download_response(id.req_id, response, cx);
self.on_lookup_result(id.lookup_id, result, "custody_download_response", cx);
}
@@ -522,14 +495,6 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
);
return;
};
let block_root = lookup.block_root();
let response = match response {
Ok(ok) => Ok(ok),
Err(err) => {
debug!(?block_root, ?id, ?err, "Payload envelope download failed");
Err(())
}
};
let result = lookup.on_payload_download_response(id.req_id, response, cx);
self.on_lookup_result(id.lookup_id, result, "payload_download_response", cx);
}
@@ -539,7 +504,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
pub fn peer_disconnected(&mut self, peer_id: &PeerId) {
for (id, lookup) in self.single_block_lookups.iter_mut() {
lookup.remove_peer(peer_id);
if lookup.has_no_peers() {
if !lookup.has_peers() {
debug!(%id, "Lookup has no peers");
}
}
@@ -901,7 +866,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
.filter(|lookup| {
// Do not drop lookup that are awaiting events to prevent inconsinstencies. If a
// lookup gets stuck, it will be eventually pruned by `drop_stuck_lookups`
lookup.has_no_peers()
!lookup.has_peers()
&& lookup.elapsed_since_created()
> Duration::from_secs(LOOKUP_MAX_DURATION_NO_PEERS_SECS)
&& !lookup.is_awaiting_event()

38
beacon_node/network/src/sync/block_lookups/single_block_lookup.rs
View File

@@ -1,8 +1,11 @@
use super::{BlockComponent, PeerId, SINGLE_BLOCK_LOOKUP_MAX_ATTEMPTS};
use crate::sync::block_lookups::{
BlobDownloadResponse, BlockDownloadResponse, CustodyDownloadResponse, PayloadDownloadResponse,
};
use crate::sync::manager::BlockProcessType;
use crate::sync::network_context::{
LookupRequestResult, PeerGroup, ReqId, RpcRequestSendError, SendErrorProcessor,
SyncNetworkContext,
LookupRequestResult, PeerGroup, ReqId, RpcRequestSendError, RpcResponseError,
SendErrorProcessor, SyncNetworkContext,
};
use beacon_chain::BeaconChainTypes;
use beacon_chain::BlockProcessStatus;
@@ -1018,11 +1021,10 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
// -- Download response handlers --
/// Handle a block download response. Updates download state and advances the lookup.
#[allow(clippy::type_complexity)]
pub fn on_block_download_response(
&mut self,
req_id: ReqId,
result: Result<(Arc<SignedBeaconBlock<T::EthSpec>>, PeerGroup, Duration), ()>,
result: BlockDownloadResponse<T::EthSpec>,
cx: &mut SyncNetworkContext<T>,
) -> Result<LookupResult, LookupRequestError> {
let BlockRequest::Downloading { state, .. } = &mut self.block_request else {
@@ -1038,7 +1040,7 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
pub fn on_blob_download_response(
&mut self,
req_id: ReqId,
result: Result<(FixedBlobSidecarList<T::EthSpec>, PeerGroup, Duration), ()>,
result: BlobDownloadResponse<T::EthSpec>,
cx: &mut SyncNetworkContext<T>,
) -> Result<LookupResult, LookupRequestError> {
let Some(DataRequest {
@@ -1058,7 +1060,7 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
pub fn on_custody_download_response(
&mut self,
req_id: ReqId,
result: Result<(DataColumnSidecarList<T::EthSpec>, PeerGroup, Duration), ()>,
result: CustodyDownloadResponse<T::EthSpec>,
cx: &mut SyncNetworkContext<T>,
) -> Result<LookupResult, LookupRequestError> {
let Some(DataRequest {
@@ -1075,18 +1077,10 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
}
/// Handle a payload envelope download response. Updates download state and advances the lookup.
#[allow(clippy::type_complexity)]
pub fn on_payload_download_response(
&mut self,
req_id: ReqId,
result: Result<
(
Arc<SignedExecutionPayloadEnvelope<T::EthSpec>>,
PeerGroup,
Duration,
),
(),
>,
result: PayloadDownloadResponse<T::EthSpec>,
cx: &mut SyncNetworkContext<T>,
) -> Result<LookupResult, LookupRequestError> {
let Some(PayloadRequest {
@@ -1142,8 +1136,14 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
}
/// Returns true if this lookup has zero peers
pub fn has_no_peers(&self) -> bool {
self.peers.read().is_empty()
pub fn has_peers(&self) -> bool {
if !self.peers.read().is_empty() {
return true;
}
let gloas_child_peers = self.gloas_child_peers.read();
!gloas_child_peers.is_empty()
&& gloas_child_peers.values().any(|set| !set.read().is_empty())
}
}
@@ -1273,7 +1273,7 @@ impl<T: Clone> SingleLookupRequestState<T> {
&mut self,
req_id: ReqId,
block_root: Hash256,
result: Result<(T, PeerGroup, Duration), ()>,
result: Result<(T, PeerGroup, Duration), RpcResponseError>,
) -> Result<(), LookupRequestError> {
match result {
Ok((value, peer_group, seen_timestamp)) => self.on_download_success(
@@ -1285,7 +1285,7 @@ impl<T: Clone> SingleLookupRequestState<T> {
peer_group,
},
),
Err(()) => self.on_download_failure(req_id),
Err(_) => self.on_download_failure(req_id),
}
}