From a8cb4ad5031fdcb7a8f4463ad3c660ce509c2b25 Mon Sep 17 00:00:00 2001 From: Alex Kouprin Date: Fri, 12 Jun 2020 10:42:59 +0300 Subject: [PATCH] feat(Store Validator): check ColTrieChanges (#2828) + GC ColTrieChanges --- Cargo.toml | 2 +- chain/chain/src/store.rs | 17 +++- chain/chain/src/store_validator.rs | 25 +++-- chain/chain/src/store_validator/validate.rs | 106 +++++++++++++++----- core/primitives/src/utils.rs | 13 ++- core/store/Cargo.toml | 1 + core/store/src/trie/mod.rs | 5 + neard/Cargo.toml | 2 +- 8 files changed, 134 insertions(+), 37 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d9a723b8f94..edd44c23f00 100644 --- a/Cargo.toml +++ b/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"] diff --git a/chain/chain/src/store.rs b/chain/chain/src/store.rs index 46e54476c2c..0fb10616997 100644 --- a/chain/chain/src/store.rs +++ b/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)); + } } } diff --git a/chain/chain/src/store_validator.rs b/chain/chain/src/store_validator.rs index fbb1f7b82b3..9f2484a3dad 100644 --- a/chain/chain/src/store_validator.rs +++ b/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); } diff --git a/chain/chain/src/store_validator/validate.rs b/chain/chain/src/store_validator/validate.rs index 1bfd5ead807..4b01196103c 100644 --- a/chain/chain/src/store_validator/validate.rs +++ b/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::(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::(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::( + 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, diff --git a/core/primitives/src/utils.rs b/core/primitives/src/utils.rs index 75130f54e9f..80a3d39a5e0 100644 --- a/core/primitives/src/utils.rs +++ b/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 res } +pub fn get_block_shard_id_rev( + key: &[u8], +) -> Result<(CryptoHash, ShardId), Box> { + let block_hash_vec: Vec = 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 = base.as_ref().to_owned(); nonce.extend(index_to_bytes(salt)); diff --git a/core/store/Cargo.toml b/core/store/Cargo.toml index 452d7ee0573..ac20d32d0a9 100644 --- a/core/store/Cargo.toml +++ b/core/store/Cargo.toml @@ -32,3 +32,4 @@ harness = false [features] default = [] no_cache = [] +adversarial = [] diff --git a/core/store/src/trie/mod.rs b/core/store/src/trie/mod.rs index 0110d7216ed..c35a8195056 100644 --- a/core/store/src/trie/mod.rs +++ b/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 { diff --git a/neard/Cargo.toml b/neard/Cargo.toml index 36bf83b387f..06e0e0d62e3 100644 --- a/neard/Cargo.toml +++ b/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"]