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

@@ -384,6 +384,9 @@ impl AccountData for TestBlockChainClient {
     fn seq(&self, address: &Address, id: BlockId) -> Option<u64> {
         match id {
             BlockId::Latest => Some(self.seqs.read().get(address).cloned().unwrap_or(0)),
+            BlockId::Hash(hash) if hash == *self.last_hash.read() => {
+                Some(self.seqs.read().get(address).cloned().unwrap_or(0))
+            }
             _ => None,
         }
     }
@@ -393,6 +396,9 @@ impl AccountData for TestBlockChainClient {
             StateOrBlock::Block(BlockId::Latest) | StateOrBlock::State(_) => {
                 Some(self.balances.read().get(address).cloned().unwrap_or(0))
             }
+            StateOrBlock::Block(BlockId::Hash(hash)) if hash == *self.last_hash.read() => {
+                Some(self.balances.read().get(address).cloned().unwrap_or(0))
+            }
             _ => None,
         }
     }

core/src/miner/mem_pool.rs

@@ -23,7 +23,7 @@ use super::TransactionImportResult;
 use crate::client::{AccountData, BlockChainTrait};
 use crate::miner::fetch_account_creator;
 use crate::transaction::{PendingSignedTransactions, SignedTransaction};
-use crate::Error as CoreError;
+use crate::{BlockId, Error as CoreError};
 use ckey::{public_to_address, Public};
 use ctypes::errors::{HistoryError, RuntimeError, SyntaxError};
 use ctypes::{BlockNumber, TxHash};
@@ -430,7 +430,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;
@@ -1524,7 +1529,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(), Default::default());
 
-        let fetch_account = fetch_account_creator(&test_client);
+        let fetch_account = fetch_account_creator(&test_client, BlockId::Latest);
         let no_timelock = TxTimelock {
             block: None,
             timestamp: None,
@@ -1647,7 +1652,7 @@ pub mod test {
         txs: Vec<SignedTransaction>,
         origin: TxOrigin,
     ) -> Vec<Result<TransactionImportResult, Error>> {
-        let fetch_account = fetch_account_creator(test_client);
+        let fetch_account = fetch_account_creator(test_client, BlockId::Latest);
         let no_timelock = TxTimelock {
             block: None,
             timestamp: None,
@@ -1782,7 +1787,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, Default::default());
 
-        let fetch_account = fetch_account_creator(&test_client);
+        let fetch_account = fetch_account_creator(&test_client, BlockId::Latest);
         let keypair = Random.generate().unwrap();
         let address = public_to_address(keypair.public());
         println!("! {}", address);

core/src/miner/miner.rs

@@ -461,7 +461,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);
 
@@ -709,10 +710,14 @@ impl Miner {
         let term_common_params = chain.term_common_params(parent_hash.into());
         let block = open_block.close(&parent_header, term_common_params.as_ref())?;
 
+        let block_id = {
+            let best_block_hash = chain.chain_info().best_block_hash;
+            BlockId::Hash(best_block_hash)
+        };
         let fetch_seq = |p: &Public| {
             let address = public_to_address(p);
-            let a = chain.latest_regular_key_owner(&address).unwrap_or(address);
-            chain.latest_seq(&a)
+            let a = chain.regular_key_owner(&address, block_id.into()).unwrap_or(address);
+            chain.seq(&a, block_id).expect("Read from best block")
         };
 
         {
@@ -886,7 +891,11 @@ impl MinerService for Miner {
 
         // ...and at the end remove the old ones
         {
-            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

@@ -197,13 +197,16 @@ pub enum TransactionImportResult {
 #[cfg(all(feature = "nightly", test))]
 mod mem_pool_benches;
 
-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 |public: &Public| {
         let address = public_to_address(public);
-        let a = client.latest_regular_key_owner(&address).unwrap_or(address);
+        let a = client.regular_key_owner(&address, block_id.into()).unwrap_or(address);
         AccountDetails {
-            seq: client.latest_seq(&a),
-            balance: client.latest_balance(&a),
+            seq: client.seq(&a, block_id).expect("We are querying sequence using trusted block id"),
+            balance: client.balance(&a, block_id.into()).expect("We are querying balance using trusted block id"),
         }
     }
 }