Fix unexpected UnrequestedBlobId and ExtraBlocksReturned errors due to race conditions.

Author: jimmygchen

beacon_node/beacon_chain/src/data_availability_checker.rs

@@ -110,8 +110,6 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
         self.processing_cache.read().get(&block_root).cloned()
     }
 
-    /// A `None` indicates blobs are not required.
-    ///
     /// If there's no block, all possible ids will be returned that don't exist in the given blobs.
     /// If there no blobs, all possible ids will be returned.
     pub fn get_missing_blob_ids<V: AvailabilityView<T::EthSpec>>(

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

@@ -371,16 +371,21 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L,
 
     fn verify_response_inner(
         &mut self,
-        _expected_block_root: Hash256,
+        expected_block_root: Hash256,
         blob: Option<Self::ResponseType>,
         peer_id: PeerId,
     ) -> Result<Option<FixedBlobSidecarList<T::EthSpec>>, LookupVerifyError> {
         match blob {
             Some(blob) => {
                 let received_id = blob.id();
                 if !self.requested_ids.contains(&received_id) {
+                    if expected_block_root == received_id.block_root {
+                        // We may have already received this blob via gossip since the lookup,
+                        // so don't return an error here.
+                        return Ok(None);
+                    }
                     self.state.register_failure_downloading();
-                    Err(LookupVerifyError::UnrequestedBlobId)
+                    Err(LookupVerifyError::UnrequestedBlobId(received_id))
                 } else {
                     // State should remain downloading until we receive the stream terminator.
                     self.requested_ids.remove(&received_id);

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

@@ -540,7 +540,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
             | ParentVerifyError::NoBlockReturned
             | ParentVerifyError::NotEnoughBlobsReturned
             | ParentVerifyError::ExtraBlocksReturned
-            | ParentVerifyError::UnrequestedBlobId
+            | ParentVerifyError::UnrequestedBlobId(_)
             | ParentVerifyError::ExtraBlobsReturned
             | ParentVerifyError::InvalidIndex(_) => {
                 let e = e.into();

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

@@ -12,6 +12,7 @@ use std::collections::VecDeque;
 use std::sync::Arc;
 use store::Hash256;
 use strum::IntoStaticStr;
+use types::blob_sidecar::BlobIdentifier;
 
 /// How many attempts we try to find a parent of a block before we give up trying.
 pub(crate) const PARENT_FAIL_TOLERANCE: u8 = 5;
@@ -36,7 +37,7 @@ pub enum ParentVerifyError {
     NoBlockReturned,
     NotEnoughBlobsReturned,
     ExtraBlocksReturned,
-    UnrequestedBlobId,
+    UnrequestedBlobId(BlobIdentifier),
     ExtraBlobsReturned,
     InvalidIndex(u64),
     PreviousFailure { parent_root: Hash256 },
@@ -242,7 +243,7 @@ impl From<LookupVerifyError> for ParentVerifyError {
             E::RootMismatch => ParentVerifyError::RootMismatch,
             E::NoBlockReturned => ParentVerifyError::NoBlockReturned,
             E::ExtraBlocksReturned => ParentVerifyError::ExtraBlocksReturned,
-            E::UnrequestedBlobId => ParentVerifyError::UnrequestedBlobId,
+            E::UnrequestedBlobId(blob_id) => ParentVerifyError::UnrequestedBlobId(blob_id),
             E::ExtraBlobsReturned => ParentVerifyError::ExtraBlobsReturned,
             E::InvalidIndex(index) => ParentVerifyError::InvalidIndex(index),
             E::NotEnoughBlobsReturned => ParentVerifyError::NotEnoughBlobsReturned,

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

@@ -16,7 +16,7 @@ use std::marker::PhantomData;
 use std::sync::Arc;
 use store::Hash256;
 use strum::IntoStaticStr;
-use types::blob_sidecar::FixedBlobSidecarList;
+use types::blob_sidecar::{BlobIdentifier, FixedBlobSidecarList};
 use types::EthSpec;
 
 #[derive(Debug, PartialEq, Eq)]
@@ -31,7 +31,7 @@ pub enum LookupVerifyError {
     RootMismatch,
     NoBlockReturned,
     ExtraBlocksReturned,
-    UnrequestedBlobId,
+    UnrequestedBlobId(BlobIdentifier),
     ExtraBlobsReturned,
     NotEnoughBlobsReturned,
     InvalidIndex(u64),
@@ -184,6 +184,7 @@ impl<L: Lookup, T: BeaconChainTypes> SingleBlockLookup<L, T> {
                 existing_components.merge_block(block);
             }
             existing_components.merge_blobs(downloaded_blobs);
+            // TODO check if we could send it to processing?
         } else {
             self.child_components = Some(components);
         }