Sort validator set in public key order

core/src/block.rs

@@ -25,7 +25,7 @@ use cstate::{FindDoubleVoteHandler, NextValidators, StateDB, StateError, StateWi
 use ctypes::errors::HistoryError;
 use ctypes::header::{Header, Seal};
 use ctypes::util::unexpected::Mismatch;
-use ctypes::{BlockNumber, TxHash};
+use ctypes::{BlockNumber, TransactionIndex, TxHash};
 use merkle_trie::skewed_merkle_root;
 use primitives::{Bytes, H256};
 use rlp::{Decodable, DecoderError, Encodable, Rlp, RlpStream};
@@ -150,6 +150,7 @@ impl OpenBlock {
         client: &C,
         parent_block_number: BlockNumber,
         parent_block_timestamp: u64,
+        transaction_index: TransactionIndex,
     ) -> Result<(), Error> {
         if self.block.transactions_set.contains(&tx.hash()) {
             return Err(HistoryError::TransactionAlreadyImported.into())
@@ -163,6 +164,7 @@ impl OpenBlock {
             parent_block_number,
             parent_block_timestamp,
             self.block.header.timestamp(),
+            transaction_index,
         ) {
             Ok(()) => {
                 self.block.transactions_set.insert(hash);
@@ -190,8 +192,14 @@ impl OpenBlock {
         parent_block_number: BlockNumber,
         parent_block_timestamp: u64,
     ) -> Result<(), Error> {
-        for tx in transactions {
-            self.push_transaction(tx.clone(), client, parent_block_number, parent_block_timestamp)?;
+        for (index, tx) in transactions.iter().enumerate() {
+            self.push_transaction(
+                tx.clone(),
+                client,
+                parent_block_number,
+                parent_block_timestamp,
+                index as TransactionIndex,
+            )?;
         }
         Ok(())
     }

core/src/blockchain/body_db.rs

@@ -19,7 +19,7 @@ use super::extras::TransactionAddress;
 use crate::db::{self, CacheUpdatePolicy, Readable, Writable};
 use crate::encoded;
 use crate::views::BlockView;
-use ctypes::{BlockHash, TxHash};
+use ctypes::{BlockHash, TransactionIndex, TxHash};
 use kvdb::{DBTransaction, KeyValueDB};
 use lru_cache::LruCache;
 use parking_lot::{Mutex, RwLock};
@@ -179,7 +179,7 @@ fn tx_hash_and_address_entries(
     tx_hashes.into_iter().enumerate().map(move |(index, tx_hash)| {
         (tx_hash, TransactionAddress {
             block_hash,
-            index,
+            index: index as TransactionIndex,
         })
     })
 }

core/src/blockchain/extras.rs

@@ -16,7 +16,7 @@
 
 use crate::db::Key;
 use crate::types::TransactionId;
-use ctypes::{BlockHash, BlockNumber, TxHash};
+use ctypes::{BlockHash, BlockNumber, TransactionIndex, TxHash};
 use primitives::{H256, H264};
 use std::ops::Deref;
 
@@ -95,7 +95,7 @@ pub struct TransactionAddress {
     /// Block hash
     pub block_hash: BlockHash,
     /// Transaction index within the block
-    pub index: usize,
+    pub index: TransactionIndex,
 }
 
 impl From<TransactionAddress> for TransactionId {

core/src/client/test_client.rs

@@ -310,7 +310,7 @@ impl TestBlockChainClient {
             pubkeys.push(public);
         }
         let fixed_validators: NextValidators =
-            pubkeys.into_iter().map(|pubkey| Validator::new(0, 0, pubkey)).collect::<Vec<_>>().into();
+            pubkeys.into_iter().map(|pubkey| Validator::new(0, 0, pubkey, 0, 0)).collect::<Vec<_>>().into();
 
         self.validators = fixed_validators;
     }

core/src/consensus/tendermint/worker.rs

@@ -1528,7 +1528,7 @@ impl Worker {
         assert_eq!(header.number(), self.height);
 
         let parent_hash = header.parent_hash();
-        let signer_index = self.validators.proposer_index(*parent_hash, self.view as usize);
+        let signer_index = self.validators.proposer_index(*parent_hash, self.view);
 
         let on = VoteOn {
             step: VoteStep::new(self.height, self.view, Step::Propose),
@@ -1555,7 +1555,7 @@ impl Worker {
         proposed_view: View,
         signature: Signature,
     ) -> Option<ConsensusMessage> {
-        let signer_index = self.validators.proposer_index(*header.parent_hash(), proposed_view as usize);
+        let signer_index = self.validators.proposer_index(*header.parent_hash(), proposed_view);
 
         let on = VoteOn {
             step: VoteStep::new(header.number(), proposed_view, Step::Propose),
@@ -1571,7 +1571,6 @@ impl Worker {
 
     fn signer_index(&self) -> Option<usize> {
         let parent = self.prev_block_hash();
-        // FIXME: More effecient way to find index
         self.signer.public().and_then(|public| self.validators.get_index(&parent, public))
     }
 

core/src/consensus/validator_set/dynamic_validator.rs

@@ -24,23 +24,36 @@ use ctypes::transaction::Validator;
 use ctypes::util::unexpected::OutOfBounds;
 use ctypes::BlockHash;
 use parking_lot::RwLock;
+use std::cmp::Reverse;
 use std::sync::{Arc, Weak};
 
 #[derive(Default)]
 pub struct DynamicValidator {
     client: RwLock<Option<Weak<dyn ConsensusClient>>>,
 }
 
+pub struct WeightOrderedValidators(Vec<Public>);
+
+pub struct WeightIndex(usize);
+
+impl WeightOrderedValidators {
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
+    pub fn get(&self, index: WeightIndex) -> Option<&Public> {
+        self.0.get(index.0)
+    }
+}
+
 impl DynamicValidator {
     fn next_validators(&self, hash: BlockHash) -> Vec<Validator> {
         let client: Arc<dyn ConsensusClient> =
             self.client.read().as_ref().and_then(Weak::upgrade).expect("Client is not initialized");
         let block_id = hash.into();
         let state = client.state_at(block_id).expect("The next validators must be called on the confirmed block");
         let validators = NextValidators::load_from_state(&state).unwrap();
-        let mut validators: Vec<_> = validators.into();
-        validators.reverse();
-        validators
+        validators.into()
     }
 
     fn current_validators(&self, hash: BlockHash) -> Vec<Validator> {
@@ -49,20 +62,31 @@ impl DynamicValidator {
         let block_id = hash.into();
         let state = client.state_at(block_id).expect("The current validators must be called on the confirmed block");
         let validators = CurrentValidators::load_from_state(&state).unwrap();
-        let mut validators: Vec<_> = validators.into();
-        validators.reverse();
-        validators
+        validators.into()
     }
 
     fn validators(&self, hash: BlockHash) -> Vec<Public> {
         let validators = self.next_validators(hash);
         validators.into_iter().map(|val| *val.pubkey()).collect()
     }
 
-    pub fn proposer_index(&self, parent: BlockHash, proposed_view: usize) -> usize {
-        let validators = self.next_validators(parent);
-        let num_validators = validators.len();
-        proposed_view % num_validators
+    fn validators_order_by_weight(&self, hash: BlockHash) -> WeightOrderedValidators {
+        let mut validators = self.next_validators(hash);
+        // Should we cache the sorted validator?
+        validators.sort_unstable_by_key(|v| {
+            (
+                Reverse(v.weight()),
+                Reverse(v.deposit()),
+                v.nominated_at_block_number(),
+                v.nominated_at_transaction_index(),
+            )
+        });
+        WeightOrderedValidators(validators.into_iter().map(|val| *val.pubkey()).collect())
+    }
+
+    pub fn proposer_index(&self, parent: BlockHash, proposed_view: u64) -> usize {
+        let propser = self.next_block_proposer(&parent, proposed_view);
+        self.get_index(&parent, &propser).expect("We know propser is included in a validator set")
     }
 
     pub fn get_current(&self, hash: &BlockHash, index: usize) -> Public {
@@ -112,17 +136,13 @@ impl ValidatorSet for DynamicValidator {
     }
 
     fn get_index(&self, parent: &BlockHash, public: &Public) -> Option<usize> {
-        self.validators(*parent)
-            .into_iter()
-            .enumerate()
-            .find(|(_index, pubkey)| pubkey == public)
-            .map(|(index, _)| index)
+        self.validators(*parent).binary_search(public).ok()
     }
 
     fn next_block_proposer(&self, parent: &BlockHash, view: u64) -> Public {
-        let validators = self.validators(*parent);
+        let validators = self.validators_order_by_weight(*parent);
         let n_validators = validators.len();
-        let index = view as usize % n_validators;
+        let index = WeightIndex(view as usize % n_validators);
         *validators.get(index).unwrap()
     }
 

core/src/miner/miner.rs

@@ -31,7 +31,7 @@ use ckey::{Ed25519Private as Private, Ed25519Public as Public, Password, Platfor
 use cstate::{FindDoubleVoteHandler, TopLevelState, TopStateView};
 use ctypes::errors::HistoryError;
 use ctypes::transaction::{IncompleteTransaction, Transaction};
-use ctypes::{BlockHash, BlockId, TxHash};
+use ctypes::{BlockHash, BlockId, TransactionIndex, TxHash};
 use kvdb::KeyValueDB;
 use parking_lot::{Mutex, RwLock};
 use primitives::Bytes;
@@ -342,8 +342,15 @@ impl Miner {
 
             let hash = tx.hash();
             let start = Instant::now();
+            let transaction_index = tx_count as TransactionIndex;
             // Check whether transaction type is allowed for sender
-            let result = open_block.push_transaction(tx, chain, parent_header.number(), parent_header.timestamp());
+            let result = open_block.push_transaction(
+                tx,
+                chain,
+                parent_header.number(),
+                parent_header.timestamp(),
+                transaction_index,
+            );
 
             match result {
                 // already have transaction - ignore
@@ -378,7 +385,8 @@ impl Miner {
             // It should use the block signer.
             let tx_signer = block_tx_signer.unwrap_or_else(Private::random);
             let mut seq = block_tx_seq.map(Ok).unwrap_or_else(|| open_block.state().seq(&tx_signer.public_key()))?;
-            for action in actions {
+            for (index, action) in actions.into_iter().enumerate() {
+                let transaction_index = (tx_count + index) as TransactionIndex;
                 let tx = Transaction {
                     network_id: chain.network_id(),
                     action,
@@ -389,7 +397,13 @@ impl Miner {
                 let tx = VerifiedTransaction::new_with_sign(tx, &tx_signer);
                 // TODO: The current code can insert more transactions than size limit.
                 // It should be fixed to pre-calculate the maximum size of the close transactions and prevent the size overflow.
-                open_block.push_transaction(tx, chain, parent_header.number(), parent_header.timestamp())?;
+                open_block.push_transaction(
+                    tx,
+                    chain,
+                    parent_header.number(),
+                    parent_header.timestamp(),
+                    transaction_index,
+                )?;
             }
         }
         let block = open_block.close()?;

core/src/transaction.rs

@@ -19,7 +19,7 @@ use ccrypto::blake256;
 use ckey::{sign, verify, Ed25519Private as Private, Ed25519Public as Public, Error as KeyError, Signature};
 use ctypes::errors::SyntaxError;
 use ctypes::transaction::Transaction;
-use ctypes::{BlockHash, BlockNumber, CommonParams, TxHash};
+use ctypes::{BlockHash, BlockNumber, CommonParams, TransactionIndex, TxHash};
 use rlp::{DecoderError, Encodable, Rlp, RlpStream};
 use std::convert::{TryFrom, TryInto};
 
@@ -222,7 +222,7 @@ pub struct LocalizedTransaction {
     /// Block hash.
     pub block_hash: BlockHash,
     /// Transaction index within block.
-    pub transaction_index: usize,
+    pub transaction_index: TransactionIndex,
     /// Cached public
     pub cached_signer_public: Option<Public>,
 }

core/src/types/transaction_id.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-use ctypes::{BlockId, TxHash};
+use ctypes::{BlockId, TransactionIndex, TxHash};
 
 /// Uniquely identifies transaction.
 #[derive(Debug, PartialEq, Clone, Hash, Eq)]
@@ -23,7 +23,7 @@ pub enum TransactionId {
     Hash(TxHash),
     /// Block id and transaction index within this block.
     /// Querying by block position is always faster.
-    Location(BlockId, usize),
+    Location(BlockId, TransactionIndex),
 }
 
 impl From<TxHash> for TransactionId {

core/src/views/block.rs

@@ -17,7 +17,7 @@
 use super::{HeaderView, TransactionView};
 use crate::transaction::{LocalizedTransaction, UnverifiedTransaction};
 use ccrypto::blake256;
-use ctypes::{BlockHash, Header, TxHash};
+use ctypes::{BlockHash, Header, TransactionIndex, TxHash};
 use rlp::Rlp;
 
 /// View onto block rlp.
@@ -82,7 +82,7 @@ impl<'a> BlockView<'a> {
                 signed,
                 block_hash,
                 block_number,
-                transaction_index,
+                transaction_index: transaction_index as TransactionIndex,
                 cached_signer_public: None,
             })
             .collect()
@@ -104,12 +104,12 @@ impl<'a> BlockView<'a> {
     }
 
     /// Returns transaction at given index without deserializing unnecessary data.
-    pub fn transaction_at(&self, index: usize) -> Option<UnverifiedTransaction> {
-        self.rlp.at(1).unwrap().iter().nth(index).map(|rlp| rlp.as_val().unwrap())
+    pub fn transaction_at(&self, index: TransactionIndex) -> Option<UnverifiedTransaction> {
+        self.rlp.at(1).unwrap().iter().nth(index as usize).map(|rlp| rlp.as_val().unwrap())
     }
 
     /// Returns localized transaction at given index.
-    pub fn localized_transaction_at(&self, transaction_index: usize) -> Option<LocalizedTransaction> {
+    pub fn localized_transaction_at(&self, transaction_index: TransactionIndex) -> Option<LocalizedTransaction> {
         let header = self.header_view();
         let block_hash = header.hash();
         let block_number = header.number();

core/src/views/body.rs

@@ -17,7 +17,7 @@
 use super::TransactionView;
 use crate::transaction::{LocalizedTransaction, UnverifiedTransaction};
 use ccrypto::blake256;
-use ctypes::{BlockHash, BlockNumber, TxHash};
+use ctypes::{BlockHash, BlockNumber, TransactionIndex, TxHash};
 use rlp::Rlp;
 
 /// View onto block rlp.
@@ -63,7 +63,7 @@ impl<'a> BodyView<'a> {
                 signed,
                 block_hash: *block_hash,
                 block_number,
-                transaction_index,
+                transaction_index: transaction_index as TransactionIndex,
                 cached_signer_public: None,
             })
             .collect()
@@ -85,16 +85,16 @@ impl<'a> BodyView<'a> {
     }
 
     /// Returns transaction at given index without deserializing unnecessary data.
-    pub fn transaction_at(&self, index: usize) -> Option<UnverifiedTransaction> {
-        self.rlp.at(0).unwrap().iter().nth(index).map(|rlp| rlp.as_val().unwrap())
+    pub fn transaction_at(&self, index: TransactionIndex) -> Option<UnverifiedTransaction> {
+        self.rlp.at(0).unwrap().iter().nth(index as usize).map(|rlp| rlp.as_val().unwrap())
     }
 
     /// Returns localized transaction at given index.
     pub fn localized_transaction_at(
         &self,
         block_hash: &BlockHash,
         block_number: BlockNumber,
-        transaction_index: usize,
+        transaction_index: TransactionIndex,
     ) -> Option<LocalizedTransaction> {
         self.transaction_at(transaction_index).map(|signed| LocalizedTransaction {
             signed,

rpc/src/v1/types/block.rs

@@ -17,7 +17,7 @@
 use super::Transaction;
 use ccore::{Block as CoreBlock, LocalizedTransaction};
 use ckey::{NetworkId, PlatformAddress};
-use ctypes::{BlockHash, BlockNumber};
+use ctypes::{BlockHash, BlockNumber, TransactionIndex};
 use primitives::H256;
 
 #[derive(Debug, Serialize)]
@@ -49,7 +49,7 @@ impl Block {
                 signed,
                 block_number,
                 block_hash,
-                transaction_index,
+                transaction_index: transaction_index as TransactionIndex,
                 cached_signer_public: None,
             });
         Block {