fix(chunks): properly handle missing block, previously causing chunk …

…signature check to fail (#3026)

Description:
For partial encoded chunks, the signature check includes looking up the chunk producer, based on the epoch ID. In addition it checks whether this chunk producer has been slashed. These checks require the node to know the parent block the chunk header references, and thus fails if it is not present. This change ensures we properly catch and handle such errors since we will be able to process the chunk later when the node eventually learns about the missing block.

Testing plan:
* New unit test for processing a chunk with an unknown parent block

chain/chunks/src/lib.rs

@@ -935,6 +935,12 @@ impl ShardsManager {
             return Err(Error::InvalidPartMessage);
         }
 
+        // check part merkle proofs
+        let num_total_parts = self.runtime_adapter.num_total_parts();
+        for part_info in forward.parts.iter() {
+            self.validate_part(forward.merkle_root, part_info, num_total_parts)?;
+        }
+
         // check signature
         let valid_signature = self.runtime_adapter.verify_chunk_signature_with_header_parts(
             &forward.chunk_hash,
@@ -948,12 +954,6 @@ impl ShardsManager {
             return Err(Error::InvalidChunkSignature);
         }
 
-        // check part merkle proofs
-        let num_total_parts = self.runtime_adapter.num_total_parts();
-        for part_info in forward.parts.iter() {
-            self.validate_part(forward.merkle_root, part_info, num_total_parts)?;
-        }
-
         Ok(())
     }
 

chain/client/src/client.rs

@@ -704,16 +704,34 @@ impl Client {
         &mut self,
         forward: PartialEncodedChunkForwardMsg,
     ) -> Result<Vec<AcceptedBlock>, Error> {
-        self.shards_mgr.validate_partial_encoded_chunk_forward(&forward)?;
+        let maybe_header = self
+            .shards_mgr
+            .validate_partial_encoded_chunk_forward(&forward)
+            .and_then(|_| self.shards_mgr.get_partial_encoded_chunk_header(&forward.chunk_hash));
 
-        let header = match self.shards_mgr.get_partial_encoded_chunk_header(&forward.chunk_hash) {
+        let header = match maybe_header {
             Ok(header) => Ok(header),
             Err(near_chunks::Error::UnknownChunk) => {
                 // We don't know this chunk yet; cache the forwarded part
                 // to be used after we get the header.
                 self.shards_mgr.insert_forwarded_chunk(forward);
                 return Err(Error::Chunk(near_chunks::Error::UnknownChunk));
             }
+            Err(near_chunks::Error::ChainError(chain_error)) => {
+                match chain_error.kind() {
+                    near_chain::ErrorKind::BlockMissing(_)
+                    | near_chain::ErrorKind::DBNotFoundErr(_) => {
+                        // We can't check if this chunk came from a valid chunk producer because
+                        // we don't know `prev_block`, however the signature is checked when
+                        // forwarded parts are later processed as partial encoded chunks, so we
+                        // can mark it as unknown for now.
+                        self.shards_mgr.insert_forwarded_chunk(forward);
+                        return Err(Error::Chunk(near_chunks::Error::UnknownChunk));
+                    }
+                    // Some other error occurred, we don't know how to handle it
+                    _ => Err(near_chunks::Error::ChainError(chain_error)),
+                }
+            }
             Err(err) => Err(err),
         }?;
         let partial_chunk =
@@ -1458,6 +1476,7 @@ mod test {
     use near_chunks::test_utils::ChunkForwardingTestFixture;
     use near_chunks::ProcessPartialEncodedChunkResult;
     use near_crypto::KeyType;
+    use near_network::types::PartialEncodedChunkForwardMsg;
     use near_primitives::block::{Approval, ApprovalInner};
     use near_primitives::hash::hash;
     use near_primitives::validator_signer::InMemoryValidatorSigner;
@@ -1502,6 +1521,10 @@ mod test {
         // change the prev_block to some unknown block
         mock_chunk.header.inner.prev_block_hash = hash(b"some_prev_block");
         mock_chunk.header.init();
+        let mock_forward = PartialEncodedChunkForwardMsg::from_header_and_parts(
+            &mock_chunk.header,
+            mock_chunk.parts.clone(),
+        );
 
         // process_partial_encoded_chunk should return Ok(NeedBlock) if the chunk is
         // based on a missing block.
@@ -1511,5 +1534,13 @@ mod test {
             &mut client.rs,
         );
         assert!(matches!(result, Ok(ProcessPartialEncodedChunkResult::NeedBlock)));
+
+        // process_partial_encoded_chunk_forward should return UnknownChunk if it is based on a
+        // a missing block.
+        let result = client.process_partial_encoded_chunk_forward(mock_forward);
+        assert!(matches!(
+            result,
+            Err(crate::types::Error::Chunk(near_chunks::Error::UnknownChunk))
+        ));
     }
 }