From 8b4bff73c13c7982c98c780a368b829416b25d89 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Thu, 19 Dec 2019 18:24:18 +0900 Subject: [PATCH 1/4] Remove should_wait_timelock in mem_pool --- core/src/miner/mem_pool.rs | 98 +++++--------------------------------- 1 file changed, 11 insertions(+), 87 deletions(-) diff --git a/core/src/miner/mem_pool.rs b/core/src/miner/mem_pool.rs index 86e736a859..1c5c5ec015 100644 --- a/core/src/miner/mem_pool.rs +++ b/core/src/miner/mem_pool.rs @@ -28,7 +28,7 @@ use table::Table; use super::backup; use super::mem_pool_types::{ AccountDetails, CurrentQueue, FutureQueue, MemPoolFees, MemPoolInput, MemPoolItem, MemPoolStatus, PoolingInstant, - QueueTag, TransactionOrder, TransactionOrderWithTag, TxOrigin, TxTimelock, + QueueTag, TransactionOrder, TransactionOrderWithTag, TxOrigin, }; use super::TransactionImportResult; use crate::client::{AccountData, BlockChainTrait}; @@ -305,22 +305,16 @@ impl MemPool { let mut first_seq = *self.first_seqs.get(&public).unwrap_or(&0); let next_seq = self.next_seqs.get(&public).cloned().unwrap_or(current_seq); - let new_next_seq = if current_seq < first_seq + let target_seq = if current_seq < first_seq || inserted_block_number < self.last_block_number || inserted_timestamp < self.last_timestamp || next_seq < current_seq { - self.check_transactions(public, current_seq, inserted_block_number, inserted_timestamp) + current_seq } else { - to_insert - .get(&public) - .and_then(|v| { - self.check_new_transactions(public, v, next_seq, inserted_block_number, inserted_timestamp) - }) - .unwrap_or_else(|| { - self.check_transactions(public, next_seq, inserted_block_number, inserted_timestamp) - }) + next_seq }; + let new_next_seq = self.check_transactions(public, target_seq); let is_this_account_local = new_local_accounts.contains(&public); // Need to update transactions because of height/origin change @@ -463,15 +457,7 @@ impl MemPool { for public in keys { let current_seq = fetch_account(&public).seq; - - let next_seq = to_insert - .get(&public) - .and_then(|v| { - self.check_new_transactions(public, v, current_seq, recover_block_number, recover_timestamp) - }) - .unwrap_or_else(|| { - self.check_transactions(public, current_seq, recover_block_number, recover_timestamp) - }); + let next_seq = self.check_transactions(public, current_seq); self.first_seqs.insert(public, current_seq); if next_seq > current_seq { @@ -554,11 +540,11 @@ impl MemPool { || current_timestamp < self.last_timestamp || next_seq < current_seq { - self.check_transactions(public, current_seq, current_block_number, current_timestamp) + self.check_transactions(public, current_seq) } else if let Some(seq) = removed.get(&public) { *seq } else { - self.check_transactions(public, next_seq, current_block_number, current_timestamp) + self.check_transactions(public, next_seq) }; // Need to update the height @@ -598,61 +584,13 @@ impl MemPool { /// Checks the timelock of transactions starting from `start_seq`. /// Returns the next seq of the last transaction which can be in the current queue - fn check_transactions( - &self, - public: Public, - mut start_seq: u64, - current_block_number: PoolingInstant, - current_timestamp: u64, - ) -> u64 { - let row = self - .by_signer_public - .row(&public) - .expect("This function should be called after checking from `self.by_signer_public.keys()`"); - - while let Some(order_with_tag) = row.get(&start_seq) { - let order = order_with_tag.order; - if Self::should_wait_timelock(&order.timelock, current_block_number, current_timestamp) { - break - } - start_seq += 1; - } - - start_seq - } - - /// Checks the timelock of transactions with the given seqs. - /// If there are transactions which should wait timelock, returns the smallest seq by Some(seq). - /// If there's no transaction which should wait timelock, returns None. - fn check_new_transactions( - &self, - public: Public, - seqs: &[u64], - next_seq: u64, - current_block_number: PoolingInstant, - current_timestamp: u64, - ) -> Option { + fn check_transactions(&self, public: Public, start_seq: u64) -> u64 { let row = self .by_signer_public .row(&public) .expect("This function should be called after checking from `self.by_signer_public.keys()`"); - let mut result = None; - - for seq in seqs { - if *seq >= next_seq { - continue - } - let order_with_tag = row.get(&seq).expect("Must exist"); - let order = order_with_tag.order; - if Self::should_wait_timelock(&order.timelock, current_block_number, current_timestamp) - && (result.is_none() || (result.is_some() && result.unwrap() > *seq)) - { - result = Some(*seq) - } - } - - result + (start_seq..).find(|s| row.get(s).is_none()).expect("Open ended range does not end") } /// Moves the transactions which of seq is in [start_seq, end_seq -1], @@ -950,21 +888,6 @@ impl MemPool { pub fn is_local_transaction(&self, tx_hash: TxHash) -> Option { self.by_hash.get(&tx_hash).map(|found_item| found_item.origin.is_local()) } - - /// Checks the given timelock with the current time/timestamp. - fn should_wait_timelock(timelock: &TxTimelock, best_block_number: BlockNumber, best_block_timestamp: u64) -> bool { - if let Some(block_number) = timelock.block { - if block_number > best_block_number { - return true - } - } - if let Some(timestamp) = timelock.timestamp { - if timestamp > best_block_timestamp { - return true - } - } - false - } } @@ -973,6 +896,7 @@ pub mod test { use std::cmp::Ordering; use crate::client::{AccountData, TestBlockChainClient}; + use crate::miner::mem_pool_types::TxTimelock; use ckey::{Generator, KeyPair, Random}; use ctypes::transaction::{Action, AssetMintOutput, Transaction}; use primitives::H160; From 4c75891947b44da5b68ad2f7d5c730026248e423 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Thu, 19 Dec 2019 18:48:54 +0900 Subject: [PATCH 2/4] Remove calculate_timelock in mem_pool --- core/src/miner/mem_pool.rs | 53 ++++++++++++------------------ core/src/miner/mem_pool_types.rs | 4 +-- core/src/miner/miner.rs | 55 ++------------------------------ 3 files changed, 24 insertions(+), 88 deletions(-) diff --git a/core/src/miner/mem_pool.rs b/core/src/miner/mem_pool.rs index 1c5c5ec015..d0ba0f6ad2 100644 --- a/core/src/miner/mem_pool.rs +++ b/core/src/miner/mem_pool.rs @@ -28,7 +28,7 @@ use table::Table; use super::backup; use super::mem_pool_types::{ AccountDetails, CurrentQueue, FutureQueue, MemPoolFees, MemPoolInput, MemPoolItem, MemPoolStatus, PoolingInstant, - QueueTag, TransactionOrder, TransactionOrderWithTag, TxOrigin, + QueueTag, TransactionOrder, TransactionOrderWithTag, TxOrigin, TxTimelock, }; use super::TransactionImportResult; use crate::client::{AccountData, BlockChainTrait}; @@ -249,7 +249,6 @@ impl MemPool { let signer_public = tx.signer_public(); let seq = tx.seq; let hash = tx.hash(); - let timelock = input.timelock; let origin = if input.origin.is_local() && !self.is_local_account.contains(&signer_public) { self.is_local_account.insert(signer_public); @@ -269,7 +268,10 @@ impl MemPool { let id = self.next_transaction_id; self.next_transaction_id += 1; - let item = MemPoolItem::new(tx, origin, inserted_block_number, inserted_timestamp, id, timelock); + let item = MemPoolItem::new(tx, origin, inserted_block_number, inserted_timestamp, id, TxTimelock { + block: None, + timestamp: None, + }); let order = TransactionOrder::for_transaction(&item, client_account.seq); let order_with_tag = TransactionOrderWithTag::new(order, QueueTag::New); @@ -1353,42 +1355,35 @@ pub mod test { balance: test_client.latest_balance(&a), } }; - let no_timelock = TxTimelock { - block: None, - timestamp: None, - }; let inserted_block_number = 1; let inserted_timestamp = 100; let mut inputs: Vec = Vec::new(); - inputs.push(create_mempool_input_with_pay(1u64, keypair, no_timelock)); - inputs.push(create_mempool_input_with_pay(3u64, keypair, TxTimelock { - block: Some(10), - timestamp: None, - })); - inputs.push(create_mempool_input_with_pay(5u64, keypair, no_timelock)); + inputs.push(create_mempool_input_with_pay(1u64, keypair)); + inputs.push(create_mempool_input_with_pay(3u64, keypair)); + inputs.push(create_mempool_input_with_pay(5u64, keypair)); mem_pool.add(inputs, inserted_block_number, inserted_timestamp, &fetch_account); let inserted_block_number = 11; let inserted_timestamp = 200; let mut inputs: Vec = Vec::new(); - inputs.push(create_mempool_input_with_pay(2u64, keypair, no_timelock)); - inputs.push(create_mempool_input_with_pay(4u64, keypair, no_timelock)); + inputs.push(create_mempool_input_with_pay(2u64, keypair)); + inputs.push(create_mempool_input_with_pay(4u64, keypair)); mem_pool.add(inputs, inserted_block_number, inserted_timestamp, &fetch_account); let inserted_block_number = 20; let inserted_timestamp = 300; let mut inputs: Vec = Vec::new(); - inputs.push(create_mempool_input_with_pay(6u64, keypair, no_timelock)); - inputs.push(create_mempool_input_with_pay(8u64, keypair, no_timelock)); - inputs.push(create_mempool_input_with_pay(10u64, keypair, no_timelock)); + inputs.push(create_mempool_input_with_pay(6u64, keypair)); + inputs.push(create_mempool_input_with_pay(8u64, keypair)); + inputs.push(create_mempool_input_with_pay(10u64, keypair)); mem_pool.add(inputs, inserted_block_number, inserted_timestamp, &fetch_account); let inserted_block_number = 21; let inserted_timestamp = 400; let mut inputs: Vec = Vec::new(); - inputs.push(create_mempool_input_with_pay(7u64, keypair, no_timelock)); + inputs.push(create_mempool_input_with_pay(7u64, keypair)); mem_pool.add(inputs, inserted_block_number, inserted_timestamp, &fetch_account); let mut mem_pool_recovered = MemPool::with_limits(8192, usize::max_value(), 3, db, Default::default()); @@ -1434,9 +1429,9 @@ pub mod test { SignedTransaction::new_with_sign(tx, keypair.private()) } - fn create_mempool_input_with_pay(seq: u64, keypair: KeyPair, timelock: TxTimelock) -> MemPoolInput { + fn create_mempool_input_with_pay(seq: u64, keypair: KeyPair) -> MemPoolInput { let signed = create_signed_pay(seq, keypair); - MemPoolInput::new(signed, TxOrigin::Local, timelock) + MemPoolInput::new(signed, TxOrigin::Local) } fn create_transaction_order(fee: u64, transaction_count: usize) -> TransactionOrder { @@ -1483,14 +1478,10 @@ pub mod test { balance: test_client.latest_balance(&a), } }; - let no_timelock = TxTimelock { - block: None, - timestamp: None, - }; let inserted_block_number = 1; let inserted_timestamp = 100; - let inputs: Vec = txs.into_iter().map(|tx| MemPoolInput::new(tx, origin, no_timelock)).collect(); + let inputs: Vec = txs.into_iter().map(|tx| MemPoolInput::new(tx, origin)).collect(); mem_pool.add(inputs, inserted_block_number, inserted_timestamp, &fetch_account) } @@ -1630,17 +1621,13 @@ pub mod test { println!("! {}", address); test_client.set_balance(address, 1_000_000_000_000); assert_eq!(1_000_000_000_000, test_client.latest_balance(&address)); - let no_timelock = TxTimelock { - block: None, - timestamp: None, - }; let inserted_block_number = 1; let inserted_timestamp = 100; let inputs = vec![ - create_mempool_input_with_pay(0, keypair, no_timelock), - create_mempool_input_with_pay(1, keypair, no_timelock), - create_mempool_input_with_pay(2, keypair, no_timelock), + create_mempool_input_with_pay(0, keypair), + create_mempool_input_with_pay(1, keypair), + create_mempool_input_with_pay(2, keypair), ]; let result = mem_pool.add(inputs, inserted_block_number, inserted_timestamp, &fetch_account); assert_eq!( diff --git a/core/src/miner/mem_pool_types.rs b/core/src/miner/mem_pool_types.rs index 1b46dabbc3..52ea317fa8 100644 --- a/core/src/miner/mem_pool_types.rs +++ b/core/src/miner/mem_pool_types.rs @@ -409,15 +409,13 @@ impl FutureQueue { pub struct MemPoolInput { pub transaction: SignedTransaction, pub origin: TxOrigin, - pub timelock: TxTimelock, } impl MemPoolInput { - pub fn new(transaction: SignedTransaction, origin: TxOrigin, timelock: TxTimelock) -> Self { + pub fn new(transaction: SignedTransaction, origin: TxOrigin) -> Self { Self { transaction, origin, - timelock, } } } diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index e4bc968757..086cf336d3 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -25,7 +25,7 @@ use std::time::{Duration, Instant}; use ckey::{public_to_address, Address, Password, PlatformAddress, Public}; use cstate::{FindActionHandler, TopLevelState}; use ctypes::errors::{HistoryError, RuntimeError}; -use ctypes::transaction::{Action, IncompleteTransaction, Timelock}; +use ctypes::transaction::{Action, IncompleteTransaction}; use ctypes::{BlockHash, TxHash}; use cvm::ChainTimeInfo; use kvdb::KeyValueDB; @@ -34,7 +34,7 @@ use primitives::Bytes; use super::mem_pool::{Error as MemPoolError, MemPool}; pub use super::mem_pool_types::MemPoolFees; -use super::mem_pool_types::{AccountDetails, MemPoolInput, TxOrigin, TxTimelock}; +use super::mem_pool_types::{AccountDetails, MemPoolInput, TxOrigin}; use super::{MinerService, MinerStatus, TransactionImportResult}; use crate::account_provider::{AccountProvider, Error as AccountProviderError}; use crate::block::{ClosedBlock, IsBlock}; @@ -255,10 +255,9 @@ impl Miner { e })?; - let timelock = self.calculate_timelock(&tx, client)?; let tx_hash = tx.hash(); - to_insert.push(MemPoolInput::new(tx, origin, timelock)); + to_insert.push(MemPoolInput::new(tx, origin)); tx_hashes.push(tx_hash); Ok(()) }) @@ -298,54 +297,6 @@ impl Miner { results } - fn calculate_timelock(&self, tx: &SignedTransaction, client: &C) -> Result { - let mut max_block = None; - let mut max_timestamp = None; - if let Action::TransferAsset { - inputs, - .. - } = &tx.action - { - for input in inputs { - if let Some(timelock) = input.timelock { - let (is_block_number, value) = match timelock { - Timelock::Block(value) => (true, value), - Timelock::BlockAge(value) => ( - true, - client.transaction_block_number(&input.prev_out.tracker).ok_or_else(|| { - Error::History(HistoryError::Timelocked { - timelock, - remaining_time: u64::max_value(), - }) - })? + value, - ), - Timelock::Time(value) => (false, value), - Timelock::TimeAge(value) => ( - false, - client.transaction_block_timestamp(&input.prev_out.tracker).ok_or_else(|| { - Error::History(HistoryError::Timelocked { - timelock, - remaining_time: u64::max_value(), - }) - })? + value, - ), - }; - if is_block_number { - if max_block.is_none() || max_block.expect("The previous guard ensures") < value { - max_block = Some(value); - } - } else if max_timestamp.is_none() || max_timestamp.expect("The previous guard ensures") < value { - max_timestamp = Some(value); - } - } - } - }; - Ok(TxTimelock { - block: max_block, - timestamp: max_timestamp, - }) - } - /// Prepares new block for sealing including top transactions from queue. fn prepare_block< C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + EngineInfo + FindActionHandler + TermInfo, From d819a4c1b62c63f7d51b53925dcda8a4aecf4aae Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Thu, 19 Dec 2019 19:13:35 +0900 Subject: [PATCH 3/4] Remove TxTimelock type --- core/src/miner/mem_pool.rs | 275 +---------------------------- core/src/miner/mem_pool_benches.rs | 8 +- core/src/miner/mem_pool_types.rs | 18 -- 3 files changed, 9 insertions(+), 292 deletions(-) diff --git a/core/src/miner/mem_pool.rs b/core/src/miner/mem_pool.rs index d0ba0f6ad2..8db3798e88 100644 --- a/core/src/miner/mem_pool.rs +++ b/core/src/miner/mem_pool.rs @@ -28,7 +28,7 @@ use table::Table; use super::backup; use super::mem_pool_types::{ AccountDetails, CurrentQueue, FutureQueue, MemPoolFees, MemPoolInput, MemPoolItem, MemPoolStatus, PoolingInstant, - QueueTag, TransactionOrder, TransactionOrderWithTag, TxOrigin, TxTimelock, + QueueTag, TransactionOrder, TransactionOrderWithTag, TxOrigin, }; use super::TransactionImportResult; use crate::client::{AccountData, BlockChainTrait}; @@ -268,10 +268,7 @@ impl MemPool { let id = self.next_transaction_id; self.next_transaction_id += 1; - let item = MemPoolItem::new(tx, origin, inserted_block_number, inserted_timestamp, id, TxTimelock { - block: None, - timestamp: None, - }); + let item = MemPoolItem::new(tx, origin, inserted_block_number, inserted_timestamp, id); let order = TransactionOrder::for_transaction(&item, client_account.seq); let order_with_tag = TransactionOrderWithTag::new(order, QueueTag::New); @@ -898,7 +895,6 @@ pub mod test { use std::cmp::Ordering; use crate::client::{AccountData, TestBlockChainClient}; - use crate::miner::mem_pool_types::TxTimelock; use ckey::{Generator, KeyPair, Random}; use ctypes::transaction::{Action, AssetMintOutput, Transaction}; use primitives::H160; @@ -917,234 +913,6 @@ pub mod test { assert_eq!(TxOrigin::External.cmp(&TxOrigin::RetractedBlock), Ordering::Greater); } - #[test] - fn timelock_ordering() { - assert_eq!( - TxTimelock { - block: None, - timestamp: None - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: None - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: None, - timestamp: None - } - .cmp(&TxTimelock { - block: None, - timestamp: Some(100) - }), - Ordering::Less - ); - - // Block is the prior condition. - assert_eq!( - TxTimelock { - block: Some(9), - timestamp: None - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: None - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: Some(9), - timestamp: None - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: Some(9), - timestamp: Some(100) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: None - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: Some(9), - timestamp: Some(99) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: Some(9), - timestamp: Some(101) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: Some(11), - timestamp: None - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: None - }), - Ordering::Greater - ); - assert_eq!( - TxTimelock { - block: Some(11), - timestamp: None - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Greater - ); - assert_eq!( - TxTimelock { - block: Some(11), - timestamp: Some(100) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: None - }), - Ordering::Greater - ); - assert_eq!( - TxTimelock { - block: Some(11), - timestamp: Some(99) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Greater - ); - assert_eq!( - TxTimelock { - block: Some(11), - timestamp: Some(101) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Greater - ); - - // Compare timestamp if blocks are equal. - assert_eq!( - TxTimelock { - block: Some(10), - timestamp: None - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: Some(10), - timestamp: Some(99) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: Some(10), - timestamp: Some(100) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Equal - ); - assert_eq!( - TxTimelock { - block: Some(10), - timestamp: Some(101) - } - .cmp(&TxTimelock { - block: Some(10), - timestamp: Some(100) - }), - Ordering::Greater - ); - assert_eq!( - TxTimelock { - block: None, - timestamp: None - } - .cmp(&TxTimelock { - block: None, - timestamp: Some(100) - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: None, - timestamp: Some(99) - } - .cmp(&TxTimelock { - block: None, - timestamp: Some(100) - }), - Ordering::Less - ); - assert_eq!( - TxTimelock { - block: None, - timestamp: Some(100) - } - .cmp(&TxTimelock { - block: None, - timestamp: Some(100) - }), - Ordering::Equal - ); - assert_eq!( - TxTimelock { - block: None, - timestamp: Some(101) - } - .cmp(&TxTimelock { - block: None, - timestamp: Some(100) - }), - Ordering::Greater - ); - } - #[test] fn mint_transaction_does_not_increase_cost() { let shard_id = 0xCCC; @@ -1169,13 +937,9 @@ pub mod test { approvals: vec![], }, }; - let timelock = TxTimelock { - block: None, - timestamp: None, - }; let keypair = Random.generate().unwrap(); let signed = SignedTransaction::new_with_sign(tx, keypair.private()); - let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0, timelock); + let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0); assert_eq!(fee, item.cost()); } @@ -1197,13 +961,9 @@ pub mod test { expiration: None, }, }; - let timelock = TxTimelock { - block: None, - timestamp: None, - }; let keypair = Random.generate().unwrap(); let signed = SignedTransaction::new_with_sign(tx, keypair.private()); - let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0, timelock); + let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0); assert_eq!(fee, item.cost()); } @@ -1223,12 +983,8 @@ pub mod test { quantity, }, }; - let timelock = TxTimelock { - block: None, - timestamp: None, - }; let signed = SignedTransaction::new_with_sign(tx, keypair.private()); - let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0, timelock); + let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0); assert_eq!(fee + quantity, item.cost()); } @@ -1276,15 +1032,6 @@ pub mod test { rlp_encode_and_decode_test!(TxOrigin::External); } - #[test] - fn txtimelock_encode_and_decode() { - let timelock = TxTimelock { - block: None, - timestamp: None, - }; - rlp_encode_and_decode_test!(timelock); - } - #[test] fn signed_transaction_encode_and_decode() { let receiver = 0u64.into(); @@ -1325,12 +1072,8 @@ pub mod test { approvals: vec![], }, }; - let timelock = TxTimelock { - block: None, - timestamp: None, - }; let signed = SignedTransaction::new_with_sign(tx, keypair.private()); - let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0, timelock); + let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0); rlp_encode_and_decode_test!(item); } @@ -1455,12 +1198,8 @@ pub mod test { approvals: vec![], }, }; - let timelock = TxTimelock { - block: None, - timestamp: None, - }; let signed = SignedTransaction::new_with_sign(tx, keypair.private()); - let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0, timelock); + let item = MemPoolItem::new(signed, TxOrigin::Local, 0, 0, 0); TransactionOrder::for_transaction(&item, 0) } diff --git a/core/src/miner/mem_pool_benches.rs b/core/src/miner/mem_pool_benches.rs index 4d51f8c175..f116206d63 100644 --- a/core/src/miner/mem_pool_benches.rs +++ b/core/src/miner/mem_pool_benches.rs @@ -25,7 +25,7 @@ use rand::thread_rng; use self::test::{black_box, Bencher}; use super::mem_pool::MemPool; -use super::mem_pool_types::{AccountDetails, MemPoolInput, PoolingInstant, TxOrigin, TxTimelock}; +use super::mem_pool_types::{AccountDetails, MemPoolInput, PoolingInstant, TxOrigin}; use crate::transaction::SignedTransaction; const NUM_TXS: usize = 5000; @@ -40,13 +40,9 @@ fn create_input(keypair: &KeyPair, seq: u64, block: Option, time quantity: 100, }, }; - let timelock = TxTimelock { - block, - timestamp, - }; let signed = SignedTransaction::new_with_sign(tx, keypair.private()); - MemPoolInput::new(signed, TxOrigin::Local, timelock) + MemPoolInput::new(signed, TxOrigin::Local) } #[bench] diff --git a/core/src/miner/mem_pool_types.rs b/core/src/miner/mem_pool_types.rs index 52ea317fa8..d2b730883d 100644 --- a/core/src/miner/mem_pool_types.rs +++ b/core/src/miner/mem_pool_types.rs @@ -100,12 +100,6 @@ impl TxOrigin { } } -#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, RlpEncodable, RlpDecodable)] -pub struct TxTimelock { - pub block: Option, - pub timestamp: Option, -} - #[derive(Clone, Copy, Debug)] /// Light structure used to identify transaction and its order pub struct TransactionOrder { @@ -126,8 +120,6 @@ pub struct TransactionOrder { pub insertion_id: u64, /// Origin of the transaction pub origin: TxOrigin, - /// Timelock - pub timelock: TxTimelock, } impl TransactionOrder { @@ -143,7 +135,6 @@ impl TransactionOrder { hash: item.hash(), insertion_id: item.insertion_id, origin: item.origin, - timelock: item.timelock, } } @@ -191,11 +182,6 @@ impl Ord for TransactionOrder { return b.fee.cmp(&self.fee) } - // Compare timelock, prefer the nearer from the present. - if self.timelock != b.timelock { - return self.timelock.cmp(&b.timelock) - } - // Lastly compare insertion_id self.insertion_id.cmp(&b.insertion_id) } @@ -214,8 +200,6 @@ pub struct MemPoolItem { pub inserted_timestamp: u64, /// ID assigned upon insertion, should be unique. pub insertion_id: u64, - /// A timelock. - pub timelock: TxTimelock, } impl MemPoolItem { @@ -225,7 +209,6 @@ impl MemPoolItem { inserted_block_number: PoolingInstant, inserted_timestamp: u64, insertion_id: u64, - timelock: TxTimelock, ) -> Self { MemPoolItem { tx, @@ -233,7 +216,6 @@ impl MemPoolItem { inserted_block_number, inserted_timestamp, insertion_id, - timelock, } } From f67556b51a16c894f2d2f58a010026b450fa5e75 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Fri, 20 Dec 2019 16:52:15 +0900 Subject: [PATCH 4/4] Remove timelock e2e test --- test/src/e2e.long/timelock.test.ts | 407 ----------------------------- test/src/e2e/mempool.test.ts | 85 ------ 2 files changed, 492 deletions(-) delete mode 100644 test/src/e2e.long/timelock.test.ts diff --git a/test/src/e2e.long/timelock.test.ts b/test/src/e2e.long/timelock.test.ts deleted file mode 100644 index 504b2716f9..0000000000 --- a/test/src/e2e.long/timelock.test.ts +++ /dev/null @@ -1,407 +0,0 @@ -// Copyright 2018-2019 Kodebox, Inc. -// This file is part of CodeChain. -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as -// published by the Free Software Foundation, either version 3 of the -// License, or (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -import { expect } from "chai"; -import { H256 } from "codechain-primitives/lib"; -import { Asset, AssetAddress, Timelock } from "codechain-sdk/lib/core/classes"; -import "mocha"; -import { wait } from "../helper/promise"; -import CodeChain from "../helper/spawn"; - -describe("Timelock", function() { - let node: CodeChain; - - beforeEach(async function() { - node = new CodeChain({ - argv: ["--force-sealing", "--no-reseal-timer"] - }); - await node.start(); - }); - - async function sendTxWithTimelock(timelock: Timelock): Promise { - const asset = await node.mintAsset({ supply: 1 }); - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs( - asset.createTransferInput({ - timelock - }) - ); - tx.addOutputs({ - quantity: 1, - assetType: asset.assetType, - shardId: asset.shardId, - recipient: await node.createP2PKHAddress() - }); - await node.signTransactionInput(tx, 0); - await node.sendAssetTransaction(tx); - return tx.tracker(); - } - - async function checkTx(tracker: H256, shouldBeConfirmed: boolean) { - const results = await node.sdk.rpc.chain.getTransactionResultsByTracker( - tracker - ); - if (shouldBeConfirmed) { - expect(results.length).to.equal(1); - expect(results[0]).to.be.true; - } else { - expect(results.length).to.equal(0); - } - } - - describe("Transaction should go into the current queue", async function() { - it(`Minted at block 1, send transfer with Timelock::Block(0)`, async function() { - const tracker = await sendTxWithTimelock({ - type: "block", - value: 0 - }); - await checkTx(tracker, true); - }); - - it(`Minted at block 1, send transfer with Timelock::BlockAge(0)`, async function() { - const tracker = await sendTxWithTimelock({ - type: "blockAge", - value: 0 - }); - await checkTx(tracker, true); - }); - - it("send transfer with Timelock::Time(0)", async function() { - const tracker = await sendTxWithTimelock({ - type: "time", - value: 0 - }); - await checkTx(tracker, true); - }); - - it("send transfer with Timelock::TimeAge(0)", async function() { - const tracker = await sendTxWithTimelock({ - type: "timeAge", - value: 0 - }); - await checkTx(tracker, true); - }); - }); - - it("A relative timelock for failed transaction's output", async function() { - const asset = await node.mintAsset({ supply: 1 }); - const failedTx = node.sdk.core.createTransferAssetTransaction(); - failedTx.addInputs(asset); - failedTx.addOutputs({ - quantity: 1, - assetType: asset.assetType, - shardId: asset.shardId, - recipient: await node.createP2PKHAddress() - }); - - await node.sendAssetTransactionExpectedToFail(failedTx); - - const output0 = failedTx.getTransferredAsset(0); - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs( - output0.createTransferInput({ - timelock: { - type: "blockAge", - value: 2 - } - }) - ); - tx.addOutputs({ - quantity: 1, - assetType: asset.assetType, - shardId: asset.shardId, - recipient: await node.createP2PKHAddress() - }); - await node.signTransactionInput(tx, 0); - try { - await node.sendAssetTransaction(tx); - expect.fail(); - } catch (e) { - expect(e.data).to.have.string("Timelocked"); - expect(e.data).to.have.string("BlockAge(2)"); - expect(e.data).to.have.string("18446744073709551615"); - } - await checkTx(tx.tracker(), false); - await node.sdk.rpc.devel.startSealing(); - }); - - describe("Transactions should go into the future queue and then move to current", async function() { - it("Minted at block 1, send transfer with Timelock::Block(3)", async function() { - const tracker = await sendTxWithTimelock({ - // available from block 3 - type: "block", - value: 3 - }); - - expect(await node.getBestBlockNumber()).to.equal(1); - await checkTx(tracker, false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - await checkTx(tracker, false); - await node.sdk.rpc.devel.startSealing(); - - expect(await node.getBestBlockNumber()).to.equal(4); - await checkTx(tracker, true); - }); - - it("Minted at block 1, send transfer with Timelock::BlockAge(3)", async function() { - const tracker = await sendTxWithTimelock({ - // available from block 4, since mintTx is at block 1. - type: "blockAge", - value: 3 - }); - - for (let i = 1; i <= 4; i++) { - expect(await node.getBestBlockNumber()).to.equal(i); - await checkTx(tracker, false); - - await node.sdk.rpc.devel.startSealing(); - } - - expect(await node.getBestBlockNumber()).to.equal(5); - await checkTx(tracker, true); - }); - }); - - async function sendTransferTx( - asset: Asset, - timelock?: Timelock, - options: { - fee?: number; - } = {} - ): Promise { - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs( - timelock - ? asset.createTransferInput({ - timelock - }) - : asset.createTransferInput() - ); - tx.addOutputs({ - quantity: 1, - assetType: asset.assetType, - shardId: asset.shardId, - recipient: await node.createP2PKHAddress() - }); - await node.signTransactionInput(tx, 0); - const { fee } = options; - await node.sendAssetTransaction(tx, { fee }); - return tx.tracker(); - } - - describe("The future items should move to the current queue", async function() { - it("Minted at block 1, send transfer with Timelock::Block(10) and then replace it with no timelock", async function() { - const asset = await node.mintAsset({ supply: 1 }); - await node.sdk.rpc.devel.stopSealing(); - const tracker1 = await sendTransferTx(asset, { - type: "block", - value: 10 - }); - const tracker2 = await sendTransferTx(asset, undefined, { - fee: 20 - }); - await checkTx(tracker1, false); - await checkTx(tracker2, false); - - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(2); - await checkTx(tracker1, false); - await checkTx(tracker2, true); - }).timeout(10_000); - }); - - describe("Multiple timelocks", async function() { - let recipient: AssetAddress; - - beforeEach(async function() { - recipient = await node.createP2PKHAddress(); - }); - - async function createUTXOs(count: number): Promise { - const asset = await node.mintAsset({ supply: count }); - const transferTx = node.sdk.core.createTransferAssetTransaction(); - transferTx.addInputs(asset); - transferTx.addOutputs( - Array.from(Array(count)).map(_ => ({ - assetType: asset.assetType, - shardId: asset.shardId, - quantity: 1, - recipient - })) - ); - await node.signTransactionInput(transferTx, 0); - await node.sendAssetTransaction(transferTx); - return transferTx.getTransferredAssets(); - } - - it("2 inputs [Block(4), Block(6)] => Block(6)", async function() { - const assets = await createUTXOs(2); - const { assetType, shardId } = assets[0]; - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs([ - assets[0].createTransferInput({ - timelock: { - type: "block", - value: 4 - } - }), - assets[1].createTransferInput({ - timelock: { - type: "block", - value: 6 - } - }) - ]); - tx.addOutputs({ quantity: 2, recipient, assetType, shardId }); - await node.signTransactionInput(tx, 0); - await node.signTransactionInput(tx, 1); - await node.sendAssetTransaction(tx); - - expect(await node.getBestBlockNumber()).to.equal(2); - await checkTx(tx.tracker(), false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(5); - await checkTx(tx.tracker(), false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(7); - await checkTx(tx.tracker(), true); - }).timeout(10_000); - - it("2 inputs [Block(6), Block(4)] => Block(4)", async function() { - const assets = await createUTXOs(2); - const { assetType, shardId } = assets[0]; - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs([ - assets[0].createTransferInput({ - timelock: { - type: "block", - value: 6 - } - }), - assets[1].createTransferInput({ - timelock: { - type: "block", - value: 4 - } - }) - ]); - tx.addOutputs({ quantity: 2, recipient, assetType, shardId }); - await node.signTransactionInput(tx, 0); - await node.signTransactionInput(tx, 1); - await node.sendAssetTransaction(tx); - - expect(await node.getBestBlockNumber()).to.equal(2); - await checkTx(tx.tracker(), false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(5); - await checkTx(tx.tracker(), false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(7); - await checkTx(tx.tracker(), true); - }).timeout(10_000); - - it("2 inputs [Time(0), Block(4)] => Block(4)", async function() { - const assets = await createUTXOs(2); - const { assetType, shardId } = assets[0]; - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs([ - assets[0].createTransferInput({ - timelock: { - type: "time", - value: 0 - } - }), - assets[1].createTransferInput({ - timelock: { - type: "block", - value: 4 - } - }) - ]); - tx.addOutputs({ quantity: 2, recipient, assetType, shardId }); - await node.signTransactionInput(tx, 0); - await node.signTransactionInput(tx, 1); - await node.sendAssetTransaction(tx); - - expect(await node.getBestBlockNumber()).to.equal(2); - await checkTx(tx.tracker(), false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(5); - await checkTx(tx.tracker(), true); - }).timeout(10_000); - - it("2 inputs [Time(now + 3 seconds), Block(4)] => Time(..)", async function() { - const assets = await createUTXOs(2); - const { assetType, shardId } = assets[0]; - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs([ - assets[0].createTransferInput({ - timelock: { - type: "time", - value: Math.floor(Date.now() / 1000) + 3 - } - }), - assets[1].createTransferInput({ - timelock: { - type: "block", - value: 4 - } - }) - ]); - tx.addOutputs({ quantity: 2, recipient, assetType, shardId }); - await node.signTransactionInput(tx, 0); - await node.signTransactionInput(tx, 1); - await node.sendAssetTransaction(tx); - - expect(await node.getBestBlockNumber()).to.equal(2); - await checkTx(tx.tracker(), false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(4); - await checkTx(tx.tracker(), false); - - await wait(3_000); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(6); - await checkTx(tx.tracker(), true); - }).timeout(10_000); - }); - - afterEach(async function() { - if (this.currentTest!.state === "failed") { - node.keepLogs(); - } - await node.clean(); - }); -}); diff --git a/test/src/e2e/mempool.test.ts b/test/src/e2e/mempool.test.ts index 66666d2e45..9638ae7e6c 100644 --- a/test/src/e2e/mempool.test.ts +++ b/test/src/e2e/mempool.test.ts @@ -15,8 +15,6 @@ // along with this program. If not, see . import { expect } from "chai"; -import { H256 } from "codechain-primitives/lib"; -import { Asset, Timelock } from "codechain-sdk/lib/core/classes"; import "mocha"; import { faucetAddress } from "../helper/constants"; import CodeChain from "../helper/spawn"; @@ -75,86 +73,3 @@ describe("Future queue", function() { await node.clean(); }); }); - -describe("Timelock", function() { - let node: CodeChain; - - beforeEach(async function() { - node = new CodeChain({ - argv: ["--force-sealing", "--no-reseal-timer"] - }); - await node.start(); - }); - - async function checkTx(txhash: H256, shouldBeConfirmed: boolean) { - const results = await node.sdk.rpc.chain.getTransactionResultsByTracker( - txhash - ); - expect(results).deep.equal(shouldBeConfirmed ? [true] : []); - } - - async function sendTransferTx( - asset: Asset, - timelock?: Timelock, - options: { - fee?: number; - } = {} - ): Promise { - const tx = node.sdk.core.createTransferAssetTransaction(); - tx.addInputs( - timelock - ? asset.createTransferInput({ - timelock - }) - : asset.createTransferInput() - ); - tx.addOutputs({ - quantity: 1, - assetType: asset.assetType, - shardId: asset.shardId, - recipient: await node.createP2PKHAddress() - }); - await node.signTransactionInput(tx, 0); - const { fee } = options; - await node.sendAssetTransaction(tx, { fee }); - return tx.tracker(); - } - - describe("The current items should move to the future queue", async function() { - it("Minted at block 1, send transfer without timelock and then replace it with Timelock::Block(3)", async function() { - const asset = await node.mintAsset({ supply: 1 }); - await node.sdk.rpc.devel.stopSealing(); - const txhash1 = await sendTransferTx(asset, undefined); - const txhash2 = await sendTransferTx( - asset, - { - type: "block", - value: 3 - }, - { - fee: 20 - } - ); - await checkTx(txhash1, false); - await checkTx(txhash2, false); - - await node.sdk.rpc.devel.startSealing(); - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(3); - await checkTx(txhash1, false); - await checkTx(txhash2, false); - - await node.sdk.rpc.devel.startSealing(); - expect(await node.getBestBlockNumber()).to.equal(4); - await checkTx(txhash1, false); - await checkTx(txhash2, true); - }); - }); - - afterEach(async function() { - if (this.currentTest!.state === "failed") { - node.keepLogs(); - } - await node.clean(); - }); -});