ensure we don't reset requested_ids during rpc download

Author: realbigsean

beacon_node/beacon_chain/src/data_availability_checker.rs

@@ -655,6 +655,14 @@ impl MissingBlobs {
             MissingBlobs::BlobsNotRequired => vec![],
         }
     }
+
+    pub fn clear(&mut self) {
+        match self {
+            MissingBlobs::KnownMissing(ref mut v) => v.clear(),
+            MissingBlobs::PossibleMissing(ref mut v) => v.clear(),
+            MissingBlobs::BlobsNotRequired => (),
+        }
+    }
 }
 
 impl Into<Vec<BlobIdentifier>> for MissingBlobs {

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

@@ -396,6 +396,8 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L,
             None => {
                 self.state.state = State::Processing { peer_id };
                 let blobs = std::mem::take(&mut self.blob_download_queue);
+                self.requested_ids.clear();
+
                 Ok(Some(blobs))
             }
         }

beacon_node/network/src/sync/block_lookups/single_block_lookup.rs

@@ -115,13 +115,19 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
         cx: &SyncNetworkContext<T>,
     ) -> Result<(), LookupRequestError> {
         let block_already_downloaded = self.block_already_downloaded();
-        let blobs_already_downloaded = self.blobs_already_downloaded();
+        let should_request_blobs = if self.blob_request_state.requested_ids.is_empty() {
+            let blob_ids = self.missing_blob_ids();
+            self.blob_request_state.requested_ids = blob_ids;
+            !self.blob_request_state.requested_ids.is_empty()
+        } else {
+            false
+        };
 
         if !block_already_downloaded {
             self.block_request_state
                 .build_request_and_send(self.id, cx)?;
         }
-        if !blobs_already_downloaded {
+        if should_request_blobs {
             self.blob_request_state
                 .build_request_and_send(self.id, cx)?;
         }
@@ -253,19 +259,6 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
         }
     }
 
-    /// Updates the `requested_ids` field of the `BlockRequestState` with the most recent picture
-    /// of which blobs still need to be requested. Returns `true` if there are no more blobs to
-    /// request.
-    pub(crate) fn blobs_already_downloaded(&mut self) -> bool {
-        self.update_blobs_request();
-        self.blob_request_state.requested_ids.is_empty()
-    }
-
-    /// Updates this request with the most recent picture of which blobs still need to be requested.
-    pub fn update_blobs_request(&mut self) {
-        self.blob_request_state.requested_ids = self.missing_blob_ids();
-    }
-
     /// If `child_components` is `Some`, we know block components won't hit the data
     /// availability cache, so we don't check its processing cache unless `child_components`
     /// is `None`.

beacon_node/network/src/sync/manager.rs

@@ -831,27 +831,37 @@ impl<T: BeaconChainTypes> SyncManager<T> {
         seen_timestamp: Duration,
     ) {
         match request_id {
-            RequestId::SingleBlock { id } => self
-                .block_lookups
-                .single_lookup_response::<BlockRequestState<Current>>(
-                    id,
-                    peer_id,
-                    block,
-                    seen_timestamp,
-                    &self.network,
-                ),
+            RequestId::SingleBlock { id } => {
+                debug!(self.log,
+                    "Peer returned block for single lookup";
+                    "peer_id" => %peer_id ,
+                );
+                self.block_lookups
+                    .single_lookup_response::<BlockRequestState<Current>>(
+                        id,
+                        peer_id,
+                        block,
+                        seen_timestamp,
+                        &self.network,
+                    )
+            }
             RequestId::SingleBlob { .. } => {
                 crit!(self.log, "Block received during blob request"; "peer_id" => %peer_id  );
             }
-            RequestId::ParentLookup { id } => self
-                .block_lookups
-                .parent_lookup_response::<BlockRequestState<Parent>>(
-                    id,
-                    peer_id,
-                    block,
-                    seen_timestamp,
-                    &self.network,
-                ),
+            RequestId::ParentLookup { id } => {
+                debug!(self.log,
+                    "Peer returned block for single lookup";
+                    "peer_id" => %peer_id ,
+                );
+                self.block_lookups
+                    .parent_lookup_response::<BlockRequestState<Parent>>(
+                        id,
+                        peer_id,
+                        block,
+                        seen_timestamp,
+                        &self.network,
+                    )
+            }
             RequestId::ParentLookupBlob { id: _ } => {
                 crit!(self.log, "Block received during parent blob request"; "peer_id" => %peer_id  );
             }