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.

Author: majecty

core/src/client/test_client.rs

@@ -366,6 +366,9 @@ impl AccountData for TestBlockChainClient {
     fn seq(&self, pubkey: &Public, id: BlockId) -> Option<u64> {
         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,
         }
     }

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<C: AccountData + BlockChainTrait>(&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<MemPoolInput> = 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![

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();

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"),
     }
 }