Skip to content

Commit 34dbb32

Browse files
dappliondapplion
authored
Nest lookup type into request id SingleBlock and SingleBlob (#5562)
* Nest lookup type into block lookup RequestId
1 parent 3d4e6e2 commit 34dbb32

6 files changed

Lines changed: 105 additions & 119 deletions

File tree

‎beacon_node/network/src/router.rs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -493,10 +493,7 @@ impl<T: BeaconChainTypes> Router<T> {
493493
) {
494494
let request_id = match request_id {
495495
RequestId::Sync(sync_id) => match sync_id {
496-
SyncId::SingleBlock { .. }
497-
| SyncId::SingleBlob { .. }
498-
| SyncId::ParentLookup { .. }
499-
| SyncId::ParentLookupBlob { .. } => {
496+
SyncId::SingleBlock { .. } | SyncId::SingleBlob { .. } => {
500497
crit!(self.log, "Block lookups do not request BBRange requests"; "peer_id" => %peer_id);
501498
return;
502499
}
@@ -561,15 +558,15 @@ impl<T: BeaconChainTypes> Router<T> {
561558
) {
562559
let request_id = match request_id {
563560
RequestId::Sync(sync_id) => match sync_id {
564-
id @ (SyncId::SingleBlock { .. } | SyncId::ParentLookup { .. }) => id,
561+
id @ SyncId::SingleBlock { .. } => id,
565562
SyncId::BackFillBlocks { .. }
566563
| SyncId::RangeBlocks { .. }
567564
| SyncId::RangeBlockAndBlobs { .. }
568565
| SyncId::BackFillBlockAndBlobs { .. } => {
569566
crit!(self.log, "Batch syncing do not request BBRoot requests"; "peer_id" => %peer_id);
570567
return;
571568
}
572-
SyncId::SingleBlob { .. } | SyncId::ParentLookupBlob { .. } => {
569+
SyncId::SingleBlob { .. } => {
573570
crit!(self.log, "Blob response to block by roots request"; "peer_id" => %peer_id);
574571
return;
575572
}
@@ -602,8 +599,8 @@ impl<T: BeaconChainTypes> Router<T> {
602599
) {
603600
let request_id = match request_id {
604601
RequestId::Sync(sync_id) => match sync_id {
605-
id @ (SyncId::SingleBlob { .. } | SyncId::ParentLookupBlob { .. }) => id,
606-
SyncId::SingleBlock { .. } | SyncId::ParentLookup { .. } => {
602+
id @ SyncId::SingleBlob { .. } => id,
603+
SyncId::SingleBlock { .. } => {
607604
crit!(self.log, "Block response to blobs by roots request"; "peer_id" => %peer_id);
608605
return;
609606
}

‎beacon_node/network/src/sync/block_lookups/common.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub enum ResponseType {
2525
Blob,
2626
}
2727

28-
#[derive(Debug, Copy, Clone)]
28+
#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
2929
pub enum LookupType {
3030
Current,
3131
Parent,
@@ -119,6 +119,7 @@ pub trait RequestState<L: Lookup, T: BeaconChainTypes> {
119119
let id = SingleLookupReqId {
120120
id,
121121
req_counter: self.get_state().req_counter,
122+
lookup_type: L::lookup_type(),
122123
};
123124
Self::make_request(id, peer_id, request, cx)
124125
}
@@ -265,7 +266,7 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlockRequestState<L>
265266
request: Self::RequestType,
266267
cx: &SyncNetworkContext<T>,
267268
) -> Result<(), LookupRequestError> {
268-
cx.block_lookup_request(id, peer_id, request, L::lookup_type())
269+
cx.block_lookup_request(id, peer_id, request)
269270
.map_err(LookupRequestError::SendFailed)
270271
}
271272

@@ -365,7 +366,7 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L,
365366
request: Self::RequestType,
366367
cx: &SyncNetworkContext<T>,
367368
) -> Result<(), LookupRequestError> {
368-
cx.blob_lookup_request(id, peer_id, request, L::lookup_type())
369+
cx.blob_lookup_request(id, peer_id, request)
369370
.map_err(LookupRequestError::SendFailed)
370371
}
371372

‎beacon_node/network/src/sync/block_lookups/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
691691
pub fn parent_lookup_failed<R: RequestState<Parent, T>>(
692692
&mut self,
693693
id: SingleLookupReqId,
694-
peer_id: PeerId,
694+
peer_id: &PeerId,
695695
cx: &SyncNetworkContext<T>,
696696
error: RPCError,
697697
) {

‎beacon_node/network/src/sync/block_lookups/tests.rs‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ impl TestRig {
296296
beacon_block: Option<Arc<SignedBeaconBlock<E>>>,
297297
) {
298298
self.send_sync_message(SyncMessage::RpcBlock {
299-
request_id: SyncRequestId::ParentLookup { id },
299+
request_id: SyncRequestId::SingleBlock { id },
300300
peer_id,
301301
beacon_block,
302302
seen_timestamp: D,
@@ -324,7 +324,7 @@ impl TestRig {
324324
blob_sidecar: Option<Arc<BlobSidecar<E>>>,
325325
) {
326326
self.send_sync_message(SyncMessage::RpcBlob {
327-
request_id: SyncRequestId::ParentLookupBlob { id },
327+
request_id: SyncRequestId::SingleBlob { id },
328328
peer_id,
329329
blob_sidecar,
330330
seen_timestamp: D,
@@ -348,7 +348,7 @@ impl TestRig {
348348
fn parent_lookup_failed(&mut self, id: SingleLookupReqId, peer_id: PeerId, error: RPCError) {
349349
self.send_sync_message(SyncMessage::RpcError {
350350
peer_id,
351-
request_id: SyncRequestId::ParentLookup { id },
351+
request_id: SyncRequestId::SingleBlock { id },
352352
error,
353353
})
354354
}
@@ -409,7 +409,11 @@ impl TestRig {
409409
peer_id: _,
410410
request: Request::BlocksByRoot(request),
411411
request_id: RequestId::Sync(SyncRequestId::SingleBlock { id }),
412-
} if request.block_roots().to_vec().contains(&for_block) => Some(*id),
412+
} if id.lookup_type == LookupType::Current
413+
&& request.block_roots().to_vec().contains(&for_block) =>
414+
{
415+
Some(*id)
416+
}
413417
_ => None,
414418
})
415419
.unwrap_or_else(|e| panic!("Expected block request for {for_block:?}: {e}"))
@@ -422,11 +426,12 @@ impl TestRig {
422426
peer_id: _,
423427
request: Request::BlobsByRoot(request),
424428
request_id: RequestId::Sync(SyncRequestId::SingleBlob { id }),
425-
} if request
426-
.blob_ids
427-
.to_vec()
428-
.iter()
429-
.any(|r| r.block_root == for_block) =>
429+
} if id.lookup_type == LookupType::Current
430+
&& request
431+
.blob_ids
432+
.to_vec()
433+
.iter()
434+
.any(|r| r.block_root == for_block) =>
430435
{
431436
Some(*id)
432437
}
@@ -441,8 +446,12 @@ impl TestRig {
441446
NetworkMessage::SendRequest {
442447
peer_id: _,
443448
request: Request::BlocksByRoot(request),
444-
request_id: RequestId::Sync(SyncRequestId::ParentLookup { id }),
445-
} if request.block_roots().to_vec().contains(&for_block) => Some(*id),
449+
request_id: RequestId::Sync(SyncRequestId::SingleBlock { id }),
450+
} if id.lookup_type == LookupType::Parent
451+
&& request.block_roots().to_vec().contains(&for_block) =>
452+
{
453+
Some(*id)
454+
}
446455
_ => None,
447456
})
448457
.unwrap_or_else(|e| panic!("Expected block parent request for {for_block:?}: {e}"))
@@ -454,12 +463,13 @@ impl TestRig {
454463
NetworkMessage::SendRequest {
455464
peer_id: _,
456465
request: Request::BlobsByRoot(request),
457-
request_id: RequestId::Sync(SyncRequestId::ParentLookupBlob { id }),
458-
} if request
459-
.blob_ids
460-
.to_vec()
461-
.iter()
462-
.all(|r| r.block_root == for_block) =>
466+
request_id: RequestId::Sync(SyncRequestId::SingleBlob { id }),
467+
} if id.lookup_type == LookupType::Parent
468+
&& request
469+
.blob_ids
470+
.to_vec()
471+
.iter()
472+
.all(|r| r.block_root == for_block) =>
463473
{
464474
Some(*id)
465475
}
@@ -1974,15 +1984,15 @@ mod deneb_only {
19741984
return;
19751985
};
19761986
let (block, blobs) = r.rand_block_and_blobs(NumBlobs::Number(2));
1977-
let parent_root = block.parent_root();
1987+
let block_root = block.canonical_root();
19781988
let blob_0 = blobs[0].clone();
19791989
let blob_1 = blobs[1].clone();
19801990
let peer_a = r.new_connected_peer();
19811991
let peer_b = r.new_connected_peer();
19821992
// Send unknown parent block lookup
19831993
r.trigger_unknown_parent_block(peer_a, block.into());
19841994
// Expect network request for blobs
1985-
let id = r.expect_blob_parent_request(parent_root);
1995+
let id = r.expect_blob_lookup_request(block_root);
19861996
// Peer responses with blob 0
19871997
r.single_lookup_blob_response(id, peer_a, Some(blob_0.into()));
19881998
// Blob 1 is received via gossip unknown parent blob from a different peer

‎beacon_node/network/src/sync/manager.rs‎

Lines changed: 62 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
//! search for the block and subsequently search for parents if needed.
3535
3636
use super::backfill_sync::{BackFillSync, ProcessResult, SyncStart};
37+
use super::block_lookups::common::LookupType;
3738
use super::block_lookups::BlockLookups;
3839
use super::network_context::{BlockOrBlob, SyncNetworkContext};
3940
use super::peer_sync_info::{remote_sync_type, PeerSyncType};
@@ -80,6 +81,7 @@ pub type Id = u32;
8081
pub struct SingleLookupReqId {
8182
pub id: Id,
8283
pub req_counter: Id,
84+
pub lookup_type: LookupType,
8385
}
8486

8587
/// Id of rpc requests sent by sync to the network.
@@ -89,12 +91,6 @@ pub enum RequestId {
8991
SingleBlock { id: SingleLookupReqId },
9092
/// Request searching for a set of blobs given a hash.
9193
SingleBlob { id: SingleLookupReqId },
92-
/// Request searching for a block's parent. The id is the chain, share with the corresponding
93-
/// blob id.
94-
ParentLookup { id: SingleLookupReqId },
95-
/// Request searching for a block's parent blobs. The id is the chain, shared with the corresponding
96-
/// block id.
97-
ParentLookupBlob { id: SingleLookupReqId },
9894
/// Request was from the backfill sync algorithm.
9995
BackFillBlocks { id: Id },
10096
/// Backfill request that is composed by both a block range request and a blob range request.
@@ -331,42 +327,42 @@ impl<T: BeaconChainTypes> SyncManager<T> {
331327
fn inject_error(&mut self, peer_id: PeerId, request_id: RequestId, error: RPCError) {
332328
trace!(self.log, "Sync manager received a failed RPC");
333329
match request_id {
334-
RequestId::SingleBlock { id } => {
335-
self.block_lookups
330+
RequestId::SingleBlock { id } => match id.lookup_type {
331+
LookupType::Current => self
332+
.block_lookups
336333
.single_block_lookup_failed::<BlockRequestState<Current>>(
337334
id,
338335
&peer_id,
339336
&self.network,
340337
error,
341-
);
342-
}
343-
RequestId::SingleBlob { id } => {
344-
self.block_lookups
345-
.single_block_lookup_failed::<BlobRequestState<Current, T::EthSpec>>(
338+
),
339+
LookupType::Parent => self
340+
.block_lookups
341+
.parent_lookup_failed::<BlockRequestState<Parent>>(
346342
id,
347343
&peer_id,
348344
&self.network,
349345
error,
350-
);
351-
}
352-
RequestId::ParentLookup { id } => {
353-
self.block_lookups
354-
.parent_lookup_failed::<BlockRequestState<Parent>>(
346+
),
347+
},
348+
RequestId::SingleBlob { id } => match id.lookup_type {
349+
LookupType::Current => self
350+
.block_lookups
351+
.single_block_lookup_failed::<BlobRequestState<Current, T::EthSpec>>(
355352
id,
356-
peer_id,
353+
&peer_id,
357354
&self.network,
358355
error,
359-
);
360-
}
361-
RequestId::ParentLookupBlob { id } => {
362-
self.block_lookups
356+
),
357+
LookupType::Parent => self
358+
.block_lookups
363359
.parent_lookup_failed::<BlobRequestState<Parent, T::EthSpec>>(
364360
id,
365-
peer_id,
361+
&peer_id,
366362
&self.network,
367363
error,
368-
);
369-
}
364+
),
365+
},
370366
RequestId::BackFillBlocks { id } => {
371367
if let Some(batch_id) = self
372368
.network
@@ -882,30 +878,29 @@ impl<T: BeaconChainTypes> SyncManager<T> {
882878
seen_timestamp: Duration,
883879
) {
884880
match request_id {
885-
RequestId::SingleBlock { id } => self
886-
.block_lookups
887-
.single_lookup_response::<BlockRequestState<Current>>(
888-
id,
889-
peer_id,
890-
block,
891-
seen_timestamp,
892-
&self.network,
893-
),
881+
RequestId::SingleBlock { id } => match id.lookup_type {
882+
LookupType::Current => self
883+
.block_lookups
884+
.single_lookup_response::<BlockRequestState<Current>>(
885+
id,
886+
peer_id,
887+
block,
888+
seen_timestamp,
889+
&self.network,
890+
),
891+
LookupType::Parent => self
892+
.block_lookups
893+
.parent_lookup_response::<BlockRequestState<Parent>>(
894+
id,
895+
peer_id,
896+
block,
897+
seen_timestamp,
898+
&self.network,
899+
),
900+
},
894901
RequestId::SingleBlob { .. } => {
895902
crit!(self.log, "Block received during blob request"; "peer_id" => %peer_id );
896903
}
897-
RequestId::ParentLookup { id } => self
898-
.block_lookups
899-
.parent_lookup_response::<BlockRequestState<Parent>>(
900-
id,
901-
peer_id,
902-
block,
903-
seen_timestamp,
904-
&self.network,
905-
),
906-
RequestId::ParentLookupBlob { id: _ } => {
907-
crit!(self.log, "Block received during parent blob request"; "peer_id" => %peer_id );
908-
}
909904
RequestId::BackFillBlocks { id } => {
910905
let is_stream_terminator = block.is_none();
911906
if let Some(batch_id) = self
@@ -966,28 +961,26 @@ impl<T: BeaconChainTypes> SyncManager<T> {
966961
RequestId::SingleBlock { .. } => {
967962
crit!(self.log, "Single blob received during block request"; "peer_id" => %peer_id );
968963
}
969-
RequestId::SingleBlob { id } => self
970-
.block_lookups
971-
.single_lookup_response::<BlobRequestState<Current, T::EthSpec>>(
972-
id,
973-
peer_id,
974-
blob,
975-
seen_timestamp,
976-
&self.network,
977-
),
978-
979-
RequestId::ParentLookup { id: _ } => {
980-
crit!(self.log, "Single blob received during parent block request"; "peer_id" => %peer_id );
981-
}
982-
RequestId::ParentLookupBlob { id } => self
983-
.block_lookups
984-
.parent_lookup_response::<BlobRequestState<Parent, T::EthSpec>>(
985-
id,
986-
peer_id,
987-
blob,
988-
seen_timestamp,
989-
&self.network,
990-
),
964+
RequestId::SingleBlob { id } => match id.lookup_type {
965+
LookupType::Current => self
966+
.block_lookups
967+
.single_lookup_response::<BlobRequestState<Current, T::EthSpec>>(
968+
id,
969+
peer_id,
970+
blob,
971+
seen_timestamp,
972+
&self.network,
973+
),
974+
LookupType::Parent => self
975+
.block_lookups
976+
.parent_lookup_response::<BlobRequestState<Parent, T::EthSpec>>(
977+
id,
978+
peer_id,
979+
blob,
980+
seen_timestamp,
981+
&self.network,
982+
),
983+
},
991984
RequestId::BackFillBlocks { id: _ } => {
992985
crit!(self.log, "Blob received during backfill block request"; "peer_id" => %peer_id );
993986
}

0 commit comments

Comments
 (0)