feat(Store Validator): check ColTrieChanges (#2828)

+ GC ColTrieChanges

Cargo.toml

@@ -95,6 +95,6 @@ opt-level = 3 # bs58 library is too slow to use in debug
 expensive_tests = ["neard/expensive_tests"]
 regression_tests = []
 old_tests = []
-adversarial = ["neard/adversarial", "near-jsonrpc/adversarial"]
+adversarial = ["neard/adversarial", "near-jsonrpc/adversarial", "near-store/adversarial"]
 no_cache = ["node-runtime/no_cache", "near-store/no_cache"]
 metric_recorder = ["neard/metric_recorder"]

chain/chain/src/store.rs

@@ -1916,6 +1916,12 @@ impl<'a> ChainStoreUpdate<'a> {
                         .map(|trie_changes: TrieChanges| {
                             tries
                                 .revert_insertions(&trie_changes, shard_id, &mut store_update)
+                                .map(|_| {
+                                    store_update.delete(
+                                        ColTrieChanges,
+                                        &get_block_shard_id(&block_hash, shard_id),
+                                    );
+                                })
                                 .map_err(|err| ErrorKind::Other(err.to_string()))
                         })
                         .unwrap_or(Ok(()))?;
@@ -1929,6 +1935,12 @@ impl<'a> ChainStoreUpdate<'a> {
                         .map(|trie_changes: TrieChanges| {
                             tries
                                 .apply_deletions(&trie_changes, shard_id, &mut store_update)
+                                .map(|_| {
+                                    store_update.delete(
+                                        ColTrieChanges,
+                                        &get_block_shard_id(&block_hash, shard_id),
+                                    );
+                                })
                                 .map_err(|err| ErrorKind::Other(err.to_string()))
                         })
                         .unwrap_or(Ok(()))?;
@@ -1937,7 +1949,10 @@ impl<'a> ChainStoreUpdate<'a> {
                 block_hash = *self.get_block_header(&block_hash)?.prev_hash();
             }
             GCMode::StateSync => {
-                // Do nothing here
+                // Not apply the data from ColTrieChanges
+                for shard_id in 0..header.chunk_mask().len() as ShardId {
+                    store_update.delete(ColTrieChanges, &get_block_shard_id(&block_hash, shard_id));
+                }
             }
         }
 

chain/chain/src/store_validator.rs

@@ -11,7 +11,8 @@ use near_primitives::borsh;
 use near_primitives::hash::CryptoHash;
 use near_primitives::sharding::{ChunkHash, ShardChunk};
 use near_primitives::types::{AccountId, BlockHeight};
-use near_store::{DBCol, Store};
+use near_primitives::utils::get_block_shard_id_rev;
+use near_store::{DBCol, Store, TrieChanges};
 
 use crate::RuntimeAdapter;
 
@@ -216,18 +217,27 @@ impl StoreValidator {
                         self.check(&validate::chunk_hash_validity, &chunk_hash, &shard_chunk, col);
                         // Chunk Height Created is not lower than Chunk Tail
                         self.check(&validate::chunk_tail_validity, &chunk_hash, &shard_chunk, col);
-                        // There is a State Root in the Trie
+                        // ShardChunk can be indexed by Height
                         self.check(
-                            &validate::chunk_state_roots_in_trie,
+                            &validate::chunk_indexed_by_height_created,
                             &chunk_hash,
                             &shard_chunk,
                             col,
                         );
-                        // ShardChunk can be indexed by Height
+                    }
+                }
+                DBCol::ColTrieChanges => {
+                    if let Some(((block_hash, shard_id), trie_changes)) = self.unwrap_kv(
+                        get_block_shard_id_rev(key_ref),
+                        TrieChanges::try_from_slice(value_ref),
+                        key_ref,
+                        col,
+                    ) {
+                        // ShardChunk should exist for current TrieChanges
                         self.check(
-                            &validate::chunk_indexed_by_height_created,
-                            &chunk_hash,
-                            &shard_chunk,
+                            &validate::trie_changes_chunk_extra_exists,
+                            &(block_hash, shard_id),
+                            &trie_changes,
                             col,
                         );
                     }
@@ -267,6 +277,7 @@ impl StoreValidator {
         // There is no more than one Block which Height is lower than Tail and not equal to Genesis
         self.check(&validate::block_height_cmp_tail, &"TAIL", &0, DBCol::ColBlockMisc);
         self.validate_col(DBCol::ColChunks);
+        self.validate_col(DBCol::ColTrieChanges);
         self.validate_col(DBCol::ColChunkHashesByHeight);
     }
 

chain/chain/src/store_validator/validate.rs

@@ -3,11 +3,12 @@ use std::collections::{HashMap, HashSet};
 use near_primitives::block::{Block, BlockHeader, Tip};
 use near_primitives::hash::CryptoHash;
 use near_primitives::sharding::{ChunkHash, ShardChunk};
-use near_primitives::types::{BlockHeight, EpochId};
-use near_primitives::utils::index_to_bytes;
+use near_primitives::types::{BlockHeight, ChunkExtra, EpochId, ShardId};
+use near_primitives::utils::{get_block_shard_id, index_to_bytes};
 use near_store::{
-    ColBlockHeader, ColBlockHeight, ColBlockMisc, ColBlockPerHeight, ColChunkHashesByHeight,
-    ColChunks, CHUNK_TAIL_KEY, HEADER_HEAD_KEY, HEAD_KEY, TAIL_KEY,
+    ColBlock, ColBlockHeader, ColBlockHeight, ColBlockMisc, ColBlockPerHeight, ColChunkExtra,
+    ColChunkHashesByHeight, ColChunks, TrieChanges, TrieIterator, CHUNK_TAIL_KEY, HEADER_HEAD_KEY,
+    HEAD_KEY, TAIL_KEY,
 };
 
 use crate::{ErrorMessage, StoreValidator};
@@ -223,28 +224,6 @@ pub(crate) fn chunk_tail_validity(
     Ok(())
 }
 
-pub(crate) fn chunk_state_roots_in_trie(
-    _sv: &mut StoreValidator,
-    _chunk_hash: &ChunkHash,
-    _shard_chunk: &ShardChunk,
-) -> Result<(), ErrorMessage> {
-    // TODO enable after fixing #2623
-    /*
-    let shard_id = shard_chunk.header.inner.shard_id;
-    let state_root = shard_chunk.header.inner.prev_state_root;
-    let trie = sv.runtime_adapter.get_trie_for_shard(shard_id);
-    let trie = unwrap_or_err!(
-        TrieIterator::new(&trie, &state_root),
-        "Trie Node Missing for ShardChunk {:?}",
-        shard_chunk
-    );
-    for item in trie {
-        unwrap_or_err!(item, "Can't find ShardChunk {:?} in Trie", shard_chunk);
-    }
-    */
-    Ok(())
-}
-
 pub(crate) fn chunk_indexed_by_height_created(
     sv: &mut StoreValidator,
     _chunk_hash: &ChunkHash,
@@ -393,6 +372,81 @@ pub(crate) fn canonical_prev_block_validity(
     Ok(())
 }
 
+pub(crate) fn trie_changes_chunk_extra_exists(
+    sv: &mut StoreValidator,
+    (block_hash, shard_id): &(CryptoHash, ShardId),
+    trie_changes: &TrieChanges,
+) -> Result<(), ErrorMessage> {
+    let new_root = trie_changes.new_root;
+    // 1. Block with `block_hash` should be available
+    let block = unwrap_or_err_db!(
+        sv.store.get_ser::<Block>(ColBlock, block_hash.as_ref()),
+        "Can't get Block from DB"
+    );
+    // 2. There should be ShardChunk with ShardId `shard_id`
+    for chunk_header in block.chunks().iter() {
+        if chunk_header.inner.shard_id == *shard_id {
+            let chunk_hash = &chunk_header.hash;
+            // 3. ShardChunk with `chunk_hash` should be available
+            unwrap_or_err_db!(
+                sv.store.get_ser::<ShardChunk>(ColChunks, chunk_hash.as_ref()),
+                "Can't get Chunk from storage with ChunkHash {:?}",
+                chunk_hash
+            );
+            // 4. Chunk Extra with `block_hash` and `shard_id` should be available
+            let chunk_extra = unwrap_or_err_db!(
+                sv.store.get_ser::<ChunkExtra>(
+                    ColChunkExtra,
+                    &get_block_shard_id(block_hash, *shard_id)
+                ),
+                "Can't get Chunk Extra from storage with key {:?} {:?}",
+                block_hash,
+                shard_id
+            );
+            let trie = sv.runtime_adapter.get_trie_for_shard(*shard_id);
+            let trie_iterator = unwrap_or_err!(
+                TrieIterator::new(&trie, &new_root),
+                "Trie Node Missing for ShardChunk {:?}",
+                chunk_header
+            );
+            // 5. ShardChunk `shard_chunk` should be available in Trie
+            for item in trie_iterator {
+                unwrap_or_err!(item, "Can't find ShardChunk {:?} in Trie", chunk_header);
+            }
+            // 6. Prev State Roots should be equal
+            // TODO #2623: enable
+            /*
+            #[cfg(feature = "adversarial")]
+            {
+                let prev_state_root = chunk_header.inner.prev_state_root;
+                let old_root = trie_changes.adv_get_old_root();
+                if prev_state_root != old_root {
+                    return err!(
+                        "Prev State Root discrepancy, {:?} != {:?}, ShardChunk {:?}",
+                        old_root,
+                        prev_state_root,
+                        chunk_header
+                    );
+                }
+            }
+            */
+            // 7. State Roots should be equal
+            let state_root = chunk_extra.state_root;
+            return if state_root == new_root {
+                Ok(())
+            } else {
+                err!(
+                    "State Root discrepancy, {:?} != {:?}, ShardChunk {:?}",
+                    new_root,
+                    state_root,
+                    chunk_header
+                )
+            };
+        }
+    }
+    err!("ShardChunk is not included into Block {:?}", block)
+}
+
 pub(crate) fn chunk_of_height_exists(
     sv: &mut StoreValidator,
     height: &BlockHeight,

core/primitives/src/utils.rs

@@ -1,5 +1,5 @@
 use std::cmp::max;
-use std::convert::AsRef;
+use std::convert::{AsRef, TryFrom};
 use std::fmt;
 
 use byteorder::{LittleEndian, WriteBytesExt};
@@ -27,6 +27,17 @@ pub fn get_block_shard_id(block_hash: &CryptoHash, shard_id: ShardId) -> Vec<u8>
     res
 }
 
+pub fn get_block_shard_id_rev(
+    key: &[u8],
+) -> Result<(CryptoHash, ShardId), Box<dyn std::error::Error>> {
+    let block_hash_vec: Vec<u8> = key[0..32].iter().cloned().collect();
+    let block_hash = CryptoHash::try_from(block_hash_vec)?;
+    let mut shard_id_arr: [u8; 8] = Default::default();
+    shard_id_arr.copy_from_slice(&key[key.len() - 8..]);
+    let shard_id = ShardId::from_le_bytes(shard_id_arr);
+    Ok((block_hash, shard_id))
+}
+
 pub fn create_nonce_with_nonce(base: &CryptoHash, salt: u64) -> CryptoHash {
     let mut nonce: Vec<u8> = base.as_ref().to_owned();
     nonce.extend(index_to_bytes(salt));

core/store/Cargo.toml

@@ -32,3 +32,4 @@ harness = false
 [features]
 default = []
 no_cache = []
+adversarial = []

core/store/src/trie/mod.rs

@@ -454,6 +454,11 @@ impl TrieChanges {
     pub fn empty(old_root: StateRoot) -> Self {
         TrieChanges { old_root, new_root: old_root, insertions: vec![], deletions: vec![] }
     }
+
+    #[cfg(feature = "adversarial")]
+    pub fn adv_get_old_root(&self) -> StateRoot {
+        self.old_root
+    }
 }
 
 impl Trie {

neard/Cargo.toml

@@ -46,7 +46,7 @@ tempfile = "3"
 testlib = { path = "../test-utils/testlib" }
 
 [features]
-adversarial =  ["near-client/adversarial", "near-network/adversarial"]
+adversarial =  ["near-client/adversarial", "near-network/adversarial", "near-store/adversarial"]
 expensive_tests = ["near-client/expensive_tests", "near-epoch-manager/expensive_tests", "near-chain/expensive_tests"]
 metric_recorder = ["near-network/metric_recorder", "near-client/metric_recorder"]