From 884ba4473ad0dc259a7b99a3cbed8d4c0a3e3013 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Fri, 17 Jan 2025 16:13:28 +1100 Subject: [PATCH 1/3] Do not send column requests if there is no blob for the block. --- beacon_node/network/src/sync/block_lookups/common.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/beacon_node/network/src/sync/block_lookups/common.rs b/beacon_node/network/src/sync/block_lookups/common.rs index 5e336d9c38e..a5781ebb3b0 100644 --- a/beacon_node/network/src/sync/block_lookups/common.rs +++ b/beacon_node/network/src/sync/block_lookups/common.rs @@ -174,9 +174,13 @@ impl RequestState for CustodyRequestState { id: Id, // TODO(das): consider selecting peers that have custody but are in this set _peer_id: PeerId, - _: usize, + expected_blobs: usize, cx: &mut SyncNetworkContext, ) -> Result { + if expected_blobs == 0 { + return Ok(LookupRequestResult::NoRequestNeeded("no columns to fetch")); + } + cx.custody_lookup_request(id, self.block_root) .map_err(LookupRequestError::SendFailedNetwork) } From e6efd356873faed5f6b1f8f4f75597154c2cb04b Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 20 Jan 2025 16:35:56 +1100 Subject: [PATCH 2/3] Address review comments --- beacon_node/network/src/sync/block_lookups/common.rs | 6 +----- beacon_node/network/src/sync/network_context.rs | 5 +++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/beacon_node/network/src/sync/block_lookups/common.rs b/beacon_node/network/src/sync/block_lookups/common.rs index a5781ebb3b0..378b4ce5909 100644 --- a/beacon_node/network/src/sync/block_lookups/common.rs +++ b/beacon_node/network/src/sync/block_lookups/common.rs @@ -177,11 +177,7 @@ impl RequestState for CustodyRequestState { expected_blobs: usize, cx: &mut SyncNetworkContext, ) -> Result { - if expected_blobs == 0 { - return Ok(LookupRequestResult::NoRequestNeeded("no columns to fetch")); - } - - cx.custody_lookup_request(id, self.block_root) + cx.custody_lookup_request(id, self.block_root, expected_blobs) .map_err(LookupRequestError::SendFailedNetwork) } diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 0a6bc8961f8..12543d21e93 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -740,7 +740,12 @@ impl SyncNetworkContext { &mut self, lookup_id: SingleLookupId, block_root: Hash256, + expected_blobs: usize, ) -> Result { + if expected_blobs == 0 { + return Ok(LookupRequestResult::NoRequestNeeded("no columns to fetch")); + } + let custody_indexes_imported = self .chain .data_availability_checker From 085197ddfec07d4290ccb024b204806aec5792ec Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 20 Jan 2025 17:53:13 +1100 Subject: [PATCH 3/3] Replace fix - the previous solution didnt work. --- beacon_node/network/src/sync/block_lookups/common.rs | 4 ++-- .../network/src/sync/block_lookups/single_block_lookup.rs | 3 +-- beacon_node/network/src/sync/network_context.rs | 5 ----- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/beacon_node/network/src/sync/block_lookups/common.rs b/beacon_node/network/src/sync/block_lookups/common.rs index 378b4ce5909..5e336d9c38e 100644 --- a/beacon_node/network/src/sync/block_lookups/common.rs +++ b/beacon_node/network/src/sync/block_lookups/common.rs @@ -174,10 +174,10 @@ impl RequestState for CustodyRequestState { id: Id, // TODO(das): consider selecting peers that have custody but are in this set _peer_id: PeerId, - expected_blobs: usize, + _: usize, cx: &mut SyncNetworkContext, ) -> Result { - cx.custody_lookup_request(id, self.block_root, expected_blobs) + cx.custody_lookup_request(id, self.block_root) .map_err(LookupRequestError::SendFailedNetwork) } diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index 9bbd2bf295b..a096efcbb22 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -215,8 +215,7 @@ impl SingleBlockLookup { let block_epoch = block.slot().epoch(T::EthSpec::slots_per_epoch()); if expected_blobs == 0 { self.component_requests = ComponentRequests::NotNeeded("no data"); - } - if cx.chain.should_fetch_blobs(block_epoch) { + } else if cx.chain.should_fetch_blobs(block_epoch) { self.component_requests = ComponentRequests::ActiveBlobRequest( BlobRequestState::new(self.block_root), expected_blobs, diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 12543d21e93..0a6bc8961f8 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -740,12 +740,7 @@ impl SyncNetworkContext { &mut self, lookup_id: SingleLookupId, block_root: Hash256, - expected_blobs: usize, ) -> Result { - if expected_blobs == 0 { - return Ok(LookupRequestResult::NoRequestNeeded("no columns to fetch")); - } - let custody_indexes_imported = self .chain .data_availability_checker