From 9196c463b1c1a6ca2dfdac37d70477e61f29530f Mon Sep 17 00:00:00 2001 From: Juhyung Park Date: Thu, 28 May 2020 16:36:52 +0900 Subject: [PATCH] Fetch consistent state in mempool functions We found that mempool mis-calculates sequence in the TPS test. After fixing the mempool to read consistent state, mempool calculates sequence well. --- core/src/client/test_client.rs | 6 ++++++ core/src/miner/mem_pool.rs | 15 +++++++++------ core/src/miner/miner.rs | 16 ++++++++++++---- core/src/miner/mod.rs | 9 ++++++--- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 321c8be73e..bcb3e24b04 100644 --- a/core/src/client/test_client.rs +++ b/core/src/client/test_client.rs @@ -366,6 +366,9 @@ impl AccountData for TestBlockChainClient { fn seq(&self, pubkey: &Public, id: BlockId) -> Option { match id { BlockId::Latest => Some(self.seqs.read().get(pubkey).cloned().unwrap_or(0)), + BlockId::Hash(hash) if hash == *self.last_hash.read() => { + Some(self.seqs.read().get(pubkey).cloned().unwrap_or(0)) + } _ => None, } } @@ -375,6 +378,9 @@ impl AccountData for TestBlockChainClient { StateOrBlock::Block(BlockId::Latest) | StateOrBlock::State(_) => { Some(self.balances.read().get(pubkey).cloned().unwrap_or(0)) } + StateOrBlock::Block(BlockId::Hash(hash)) if hash == *self.last_hash.read() => { + Some(self.balances.read().get(pubkey).cloned().unwrap_or(0)) + } _ => None, } } diff --git a/core/src/miner/mem_pool.rs b/core/src/miner/mem_pool.rs index b54bddf7b6..78298b3d4a 100644 --- a/core/src/miner/mem_pool.rs +++ b/core/src/miner/mem_pool.rs @@ -26,7 +26,7 @@ use crate::transaction::{PendingVerifiedTransactions, VerifiedTransaction}; use crate::Error as CoreError; use ckey::Ed25519Public as Public; use ctypes::errors::{HistoryError, RuntimeError, SyntaxError}; -use ctypes::{BlockNumber, TxHash}; +use ctypes::{BlockId, BlockNumber, TxHash}; use kvdb::{DBTransaction, KeyValueDB}; use std::cmp::max; use std::collections::{BTreeSet, HashMap, HashSet}; @@ -404,7 +404,12 @@ impl MemPool { // Recover MemPool state from db stored data pub fn recover_from_db(&mut self, client: &C) { - let fetch_account = fetch_account_creator(client); + let recover_block_id = { + let recover_block_hash = client.chain_info().best_block_hash; + BlockId::Hash(recover_block_hash) + }; + let fetch_account = fetch_account_creator(client, recover_block_id); + let by_hash = backup::recover_to_data(self.db.as_ref()); let recover_block_number = client.chain_info().best_block_number; @@ -1007,8 +1012,7 @@ pub mod test { let db = Arc::new(kvdb_memorydb::create(crate::db::NUM_COLUMNS.unwrap_or(0))); let mut mem_pool = MemPool::with_limits(8192, usize::max_value(), 3, db.clone()); - let fetch_account = fetch_account_creator(&test_client); - + let fetch_account = fetch_account_creator(&test_client, BlockId::Latest); let inserted_block_number = 1; let inserted_timestamp = 100; let mut inputs: Vec = Vec::new(); @@ -1081,12 +1085,11 @@ pub mod test { let db = Arc::new(kvdb_memorydb::create(crate::db::NUM_COLUMNS.unwrap_or(0))); let mut mem_pool = MemPool::with_limits(8192, usize::max_value(), 3, db); - let fetch_account = fetch_account_creator(&test_client); + let fetch_account = fetch_account_creator(&test_client, BlockId::Latest); let keypair: KeyPair = Random.generate().unwrap(); let pubkey = keypair.public(); test_client.set_balance(*pubkey, 1_000_000_000_000); assert_eq!(1_000_000_000_000, test_client.latest_balance(&pubkey)); - let inserted_block_number = 1; let inserted_timestamp = 100; let inputs = vec![ diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index fa4c158040..2e4f178995 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -292,7 +292,8 @@ impl Miner { }) .collect(); - let fetch_account = fetch_account_creator(client); + let block_id = BlockId::Hash(best_header.hash()); + let fetch_account = fetch_account_creator(client, block_id); let insertion_results = mem_pool.add(to_insert, current_block_number, current_timestamp, &fetch_account); @@ -469,8 +470,11 @@ impl Miner { } let block = open_block.close()?; - let fetch_seq = |p: &Public| chain.latest_seq(p); - + let block_id = { + let best_block_hash = chain.chain_info().best_block_hash; + BlockId::Hash(best_block_hash) + }; + let fetch_seq = |p: &Public| chain.seq(p, block_id).expect("Read from best block"); { let mut mem_pool = self.mem_pool.write(); mem_pool.remove( @@ -551,7 +555,11 @@ impl MinerService for Miner { ctrace!(MINER, "chain_new_blocks"); { - let fetch_account = fetch_account_creator(chain); + let block_id = { + let current_block_hash = chain.chain_info().best_block_hash; + BlockId::Hash(current_block_hash) + }; + let fetch_account = fetch_account_creator(chain, block_id); let current_block_number = chain.chain_info().best_block_number; let current_timestamp = chain.chain_info().best_block_timestamp; let mut mem_pool = self.mem_pool.write(); diff --git a/core/src/miner/mod.rs b/core/src/miner/mod.rs index 6fad9ea4ef..fd9f15e0af 100644 --- a/core/src/miner/mod.rs +++ b/core/src/miner/mod.rs @@ -156,9 +156,12 @@ pub enum TransactionImportResult { Future, } -fn fetch_account_creator<'c>(client: &'c dyn AccountData) -> impl Fn(&Public) -> AccountDetails + 'c { +fn fetch_account_creator<'c>( + client: &'c dyn AccountData, + block_id: BlockId, +) -> impl Fn(&Public) -> AccountDetails + 'c { move |pubkey: &Public| AccountDetails { - seq: client.latest_seq(&pubkey), - balance: client.latest_balance(&pubkey), + seq: client.seq(&pubkey, block_id).expect("We are querying sequence using trusted block id"), + balance: client.balance(&pubkey, block_id.into()).expect("We are querying balance using trusted block id"), } }