Some lints and bugfixes (#23)

* fix lints * bug fixes
sigp · Apr 16, 2024 · 71cceee · 71cceee
1 parent 2ef22b3
commit 71cceee
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 16 deletions.
diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs
@@ -392,7 +392,6 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                         .get_state_mut()
                         .on_download_success()
                         .map_err(LookupRequestError::BadState)?;
-
                     self.send_block_for_processing(
                         block_root,
                         block,
@@ -403,6 +402,10 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                 }
             }
             CachedChild::DownloadIncomplete => {
+                R::request_state_mut(lookup)
+                    .get_state_mut()
+                    .on_download_success()
+                    .map_err(LookupRequestError::BadState)?;
                 // If this was the result of a block request, we can't determine if the block peer
                 // did anything wrong. If we already had both a block and blobs response processed,
                 // we should penalize the blobs peer because they did not provide all blobs on the

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
@@ -75,6 +75,7 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
     /// the next parent.
     pub fn update_requested_parent_block(&mut self, block_root: Hash256) {
         self.block_request_state.requested_block_root = block_root;
+        self.blob_request_state.block_root = block_root;
         self.block_request_state.state.state = State::AwaitingDownload;
         self.blob_request_state.state.state = State::AwaitingDownload;
         self.child_components = Some(ChildComponents::empty(block_root));
@@ -450,11 +451,9 @@ impl SingleLookupRequestState {
                 self.state = State::Processing { peer_id: *peer_id };
                 Ok(())
             }
-            other => {
-                return Err(format!(
-                    "request bad state, expected downloading got {other}"
-                ))
-            }
+            other => Err(format!(
+                "request bad state, expected downloading got {other}"
+            )),
         }
     }
 

diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs
@@ -51,6 +51,8 @@ pub enum RpcEvent<T> {
     RPCError(RPCError),
 }
 
+pub type RpcProcessingResult<T> = Option<Result<(T, Duration), RPCError>>;
+
 /// Wraps a Network channel to employ various RPC related network functionality for the Sync manager. This includes management of a global RPC request Id.
 pub struct SyncNetworkContext<T: BeaconChainTypes> {
     /// The network channel to relay messages to the Network service.
@@ -307,8 +309,8 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
         );
 
         self.send_network_msg(NetworkMessage::SendRequest {
-            peer_id: peer_id,
-            request: Request::BlobsByRoot(request.into_request(&self.chain.spec)),
+            peer_id,
+            request: Request::BlobsByRoot(request.clone().into_request(&self.chain.spec)),
             request_id: RequestId::Sync(SyncRequestId::SingleBlob { id }),
         })?;
 
@@ -426,7 +428,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
         &mut self,
         request_id: SingleLookupReqId,
         block: RpcEvent<Arc<SignedBeaconBlock<T::EthSpec>>>,
-    ) -> Option<Result<(Arc<SignedBeaconBlock<T::EthSpec>>, Duration), RPCError>> {
+    ) -> RpcProcessingResult<Arc<SignedBeaconBlock<T::EthSpec>>> {
         let Entry::Occupied(mut request) = self.blocks_by_root_requests.entry(request_id) else {
             return None;
         };
@@ -459,7 +461,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
         &mut self,
         request_id: SingleLookupReqId,
         blob: RpcEvent<Arc<BlobSidecar<T::EthSpec>>>,
-    ) -> Option<Result<(FixedBlobSidecarList<T::EthSpec>, Duration), RPCError>> {
+    ) -> RpcProcessingResult<FixedBlobSidecarList<T::EthSpec>> {
         let Entry::Occupied(mut request) = self.blobs_by_root_requests.entry(request_id) else {
             return None;
         };

diff --git a/beacon_node/network/src/sync/network_context/requests.rs b/beacon_node/network/src/sync/network_context/requests.rs
@@ -50,28 +50,29 @@ impl ActiveBlocksByRootRequest {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub struct BlocksByRootSingleRequest(pub Hash256);
 
 impl BlocksByRootSingleRequest {
-    pub fn into_request(&self, spec: &ChainSpec) -> BlocksByRootRequest {
+    pub fn into_request(self, spec: &ChainSpec) -> BlocksByRootRequest {
         BlocksByRootRequest::new(vec![self.0], spec)
     }
 }
 
+#[derive(Debug, Clone)]
 pub struct BlobsByRootSingleBlockRequest {
     pub block_root: Hash256,
     pub indices: Vec<u64>,
 }
 
 impl BlobsByRootSingleBlockRequest {
-    pub fn into_request(&self, spec: &ChainSpec) -> BlobsByRootRequest {
+    pub fn into_request(self, spec: &ChainSpec) -> BlobsByRootRequest {
         BlobsByRootRequest::new(
             self.indices
-                .iter()
+                .into_iter()
                 .map(|index| BlobIdentifier {
                     block_root: self.block_root,
-                    index: *index,
+                    index,
                 })
                 .collect(),
             spec,
@@ -136,7 +137,7 @@ impl<E: EthSpec> ActiveBlobsByRootRequest<E> {
 
     pub fn terminate(self) -> Option<Vec<Arc<BlobSidecar<E>>>> {
         if self.resolved {
-            return None;
+            None
         } else {
             Some(self.blobs)
         }