From 5fa291240ec1d5e3d094aad82ff0bb68d74c57bc Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Fri, 18 Oct 2024 17:28:09 +0300 Subject: [PATCH] Address review --- beacon_node/network/src/sync/manager.rs | 20 +++++++---- .../network/src/sync/network_context.rs | 36 +++++++------------ .../network/src/sync/range_sync/range.rs | 14 ++++---- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 99c0e27c47..8a4db9fa8f 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -36,7 +36,7 @@ use super::backfill_sync::{BackFillSync, ProcessResult, SyncStart}; use super::block_lookups::BlockLookups; use super::network_context::{ - BlockOrBlob, CustodyByRootResult, RangeRequestId, RpcEvent, SyncNetworkContext, + CustodyByRootResult, RangeBlockComponent, RangeRequestId, RpcEvent, SyncNetworkContext, }; use super::peer_sampling::{Sampling, SamplingConfig, SamplingResult}; use super::peer_sync_info::{remote_sync_type, PeerSyncType}; @@ -1150,7 +1150,11 @@ impl SyncManager { block: RpcEvent>>, ) { if let Some(resp) = self.network.on_blocks_by_range_response(id, peer_id, block) { - self.on_range_components_response(id.requester, peer_id, BlockOrBlob::Block(resp)); + self.on_range_components_response( + id.requester, + peer_id, + RangeBlockComponent::Block(resp), + ); } } @@ -1161,7 +1165,11 @@ impl SyncManager { blob: RpcEvent>>, ) { if let Some(resp) = self.network.on_blobs_by_range_response(id, peer_id, blob) { - self.on_range_components_response(id.requester, peer_id, BlockOrBlob::Blob(resp)); + self.on_range_components_response( + id.requester, + peer_id, + RangeBlockComponent::Blob(resp), + ); } } @@ -1178,7 +1186,7 @@ impl SyncManager { self.on_range_components_response( id.requester, peer_id, - BlockOrBlob::CustodyColumns(resp), + RangeBlockComponent::CustodyColumns(resp), ); } } @@ -1237,11 +1245,11 @@ impl SyncManager { &mut self, id: ComponentsByRangeRequestId, peer_id: PeerId, - block_or_blob: BlockOrBlob, + block_or_blob: RangeBlockComponent, ) { if let Some(resp) = self .network - .range_block_and_blob_response(id, block_or_blob) + .range_block_component_response(id, block_or_blob) { match resp { Ok(blocks) => { diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index aa25cdf734..2a69b7e265 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -208,7 +208,7 @@ pub struct SyncNetworkContext { } /// Small enumeration to make dealing with block and blob requests easier. -pub enum BlockOrBlob { +pub enum RangeBlockComponent { Block(RpcResponseResult>>>), Blob(RpcResponseResult>>>), CustodyColumns(RpcResponseResult>>>), @@ -449,10 +449,10 @@ impl SyncNetworkContext { /// Received a blocks by range or blobs by range response for a request that couples blocks ' /// and blobs. - pub fn range_block_and_blob_response( + pub fn range_block_component_response( &mut self, id: ComponentsByRangeRequestId, - block_or_blob: BlockOrBlob, + block_or_blob: RangeBlockComponent, ) -> Option>, RpcResponseError>> { let Entry::Occupied(mut entry) = self.components_by_range_requests.entry(id) else { metrics::inc_counter_vec(&metrics::SYNC_UNKNOWN_NETWORK_REQUESTS, &["range_blocks"]); @@ -462,27 +462,15 @@ impl SyncNetworkContext { if let Err(e) = { let request = entry.get_mut(); match block_or_blob { - BlockOrBlob::Block(resp) => match resp { - Ok((blocks, _)) => { - request.add_blocks(blocks); - Ok(()) - } - Err(e) => Err(e), - }, - BlockOrBlob::Blob(resp) => match resp { - Ok((blobs, _)) => { - request.add_blobs(blobs); - Ok(()) - } - Err(e) => Err(e), - }, - BlockOrBlob::CustodyColumns(resp) => match resp { - Ok((custody_columns, _)) => { - request.add_custody_columns(custody_columns); - Ok(()) - } - Err(e) => Err(e), - }, + RangeBlockComponent::Block(resp) => resp.map(|(blocks, _)| { + request.add_blocks(blocks); + }), + RangeBlockComponent::Blob(resp) => resp.map(|(blobs, _)| { + request.add_blobs(blobs); + }), + RangeBlockComponent::CustodyColumns(resp) => resp.map(|(custody_columns, _)| { + request.add_custody_columns(custody_columns); + }), } } { entry.remove(); diff --git a/beacon_node/network/src/sync/range_sync/range.rs b/beacon_node/network/src/sync/range_sync/range.rs index cb511c08c1..4f88132833 100644 --- a/beacon_node/network/src/sync/range_sync/range.rs +++ b/beacon_node/network/src/sync/range_sync/range.rs @@ -390,7 +390,7 @@ mod tests { use crate::NetworkMessage; use super::*; - use crate::sync::network_context::{BlockOrBlob, RangeRequestId, RpcResponseResult}; + use crate::sync::network_context::{RangeBlockComponent, RangeRequestId, RpcResponseResult}; use beacon_chain::builder::Witness; use beacon_chain::eth1_chain::CachingEth1Backend; use beacon_chain::parking_lot::RwLock; @@ -569,17 +569,17 @@ mod tests { if blob_req_opt.is_some() { match block_req { AppRequestId::Sync(SyncRequestId::BlocksByRange(id)) => { - let result = self.cx.range_block_and_blob_response( + let result = self.cx.range_block_component_response( id.requester, - BlockOrBlob::Block(empty_response()), + RangeBlockComponent::Block(empty_response()), ); if result.is_some() { panic!("range components should not complete yet"); } self.cx - .range_block_and_blob_response( + .range_block_component_response( id.requester, - BlockOrBlob::Blob(empty_response()), + RangeBlockComponent::Blob(empty_response()), ) .unwrap() .unwrap(); @@ -593,9 +593,9 @@ mod tests { match block_req { AppRequestId::Sync(SyncRequestId::BlocksByRange(id)) => { self.cx - .range_block_and_blob_response( + .range_block_component_response( id.requester, - BlockOrBlob::Block(empty_response()), + RangeBlockComponent::Block(empty_response()), ) .unwrap() .unwrap();