From 0b083bdaafee9f9327c1316b3d6cc43d097910db Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 13 May 2020 18:09:18 +0900 Subject: [PATCH 1/3] Add TransactionIndex type --- core/src/blockchain/body_db.rs | 4 ++-- core/src/blockchain/extras.rs | 4 ++-- core/src/transaction.rs | 4 ++-- core/src/types/transaction_id.rs | 4 ++-- core/src/views/block.rs | 10 +++++----- core/src/views/body.rs | 10 +++++----- rpc/src/v1/types/block.rs | 4 ++-- rpc/src/v1/types/transaction.rs | 4 ++-- types/src/lib.rs | 1 + 9 files changed, 23 insertions(+), 22 deletions(-) diff --git a/core/src/blockchain/body_db.rs b/core/src/blockchain/body_db.rs index e6af296a90..c28568cb30 100644 --- a/core/src/blockchain/body_db.rs +++ b/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, }) }) } diff --git a/core/src/blockchain/extras.rs b/core/src/blockchain/extras.rs index d78ae0052c..2a8507ad8d 100644 --- a/core/src/blockchain/extras.rs +++ b/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 for TransactionId { diff --git a/core/src/transaction.rs b/core/src/transaction.rs index 695dd1af3c..433b2c82ad 100644 --- a/core/src/transaction.rs +++ b/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, } diff --git a/core/src/types/transaction_id.rs b/core/src/types/transaction_id.rs index 12d9eb3b88..6b0f1b6ac8 100644 --- a/core/src/types/transaction_id.rs +++ b/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 . -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 for TransactionId { diff --git a/core/src/views/block.rs b/core/src/views/block.rs index 2a53f46107..2533202dbb 100644 --- a/core/src/views/block.rs +++ b/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 { - self.rlp.at(1).unwrap().iter().nth(index).map(|rlp| rlp.as_val().unwrap()) + pub fn transaction_at(&self, index: TransactionIndex) -> Option { + 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 { + pub fn localized_transaction_at(&self, transaction_index: TransactionIndex) -> Option { let header = self.header_view(); let block_hash = header.hash(); let block_number = header.number(); diff --git a/core/src/views/body.rs b/core/src/views/body.rs index 460b119ff1..939a8e01d4 100644 --- a/core/src/views/body.rs +++ b/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,8 +85,8 @@ impl<'a> BodyView<'a> { } /// Returns transaction at given index without deserializing unnecessary data. - pub fn transaction_at(&self, index: usize) -> Option { - self.rlp.at(0).unwrap().iter().nth(index).map(|rlp| rlp.as_val().unwrap()) + pub fn transaction_at(&self, index: TransactionIndex) -> Option { + self.rlp.at(0).unwrap().iter().nth(index as usize).map(|rlp| rlp.as_val().unwrap()) } /// Returns localized transaction at given index. @@ -94,7 +94,7 @@ impl<'a> BodyView<'a> { &self, block_hash: &BlockHash, block_number: BlockNumber, - transaction_index: usize, + transaction_index: TransactionIndex, ) -> Option { self.transaction_at(transaction_index).map(|signed| LocalizedTransaction { signed, diff --git a/rpc/src/v1/types/block.rs b/rpc/src/v1/types/block.rs index 3921723520..4b1993db2a 100644 --- a/rpc/src/v1/types/block.rs +++ b/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 { diff --git a/rpc/src/v1/types/transaction.rs b/rpc/src/v1/types/transaction.rs index 09a426bf36..877910d174 100644 --- a/rpc/src/v1/types/transaction.rs +++ b/rpc/src/v1/types/transaction.rs @@ -18,14 +18,14 @@ use super::Action; use ccore::{LocalizedTransaction, PendingVerifiedTransactions, VerifiedTransaction}; use cjson::uint::Uint; use ckey::{NetworkId, Signature}; -use ctypes::{BlockHash, TxHash}; +use ctypes::{BlockHash, TransactionIndex, TxHash}; #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct Transaction { pub block_number: Option, pub block_hash: Option, - pub transaction_index: Option, + pub transaction_index: Option, pub result: Option, pub seq: u64, pub fee: Uint, diff --git a/types/src/lib.rs b/types/src/lib.rs index d057ca53b0..fa25acb81e 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -33,6 +33,7 @@ pub mod transaction; pub mod util; pub type BlockNumber = u64; +pub type TransactionIndex = u32; pub type ShardId = u16; pub type StorageId = u16; From 6e7b934ee5ff39690373718e75b764dbf460169d Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 13 May 2020 18:58:02 +0900 Subject: [PATCH 2/3] Save nomination_starts_at in Validator and Candidate If two validators have the same delegation and deposit, the one who became the candidate first will be validator first. We didn't change the rule in this commit. Previously we kept the order of Candidates and Validators to remember who became the candidate first. Now we are saving nomination_starts_at variable to remember it. By saving the nomination_starts_at, we can sort validators using public keys. Sorting validators using public key makes validator set hash consistent. --- core/src/block.rs | 14 ++- core/src/client/test_client.rs | 2 +- core/src/miner/miner.rs | 22 +++- state/src/impls/top_level.rs | 28 +++-- state/src/item/stake.rs | 140 ++++++++++++++++++++++--- state/src/stake/actions.rs | 157 +++++++++++++++++++++++------ state/src/stake/mod.rs | 93 ++++++++++++++--- test/src/stakeholder/actionData.ts | 4 +- types/src/lib.rs | 6 ++ types/src/transaction/action.rs | 5 +- types/src/transaction/validator.rs | 15 ++- 11 files changed, 411 insertions(+), 75 deletions(-) diff --git a/core/src/block.rs b/core/src/block.rs index 585b23e67b..8e1a6b41ec 100644 --- a/core/src/block.rs +++ b/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(()) } diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 41004d9cb3..321c8be73e 100644 --- a/core/src/client/test_client.rs +++ b/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::>().into(); + pubkeys.into_iter().map(|pubkey| Validator::new(0, 0, pubkey, 0, 0)).collect::>().into(); self.validators = fixed_validators; } diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 15d8833bc0..88dda2a26a 100644 --- a/core/src/miner/miner.rs +++ b/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()?; diff --git a/state/src/impls/top_level.rs b/state/src/impls/top_level.rs index edc24ab529..80dfd864d3 100644 --- a/state/src/impls/top_level.rs +++ b/state/src/impls/top_level.rs @@ -51,7 +51,7 @@ use ckey::{Ed25519Public as Public, NetworkId}; use ctypes::errors::RuntimeError; use ctypes::transaction::{Action, ShardTransaction, Transaction}; use ctypes::util::unexpected::Mismatch; -use ctypes::{BlockNumber, CommonParams, ShardId, StorageId, TxHash}; +use ctypes::{BlockNumber, CommonParams, ShardId, StorageId, TransactionIndex, TransactionLocation, TxHash}; use kvdb::DBTransaction; use merkle_trie::{Result as TrieResult, TrieError, TrieFactory}; use primitives::{Bytes, H256}; @@ -285,6 +285,7 @@ impl TopLevelState { parent_block_number: BlockNumber, parent_block_timestamp: u64, current_block_timestamp: u64, + transaction_index: TransactionIndex, ) -> StateResult<()> { self.create_checkpoint(FEE_CHECKPOINT); let result = self.apply_internal( @@ -294,6 +295,7 @@ impl TopLevelState { parent_block_number, parent_block_timestamp, current_block_timestamp, + transaction_index, ); match result { Ok(()) => { @@ -314,6 +316,7 @@ impl TopLevelState { parent_block_number: BlockNumber, parent_block_timestamp: u64, current_block_timestamp: u64, + transaction_index: TransactionIndex, ) -> StateResult<()> { let seq = self.seq(sender)?; @@ -341,6 +344,7 @@ impl TopLevelState { parent_block_number, parent_block_timestamp, current_block_timestamp, + transaction_index, ); match &result { Ok(()) => { @@ -364,6 +368,7 @@ impl TopLevelState { parent_block_number: BlockNumber, parent_block_timestamp: u64, _current_block_timestamp: u64, + transaction_index: TransactionIndex, ) -> StateResult<()> { let (transaction, approvers) = match action { Action::ShardStore { @@ -410,7 +415,18 @@ impl TopLevelState { let nomination_ends_at = current_term + expiration; (current_term, nomination_ends_at) }; - return self_nominate(self, sender, *deposit, current_term, nomination_ends_at, metadata.clone()) + return self_nominate( + self, + sender, + *deposit, + current_term, + nomination_ends_at, + TransactionLocation { + block_number: parent_block_number + 1, + transaction_index, + }, + metadata.clone(), + ) } Action::ChangeParams { metadata_seq, @@ -1151,7 +1167,7 @@ mod tests_tx { expected: 0, found: 2 }))), - state.apply(&tx, &sender, &get_test_client(), 0, 0, 0) + state.apply(&tx, &sender, &get_test_client(), 0, 0, 0, 0) ); check_top_level_state!(state, [ @@ -1174,7 +1190,7 @@ mod tests_tx { cost: 5, } .into()), - state.apply(&tx, &sender, &get_test_client(), 0, 0, 0) + state.apply(&tx, &sender, &get_test_client(), 0, 0, 0, 0) ); check_top_level_state!(state, [ @@ -1191,7 +1207,7 @@ mod tests_tx { let receiver = 1u64.into(); let tx = transaction!(fee: 5, pay!(receiver, 10)); - assert_eq!(Ok(()), state.apply(&tx, &sender, &get_test_client(), 0, 0, 0)); + assert_eq!(Ok(()), state.apply(&tx, &sender, &get_test_client(), 0, 0, 0, 0)); check_top_level_state!(state, [ (account: sender => seq: 1, balance: 5), @@ -1217,7 +1233,7 @@ mod tests_tx { cost: 30, } .into()), - state.apply(&tx, &sender, &get_test_client(), 0, 0, 0) + state.apply(&tx, &sender, &get_test_client(), 0, 0, 0, 0) ); check_top_level_state!(state, [ diff --git a/state/src/item/stake.rs b/state/src/item/stake.rs index 90622e491d..5d539cb4c1 100644 --- a/state/src/item/stake.rs +++ b/state/src/item/stake.rs @@ -18,7 +18,7 @@ use crate::{ActionData, StakeKeyBuilder, StateResult, TopLevelState, TopState, T use ckey::Ed25519Public as Public; use ctypes::errors::RuntimeError; use ctypes::transaction::Validator; -use ctypes::{CompactValidatorEntry, CompactValidatorSet}; +use ctypes::{BlockNumber, CompactValidatorEntry, CompactValidatorSet, TransactionIndex, TransactionLocation}; use primitives::{Bytes, H256}; use rlp::{decode_list, encode_list, Decodable, Encodable, Rlp, RlpStream}; use std::cmp::Ordering; @@ -428,6 +428,8 @@ pub struct Candidate { pub pubkey: Public, pub deposit: DepositQuantity, pub nomination_ends_at: u64, + pub nomination_starts_at_block_number: BlockNumber, + pub nomination_starts_at_transaction_index: TransactionIndex, pub metadata: Bytes, } @@ -459,7 +461,13 @@ impl Candidates { let mut result = Vec::new(); for candidate in candidates.into_iter().filter(|c| c.deposit >= min_deposit) { if let Some(delegation) = delegations.get(&candidate.pubkey).cloned() { - result.push(Validator::new(delegation, candidate.deposit, candidate.pubkey)); + result.push(Validator::new( + delegation, + candidate.deposit, + candidate.pubkey, + candidate.nomination_starts_at_block_number, + candidate.nomination_starts_at_transaction_index, + )); } } // Candidates are sorted in low priority: low index, high priority: high index @@ -491,6 +499,7 @@ impl Candidates { pubkey: &Public, quantity: DepositQuantity, nomination_ends_at: u64, + nomination_starts_at: TransactionLocation, metadata: Bytes, ) { if let Some(index) = self.0.iter().position(|c| c.pubkey == *pubkey) { @@ -505,6 +514,8 @@ impl Candidates { pubkey: *pubkey, deposit: quantity, nomination_ends_at, + nomination_starts_at_block_number: nomination_starts_at.block_number, + nomination_starts_at_transaction_index: nomination_starts_at.transaction_index, metadata, }); }; @@ -1084,9 +1095,14 @@ mod tests { let pubkey = Public::random(); let deposits = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + let nomination_ends_at = 0; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; for deposit in deposits.iter() { let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, *deposit, 0, b"".to_vec()); + candidates.add_deposit(&pubkey, *deposit, nomination_ends_at, nomination_starts_at, b"".to_vec()); candidates.save_to_state(&mut state).unwrap(); } @@ -1105,7 +1121,13 @@ mod tests { let pubkey = Public::random(); let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, 10, 0, b"metadata".to_vec()); + let deposit = 10; + let nomination_ends_at = 0; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + candidates.add_deposit(&pubkey, deposit, nomination_ends_at, nomination_starts_at, b"metadata".to_vec()); candidates.save_to_state(&mut state).unwrap(); // Assert @@ -1123,11 +1145,16 @@ mod tests { let pubkey = Public::random(); let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, 10, 0, b"metadata".to_vec()); + let nomination_ends_at = 0; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + candidates.add_deposit(&pubkey, 10, nomination_ends_at, nomination_starts_at, b"metadata".to_vec()); candidates.save_to_state(&mut state).unwrap(); let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, 10, 0, b"metadata-updated".to_vec()); + candidates.add_deposit(&pubkey, 10, nomination_ends_at, nomination_starts_at, b"metadata-updated".to_vec()); candidates.save_to_state(&mut state).unwrap(); // Assert @@ -1144,7 +1171,12 @@ mod tests { // Prepare let pubkey = Public::random(); let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, 0, 10, b"".to_vec()); + let nomination_ends_at = 10; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + candidates.add_deposit(&pubkey, 0, nomination_ends_at, nomination_starts_at, b"".to_vec()); candidates.save_to_state(&mut state).unwrap(); // Assert @@ -1163,11 +1195,16 @@ mod tests { let pubkey = Public::random(); let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, 10, 0, b"metadata".to_vec()); + let nomination_ends_at = 0; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + candidates.add_deposit(&pubkey, 10, nomination_ends_at, nomination_starts_at, b"metadata".to_vec()); candidates.save_to_state(&mut state).unwrap(); let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, 0, 0, b"metadata-updated".to_vec()); + candidates.add_deposit(&pubkey, 0, nomination_ends_at, nomination_starts_at, b"metadata-updated".to_vec()); candidates.save_to_state(&mut state).unwrap(); // Assert @@ -1185,9 +1222,13 @@ mod tests { let pubkey = Public::random(); let deposit_and_nomination_ends_at = [(10, 11), (20, 22), (30, 33), (0, 44)]; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; for (deposit, nomination_ends_at) in &deposit_and_nomination_ends_at { let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, *deposit, *nomination_ends_at, b"".to_vec()); + candidates.add_deposit(&pubkey, *deposit, *nomination_ends_at, nomination_starts_at, b"".to_vec()); candidates.save_to_state(&mut state).unwrap(); } @@ -1218,24 +1259,32 @@ mod tests { pubkey: pubkey0, deposit: 20, nomination_ends_at: 11, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, Candidate { pubkey: pubkey1, deposit: 30, nomination_ends_at: 22, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, Candidate { pubkey: pubkey2, deposit: 40, nomination_ends_at: 33, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, Candidate { pubkey: pubkey3, deposit: 50, nomination_ends_at: 44, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, ]; @@ -1244,11 +1293,22 @@ mod tests { pubkey, deposit, nomination_ends_at, + nomination_starts_at_block_number, + nomination_starts_at_transaction_index, metadata, } in &candidates_prepared { let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, *deposit, *nomination_ends_at, metadata.clone()); + candidates.add_deposit( + &pubkey, + *deposit, + *nomination_ends_at, + TransactionLocation { + block_number: *nomination_starts_at_block_number, + transaction_index: *nomination_starts_at_transaction_index, + }, + metadata.clone(), + ); candidates.save_to_state(&mut state).unwrap(); } @@ -1283,24 +1343,32 @@ mod tests { pubkey: pubkey0, deposit: 20, nomination_ends_at: 11, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, Candidate { pubkey: pubkey1, deposit: 30, nomination_ends_at: 22, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, Candidate { pubkey: pubkey2, deposit: 40, nomination_ends_at: 33, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, Candidate { pubkey: pubkey3, deposit: 50, nomination_ends_at: 44, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, ]; @@ -1309,11 +1377,22 @@ mod tests { pubkey, deposit, nomination_ends_at, + nomination_starts_at_block_number, + nomination_starts_at_transaction_index, metadata, } in &candidates_prepared { let mut candidates = Candidates::load_from_state(&state).unwrap(); - candidates.add_deposit(&pubkey, *deposit, *nomination_ends_at, metadata.clone()); + candidates.add_deposit( + &pubkey, + *deposit, + *nomination_ends_at, + TransactionLocation { + block_number: *nomination_starts_at_block_number, + transaction_index: *nomination_starts_at_transaction_index, + }, + metadata.clone(), + ); candidates.save_to_state(&mut state).unwrap(); } @@ -1344,6 +1423,8 @@ mod tests { pubkey, deposit: 100, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 10, @@ -1370,6 +1451,8 @@ mod tests { pubkey, deposit: 100, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 10, @@ -1396,6 +1479,8 @@ mod tests { pubkey, deposit: 100, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 10, @@ -1438,6 +1523,8 @@ mod tests { pubkey: pubkey1, deposit: 100, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 10, @@ -1448,6 +1535,8 @@ mod tests { pubkey: pubkey2, deposit: 200, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 15, @@ -1482,6 +1571,8 @@ mod tests { pubkey: pubkey1, deposit: 100, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 10, @@ -1492,6 +1583,8 @@ mod tests { pubkey: pubkey2, deposit: 200, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 15, @@ -1535,6 +1628,8 @@ mod tests { pubkey: pubkey1, deposit: 100, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 10, @@ -1545,6 +1640,8 @@ mod tests { pubkey: pubkey2, deposit: 200, nomination_ends_at: 0, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }, 15, @@ -1617,15 +1714,20 @@ mod tests { let pubkeys = (0..10).map(|_| Public::random()).collect::>(); let mut candidates = Candidates::load_from_state(&state).unwrap(); + let nomination_ends_at = 0; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; for _ in 0..10 { // Random pre-fill let i = rand::thread_rng().gen_range(0, pubkeys.len()); let pubkey = &pubkeys[i]; - candidates.add_deposit(pubkey, 0, 0, Bytes::new()); + candidates.add_deposit(pubkey, 0, nomination_ends_at, nomination_starts_at, Bytes::new()); } // Inserting pubkey in this order, they'll get sorted. for pubkey in &pubkeys { - candidates.add_deposit(pubkey, 10, 0, Bytes::new()); + candidates.add_deposit(pubkey, 10, nomination_ends_at, nomination_starts_at, Bytes::new()); } candidates.save_to_state(&mut state).unwrap(); @@ -1643,12 +1745,18 @@ mod tests { let pubkeys = (0..10).map(|_| Public::random()).collect::>(); let mut candidates = Candidates::load_from_state(&state).unwrap(); + let nomination_ends_at = 0; + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; for pubkey in &pubkeys { - candidates.add_deposit(pubkey, 10, 0, Bytes::new()); + candidates.add_deposit(pubkey, 10, nomination_ends_at, nomination_starts_at, Bytes::new()); } candidates.save_to_state(&mut state).unwrap(); - let dummy_validators = pubkeys[0..5].iter().map(|pubkey| Validator::new(0, 0, *pubkey)).collect::>(); + let dummy_validators = + pubkeys[0..5].iter().map(|pubkey| Validator::new(0, 0, *pubkey, 0, 0)).collect::>(); let dummy_banned = Banned::load_from_state(&state).unwrap(); candidates.renew_candidates(&dummy_validators, 0, &[], &dummy_banned); diff --git a/state/src/stake/actions.rs b/state/src/stake/actions.rs index 2b78bb191e..e043b00e8c 100644 --- a/state/src/stake/actions.rs +++ b/state/src/stake/actions.rs @@ -20,7 +20,7 @@ use ckey::Ed25519Public as Public; use ctypes::errors::RuntimeError; use ctypes::transaction::{Approval, Validator}; use ctypes::util::unexpected::Mismatch; -use ctypes::{CommonParams, Deposit}; +use ctypes::{CommonParams, Deposit, TransactionIndex, TransactionLocation}; use primitives::Bytes; use std::collections::HashMap; @@ -81,11 +81,19 @@ pub fn init_stake( let mut values: Vec<_> = genesis_candidates.values().collect(); values.sort_unstable(); // The insertion order of candidates is important. - for candidate in values { + for (index, candidate) in values.iter().enumerate() { + let block_number = 0; + // `index` is not a real transaction index. + // Since Candidate struct requires transaction index to order candidates, let's add a fake one. + let transaction_index = index as TransactionIndex; candidates.add_deposit( &candidate.pubkey, candidate.deposit, candidate.nomination_ends_at, + TransactionLocation { + block_number, + transaction_index, + }, candidate.metadata.clone(), ); } @@ -211,6 +219,7 @@ pub fn self_nominate( deposit: u64, current_term: u64, nomination_ends_at: u64, + nomination_starts_at: TransactionLocation, metadata: Bytes, ) -> StateResult<()> { let blacklist = Banned::load_from_state(state)?; @@ -232,7 +241,7 @@ pub fn self_nominate( let mut candidates = Candidates::load_from_state(&state)?; state.sub_balance(nominee, deposit)?; - candidates.add_deposit(nominee, total_deposit, nomination_ends_at, metadata); + candidates.add_deposit(nominee, total_deposit, nomination_ends_at, nomination_starts_at, metadata); jail.save_to_state(state)?; candidates.save_to_state(state)?; @@ -527,7 +536,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap(); @@ -564,7 +577,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 100; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap(); @@ -620,7 +637,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 200; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap_err(); @@ -639,7 +660,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap(); @@ -661,7 +686,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap(); @@ -683,7 +712,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap(); @@ -711,7 +744,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap(); @@ -739,7 +776,11 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &delegatee, quantity).unwrap(); @@ -765,8 +806,12 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &prev_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); - self_nominate(&mut state, &next_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &prev_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); + self_nominate(&mut state, &next_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &prev_delegatee, quantity).unwrap(); @@ -795,8 +840,12 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &prev_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); - self_nominate(&mut state, &next_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &prev_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); + self_nominate(&mut state, &next_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &prev_delegatee, quantity).unwrap(); @@ -825,8 +874,12 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &prev_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); - self_nominate(&mut state, &next_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &prev_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); + self_nominate(&mut state, &next_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 50; delegate_ccs(&mut state, &delegator, &prev_delegatee, quantity).unwrap(); @@ -856,7 +909,11 @@ mod tests { }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &prev_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &prev_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &prev_delegatee, quantity).unwrap(); @@ -881,8 +938,12 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &prev_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); - self_nominate(&mut state, &criminal, 100, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &prev_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); + self_nominate(&mut state, &criminal, 100, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &criminal, quantity).unwrap(); @@ -919,10 +980,14 @@ mod tests { genesis_stakes }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); - self_nominate(&mut state, &prev_delegatee, 0, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &prev_delegatee, 0, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let deposit = 200; - self_nominate(&mut state, &jail_pubkey, deposit, 0, 5, b"".to_vec()).unwrap(); + self_nominate(&mut state, &jail_pubkey, deposit, 0, 5, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &prev_delegatee, quantity).unwrap(); @@ -951,8 +1016,12 @@ mod tests { init_stake(&mut state, Default::default(), Default::default(), Default::default()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; // TODO: change with stake::execute() - self_nominate(&mut state, &pubkey, 0, 0, 5, b"metadata1".to_vec()).unwrap(); + self_nominate(&mut state, &pubkey, 0, 0, 5, nomination_starts_at, b"metadata1".to_vec()).unwrap(); assert_eq!(state.balance(&pubkey).unwrap(), 1000); let candidates = Candidates::load_from_state(&state).unwrap(); @@ -962,12 +1031,18 @@ mod tests { pubkey, deposit: 0, nomination_ends_at: 5, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"metadata1".to_vec(), }), "nomination_ends_at should be updated even if candidate deposits 0" ); - self_nominate(&mut state, &pubkey, 200, 0, 10, b"metadata2".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, 200, 0, 10, nomination_starts_at, b"metadata2".to_vec()).unwrap(); assert_eq!(state.balance(&pubkey).unwrap(), 800); let candidates = Candidates::load_from_state(&state).unwrap(); @@ -977,11 +1052,17 @@ mod tests { pubkey, deposit: 200, nomination_ends_at: 10, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"metadata2".to_vec(), }) ); - self_nominate(&mut state, &pubkey, 0, 0, 15, b"metadata3".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, 0, 0, 15, nomination_starts_at, b"metadata3".to_vec()).unwrap(); assert_eq!(state.balance(&pubkey).unwrap(), 800); let candidates = Candidates::load_from_state(&state).unwrap(); @@ -991,6 +1072,8 @@ mod tests { pubkey, deposit: 200, nomination_ends_at: 15, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"metadata3".to_vec(), }), "nomination_ends_at should be updated even if candidate deposits 0" @@ -1006,8 +1089,12 @@ mod tests { init_stake(&mut state, Default::default(), Default::default(), Default::default()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; // TODO: change with stake::execute() - let result = self_nominate(&mut state, &pubkey, 2000, 0, 5, b"".to_vec()); + let result = self_nominate(&mut state, &pubkey, 2000, 0, 5, nomination_starts_at, b"".to_vec()); assert!(result.is_err(), "Cannot self-nominate without a sufficient balance"); } @@ -1022,7 +1109,11 @@ mod tests { // TODO: change with stake::execute() let deposit = 200; - self_nominate(&mut state, &pubkey, deposit, 0, 5, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, deposit, 0, 5, nomination_starts_at, b"".to_vec()).unwrap(); let custody_until = 10; let released_at = 20; @@ -1064,7 +1155,11 @@ mod tests { init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); let deposit = 100; - self_nominate(&mut state, &criminal, deposit, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &criminal, deposit, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &criminal, quantity).unwrap(); @@ -1096,7 +1191,11 @@ mod tests { assert_eq!(Ok(()), state.add_balance(&criminal, 100)); let deposit = 10; - self_nominate(&mut state, &criminal, deposit, 0, 10, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &criminal, deposit, 0, 10, nomination_starts_at, b"".to_vec()).unwrap(); let custody_until = 10; let released_at = 20; jail(&mut state, &[criminal], custody_until, released_at).unwrap(); diff --git a/state/src/stake/mod.rs b/state/src/stake/mod.rs index bdce4ce123..7060dd8532 100644 --- a/state/src/stake/mod.rs +++ b/state/src/stake/mod.rs @@ -90,7 +90,7 @@ mod tests { use crate::tests::helpers; use crate::{NextValidators, TopLevelState, TopState, TopStateView}; use ckey::Ed25519Public as Public; - use ctypes::CommonParams; + use ctypes::{CommonParams, TransactionLocation}; use std::collections::HashMap; #[test] @@ -130,8 +130,12 @@ mod tests { init_stake(&mut state, Default::default(), Default::default(), Default::default()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; // TODO: change with stake::execute() - self_nominate(&mut state, &pubkey, 200, 0, 30, b"".to_vec()).unwrap(); + self_nominate(&mut state, &pubkey, 200, 0, 30, nomination_starts_at, b"".to_vec()).unwrap(); let next_validators = Vec::from(NextValidators::load_from_state(&state).unwrap()); close_term(&mut state, &next_validators, &[]).unwrap(); @@ -150,6 +154,8 @@ mod tests { pubkey, deposit: 200, nomination_ends_at: 30, + nomination_starts_at_block_number: 0, + nomination_starts_at_transaction_index: 0, metadata: b"".to_vec(), }), "Keep deposit before expiration", @@ -185,8 +191,12 @@ mod tests { }; init_stake(&mut state, genesis_stakes, Default::default(), Default::default()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; // TODO: change with stake::execute() - self_nominate(&mut state, &pubkey, 0, 0, 30, b"".to_vec()).unwrap(); + self_nominate(&mut state, &pubkey, 0, 0, 30, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &pubkey, quantity).unwrap(); @@ -234,12 +244,27 @@ mod tests { let nominate_expire = 5; let custody_until = 10; let released_at = 20; - self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, nomination_starts_at, b"".to_vec()).unwrap(); jail(&mut state, &[pubkey], custody_until, released_at).unwrap(); for current_term in 0..=custody_until { - let result = - self_nominate(&mut state, &pubkey, 0, current_term, current_term + nominate_expire, b"".to_vec()); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + let result = self_nominate( + &mut state, + &pubkey, + 0, + current_term, + current_term + nominate_expire, + nomination_starts_at, + b"".to_vec(), + ); assert!( result.is_err(), "Shouldn't nominate while current_term({}) <= custody_until({})", @@ -271,7 +296,20 @@ mod tests { let nominate_expire = 5; let custody_until = 10; let released_at = 20; - self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, b"metadata-before".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate( + &mut state, + &pubkey, + deposit, + 0, + nominate_expire, + nomination_starts_at, + b"metadata-before".to_vec(), + ) + .unwrap(); jail(&mut state, &[pubkey], custody_until, released_at).unwrap(); for current_term in 0..=custody_until { let next_validators = Vec::from(NextValidators::load_from_state(&state).unwrap()); @@ -292,6 +330,7 @@ mod tests { additional_deposit, current_term, current_term + nominate_expire, + nomination_starts_at, b"metadata-after".to_vec(), ); assert!(result.is_ok()); @@ -303,6 +342,8 @@ mod tests { deposit: deposit + additional_deposit, nomination_ends_at: current_term + nominate_expire, pubkey, + nomination_starts_at_block_number: nomination_starts_at.block_number, + nomination_starts_at_transaction_index: nomination_starts_at.transaction_index, metadata: "metadata-after".into() }), "The prisoner is become a candidate", @@ -328,7 +369,11 @@ mod tests { let nominate_expire = 5; let custody_until = 10; let released_at = 20; - self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, nomination_starts_at, b"".to_vec()).unwrap(); jail(&mut state, &[pubkey], custody_until, released_at).unwrap(); for current_term in 0..released_at { @@ -386,7 +431,11 @@ mod tests { let nominate_expire = 5; let custody_until = 10; let released_at = 20; - self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, nomination_starts_at, b"".to_vec()).unwrap(); jail(&mut state, &[pubkey], custody_until, released_at).unwrap(); for current_term in 0..=released_at { @@ -427,7 +476,11 @@ mod tests { let nominate_expire = 5; let custody_until = 10; let released_at = 20; - self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, deposit, 0, nominate_expire, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &pubkey, quantity).unwrap(); @@ -471,7 +524,11 @@ mod tests { let nominate_expire = 5; let custody_until = 10; let released_at = 20; - self_nominate(&mut state, &pubkey, 0, 0, nominate_expire, b"".to_vec()).unwrap(); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + self_nominate(&mut state, &pubkey, 0, 0, nominate_expire, nomination_starts_at, b"".to_vec()).unwrap(); let quantity = 40; delegate_ccs(&mut state, &delegator, &pubkey, quantity).unwrap(); @@ -490,7 +547,19 @@ mod tests { } let current_term = custody_until + 1; - let result = self_nominate(&mut state, &pubkey, 0, current_term, current_term + nominate_expire, b"".to_vec()); + let nomination_starts_at = TransactionLocation { + block_number: 0, + transaction_index: 0, + }; + let result = self_nominate( + &mut state, + &pubkey, + 0, + current_term, + current_term + nominate_expire, + nomination_starts_at, + b"".to_vec(), + ); assert!(result.is_ok()); let delegation = Delegation::load_from_state(&state, &delegator).unwrap(); diff --git a/test/src/stakeholder/actionData.ts b/test/src/stakeholder/actionData.ts index c381553a1f..5745580807 100644 --- a/test/src/stakeholder/actionData.ts +++ b/test/src/stakeholder/actionData.ts @@ -115,7 +115,7 @@ export async function getCandidates( } const decoded = RLP.decode(Buffer.from(data, "hex")); function isCandidateShape(entry: any): entry is Buffer[] { - return entry != null && Array.isArray(entry) && entry.length === 4; + return entry != null && Array.isArray(entry) && entry.length === 6; } if (!isArrayOf(decoded, isCandidateShape)) { throw new Error( @@ -263,7 +263,7 @@ export async function getValidators( } const decoded = RLP.decode(Buffer.from(data, "hex")); function isValidatorShape(entry: any): entry is Buffer[] { - return entry != null && Array.isArray(entry) && entry.length === 4; + return entry != null && Array.isArray(entry) && entry.length === 6; } if (!isArrayOf(decoded, isValidatorShape)) { throw new Error( diff --git a/types/src/lib.rs b/types/src/lib.rs index fa25acb81e..3953176cf0 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -37,6 +37,12 @@ pub type TransactionIndex = u32; pub type ShardId = u16; pub type StorageId = u16; +#[derive(Copy, Clone)] +pub struct TransactionLocation { + pub block_number: BlockNumber, + pub transaction_index: TransactionIndex, +} + pub use block_hash::BlockHash; pub use block_id::BlockId; pub use common_params::CommonParams; diff --git a/types/src/transaction/action.rs b/types/src/transaction/action.rs index b805ed3b31..3213ca106e 100644 --- a/types/src/transaction/action.rs +++ b/types/src/transaction/action.rs @@ -574,7 +574,10 @@ mod tests { #[test] fn rlp_of_update_validators() { rlp_encode_and_decode_test!(Action::UpdateValidators { - validators: vec![Validator::new(1, 2, Public::random()), Validator::new(3, 4, Public::random())], + validators: vec![ + Validator::new(1, 2, Public::random(), 0, 0), + Validator::new(3, 4, Public::random(), 0, 0) + ], }); } diff --git a/types/src/transaction/validator.rs b/types/src/transaction/validator.rs index e2752481e4..fd7f5c915f 100644 --- a/types/src/transaction/validator.rs +++ b/types/src/transaction/validator.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +use crate::{BlockNumber, TransactionIndex}; use ckey::Ed25519Public as Public; pub type StakeQuantity = u64; @@ -25,15 +26,25 @@ pub struct Validator { delegation: StakeQuantity, deposit: DepositQuantity, pubkey: Public, + nominated_at_block_number: BlockNumber, + nominated_at_transaction_index: TransactionIndex, } impl Validator { - pub fn new(delegation: StakeQuantity, deposit: DepositQuantity, pubkey: Public) -> Self { + pub fn new( + delegation: StakeQuantity, + deposit: DepositQuantity, + pubkey: Public, + nominated_at_block_number: BlockNumber, + nominated_at_transaction_index: TransactionIndex, + ) -> Self { Self { weight: delegation, delegation, deposit, pubkey, + nominated_at_block_number, + nominated_at_transaction_index, } } @@ -75,6 +86,8 @@ mod tests { delegation: 2, deposit: 3, pubkey: Public::random(), + nominated_at_block_number: 3, + nominated_at_transaction_index: 2 }); } } From ccf8b4bac5ff520f9e2ab63d3c27b793c5c22b62 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Thu, 14 May 2020 11:35:28 +0900 Subject: [PATCH 3/3] Save validator set in public key order By sorting the validator set using public keys, we can make validator set hash consistent. --- core/src/consensus/tendermint/worker.rs | 5 +- .../validator_set/dynamic_validator.rs | 54 +++++++++++++------ state/src/item/stake.rs | 53 ++++++++++++------ test/src/e2e.long/tendermint.test.ts | 2 +- types/src/transaction/validator.rs | 10 +++- 5 files changed, 85 insertions(+), 39 deletions(-) diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index badea501c3..b75f2cdc6d 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/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 { - 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 { 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)) } diff --git a/core/src/consensus/validator_set/dynamic_validator.rs b/core/src/consensus/validator_set/dynamic_validator.rs index eb236be151..eecd92916a 100644 --- a/core/src/consensus/validator_set/dynamic_validator.rs +++ b/core/src/consensus/validator_set/dynamic_validator.rs @@ -24,6 +24,7 @@ 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)] @@ -31,6 +32,20 @@ pub struct DynamicValidator { client: RwLock>>, } +pub struct WeightOrderedValidators(Vec); + +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 { let client: Arc = @@ -38,9 +53,7 @@ impl DynamicValidator { 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 { @@ -49,9 +62,7 @@ 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 { @@ -59,10 +70,23 @@ impl DynamicValidator { 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 { - 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() } diff --git a/state/src/item/stake.rs b/state/src/item/stake.rs index 5d539cb4c1..a607e2068a 100644 --- a/state/src/item/stake.rs +++ b/state/src/item/stake.rs @@ -21,7 +21,7 @@ use ctypes::transaction::Validator; use ctypes::{BlockNumber, CompactValidatorEntry, CompactValidatorSet, TransactionIndex, TransactionLocation}; use primitives::{Bytes, H256}; use rlp::{decode_list, encode_list, Decodable, Encodable, Rlp, RlpStream}; -use std::cmp::Ordering; +use std::cmp::{Ordering, Reverse}; use std::collections::btree_map::{BTreeMap, Entry}; use std::collections::btree_set::{self, BTreeSet}; use std::collections::{btree_map, HashMap, HashSet}; @@ -235,10 +235,8 @@ impl NextValidators { } pub fn create_compact_validator_set(&self) -> CompactValidatorSet { - let mut reversed = self.0.clone(); - reversed.reverse(); CompactValidatorSet::new( - reversed + self.0 .iter() .map(|x| CompactValidatorEntry { public_key: *x.pubkey(), @@ -263,9 +261,8 @@ impl NextValidators { let delegatees = Stakeholders::delegatees(&state)?; // Step 1 & 2. + // Ordered by (delegation DESC, deposit DESC, nomination_starts_at ASC) let mut validators = Candidates::prepare_validators(&state, min_deposit, &delegatees)?; - // validators are now sorted in descending order of (delegation, deposit, priority) - validators.reverse(); let banned = Banned::load_from_state(&state)?; for validator in &validators { @@ -289,7 +286,8 @@ impl NextValidators { let over_threshold = rest.iter().filter(|c| c.delegation() >= delegation_threshold); let mut result: Vec<_> = minimum.iter().chain(over_threshold).cloned().collect(); - result.reverse(); // Ascending order of (delegation, deposit, priority) + result.sort_unstable_by_key(|v| *v.pubkey()); + Ok(Self(result)) } @@ -306,7 +304,16 @@ impl NextValidators { pub fn update_weight(state: &TopLevelState, block_author: &Public) -> StateResult { let mut validators = Self::load_from_state(state)?; let min_delegation = validators.min_delegation(); - for validator in validators.0.iter_mut().rev() { + let mut sorted_validators_view: Vec<&mut Validator> = validators.0.iter_mut().collect(); + sorted_validators_view.sort_unstable_by_key(|val| { + ( + Reverse(val.weight()), + Reverse(val.deposit()), + val.nominated_at_block_number(), + val.nominated_at_transaction_index(), + ) + }); + for validator in sorted_validators_view.iter_mut() { if *validator.pubkey() == *block_author { // block author validator.set_weight(validator.weight().saturating_sub(min_delegation)); @@ -318,7 +325,6 @@ impl NextValidators { if validators.0.iter().all(|validator| validator.weight() == 0) { validators.0.iter_mut().for_each(Validator::reset); } - validators.0.sort_unstable(); Ok(validators) } @@ -369,7 +375,7 @@ pub struct CurrentValidators(Vec); impl CurrentValidators { pub fn load_from_state(state: &TopLevelState) -> StateResult { let key = &*CURRENT_VALIDATORS_KEY; - let validators = state.action_data(&key)?.map(|data| decode_list(&data)).unwrap_or_default(); + let validators: Vec = state.action_data(&key)?.map(|data| decode_list(&data)).unwrap_or_default(); Ok(Self(validators)) } @@ -384,7 +390,17 @@ impl CurrentValidators { Ok(()) } + /// validator should be sorted by public key pub fn update(&mut self, validators: Vec) { + debug_assert_eq!( + validators, + { + let mut cloned = validators.clone(); + cloned.sort_unstable_by_key(|v| *v.pubkey()); + cloned + }, + "CurrentValidator is always sorted by public key" + ); self.0 = validators; } @@ -393,10 +409,8 @@ impl CurrentValidators { } pub fn create_compact_validator_set(&self) -> CompactValidatorSet { - let mut reversed = self.0.clone(); - reversed.reverse(); CompactValidatorSet::new( - reversed + self.0 .iter() .map(|x| CompactValidatorEntry { public_key: *x.pubkey(), @@ -470,10 +484,15 @@ impl Candidates { )); } } - // Candidates are sorted in low priority: low index, high priority: high index - // so stable sorting with the key (delegation, deposit) preserves its priority order. - // ascending order of (delegation, deposit, priority) - result.sort_by_key(|v| (v.delegation(), v.deposit())); + // Descending order of (delegation, deposit, priority) + result.sort_unstable_by_key(|v| { + ( + Reverse(v.delegation()), + Reverse(v.deposit()), + v.nominated_at_block_number(), + v.nominated_at_transaction_index(), + ) + }); Ok(result) } diff --git a/test/src/e2e.long/tendermint.test.ts b/test/src/e2e.long/tendermint.test.ts index fc25cb53c1..aa8eb1ad01 100644 --- a/test/src/e2e.long/tendermint.test.ts +++ b/test/src/e2e.long/tendermint.test.ts @@ -63,8 +63,8 @@ describe("Tendermint ", function() { describe("getPossibleAuthors", function() { it("latest", async function() { const validators = [ - "rjmxg19kCmkCxROEoV0QYsrDpOYsjQwusCtN5_oKMEzk-I6kgtAtc0", "szff1322BHP3gsOuwFPDf-K8zvqSmNz4rj3CJirlQKFKWA_3c-Ytc0", + "rjmxg19kCmkCxROEoV0QYsrDpOYsjQwusCtN5_oKMEzk-I6kgtAtc0", "qwfj0xwkJQLV5iEGeaGeRfPA-TJX56Mnuq9fQD9coasmhanhck4tc0", "dbqtds3w6QnzEf0RXuQS7c_N6IzFBzcBAfdjWme5y0U5DxzLS14tc0" ]; diff --git a/types/src/transaction/validator.rs b/types/src/transaction/validator.rs index fd7f5c915f..7b8671cae8 100644 --- a/types/src/transaction/validator.rs +++ b/types/src/transaction/validator.rs @@ -20,7 +20,7 @@ use ckey::Ed25519Public as Public; pub type StakeQuantity = u64; pub type DepositQuantity = u64; -#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, RlpDecodable, RlpEncodable)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, RlpDecodable, RlpEncodable)] pub struct Validator { weight: StakeQuantity, delegation: StakeQuantity, @@ -71,6 +71,14 @@ impl Validator { pub fn deposit(&self) -> DepositQuantity { self.deposit } + + pub fn nominated_at_block_number(&self) -> BlockNumber { + self.nominated_at_block_number + } + + pub fn nominated_at_transaction_index(&self) -> TransactionIndex { + self.nominated_at_transaction_index + } } #[cfg(test)]