Fix crash when client is not a block producer (#1275)

* Fix crash when not a block producer. Add more tests for sharding in near. Clean up configs * Make all features compile. Support non block hash for tx * Fix deserialize tesnet * Mark testnet.json as binary file * Mark testnet.json as binary * cross_shard_tx.rs fixing * Add sized cache around requested chunks in ChunkManager * Fix usage of setup_.. * Change to request chunks with explicitly interested shards for receips. Adding tests for tracking shards specifically. * Add chunk fetching via ViewClient. Test in track_shards fails * Fix chunk request/response logic for non-validators * Fix repro_1183 test
near · Sep 12, 2019 · 3483659 · 3483659
1 parent 543743c
commit 3483659
Show file tree

Hide file tree

Showing 46 changed files with 47,945 additions and 21,421 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -1,6 +1,3 @@
 Cargo.lock linguist-generated=true -diff
-core/protos/src/autogenerated/* linguist-generated=true -diff
-nearlib/protos.js linguist-generated=true -diff
-pynear/**/protos/* linguist-generated=true -diff
 **/package-lock.json linguist-generated=true -diff
-nearlib/dist/** linguist-generated=true -diff
+near/res/testnet.json linguist-generated=true binary
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/chain/chain/src/chain.rs b/chain/chain/src/chain.rs
@@ -9,7 +9,7 @@ use log::{debug, info};
 use near_primitives::hash::CryptoHash;
 use near_primitives::merkle::merklize;
 use near_primitives::receipt::Receipt;
-use near_primitives::sharding::{ShardChunk, ShardChunkHeader};
+use near_primitives::sharding::{ChunkHash, ShardChunk, ShardChunkHeader};
 use near_primitives::transaction::{check_tx_history, TransactionResult};
 use near_primitives::types::{AccountId, Balance, BlockIndex, ChunkExtra, Gas, ShardId};
 use near_store::Store;
@@ -345,8 +345,6 @@ impl Chain {
             {
                 return Ok(Some(new_res));
             }
-        } else {
-            crate::test_utils::display_chain(self);
         }
         res
     }
@@ -991,10 +989,19 @@ impl Chain {
         self.store.get_block(hash)
     }
 
-    /// Gets a chunk by hash.
+    /// Get a chunk from hash.
     #[inline]
-    pub fn get_chunk(&mut self, header: &ShardChunkHeader) -> Result<&ShardChunk, Error> {
-        self.store.get_chunk(header)
+    pub fn get_chunk(&mut self, chunk_hash: &ChunkHash) -> Result<&ShardChunk, Error> {
+        self.store.get_chunk(chunk_hash)
+    }
+
+    /// Gets a chunk from header.
+    #[inline]
+    pub fn get_chunk_from_header(
+        &mut self,
+        header: &ShardChunkHeader,
+    ) -> Result<&ShardChunk, Error> {
+        self.store.get_chunk_from_header(header)
     }
 
     /// Gets a block from the current chain by height.
@@ -1275,7 +1282,8 @@ impl<'a> ChainUpdate<'a> {
                         .cloned()
                         .collect::<Vec<_>>();
 
-                    let chunk = self.chain_store_update.get_chunk(&chunk_header)?.clone();
+                    let chunk =
+                        self.chain_store_update.get_chunk_from_header(&chunk_header)?.clone();
                     transactions.extend(chunk.transactions.iter().cloned());
 
                     if transactions.iter().any(|t| {
@@ -1343,6 +1351,7 @@ impl<'a> ChainUpdate<'a> {
                         .get_chunk_extra(&prev_block.hash(), shard_id)?
                         .clone();
 
+                    // TODO(1306): Transactions directly included in the block are not getting applied if chunk is skipped.
                     let apply_result = self
                         .runtime_adapter
                         .apply_transactions(

diff --git a/chain/chain/src/error.rs b/chain/chain/src/error.rs
@@ -3,7 +3,7 @@ use std::io;
 
 use chrono::{DateTime, Utc};
 use failure::{Backtrace, Context, Fail};
-use near_primitives::sharding::ShardChunkHeader;
+use near_primitives::sharding::{ChunkHash, ShardChunkHeader};
 
 #[derive(Debug)]
 pub struct Error {
@@ -18,7 +18,9 @@ pub enum ErrorKind {
     /// Orphan block.
     #[fail(display = "Orphan")]
     Orphan,
-    /// Chunks missing.
+    #[fail(display = "Chunk Missing: {:?}", _0)]
+    ChunkMissing(ChunkHash),
+    /// Chunks missing with header info.
     #[fail(display = "Chunks Missing: {:?}", _0)]
     ChunksMissing(Vec<(ShardChunkHeader)>),
     /// Peer abusively sending us an old block we already have
@@ -121,6 +123,7 @@ impl Error {
         match self.kind() {
             ErrorKind::Unfit(_)
             | ErrorKind::Orphan
+            | ErrorKind::ChunkMissing(_)
             | ErrorKind::ChunksMissing(_)
             | ErrorKind::IOErr(_)
             | ErrorKind::Other(_)

diff --git a/chain/chain/src/store.rs b/chain/chain/src/store.rs
@@ -73,7 +73,12 @@ pub trait ChainStoreAccess {
     /// Get full block.
     fn get_block(&mut self, h: &CryptoHash) -> Result<&Block, Error>;
     /// Get full chunk.
-    fn get_chunk(&mut self, header: &ShardChunkHeader) -> Result<&ShardChunk, Error>;
+    fn get_chunk(&mut self, chunk_hash: &ChunkHash) -> Result<&ShardChunk, Error>;
+    /// Get full chunk from header, with possible error that contains the header for further retreival.
+    fn get_chunk_from_header(&mut self, header: &ShardChunkHeader) -> Result<&ShardChunk, Error> {
+        self.get_chunk(&header.chunk_hash())
+            .map_err(|_| ErrorKind::ChunksMissing(vec![header.clone()]).into())
+    }
     /// Get chunk one part.
     fn get_chunk_one_part(&mut self, header: &ShardChunkHeader) -> Result<&ChunkOnePart, Error>;
     /// Get a collection of chunks and chunk_one_parts for a given height
@@ -256,16 +261,15 @@ impl ChainStoreAccess for ChainStore {
     }
 
     /// Get full chunk.
-    fn get_chunk(&mut self, header: &ShardChunkHeader) -> Result<&ShardChunk, Error> {
-        let chunk_hash = header.chunk_hash();
+    fn get_chunk(&mut self, chunk_hash: &ChunkHash) -> Result<&ShardChunk, Error> {
         let entry = self.chunks.entry(chunk_hash.clone());
         match entry {
             Entry::Occupied(s) => Ok(s.into_mut()),
             Entry::Vacant(s) => {
                 if let Ok(Some(chunk)) = self.store.get_ser(COL_CHUNKS, chunk_hash.as_ref()) {
                     Ok(s.insert(chunk))
                 } else {
-                    Err(ErrorKind::ChunksMissing(vec![header.clone()]).into())
+                    Err(ErrorKind::ChunkMissing(chunk_hash.clone()).into())
                 }
             }
         }
@@ -711,8 +715,12 @@ impl<'a, T: ChainStoreAccess> ChainStoreAccess for ChainStoreUpdate<'a, T> {
         self.chain_store.get_chunks_or_one_parts(me, parent_hash, height, runtime_adapter, headers)
     }
 
-    fn get_chunk(&mut self, header: &ShardChunkHeader) -> Result<&ShardChunk, Error> {
-        self.chain_store.get_chunk(header)
+    fn get_chunk(&mut self, chunk_hash: &ChunkHash) -> Result<&ShardChunk, Error> {
+        self.chain_store.get_chunk(chunk_hash)
+    }
+
+    fn get_chunk_from_header(&mut self, header: &ShardChunkHeader) -> Result<&ShardChunk, Error> {
+        self.chain_store.get_chunk_from_header(header)
     }
 
     fn get_chunk_one_part(&mut self, header: &ShardChunkHeader) -> Result<&ChunkOnePart, Error> {

diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs
@@ -165,10 +165,10 @@ impl KeyValueRuntime {
         if prev_hash == CryptoHash::default() {
             return Ok((EpochId(prev_hash), 0, EpochId(prev_hash)));
         }
-        let prev_block_header = self
-            .store
-            .get_ser::<BlockHeader>(COL_BLOCK_HEADER, prev_hash.as_ref())?
-            .ok_or(ErrorKind::Other("Missing block when computing the epoch".to_string()))?;
+        let prev_block_header =
+            self.store.get_ser::<BlockHeader>(COL_BLOCK_HEADER, prev_hash.as_ref())?.ok_or(
+                ErrorKind::Other(format!("Missing block {} when computing the epoch", prev_hash)),
+            )?;
 
         let mut hash_to_epoch = self.hash_to_epoch.write().unwrap();
         let mut hash_to_next_epoch = self.hash_to_next_epoch.write().unwrap();
@@ -348,7 +348,11 @@ impl RuntimeAdapter for KeyValueRuntime {
         shard_id: ShardId,
         _is_me: bool,
     ) -> bool {
-        let validators = &self.validators[self.get_epoch_and_valset(*parent_hash).unwrap().1];
+        let epoch_valset = match self.get_epoch_and_valset(*parent_hash) {
+            Ok(epoch_valset) => epoch_valset,
+            Err(_) => return false,
+        };
+        let validators = &self.validators[epoch_valset.1];
         assert_eq!((validators.len() as u64) % self.num_shards(), 0);
         assert_eq!(0, validators.len() as u64 % self.validator_groups);
         let validators_per_shard = validators.len() as ShardId / self.validator_groups;
@@ -372,8 +376,11 @@ impl RuntimeAdapter for KeyValueRuntime {
         shard_id: ShardId,
         _is_me: bool,
     ) -> bool {
-        let validators = &self.validators
-            [(self.get_epoch_and_valset(*parent_hash).unwrap().1 + 1) % self.validators.len()];
+        let epoch_valset = match self.get_epoch_and_valset(*parent_hash) {
+            Ok(epoch_valset) => epoch_valset,
+            Err(_) => return false,
+        };
+        let validators = &self.validators[(epoch_valset.1 + 1) % self.validators.len()];
         assert_eq!((validators.len() as u64) % self.num_shards(), 0);
         assert_eq!(0, validators.len() as u64 % self.validator_groups);
         let validators_per_shard = validators.len() as ShardId / self.validator_groups;
@@ -703,8 +710,8 @@ pub fn format_hash(h: CryptoHash) -> String {
     to_base(&h)[..6].to_string()
 }
 
-// Displays chain from given store.
-pub fn display_chain(chain: &mut Chain) {
+/// Displays chain from given store.
+pub fn display_chain(chain: &mut Chain, tail: bool) {
     let runtime_adapter = chain.runtime_adapter();
     let chain_store = chain.mut_store();
     let head = chain_store.head().unwrap();
@@ -715,7 +722,9 @@ pub fn display_chain(chain: &mut Chain) {
             .get_block_header(&CryptoHash::try_from(key.as_ref()).unwrap())
             .unwrap()
             .clone();
-        headers.push(header);
+        if !tail || header.inner.height + 10 > head.height {
+            headers.push(header);
+        }
     }
     headers.sort_by(|h_left, h_right| {
         if h_left.inner.height > h_right.inner.height {
@@ -758,7 +767,7 @@ pub fn display_chain(chain: &mut Chain) {
                             chunk_header.inner.shard_id,
                         )
                         .unwrap();
-                    if let Ok(chunk) = chain_store.get_chunk(&chunk_header) {
+                    if let Ok(chunk) = chain_store.get_chunk(&chunk_header.chunk_hash()) {
                         debug!(
                             "    {: >3} {} | {} | {: >10} | tx = {: >2}, receipts = {: >2}",
                             chunk_header.inner.height_created,

diff --git a/chain/chain/src/types.rs b/chain/chain/src/types.rs
@@ -165,6 +165,7 @@ pub trait RuntimeAdapter: Send + Sync {
     /// Account Id to Shard Id mapping, given current number of shards.
     fn account_id_to_shard_id(&self, account_id: &AccountId) -> ShardId;
 
+    /// Returns `account_id` that suppose to have the `part_id` of all chunks given previous block hash.
     fn get_part_owner(&self, parent_hash: &CryptoHash, part_id: u64) -> Result<AccountId, Error>;
 
     /// Whether the client cares about some shard right now.