Problem (Fix #1069): blocks with multiple txs may fail light client a…

…pp hash check

Solution:
- Change BTreeMap to IndexMap

client-common/Cargo.toml

@@ -29,6 +29,7 @@ parity-scale-codec = { features = ["derive"], version = "1.1" }
 websocket = { version = "0.24", default-features = false, features = ["sync"], optional = true }
 tendermint = { git = "https://github.com/crypto-com/tendermint-rs.git", default-features = false, rev = "db982c2437fe72c7a03942fc2bddf490f2332364" }
 itertools = "0.8"
+indexmap = "1.3"
 
 [dev-dependencies]
 quickcheck = "0.9"

client-common/src/tendermint/types/block_results.rs

@@ -1,9 +1,9 @@
 #![allow(missing_docs)]
-use std::collections::BTreeMap;
 use std::convert::TryFrom;
 use std::str::{from_utf8, FromStr};
 
 use base64;
+use indexmap::IndexMap;
 use serde::Deserialize;
 
 use chain_core::common::{TendermintEventKey, TendermintEventType};
@@ -56,11 +56,11 @@ pub struct Attribute {
 
 impl BlockResults {
     /// Returns transaction ids and the corresponding fees in block results
-    pub fn fees(&self) -> Result<BTreeMap<TxId, Fee>> {
+    pub fn fees(&self) -> Result<IndexMap<TxId, Fee>> {
         match &self.results.deliver_tx {
-            None => Ok(BTreeMap::default()),
+            None => Ok(IndexMap::default()),
             Some(deliver_tx) => {
-                let mut fees: BTreeMap<TxId, Fee> = BTreeMap::new();
+                let mut fees: IndexMap<TxId, Fee> = IndexMap::new();
 
                 for transaction in deliver_tx.iter() {
                     for event in transaction.events.iter() {

client-core/src/wallet/syncer.rs

@@ -1,4 +1,5 @@
 #![allow(missing_docs)]
+use indexmap::IndexMap;
 use itertools::{izip, Itertools};
 use non_empty_vec::NonEmpty;
 use std::sync::mpsc::Sender;
@@ -18,7 +19,6 @@ use super::syncer_logic::handle_blocks;
 use crate::service;
 use crate::service::{KeyService, SyncState, Wallet, WalletState, WalletStateMemento};
 use crate::TransactionObfuscation;
-use std::collections::BTreeMap;
 
 /// Transaction decryptor interface for wallet synchronizer
 pub trait TxDecryptor: Clone + Send + Sync {
@@ -484,7 +484,7 @@ pub(crate) struct FilteredBlock {
     /// Block time
     pub block_time: Time,
     /// List of successfully committed transaction ids in this block and their fees
-    pub valid_transaction_fees: BTreeMap<TxId, Fee>,
+    pub valid_transaction_fees: IndexMap<TxId, Fee>,
     /// Bloom filter for view keys and staking addresses
     pub block_filter: BlockFilter,
     /// List of successfully committed transaction of transactions that may need to be queried against

client-core/src/wallet/syncer_logic.rs

@@ -228,6 +228,7 @@ fn calculate_balance_change<'a>(
 
 #[cfg(test)]
 mod tests {
+    use indexmap::IndexMap;
     use secstr::SecUtf8;
     use std::str::FromStr;
 
@@ -243,7 +244,6 @@ mod tests {
     use crate::service::load_wallet;
     use crate::types::WalletKind;
     use crate::wallet::{DefaultWalletClient, WalletClient};
-    use std::collections::BTreeMap;
 
     fn create_test_wallet(n: usize) -> Result<Vec<Wallet>> {
         let storage = MemoryStorage::default();
@@ -302,7 +302,7 @@ mod tests {
             .map(|tx| tx.id())
             .chain(other_txs.iter().map(|tx| tx.id()))
             .collect();
-        let mut valid_transaction_fees = BTreeMap::new();
+        let mut valid_transaction_fees = IndexMap::new();
         for txid in valid_transaction_ids.iter() {
             valid_transaction_fees.insert(*txid, Fee::new(Coin::one()));
         }

integration-tests/bot/chainrpc.py

@@ -169,22 +169,21 @@ def sync_unlock(self, name=DEFAULT_WALLET, enckey=None):
     def sync_stop(self, name=DEFAULT_WALLET, enckey=None):
         return self.call('sync_stop', [name, enckey or get_enckey()])
 
-    def build_raw_transfer_tx(self, to_address, amount, name = DEFAULT_WALLET,  enckey=None, viewkeys=[]):
+    def build_raw_transfer_tx(self, to_address, amount, name=DEFAULT_WALLET,  enckey=None, viewkeys=[]):
         """
         build a raw transfer tx on watch-only wallet
         :return: unsigned raw transaction info encoded in base64 string
         """
         return self.call('wallet_buildRawTransferTransaction', [name, enckey or get_enckey()], to_address, str(amount), viewkeys)
 
-    def broadcast_signed_transfer_tx(self, signed_tx, name = DEFAULT_WALLET, enckey=None):
+    def broadcast_signed_transfer_tx(self, signed_tx, name=DEFAULT_WALLET, enckey=None):
         """
         send a transfer tx signed by offline wallet
         :return:
         """
         return self.call('wallet_broadcastSignedTransferTransaction', [name, enckey or get_enckey()], signed_tx)
 
 
-
 class Staking(BaseService):
     def deposit(self, to_address, inputs, name=DEFAULT_WALLET, enckey=None):
         return self.call('staking_depositStake', [name, enckey or get_enckey()], fix_address(to_address), inputs)
@@ -211,28 +210,29 @@ def unjail(self, address, name=DEFAULT_WALLET, enckey=None):
     def join(self, node_name, node_pubkey, node_staking_address, name=DEFAULT_WALLET, enckey=None):
         return self.call('staking_validatorNodeJoin', [name, enckey or get_enckey()], node_name, node_pubkey,  fix_address(node_staking_address))
 
-    def build_raw_transfer_tx(self, to_address, amount, name = DEFAULT_WALLET,  enckey=None, viewkeys=[]):
+    def build_raw_transfer_tx(self, to_address, amount, name=DEFAULT_WALLET,  enckey=None, viewkeys=[]):
         return self.call('wallet_buildRawTransferTx', [name, enckey or get_enckey()], to_address, amount, viewkeys)
 
-    def broadcast_raw_transfer_tx(self, signed_tx, name = DEFAULT_WALLET, enckey=None):
+    def broadcast_raw_transfer_tx(self, signed_tx, name=DEFAULT_WALLET, enckey=None):
         return self.call('wallet_broadcastSignedTransferTx', [name, enckey or get_enckey()], signed_tx)
 
 
-
 class MultiSig(BaseService):
     def create_address(self, public_keys, self_public_key, required_signatures, name=DEFAULT_WALLET, enckey=None):
-        return self.call('multiSig_createAddress',
-                    [name, enckey or get_enckey()],
-                    public_keys,
-                    self_public_key,
-                    required_signatures)
+        return self.call(
+            'multiSig_createAddress',
+            [name, enckey or get_enckey()],
+            public_keys,
+            self_public_key,
+            required_signatures)
 
     def new_session(self, message, signer_public_keys, self_public_key, name=DEFAULT_WALLET, enckey=None):
-        return self.call('multiSig_newSession',
-                    [name, enckey or get_enckey()],
-                    message,
-                    signer_public_keys,
-                    self_public_key)
+        return self.call(
+            'multiSig_newSession',
+            [name, enckey or get_enckey()],
+            message,
+            signer_public_keys,
+            self_public_key)
 
     def nonce_commitment(self, session_id, passphrase):
         return self.call('multiSig_nonceCommitment', session_id, passphrase)
@@ -256,10 +256,11 @@ def signature(self, session_id, passphrase):
         return self.call('multiSig_signature', session_id, passphrase)
 
     def broadcast_with_signature(self, session_id, unsigned_transaction, name=DEFAULT_WALLET, enckey=None):
-        return self.call('multiSig_broadcastWithSignature',
-                    [name, enckey or get_enckey()],
-                    session_id,
-                    unsigned_transaction)
+        return self.call(
+            'multiSig_broadcastWithSignature',
+            [name, enckey or get_enckey()],
+            session_id,
+            unsigned_transaction)
 
 
 class Blockchain(BaseService):

integration-tests/multinode/common.py

@@ -44,8 +44,8 @@ def latest_block_height(rpc):
     return int(rpc.chain.status()['sync_info']['latest_block_height'])
 
 
-def wait_for_blocks(rpc, n):
-    height = latest_block_height(rpc)
+def wait_for_blocks(rpc, n, height=None):
+    height = height if height is not None else latest_block_height(rpc)
     while True:
         time.sleep(1)
         delta = latest_block_height(rpc) - height

integration-tests/multinode/multitx_cluster.json

@@ -0,0 +1,53 @@
+{
+    "mock_mode": false,
+    "root_path": "./data",
+    "chain_id": "test-chain-y3m1e6-AB",
+    "genesis_time": "2019-11-20T08:56:48.618137Z",
+    "expansion_cap": 1000000000000000000,
+    "nodes": [
+        {
+            "name": "node0",
+            "hostname": "127.0.0.1",
+            "mnemonic": "analyst salon domain idea mango loyal depart utility vicious afraid double visit frog place bench",
+            "validator_seed": "14d5136b842b5bdbe6df065f78ef8b92f3f67597a77f6fd16f13b7513331c3a0",
+            "node_seed": "0360e3074ebe3ef45660514e3723670cb5a20c231778abaf4b21ebd4a8cdfac1",
+            "bonded_coin": 250000000000000000,
+            "unbonded_coin": 250000000000000000,
+            "base_port": 26650
+        },
+        {
+            "name": "node1",
+            "hostname": "127.0.0.1",
+            "mnemonic": "afford citizen lyrics field stumble globe brisk muffin speak scare gift million isolate nuclear wait",
+            "validator_seed": "5191100898b7eba044ff67cf7bf496ee49c1fd553755e7966d6dcd9f29d1e686",
+            "node_seed": "6a7833dc02885693c7f9704a5434facb7eb7a288bbaa0edb2145ab3e7a819ff3",
+            "bonded_coin": 250000000000000000,
+            "unbonded_coin": 250000000000000000,
+            "base_port": 26660
+        }
+    ],
+    "chain_config_patch": [
+        {
+            "op": "replace",
+            "path": "/initial_fee_policy/base_fee",
+            "value": "0"
+        },
+        {
+            "op": "replace",
+            "path": "/initial_fee_policy/per_byte_fee",
+            "value": "0"
+        }
+    ],
+    "tendermint_config_patch": [
+        {
+            "op": "replace",
+            "path": "/consensus/create_empty_blocks",
+            "value": false
+        },
+        {
+            "op": "add",
+            "path": "/consensus/create_empty_blocks_interval",
+            "value": "0s"
+        }
+    ]
+}

integration-tests/multinode/multitx_test.py

@@ -0,0 +1,70 @@
+import os
+import time
+from chainrpc import RPC
+from common import (
+    UnixStreamXMLRPCClient, wait_for_validators, stop_node,
+    wait_for_tx, latest_block_height, wait_for_blocks
+)
+
+'''
+Env:
+- Two nodes, each with half power
+
+Procedure:
+- stop node1
+- send tx to node0
+- start node1
+- repeat above
+- test wallet sync
+'''
+
+
+BASE_PORT = int(os.environ.get('BASE_PORT', 25560))
+supervisor = UnixStreamXMLRPCClient('data/supervisor.sock')
+rpc = RPC(BASE_PORT)
+
+print('Wait for 2 validators online')
+wait_for_validators(rpc, 2)
+
+os.environ['ENCKEY'] = rpc.wallet.enckey()
+
+print('Prepare node0 transfer addresses')
+unbonded = rpc.address.list()[1]
+transfer1 = rpc.address.list(type='transfer')[0]
+
+time.sleep(3)  # wait for at least one block, FIXME remove after #828 fixed
+txid = rpc.staking.withdraw_all_unbonded(unbonded, transfer1)
+wait_for_tx(rpc, txid)
+rpc.wallet.sync()
+
+addresses = [rpc.address.create(type='transfer') for i in range(10)]
+amount = 100000000
+for addr in addresses:
+    txid = rpc.wallet.send(addr, amount)
+    wait_for_tx(rpc, txid)
+    rpc.wallet.sync()
+
+print('Stop node1')
+stop_node(supervisor, 'node1')
+last_block_height = latest_block_height(rpc)
+
+print('Send multiple tx')
+pending_txs = [rpc.wallet.send(transfer1, amount) for _ in addresses]
+time.sleep(2)  # wait for the tx processing
+
+print('Start node1')
+supervisor.supervisor.startProcessGroup('node1')
+
+print('Wait for one block')
+wait_for_blocks(rpc, 1, height=last_block_height)
+block = rpc.chain.block(last_block_height+1)
+print(block['block_meta']['header']['num_txs'], block)
+assert block['block_meta']['header']['num_txs'] == '10'
+
+print('Check sync ok')
+rpc.wallet.sync()
+assert rpc.wallet.balance() == {
+    'total': '250000000000000000',
+    'pending': '0',
+    'available': '250000000000000000',
+}

integration-tests/run_multinode.sh

@@ -82,5 +82,6 @@ runtest "jail"
 runtest "join"
 runtest "byzantine"
 runtest "proportional"
+runtest "multitx"
 
 ./cleanup.sh