From 6c1802e412ab8de8bf9f0001a3a4280fce9c226a Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 17:23:54 +0200 Subject: [PATCH 01/10] Allow configuration of when to reseal blocks. --- ethcore/src/miner/miner.rs | 37 +++++++++++++++++++++++++++++-------- ethcore/src/miner/mod.rs | 2 +- parity/cli.rs | 7 +++++++ parity/configuration.rs | 16 ++++++++++++++++ parity/main.rs | 2 +- rpc/src/v1/tests/eth.rs | 12 ++++++++++-- sync/src/lib.rs | 2 +- 7 files changed, 65 insertions(+), 13 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 401a5314a..1e76e6288 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -29,6 +29,27 @@ use spec::Spec; use engine::Engine; use miner::{MinerService, MinerStatus, TransactionQueue, AccountDetails, TransactionImportResult, TransactionOrigin}; +/// Configures the behaviour of the miner. +#[derive(Debug)] +pub struct MinerOptions { + /// Force the miner to reseal, even when nobody has asked for work. + pub force_sealing: bool, + /// Reseal on receipt of new external transactions. + pub reseal_on_external_tx: bool, + /// Reseal on receipt of new local transactions. + pub reseal_on_own_tx: bool, +} + +impl Default for MinerOptions { + fn default() -> Self { + MinerOptions { + force_sealing: false, + reseal_on_external_tx: true, + reseal_on_own_tx: true, + } + } +} + /// Keeps track of transactions using priority queue and holds currently mined block. pub struct Miner { // NOTE [ToDr] When locking always lock in this order! @@ -36,7 +57,7 @@ pub struct Miner { sealing_work: Mutex>, // for sealing... - force_sealing: bool, + options: MinerOptions, sealing_enabled: AtomicBool, sealing_block_last_request: Mutex, gas_range_target: RwLock<(U256, U256)>, @@ -52,7 +73,7 @@ impl Miner { pub fn with_spec(spec: Spec) -> Miner { Miner { transaction_queue: Mutex::new(TransactionQueue::new()), - force_sealing: false, + options: Default::default(), sealing_enabled: AtomicBool::new(false), sealing_block_last_request: Mutex::new(0), sealing_work: Mutex::new(UsingQueue::new(5)), @@ -65,11 +86,11 @@ impl Miner { } /// Creates new instance of miner - pub fn new(force_sealing: bool, spec: Spec, accounts: Option>) -> Arc { + pub fn new(options: MinerOptions, spec: Spec, accounts: Option>) -> Arc { Arc::new(Miner { transaction_queue: Mutex::new(TransactionQueue::new()), - force_sealing: force_sealing, - sealing_enabled: AtomicBool::new(force_sealing), + sealing_enabled: AtomicBool::new(options.force_sealing), + options: options, sealing_block_last_request: Mutex::new(0), sealing_work: Mutex::new(UsingQueue::new(5)), gas_range_target: RwLock::new((U256::zero(), U256::zero())), @@ -385,7 +406,7 @@ impl MinerService for Miner { .map(|tx| transaction_queue.add(tx, &fetch_account, TransactionOrigin::External)) .collect() }; - if !results.is_empty() { + if !results.is_empty() && self.options.reseal_on_external_tx { self.update_sealing(chain); } results @@ -420,7 +441,7 @@ impl MinerService for Miner { import }; - if imported.is_ok() { + if imported.is_ok() && self.options.reseal_on_own_tx { // Make sure to do it after transaction is imported and lock is droped. // We need to create pending block and enable sealing let prepared = self.enable_and_prepare_sealing(chain); @@ -494,7 +515,7 @@ impl MinerService for Miner { let current_no = chain.chain_info().best_block_number; let has_local_transactions = self.transaction_queue.lock().unwrap().has_local_pending_transactions(); let last_request = *self.sealing_block_last_request.lock().unwrap(); - let should_disable_sealing = !self.force_sealing + let should_disable_sealing = !self.options.force_sealing && !has_local_transactions && current_no > last_request && current_no - last_request > SEALING_TIMEOUT_IN_BLOCKS; diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 6e415a602..8a282490c 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -47,7 +47,7 @@ mod external; mod transaction_queue; pub use self::transaction_queue::{TransactionQueue, AccountDetails, TransactionImportResult, TransactionOrigin}; -pub use self::miner::{Miner}; +pub use self::miner::{Miner, MinerOptions}; pub use self::external::{ExternalMiner, ExternalMinerService}; use std::collections::BTreeMap; diff --git a/parity/cli.rs b/parity/cli.rs index 29333fe9a..a3b3bb5df 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -133,6 +133,12 @@ Sealing/Mining Options: NOTE: MINING WILL NOT WORK WITHOUT THIS OPTION. --force-sealing Force the node to author new blocks as if it were always sealing/mining. + --reseal-on-txs Specify which transactions should force the node + to reseal a block. One of: + none - never reseal on new transactions; + own - reseal only on a new local transaction; + ext - reseal only on a new external transaction; + all - reseal on all new transactions [default: all]. --usd-per-tx USD Amount of USD to be paid for a basic transaction [default: 0.005]. The minimum gas price is set accordingly. @@ -283,6 +289,7 @@ pub struct Args { pub flag_signer_path: String, pub flag_no_token: bool, pub flag_force_sealing: bool, + pub flag_reseal_on_txs: String, pub flag_author: Option, pub flag_usd_per_tx: String, pub flag_usd_per_eth: String, diff --git a/parity/configuration.rs b/parity/configuration.rs index 43a7fcd3c..39893622a 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -27,6 +27,7 @@ use util::*; use ethcore::account_provider::AccountProvider; use util::network_settings::NetworkSettings; use ethcore::client::{append_path, get_db_path, ClientConfig, Switch, VMType}; +use ethcore::miner::MinerOptions; use ethcore::ethereum; use ethcore::spec::Spec; use ethsync::SyncConfig; @@ -67,6 +68,21 @@ impl Configuration { self.args.flag_maxpeers.unwrap_or(self.args.flag_peers) as u32 } + pub fn miner_options(&self) -> MinerOptions { + let (own, ext) = match self.args.flag_reseal_on_txs.as_str() { + "none" => (false, false), + "own" => (true, false), + "ext" => (false, true), + "all" => (true, true), + x => die!("{}: Invalid value for --reseal option. Use --help for more information.", x) + }; + MinerOptions { + force_sealing: self.args.flag_force_sealing, + reseal_on_external_tx: ext, + reseal_on_own_tx: own, + } + } + pub fn author(&self) -> Option
{ self.args.flag_etherbase.as_ref() .or(self.args.flag_author.as_ref()) diff --git a/parity/main.rs b/parity/main.rs index 62262758d..e3c20caf4 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -208,7 +208,7 @@ fn execute_client(conf: Configuration, spec: Spec, client_config: ClientConfig) let account_service = Arc::new(conf.account_service()); // Miner - let miner = Miner::new(conf.args.flag_force_sealing, conf.spec(), Some(account_service.clone())); + let miner = Miner::new(conf.miner_options(), conf.spec(), Some(account_service.clone())); miner.set_author(conf.author().unwrap_or_default()); miner.set_gas_floor_target(conf.gas_floor_target()); miner.set_gas_ceil_target(conf.gas_ceil_target()); diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 6e2256e24..2ce482a90 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -24,7 +24,7 @@ use ethcore::spec::{Genesis, Spec}; use ethcore::block::Block; use ethcore::views::BlockView; use ethcore::ethereum; -use ethcore::miner::{MinerService, ExternalMiner, Miner}; +use ethcore::miner::{MinerOptions, MinerService, ExternalMiner, Miner}; use ethcore::account_provider::AccountProvider; use devtools::RandomTempPath; use util::Hashable; @@ -49,7 +49,15 @@ fn sync_provider() -> Arc { } fn miner_service(spec: Spec, accounts: Arc) -> Arc { - Miner::new(true, spec, Some(accounts)) + Miner::new( + MinerOptions { + force_sealing: true, + reseal_on_external_tx: true, + reseal_on_own_tx: true, + }, + spec, + Some(accounts) + ) } fn make_spec(chain: &BlockChain) -> Spec { diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 3f899c261..928496326 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -44,7 +44,7 @@ //! let mut service = NetworkService::new(NetworkConfiguration::new()).unwrap(); //! service.start().unwrap(); //! let dir = env::temp_dir(); -//! let miner = Miner::new(false, ethereum::new_frontier(true), None); +//! let miner = Miner::new(MinerOptions::default(), ethereum::new_frontier(true), None); //! let client = Client::new( //! ClientConfig::default(), //! ethereum::new_frontier(true), From 1667808ecb67dc0c45f51f83b3a95369b075643a Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 18:27:06 +0200 Subject: [PATCH 02/10] More miner options. - Optional limit for the amount of gas transactions may have; - option to restruct transactions returned/queried to only those which have been executed. --- ethcore/src/client/client.rs | 3 ++- ethcore/src/miner/miner.rs | 24 ++++++++++++++---------- ethcore/src/miner/transaction_queue.rs | 16 ++++++++++++++++ parity/cli.rs | 13 ++++++++++--- parity/configuration.rs | 14 ++++++++++++++ parity/main.rs | 2 +- 6 files changed, 57 insertions(+), 15 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 16422250a..2c4499e57 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -149,7 +149,8 @@ impl Client where V: Verifier { let mut state_db = journaldb::new( &append_path(&path, "state"), config.pruning, - config.db_cache_size); + config.db_cache_size + ); if state_db.is_empty() && spec.ensure_db_good(state_db.as_hashdb_mut()) { state_db.commit(0, &spec.genesis_header().hash(), None).expect("Error commiting genesis state to state DB"); diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 1e76e6288..e5ecff8c4 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -38,6 +38,11 @@ pub struct MinerOptions { pub reseal_on_external_tx: bool, /// Reseal on receipt of new local transactions. pub reseal_on_own_tx: bool, + /// Specify maximum amount of gas to bother considering for block insertion. + /// If `None`, then no limit. + pub max_tx_gas: Option, + /// Whether we should fallback to providing all the queue's transactions or just pending. + pub strict_valid_pending: bool, } impl Default for MinerOptions { @@ -46,6 +51,8 @@ impl Default for MinerOptions { force_sealing: false, reseal_on_external_tx: true, reseal_on_own_tx: true, + max_tx_gas: None, + strict_valid_pending: false, } } } @@ -112,7 +119,7 @@ impl Miner { trace!(target: "miner", "prepare_sealing: entering"); let (transactions, mut open_block) = { - let transactions = {self.transaction_queue.lock().unwrap().top_transactions()}; + let transactions = {self.transaction_queue.lock().unwrap().top_transactions_maybe_limit(&self.options.max_tx_gas)}; let mut sealing_work = self.sealing_work.lock().unwrap(); let best_hash = chain.best_block_header().sha3(); /* @@ -459,9 +466,8 @@ impl MinerService for Miner { let queue = self.transaction_queue.lock().unwrap(); match (self.sealing_enabled.load(atomic::Ordering::Relaxed), self.sealing_work.lock().unwrap().peek_last_ref()) { (true, Some(pending)) => pending.transactions().iter().map(|t| t.hash()).collect(), - _ => { - queue.pending_hashes() - } + _ if self.options.strict_valid_pending => Vec::new(), + _ => queue.pending_hashes(), } } @@ -469,9 +475,8 @@ impl MinerService for Miner { let queue = self.transaction_queue.lock().unwrap(); match (self.sealing_enabled.load(atomic::Ordering::Relaxed), self.sealing_work.lock().unwrap().peek_last_ref()) { (true, Some(pending)) => pending.transactions().iter().find(|t| &t.hash() == hash).cloned(), - _ => { - queue.find(hash) - } + _ if self.options.strict_valid_pending => None, + _ => queue.find(hash), } } @@ -485,9 +490,8 @@ impl MinerService for Miner { // TODO: should only use the sealing_work when it's current (it could be an old block) match (self.sealing_enabled.load(atomic::Ordering::Relaxed), self.sealing_work.lock().unwrap().peek_last_ref()) { (true, Some(pending)) => pending.transactions().clone(), - _ => { - queue.top_transactions() - } + _ if self.options.strict_valid_pending => Vec::new(), + _ => queue.top_transactions(), } } diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index b2ca31e0d..9159cc6b3 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -583,6 +583,22 @@ impl TransactionQueue { .collect() } + /// Returns top transactions from the queue ordered by priority with a maximum gas limit. + pub fn top_transactions_with_limit(&self, max_tx_gas: &U256) -> Vec { + self.current.by_priority + .iter() + .map(|t| self.by_hash.get(&t.hash).expect("All transactions in `current` and `future` are always included in `by_hash`")) + .filter_map(|t| if &t.transaction.gas <= max_tx_gas { Some(t.transaction.clone()) } else { None }) + .collect() + } + + /// Returns top transactions from the queue ordered by priority with a maximum gas limit. + pub fn top_transactions_maybe_limit(&self, max_tx_gas: &Option) -> Vec { + match *max_tx_gas { + None => self.top_transactions(), + Some(ref m) => self.top_transactions_with_limit(m), + } + } /// Returns hashes of all transactions from current, ordered by priority. pub fn pending_hashes(&self) -> Vec { self.current.by_priority diff --git a/parity/cli.rs b/parity/cli.rs index a3b3bb5df..3090e83c5 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -139,6 +139,11 @@ Sealing/Mining Options: own - reseal only on a new local transaction; ext - reseal only on a new external transaction; all - reseal on all new transactions [default: all]. + --max-tx-gas GAS Apply a limit of GAS as the maximum amount of gas + a single transaction may have for it to be mined. + --relay-validity REQ Requirements for relaying. REQ may be: + cheap - Relay only after cheap checks; + strict - Relay only once executed [default: cheap]. --usd-per-tx USD Amount of USD to be paid for a basic transaction [default: 0.005]. The minimum gas price is set accordingly. @@ -152,8 +157,8 @@ Sealing/Mining Options: block due to transaction volume [default: 3141592]. --extra-data STRING Specify a custom extra-data for authored blocks, no more than 32 characters. - --tx-limit LIMIT Limit of transactions kept in the queue (waiting to - be included in next block) [default: 1024]. + --tx-queue-size LIMIT Maxmimum amount of transactions in the queue (waiting + to be included in next block) [default: 1024]. Footprint Options: --tracing BOOL Indicates if full transaction tracing should be @@ -290,13 +295,15 @@ pub struct Args { pub flag_no_token: bool, pub flag_force_sealing: bool, pub flag_reseal_on_txs: String, + pub flag_max_tx_gas: Option, + pub flag_relay_validity: String, pub flag_author: Option, pub flag_usd_per_tx: String, pub flag_usd_per_eth: String, pub flag_gas_floor_target: String, pub flag_gas_cap: String, pub flag_extra_data: Option, - pub flag_tx_limit: usize, + pub flag_tx_queue_size: usize, pub flag_logging: Option, pub flag_version: bool, pub flag_from: String, diff --git a/parity/configuration.rs b/parity/configuration.rs index 39893622a..57c62f839 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -68,6 +68,14 @@ impl Configuration { self.args.flag_maxpeers.unwrap_or(self.args.flag_peers) as u32 } + fn decode_u256(d: &str, argument: &str) -> U256 { + U256::from_dec_str(d).unwrap_or_else(|_| + U256::from_str(clean_0x(d)).unwrap_or_else(|_| + die!("{}: Invalid numeric value for {}. Must be either a decimal or a hex number.", d, argument) + ) + ) + } + pub fn miner_options(&self) -> MinerOptions { let (own, ext) = match self.args.flag_reseal_on_txs.as_str() { "none" => (false, false), @@ -80,6 +88,12 @@ impl Configuration { force_sealing: self.args.flag_force_sealing, reseal_on_external_tx: ext, reseal_on_own_tx: own, + max_tx_gas: self.args.flag_max_tx_gas.as_ref().map(|d| Self::decode_u256(d, "--max-tx-gas")), + strict_valid_pending: match self.args.flag_relay_validity.as_str() { + "cheap" => false, + "strict" => true, + x => die!("{}: Invalid value for --relay-validity option. Use --help for more information.", x) + }, } } diff --git a/parity/main.rs b/parity/main.rs index e3c20caf4..047338bc8 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -214,7 +214,7 @@ fn execute_client(conf: Configuration, spec: Spec, client_config: ClientConfig) miner.set_gas_ceil_target(conf.gas_ceil_target()); miner.set_extra_data(conf.extra_data()); miner.set_minimal_gas_price(conf.gas_price()); - miner.set_transactions_limit(conf.args.flag_tx_limit); + miner.set_transactions_limit(conf.args.flag_tx_queue_size); // Build client let mut service = ClientService::start( From 2a51a30d41ca4c1361b5398400ed2eb60f7f011b Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 19:06:54 +0200 Subject: [PATCH 03/10] Fix up the pending set stuff. --- ethcore/src/client/client.rs | 4 +- ethcore/src/client/mod.rs | 2 +- ethcore/src/client/test_client.rs | 4 +- ethcore/src/miner/miner.rs | 72 ++++++++++++++++++++----------- ethcore/src/miner/mod.rs | 2 +- parity/cli.rs | 13 ++++-- parity/configuration.rs | 11 ++--- sync/src/chain.rs | 2 +- 8 files changed, 70 insertions(+), 40 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 2c4499e57..0082aa4c5 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -792,8 +792,8 @@ impl BlockChainClient for Client where V: Verifier { } } - fn all_transactions(&self) -> Vec { - self.miner.all_transactions() + fn pending_transactions(&self) -> Vec { + self.miner.pending_transactions() } } diff --git a/ethcore/src/client/mod.rs b/ethcore/src/client/mod.rs index 80625ad85..c05874e64 100644 --- a/ethcore/src/client/mod.rs +++ b/ethcore/src/client/mod.rs @@ -197,7 +197,7 @@ pub trait BlockChainClient : Sync + Send { fn queue_transactions(&self, transactions: Vec); /// list all transactions - fn all_transactions(&self) -> Vec; + fn pending_transactions(&self) -> Vec; /// Get the gas price distribution. fn gas_price_statistics(&self, sample_size: usize, distribution_size: usize) -> Result, ()> { diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 812c15655..a7f508a51 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -500,7 +500,7 @@ impl BlockChainClient for TestBlockChainClient { self.import_transactions(tx); } - fn all_transactions(&self) -> Vec { - self.miner.all_transactions() + fn pending_transactions(&self) -> Vec { + self.miner.pending_transactions() } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index e5ecff8c4..72228a959 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -29,6 +29,18 @@ use spec::Spec; use engine::Engine; use miner::{MinerService, MinerStatus, TransactionQueue, AccountDetails, TransactionImportResult, TransactionOrigin}; +/// Different possible definitions for pending transaction set. +#[derive(Debug)] +pub enum PendingSet { + /// Always just the transactions in the queue. These have had only cheap checks. + AlwaysQueue, + /// Always just the transactions in the sealing block. These have had full checks but + /// may be empty if the node is not actively mining or has force_sealing enabled. + AlwaysSealing, + /// Try the sealing block, but if it is not currently sealing, fallback to the queue. + SealingOrElseQueue, +} + /// Configures the behaviour of the miner. #[derive(Debug)] pub struct MinerOptions { @@ -42,7 +54,7 @@ pub struct MinerOptions { /// If `None`, then no limit. pub max_tx_gas: Option, /// Whether we should fallback to providing all the queue's transactions or just pending. - pub strict_valid_pending: bool, + pub pending_set: PendingSet, } impl Default for MinerOptions { @@ -52,7 +64,7 @@ impl Default for MinerOptions { reseal_on_external_tx: true, reseal_on_own_tx: true, max_tx_gas: None, - strict_valid_pending: false, + pending_set: PendingSet::AlwaysQueue, } } } @@ -462,24 +474,6 @@ impl MinerService for Miner { imported } - fn pending_transactions_hashes(&self) -> Vec { - let queue = self.transaction_queue.lock().unwrap(); - match (self.sealing_enabled.load(atomic::Ordering::Relaxed), self.sealing_work.lock().unwrap().peek_last_ref()) { - (true, Some(pending)) => pending.transactions().iter().map(|t| t.hash()).collect(), - _ if self.options.strict_valid_pending => Vec::new(), - _ => queue.pending_hashes(), - } - } - - fn transaction(&self, hash: &H256) -> Option { - let queue = self.transaction_queue.lock().unwrap(); - match (self.sealing_enabled.load(atomic::Ordering::Relaxed), self.sealing_work.lock().unwrap().peek_last_ref()) { - (true, Some(pending)) => pending.transactions().iter().find(|t| &t.hash() == hash).cloned(), - _ if self.options.strict_valid_pending => None, - _ => queue.find(hash), - } - } - fn all_transactions(&self) -> Vec { let queue = self.transaction_queue.lock().unwrap(); queue.top_transactions() @@ -487,11 +481,41 @@ impl MinerService for Miner { fn pending_transactions(&self) -> Vec { let queue = self.transaction_queue.lock().unwrap(); + let sw = self.sealing_work.lock().unwrap(); // TODO: should only use the sealing_work when it's current (it could be an old block) - match (self.sealing_enabled.load(atomic::Ordering::Relaxed), self.sealing_work.lock().unwrap().peek_last_ref()) { - (true, Some(pending)) => pending.transactions().clone(), - _ if self.options.strict_valid_pending => Vec::new(), - _ => queue.top_transactions(), + let sealing_set = match self.sealing_enabled.load(atomic::Ordering::Relaxed) { + true => sw.peek_last_ref(), + false => None, + }; + match (&self.options.pending_set, sealing_set) { + (&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.top_transactions(), + (_, sealing) => sealing.map(|s| s.transactions().clone()).unwrap_or(Vec::new()), + } + } + + fn pending_transactions_hashes(&self) -> Vec { + let queue = self.transaction_queue.lock().unwrap(); + let sw = self.sealing_work.lock().unwrap(); + let sealing_set = match self.sealing_enabled.load(atomic::Ordering::Relaxed) { + true => sw.peek_last_ref(), + false => None, + }; + match (&self.options.pending_set, sealing_set) { + (&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.pending_hashes(), + (_, sealing) => sealing.map(|s| s.transactions().iter().map(|t| t.hash()).collect()).unwrap_or(Vec::new()), + } + } + + fn transaction(&self, hash: &H256) -> Option { + let queue = self.transaction_queue.lock().unwrap(); + let sw = self.sealing_work.lock().unwrap(); + let sealing_set = match self.sealing_enabled.load(atomic::Ordering::Relaxed) { + true => sw.peek_last_ref(), + false => None, + }; + match (&self.options.pending_set, sealing_set) { + (&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.find(hash), + (_, sealing) => sealing.and_then(|s| s.transactions().iter().find(|t| &t.hash() == hash).cloned()), } } diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index 8a282490c..f02925438 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -47,7 +47,7 @@ mod external; mod transaction_queue; pub use self::transaction_queue::{TransactionQueue, AccountDetails, TransactionImportResult, TransactionOrigin}; -pub use self::miner::{Miner, MinerOptions}; +pub use self::miner::{Miner, MinerOptions, PendingSet}; pub use self::external::{ExternalMiner, ExternalMinerService}; use std::collections::BTreeMap; diff --git a/parity/cli.rs b/parity/cli.rs index 3090e83c5..2fe9186f5 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -141,9 +141,14 @@ Sealing/Mining Options: all - reseal on all new transactions [default: all]. --max-tx-gas GAS Apply a limit of GAS as the maximum amount of gas a single transaction may have for it to be mined. - --relay-validity REQ Requirements for relaying. REQ may be: - cheap - Relay only after cheap checks; - strict - Relay only once executed [default: cheap]. + --relay-set SET Set of transactions to relay. SET may be: + cheap - Relay any transaction in the queue (this + may include invalid transactions); + strict - Relay only executed transactions (this + guarantees we don't relay invalid transactions, but + means we relay nothing if not mining); + lenient - Same as struct when mining, and cheap + when not [default: cheap]. --usd-per-tx USD Amount of USD to be paid for a basic transaction [default: 0.005]. The minimum gas price is set accordingly. @@ -296,7 +301,7 @@ pub struct Args { pub flag_force_sealing: bool, pub flag_reseal_on_txs: String, pub flag_max_tx_gas: Option, - pub flag_relay_validity: String, + pub flag_relay_set: String, pub flag_author: Option, pub flag_usd_per_tx: String, pub flag_usd_per_eth: String, diff --git a/parity/configuration.rs b/parity/configuration.rs index 57c62f839..501cc68cb 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -27,7 +27,7 @@ use util::*; use ethcore::account_provider::AccountProvider; use util::network_settings::NetworkSettings; use ethcore::client::{append_path, get_db_path, ClientConfig, Switch, VMType}; -use ethcore::miner::MinerOptions; +use ethcore::miner::{MinerOptions, PendingSet}; use ethcore::ethereum; use ethcore::spec::Spec; use ethsync::SyncConfig; @@ -89,10 +89,11 @@ impl Configuration { reseal_on_external_tx: ext, reseal_on_own_tx: own, max_tx_gas: self.args.flag_max_tx_gas.as_ref().map(|d| Self::decode_u256(d, "--max-tx-gas")), - strict_valid_pending: match self.args.flag_relay_validity.as_str() { - "cheap" => false, - "strict" => true, - x => die!("{}: Invalid value for --relay-validity option. Use --help for more information.", x) + pending_set: match self.args.flag_relay_set.as_str() { + "cheap" => PendingSet::AlwaysQueue, + "strict" => PendingSet::AlwaysSealing, + "lenient" => PendingSet::SealingOrElseQueue, + x => die!("{}: Invalid value for --relay-set option. Use --help for more information.", x) }, } } diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 5d0976aac..aa3657419 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -1314,7 +1314,7 @@ impl ChainSync { return 0; } - let mut transactions = io.chain().all_transactions(); + let mut transactions = io.chain().pending_transactions(); if transactions.is_empty() { return 0; } From a102015ecf253a80f7431f46177c1846e712cc61 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 19:16:26 +0200 Subject: [PATCH 04/10] Fix doc test. --- parity/cli.rs | 4 ++-- rpc/src/v1/tests/eth.rs | 4 +++- sync/src/lib.rs | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/parity/cli.rs b/parity/cli.rs index 2fe9186f5..86ab2037d 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -133,8 +133,8 @@ Sealing/Mining Options: NOTE: MINING WILL NOT WORK WITHOUT THIS OPTION. --force-sealing Force the node to author new blocks as if it were always sealing/mining. - --reseal-on-txs Specify which transactions should force the node - to reseal a block. One of: + --reseal-on-txs SET Specify which transactions should force the node + to reseal a block. SET is one of: none - never reseal on new transactions; own - reseal only on a new local transaction; ext - reseal only on a new external transaction; diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 2ce482a90..1a0cc26c8 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -24,7 +24,7 @@ use ethcore::spec::{Genesis, Spec}; use ethcore::block::Block; use ethcore::views::BlockView; use ethcore::ethereum; -use ethcore::miner::{MinerOptions, MinerService, ExternalMiner, Miner}; +use ethcore::miner::{MinerOptions, MinerService, ExternalMiner, Miner, PendingSet}; use ethcore::account_provider::AccountProvider; use devtools::RandomTempPath; use util::Hashable; @@ -54,6 +54,8 @@ fn miner_service(spec: Spec, accounts: Arc) -> Arc { force_sealing: true, reseal_on_external_tx: true, reseal_on_own_tx: true, + max_tx_gas: None, + pending_set: PendingSet::SealingOrElseQueue, }, spec, Some(accounts) diff --git a/sync/src/lib.rs b/sync/src/lib.rs index 928496326..ced1bf6a2 100644 --- a/sync/src/lib.rs +++ b/sync/src/lib.rs @@ -44,7 +44,7 @@ //! let mut service = NetworkService::new(NetworkConfiguration::new()).unwrap(); //! service.start().unwrap(); //! let dir = env::temp_dir(); -//! let miner = Miner::new(MinerOptions::default(), ethereum::new_frontier(true), None); +//! let miner = Miner::new(Default::default(), ethereum::new_frontier(true), None); //! let client = Client::new( //! ClientConfig::default(), //! ethereum::new_frontier(true), From 10aa32b0f5eda6a96995558db5dc8ef22e672a46 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 20:19:01 +0200 Subject: [PATCH 05/10] Include RPC configurability for max tx gas limit. Also Move the gas limit into the transaction queue from the miner. --- ethcore/src/miner/miner.rs | 9 ++++- ethcore/src/miner/mod.rs | 3 ++ ethcore/src/miner/transaction_queue.rs | 40 ++++++++++++++++------- parity/configuration.rs | 1 + rpc/src/v1/impls/ethcore_set.rs | 9 ++++- rpc/src/v1/tests/helpers/miner_service.rs | 6 ++++ rpc/src/v1/traits/ethcore_set.rs | 4 +++ rpc/src/v1/types/optionals.rs | 26 +++++++++++++-- 8 files changed, 82 insertions(+), 16 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 72228a959..42971850a 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -53,6 +53,8 @@ pub struct MinerOptions { /// Specify maximum amount of gas to bother considering for block insertion. /// If `None`, then no limit. pub max_tx_gas: Option, + /// Maximum size of the transaction queue. + pub tx_queue_size: usize, /// Whether we should fallback to providing all the queue's transactions or just pending. pub pending_set: PendingSet, } @@ -64,6 +66,7 @@ impl Default for MinerOptions { reseal_on_external_tx: true, reseal_on_own_tx: true, max_tx_gas: None, + tx_queue_size: 1024, pending_set: PendingSet::AlwaysQueue, } } @@ -107,7 +110,7 @@ impl Miner { /// Creates new instance of miner pub fn new(options: MinerOptions, spec: Spec, accounts: Option>) -> Arc { Arc::new(Miner { - transaction_queue: Mutex::new(TransactionQueue::new()), + transaction_queue: Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, options.max_tx_gas)), sealing_enabled: AtomicBool::new(options.force_sealing), options: options, sealing_block_last_request: Mutex::new(0), @@ -396,6 +399,10 @@ impl MinerService for Miner { self.transaction_queue.lock().unwrap().set_limit(limit) } + fn set_tx_gas_limit(&self, limit: Option) { + self.transaction_queue.lock().unwrap().set_tx_gas_limit(limit) + } + /// Get the author that we will seal blocks as. fn author(&self) -> Address { *self.author.read().unwrap() diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index f02925438..a56f881a4 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -101,6 +101,9 @@ pub trait MinerService : Send + Sync { /// Set maximal number of transactions kept in the queue (both current and future). fn set_transactions_limit(&self, limit: usize); + /// Set maximum amount of gas allowed for any single transaction to mine. + fn set_tx_gas_limit(&self, limit: Option); + /// Imports transactions to transaction queue. fn import_transactions(&self, chain: &MiningBlockChainClient, transactions: Vec, fetch_account: T) -> Vec> diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index 9159cc6b3..26ec441fe 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -334,6 +334,8 @@ const GAS_LIMIT_HYSTERESIS: usize = 10; // % pub struct TransactionQueue { /// Gas Price threshold for transactions that can be imported to this queue (defaults to 0) minimal_gas_price: U256, + /// If `Some`, then the maximum amount of gas any individual transaction may use. + tx_gas_limit: Option, /// Current gas limit (block gas limit * factor). Transactions above the limit will not be accepted (default to !0) gas_limit: U256, /// Priority queue for transactions that can go to block @@ -355,11 +357,11 @@ impl Default for TransactionQueue { impl TransactionQueue { /// Creates new instance of this Queue pub fn new() -> Self { - Self::with_limit(1024) + Self::with_limits(1024, None) } /// Create new instance of this Queue with specified limits - pub fn with_limit(limit: usize) -> Self { + pub fn with_limits(limit: usize, tx_gas_limit: Option) -> Self { let current = TransactionSet { by_priority: BTreeSet::new(), by_address: Table::new(), @@ -374,6 +376,7 @@ impl TransactionQueue { TransactionQueue { minimal_gas_price: U256::zero(), + tx_gas_limit: tx_gas_limit, gas_limit: !U256::zero(), current: current, future: future, @@ -418,6 +421,12 @@ impl TransactionQueue { }; } + /// Set the new limit for the amount of gas any individual transaction may have. + /// Any transaction already imported to the queue is not affected. + pub fn set_tx_gas_limit(&mut self, limit: Option) { + self.tx_gas_limit = limit; + } + /// Returns current status for this queue pub fn status(&self) -> TransactionQueueStatus { TransactionQueueStatus { @@ -435,7 +444,9 @@ impl TransactionQueue { if tx.gas_price < self.minimal_gas_price { trace!(target: "miner", "Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})", - tx.hash(), tx.gas_price, self.minimal_gas_price + tx.hash(), + tx.gas_price, + self.minimal_gas_price ); return Err(Error::Transaction(TransactionError::InsufficientGasPrice { @@ -446,10 +457,12 @@ impl TransactionQueue { try!(tx.check_low_s()); - if tx.gas > self.gas_limit { + if tx.gas > self.gas_limit || self.tx_gas_limit.map(|l| tx.gas > l).unwrap_or(false) { trace!(target: "miner", "Dropping transaction above gas limit: {:?} ({} > {})", - tx.hash(), tx.gas, self.gas_limit + tx.hash(), + tx.gas, + self.gas_limit ); return Err(Error::Transaction(TransactionError::GasLimitExceeded { @@ -463,8 +476,13 @@ impl TransactionQueue { let cost = vtx.transaction.value + vtx.transaction.gas_price * vtx.transaction.gas; if client_account.balance < cost { - trace!(target: "miner", "Dropping transaction without sufficient balance: {:?} ({} < {})", - vtx.hash(), client_account.balance, cost); + trace!(target: "miner", + "Dropping transaction without sufficient balance: {:?} ({} < {})", + vtx.hash(), + client_account.balance, + cost + ); + return Err(Error::Transaction(TransactionError::InsufficientBalance { cost: cost, balance: client_account.balance @@ -1304,7 +1322,7 @@ mod test { #[test] fn should_drop_old_transactions_when_hitting_the_limit() { // given - let mut txq = TransactionQueue::with_limit(1); + let mut txq = TransactionQueue::with_limit(1, None); let (tx, tx2) = new_txs(U256::one()); let sender = tx.sender().unwrap(); let nonce = tx.nonce; @@ -1326,7 +1344,7 @@ mod test { #[test] fn should_return_correct_nonces_when_dropped_because_of_limit() { // given - let mut txq = TransactionQueue::with_limit(2); + let mut txq = TransactionQueue::with_limit(2, None); let tx = new_tx(); let (tx1, tx2) = new_txs(U256::one()); let sender = tx1.sender().unwrap(); @@ -1347,7 +1365,7 @@ mod test { #[test] fn should_limit_future_transactions() { - let mut txq = TransactionQueue::with_limit(1); + let mut txq = TransactionQueue::with_limit(1, None); txq.current.set_limit(10); let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1)); let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2)); @@ -1607,7 +1625,7 @@ mod test { #[test] fn should_keep_right_order_in_future() { // given - let mut txq = TransactionQueue::with_limit(1); + let mut txq = TransactionQueue::with_limit(1, None); let (tx1, tx2) = new_txs(U256::from(1)); let prev_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce - U256::one(), balance: default_nonce(a).balance }; diff --git a/parity/configuration.rs b/parity/configuration.rs index 501cc68cb..4469eefaf 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -89,6 +89,7 @@ impl Configuration { reseal_on_external_tx: ext, reseal_on_own_tx: own, max_tx_gas: self.args.flag_max_tx_gas.as_ref().map(|d| Self::decode_u256(d, "--max-tx-gas")), + tx_queue_size: self.args.flag_tx_queue_size, pending_set: match self.args.flag_relay_set.as_str() { "cheap" => PendingSet::AlwaysQueue, "strict" => PendingSet::AlwaysSealing, diff --git a/rpc/src/v1/impls/ethcore_set.rs b/rpc/src/v1/impls/ethcore_set.rs index 55164217c..d7624442a 100644 --- a/rpc/src/v1/impls/ethcore_set.rs +++ b/rpc/src/v1/impls/ethcore_set.rs @@ -22,7 +22,7 @@ use jsonrpc_core::*; use ethcore::miner::MinerService; use ethcore::service::SyncMessage; use v1::traits::EthcoreSet; -use v1::types::{Bytes}; +use v1::types::{OptionalValue, Bytes}; /// Ethcore-specific rpc interface for operations altering the settings. pub struct EthcoreSetClient where @@ -86,6 +86,13 @@ impl EthcoreSet for EthcoreSetClient where M: MinerService + 'static { }) } + fn set_tx_gas_limit(&self, params: Params) -> Result { + from_params::<(OptionalValue,)>(params).and_then(|(limit,)| { + take_weak!(self.miner).set_tx_gas_limit(limit.into()); + to_value(&true) + }) + } + fn add_reserved_peer(&self, params: Params) -> Result { from_params::<(String,)>(params).and_then(|(peer,)| { match take_weak!(self.net).add_reserved_peer(&peer) { diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index b87b650e1..4a9b22f42 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -43,6 +43,7 @@ pub struct TestMinerService { author: RwLock
, extra_data: RwLock, limit: RwLock, + tx_gas_limit: RwLock>, } impl Default for TestMinerService { @@ -58,6 +59,7 @@ impl Default for TestMinerService { author: RwLock::new(Address::zero()), extra_data: RwLock::new(vec![1, 2, 3, 4]), limit: RwLock::new(1024), + tx_gas_limit: RwLock::new(None), } } } @@ -99,6 +101,10 @@ impl MinerService for TestMinerService { *self.limit.write().unwrap() = limit; } + fn set_tx_gas_limit(&self, limit: Option) { + *self.tx_gas_limit.write().unwrap() = limit; + } + fn transactions_limit(&self) -> usize { *self.limit.read().unwrap() } diff --git a/rpc/src/v1/traits/ethcore_set.rs b/rpc/src/v1/traits/ethcore_set.rs index d00295036..8afbcdab9 100644 --- a/rpc/src/v1/traits/ethcore_set.rs +++ b/rpc/src/v1/traits/ethcore_set.rs @@ -40,6 +40,9 @@ pub trait EthcoreSet: Sized + Send + Sync + 'static { /// Sets the limits for transaction queue. fn set_transactions_limit(&self, _: Params) -> Result; + /// Sets the maximum amount of gas a single transaction may consume. + fn set_tx_gas_limit(&self, _: Params) -> Result; + /// Add a reserved peer. fn add_reserved_peer(&self, _: Params) -> Result; @@ -60,6 +63,7 @@ pub trait EthcoreSet: Sized + Send + Sync + 'static { delegate.add_method("ethcore_setGasCeilTarget", EthcoreSet::set_gas_ceil_target); delegate.add_method("ethcore_setExtraData", EthcoreSet::set_extra_data); delegate.add_method("ethcore_setAuthor", EthcoreSet::set_author); + delegate.add_method("ethcore_setMaxTransactionGas", EthcoreSet::set_tx_gas_limit); delegate.add_method("ethcore_setTransactionsLimit", EthcoreSet::set_transactions_limit); delegate.add_method("ethcore_addReservedPeer", EthcoreSet::add_reserved_peer); delegate.add_method("ethcore_removeReservedPeer", EthcoreSet::remove_reserved_peer); diff --git a/rpc/src/v1/types/optionals.rs b/rpc/src/v1/types/optionals.rs index 2ed272ade..5f62dc4a0 100644 --- a/rpc/src/v1/types/optionals.rs +++ b/rpc/src/v1/types/optionals.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use serde::{Serialize, Serializer}; +use serde::{Serialize, Serializer, Deserialize, Deserializer}; use serde_json::Value; /// Optional value @@ -26,13 +26,22 @@ pub enum OptionalValue where T: Serialize { Null } -impl Default for OptionalValue where T: Serialize { +impl Default for OptionalValue where T: Serialize + Deserialize { fn default() -> Self { OptionalValue::Null } } -impl Serialize for OptionalValue where T: Serialize { +impl Into> for OptionalValue where T: Serialize + Deserialize { + fn into(self) -> Option { + match self { + OptionalValue::Null => None, + OptionalValue::Value(t) => Some(t), + } + } +} + +impl Serialize for OptionalValue where T: Serialize + Deserialize { fn serialize(&self, serializer: &mut S) -> Result<(), S::Error> where S: Serializer { match *self { @@ -42,6 +51,17 @@ impl Serialize for OptionalValue where T: Serialize { } } +impl Deserialize for OptionalValue where T: Serialize + Deserialize { + fn deserialize(deserializer: &mut D) -> Result, D::Error> + where D: Deserializer { + let deser_result: Result = Deserialize::deserialize(deserializer); + match deser_result { + Ok(t) => Ok(OptionalValue::Value(t)), + Err(_) => Ok(OptionalValue::Null), + } + } +} + #[cfg(test)] mod tests { use serde_json; From 5919c660e570d79f3029589db230ba358d774c08 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 21:06:10 +0200 Subject: [PATCH 06/10] Fix typo [ci:skip] --- parity/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parity/cli.rs b/parity/cli.rs index 86ab2037d..3fd802658 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -147,7 +147,7 @@ Sealing/Mining Options: strict - Relay only executed transactions (this guarantees we don't relay invalid transactions, but means we relay nothing if not mining); - lenient - Same as struct when mining, and cheap + lenient - Same as strict when mining, and cheap when not [default: cheap]. --usd-per-tx USD Amount of USD to be paid for a basic transaction [default: 0.005]. The minimum gas price is set From ff788e4199d4143a55aafeea27757a73e1808c7d Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 21:06:40 +0200 Subject: [PATCH 07/10] Fix another typo [ci:skip] --- parity/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parity/cli.rs b/parity/cli.rs index 3fd802658..3eeae7fae 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -162,7 +162,7 @@ Sealing/Mining Options: block due to transaction volume [default: 3141592]. --extra-data STRING Specify a custom extra-data for authored blocks, no more than 32 characters. - --tx-queue-size LIMIT Maxmimum amount of transactions in the queue (waiting + --tx-queue-size LIMIT Maximum amount of transactions in the queue (waiting to be included in next block) [default: 1024]. Footprint Options: From c221f69ccdf2f811fa0f593ee77d52ae6b400d25 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 28 Jun 2016 10:00:28 +0200 Subject: [PATCH 08/10] Clean up some of the FP stuff. --- ethcore/src/miner/miner.rs | 4 ++-- ethcore/src/miner/transaction_queue.rs | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 42971850a..20c9af3f7 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -496,7 +496,7 @@ impl MinerService for Miner { }; match (&self.options.pending_set, sealing_set) { (&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.top_transactions(), - (_, sealing) => sealing.map(|s| s.transactions().clone()).unwrap_or(Vec::new()), + (_, sealing) => sealing.map_or_else(Vec::new, |s| s.transactions().clone()), } } @@ -509,7 +509,7 @@ impl MinerService for Miner { }; match (&self.options.pending_set, sealing_set) { (&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.pending_hashes(), - (_, sealing) => sealing.map(|s| s.transactions().iter().map(|t| t.hash()).collect()).unwrap_or(Vec::new()), + (_, sealing) => sealing.map_or_else(Vec::new, |s| s.transactions().iter().map(|t| t.hash()).collect()), } } diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index 26ec441fe..6ebbf0f6a 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -459,10 +459,11 @@ impl TransactionQueue { if tx.gas > self.gas_limit || self.tx_gas_limit.map(|l| tx.gas > l).unwrap_or(false) { trace!(target: "miner", - "Dropping transaction above gas limit: {:?} ({} > {})", + "Dropping transaction above gas limit: {:?} ({} > min({}, {:?}))", tx.hash(), tx.gas, - self.gas_limit + self.gas_limit, + self.tx_gas_limit ); return Err(Error::Transaction(TransactionError::GasLimitExceeded { @@ -606,7 +607,7 @@ impl TransactionQueue { self.current.by_priority .iter() .map(|t| self.by_hash.get(&t.hash).expect("All transactions in `current` and `future` are always included in `by_hash`")) - .filter_map(|t| if &t.transaction.gas <= max_tx_gas { Some(t.transaction.clone()) } else { None }) + .map(|t| &t.transaction).filter(|t| t.gas <= *max_tx_gas).cloned() .collect() } From 31de739122d4ee86ba6b1b186cf03d3eb6105f72 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 28 Jun 2016 10:21:29 +0200 Subject: [PATCH 09/10] U256 instead of Option. Fix up tests. --- ethcore/src/miner/miner.rs | 11 +++---- ethcore/src/miner/mod.rs | 2 +- ethcore/src/miner/transaction_queue.rs | 38 +++++++---------------- parity/configuration.rs | 2 +- rpc/src/v1/impls/ethcore_set.rs | 4 +-- rpc/src/v1/tests/eth.rs | 5 +-- rpc/src/v1/tests/helpers/miner_service.rs | 6 ++-- 7 files changed, 26 insertions(+), 42 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 20c9af3f7..2623ce3d9 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -50,9 +50,8 @@ pub struct MinerOptions { pub reseal_on_external_tx: bool, /// Reseal on receipt of new local transactions. pub reseal_on_own_tx: bool, - /// Specify maximum amount of gas to bother considering for block insertion. - /// If `None`, then no limit. - pub max_tx_gas: Option, + /// Maximum amount of gas to bother considering for block insertion. + pub max_tx_gas: U256, /// Maximum size of the transaction queue. pub tx_queue_size: usize, /// Whether we should fallback to providing all the queue's transactions or just pending. @@ -65,7 +64,7 @@ impl Default for MinerOptions { force_sealing: false, reseal_on_external_tx: true, reseal_on_own_tx: true, - max_tx_gas: None, + max_tx_gas: !U256::zero(), tx_queue_size: 1024, pending_set: PendingSet::AlwaysQueue, } @@ -134,7 +133,7 @@ impl Miner { trace!(target: "miner", "prepare_sealing: entering"); let (transactions, mut open_block) = { - let transactions = {self.transaction_queue.lock().unwrap().top_transactions_maybe_limit(&self.options.max_tx_gas)}; + let transactions = {self.transaction_queue.lock().unwrap().top_transactions()}; let mut sealing_work = self.sealing_work.lock().unwrap(); let best_hash = chain.best_block_header().sha3(); /* @@ -399,7 +398,7 @@ impl MinerService for Miner { self.transaction_queue.lock().unwrap().set_limit(limit) } - fn set_tx_gas_limit(&self, limit: Option) { + fn set_tx_gas_limit(&self, limit: U256) { self.transaction_queue.lock().unwrap().set_tx_gas_limit(limit) } diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index a56f881a4..e65d6048a 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -102,7 +102,7 @@ pub trait MinerService : Send + Sync { fn set_transactions_limit(&self, limit: usize); /// Set maximum amount of gas allowed for any single transaction to mine. - fn set_tx_gas_limit(&self, limit: Option); + fn set_tx_gas_limit(&self, limit: U256); /// Imports transactions to transaction queue. fn import_transactions(&self, chain: &MiningBlockChainClient, transactions: Vec, fetch_account: T) -> diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index 6ebbf0f6a..17ca18272 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -334,8 +334,8 @@ const GAS_LIMIT_HYSTERESIS: usize = 10; // % pub struct TransactionQueue { /// Gas Price threshold for transactions that can be imported to this queue (defaults to 0) minimal_gas_price: U256, - /// If `Some`, then the maximum amount of gas any individual transaction may use. - tx_gas_limit: Option, + /// The maximum amount of gas any individual transaction may use. + tx_gas_limit: U256, /// Current gas limit (block gas limit * factor). Transactions above the limit will not be accepted (default to !0) gas_limit: U256, /// Priority queue for transactions that can go to block @@ -357,11 +357,11 @@ impl Default for TransactionQueue { impl TransactionQueue { /// Creates new instance of this Queue pub fn new() -> Self { - Self::with_limits(1024, None) + Self::with_limits(1024, !U256::zero()) } /// Create new instance of this Queue with specified limits - pub fn with_limits(limit: usize, tx_gas_limit: Option) -> Self { + pub fn with_limits(limit: usize, tx_gas_limit: U256) -> Self { let current = TransactionSet { by_priority: BTreeSet::new(), by_address: Table::new(), @@ -423,7 +423,7 @@ impl TransactionQueue { /// Set the new limit for the amount of gas any individual transaction may have. /// Any transaction already imported to the queue is not affected. - pub fn set_tx_gas_limit(&mut self, limit: Option) { + pub fn set_tx_gas_limit(&mut self, limit: U256) { self.tx_gas_limit = limit; } @@ -457,9 +457,9 @@ impl TransactionQueue { try!(tx.check_low_s()); - if tx.gas > self.gas_limit || self.tx_gas_limit.map(|l| tx.gas > l).unwrap_or(false) { + if tx.gas > self.gas_limit || tx.gas > self.tx_gas_limit { trace!(target: "miner", - "Dropping transaction above gas limit: {:?} ({} > min({}, {:?}))", + "Dropping transaction above gas limit: {:?} ({} > min({}, {}))", tx.hash(), tx.gas, self.gas_limit, @@ -602,22 +602,6 @@ impl TransactionQueue { .collect() } - /// Returns top transactions from the queue ordered by priority with a maximum gas limit. - pub fn top_transactions_with_limit(&self, max_tx_gas: &U256) -> Vec { - self.current.by_priority - .iter() - .map(|t| self.by_hash.get(&t.hash).expect("All transactions in `current` and `future` are always included in `by_hash`")) - .map(|t| &t.transaction).filter(|t| t.gas <= *max_tx_gas).cloned() - .collect() - } - - /// Returns top transactions from the queue ordered by priority with a maximum gas limit. - pub fn top_transactions_maybe_limit(&self, max_tx_gas: &Option) -> Vec { - match *max_tx_gas { - None => self.top_transactions(), - Some(ref m) => self.top_transactions_with_limit(m), - } - } /// Returns hashes of all transactions from current, ordered by priority. pub fn pending_hashes(&self) -> Vec { self.current.by_priority @@ -1323,7 +1307,7 @@ mod test { #[test] fn should_drop_old_transactions_when_hitting_the_limit() { // given - let mut txq = TransactionQueue::with_limit(1, None); + let mut txq = TransactionQueue::with_limits(1, !U256::zero()); let (tx, tx2) = new_txs(U256::one()); let sender = tx.sender().unwrap(); let nonce = tx.nonce; @@ -1345,7 +1329,7 @@ mod test { #[test] fn should_return_correct_nonces_when_dropped_because_of_limit() { // given - let mut txq = TransactionQueue::with_limit(2, None); + let mut txq = TransactionQueue::with_limits(2, !U256::zero()); let tx = new_tx(); let (tx1, tx2) = new_txs(U256::one()); let sender = tx1.sender().unwrap(); @@ -1366,7 +1350,7 @@ mod test { #[test] fn should_limit_future_transactions() { - let mut txq = TransactionQueue::with_limit(1, None); + let mut txq = TransactionQueue::with_limits(1, !U256::zero()); txq.current.set_limit(10); let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1)); let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2)); @@ -1626,7 +1610,7 @@ mod test { #[test] fn should_keep_right_order_in_future() { // given - let mut txq = TransactionQueue::with_limit(1, None); + let mut txq = TransactionQueue::with_limits(1, !U256::zero()); let (tx1, tx2) = new_txs(U256::from(1)); let prev_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce - U256::one(), balance: default_nonce(a).balance }; diff --git a/parity/configuration.rs b/parity/configuration.rs index d5f41c471..acae3b5f4 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -88,7 +88,7 @@ impl Configuration { force_sealing: self.args.flag_force_sealing, reseal_on_external_tx: ext, reseal_on_own_tx: own, - max_tx_gas: self.args.flag_max_tx_gas.as_ref().map(|d| Self::decode_u256(d, "--max-tx-gas")), + max_tx_gas: self.args.flag_max_tx_gas.as_ref().map_or(!U256::zero(), |d| Self::decode_u256(d, "--max-tx-gas")), tx_queue_size: self.args.flag_tx_queue_size, pending_set: match self.args.flag_relay_set.as_str() { "cheap" => PendingSet::AlwaysQueue, diff --git a/rpc/src/v1/impls/ethcore_set.rs b/rpc/src/v1/impls/ethcore_set.rs index d7624442a..baf5bf134 100644 --- a/rpc/src/v1/impls/ethcore_set.rs +++ b/rpc/src/v1/impls/ethcore_set.rs @@ -22,7 +22,7 @@ use jsonrpc_core::*; use ethcore::miner::MinerService; use ethcore::service::SyncMessage; use v1::traits::EthcoreSet; -use v1::types::{OptionalValue, Bytes}; +use v1::types::Bytes; /// Ethcore-specific rpc interface for operations altering the settings. pub struct EthcoreSetClient where @@ -87,7 +87,7 @@ impl EthcoreSet for EthcoreSetClient where M: MinerService + 'static { } fn set_tx_gas_limit(&self, params: Params) -> Result { - from_params::<(OptionalValue,)>(params).and_then(|(limit,)| { + from_params::<(U256,)>(params).and_then(|(limit,)| { take_weak!(self.miner).set_tx_gas_limit(limit.into()); to_value(&true) }) diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 1a0cc26c8..e3aed553f 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -29,7 +29,7 @@ use ethcore::account_provider::AccountProvider; use devtools::RandomTempPath; use util::Hashable; use util::io::IoChannel; -use util::{U256, H256}; +use util::{U256, H256, Uint}; use jsonrpc_core::IoHandler; use ethjson::blockchain::BlockChain; @@ -54,7 +54,8 @@ fn miner_service(spec: Spec, accounts: Arc) -> Arc { force_sealing: true, reseal_on_external_tx: true, reseal_on_own_tx: true, - max_tx_gas: None, + tx_queue_size: 1024, + max_tx_gas: !U256::zero(), pending_set: PendingSet::SealingOrElseQueue, }, spec, diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 4a9b22f42..6329be7bd 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -43,7 +43,7 @@ pub struct TestMinerService { author: RwLock
, extra_data: RwLock, limit: RwLock, - tx_gas_limit: RwLock>, + tx_gas_limit: RwLock, } impl Default for TestMinerService { @@ -59,7 +59,7 @@ impl Default for TestMinerService { author: RwLock::new(Address::zero()), extra_data: RwLock::new(vec![1, 2, 3, 4]), limit: RwLock::new(1024), - tx_gas_limit: RwLock::new(None), + tx_gas_limit: RwLock::new(!U256::zero()), } } } @@ -101,7 +101,7 @@ impl MinerService for TestMinerService { *self.limit.write().unwrap() = limit; } - fn set_tx_gas_limit(&self, limit: Option) { + fn set_tx_gas_limit(&self, limit: U256) { *self.tx_gas_limit.write().unwrap() = limit; } From 599a6104b7fd794a132316466995720976f163bd Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 28 Jun 2016 10:40:35 +0200 Subject: [PATCH 10/10] Minor renaming. --- ethcore/src/miner/miner.rs | 6 +++--- parity/cli.rs | 4 ++-- parity/configuration.rs | 2 +- rpc/src/v1/tests/eth.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 2623ce3d9..803706c56 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -51,7 +51,7 @@ pub struct MinerOptions { /// Reseal on receipt of new local transactions. pub reseal_on_own_tx: bool, /// Maximum amount of gas to bother considering for block insertion. - pub max_tx_gas: U256, + pub tx_gas_limit: U256, /// Maximum size of the transaction queue. pub tx_queue_size: usize, /// Whether we should fallback to providing all the queue's transactions or just pending. @@ -64,7 +64,7 @@ impl Default for MinerOptions { force_sealing: false, reseal_on_external_tx: true, reseal_on_own_tx: true, - max_tx_gas: !U256::zero(), + tx_gas_limit: !U256::zero(), tx_queue_size: 1024, pending_set: PendingSet::AlwaysQueue, } @@ -109,7 +109,7 @@ impl Miner { /// Creates new instance of miner pub fn new(options: MinerOptions, spec: Spec, accounts: Option>) -> Arc { Arc::new(Miner { - transaction_queue: Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, options.max_tx_gas)), + transaction_queue: Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, options.tx_gas_limit)), sealing_enabled: AtomicBool::new(options.force_sealing), options: options, sealing_block_last_request: Mutex::new(0), diff --git a/parity/cli.rs b/parity/cli.rs index 4999caf65..3ee6f31c8 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -139,7 +139,7 @@ Sealing/Mining Options: own - reseal only on a new local transaction; ext - reseal only on a new external transaction; all - reseal on all new transactions [default: all]. - --max-tx-gas GAS Apply a limit of GAS as the maximum amount of gas + --tx-gas-limit GAS Apply a limit of GAS as the maximum amount of gas a single transaction may have for it to be mined. --relay-set SET Set of transactions to relay. SET may be: cheap - Relay any transaction in the queue (this @@ -305,7 +305,7 @@ pub struct Args { pub flag_no_token: bool, pub flag_force_sealing: bool, pub flag_reseal_on_txs: String, - pub flag_max_tx_gas: Option, + pub flag_tx_gas_limit: Option, pub flag_relay_set: String, pub flag_author: Option, pub flag_usd_per_tx: String, diff --git a/parity/configuration.rs b/parity/configuration.rs index acae3b5f4..4bd0cd493 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -88,7 +88,7 @@ impl Configuration { force_sealing: self.args.flag_force_sealing, reseal_on_external_tx: ext, reseal_on_own_tx: own, - max_tx_gas: self.args.flag_max_tx_gas.as_ref().map_or(!U256::zero(), |d| Self::decode_u256(d, "--max-tx-gas")), + tx_gas_limit: self.args.flag_tx_gas_limit.as_ref().map_or(!U256::zero(), |d| Self::decode_u256(d, "--tx-gas-limit")), tx_queue_size: self.args.flag_tx_queue_size, pending_set: match self.args.flag_relay_set.as_str() { "cheap" => PendingSet::AlwaysQueue, diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index e3aed553f..88a12a74f 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -55,7 +55,7 @@ fn miner_service(spec: Spec, accounts: Arc) -> Arc { reseal_on_external_tx: true, reseal_on_own_tx: true, tx_queue_size: 1024, - max_tx_gas: !U256::zero(), + tx_gas_limit: !U256::zero(), pending_set: PendingSet::SealingOrElseQueue, }, spec,