From 6c1802e412ab8de8bf9f0001a3a4280fce9c226a Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 27 Jun 2016 17:23:54 +0200 Subject: [PATCH 01/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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, From 4f56f8b27c5e2cd0d0b1a2b2245e1454621c18a4 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Tue, 28 Jun 2016 11:52:59 +0200 Subject: [PATCH 11/12] removed unsafe code (#1466) --- util/bigint/src/uint.rs | 10 +--------- util/src/hash.rs | 11 +++-------- util/src/rlp/bytes.rs | 9 +-------- 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/util/bigint/src/uint.rs b/util/bigint/src/uint.rs index 3e629032e..2b9863135 100644 --- a/util/bigint/src/uint.rs +++ b/util/bigint/src/uint.rs @@ -557,7 +557,7 @@ macro_rules! construct_uint { ($name:ident, $n_words:expr) => ( /// Little-endian large integer type #[repr(C)] - #[derive(Copy, Clone, Eq, PartialEq)] + #[derive(Copy, Clone, Eq, PartialEq, Hash)] pub struct $name(pub [u64; $n_words]); impl Uint for $name { @@ -1126,14 +1126,6 @@ macro_rules! construct_uint { Ok(()) } } - - #[cfg_attr(feature="dev", allow(derive_hash_xor_eq))] // We are pretty sure it's ok. - impl Hash for $name { - fn hash(&self, state: &mut H) where H: Hasher { - unsafe { state.write(::std::slice::from_raw_parts(self.0.as_ptr() as *mut u8, self.0.len() * 8)); } - state.finish(); - } - } ); } diff --git a/util/src/hash.rs b/util/src/hash.rs index 6c1f8b2a4..16f0bde9e 100644 --- a/util/src/hash.rs +++ b/util/src/hash.rs @@ -132,15 +132,10 @@ macro_rules! impl_hash { $size } - // TODO: remove once slice::clone_from_slice is stable #[inline] fn clone_from_slice(&mut self, src: &[u8]) -> usize { - let min = ::std::cmp::min($size, src.len()); - let dst = &mut self.deref_mut()[.. min]; - let src = &src[.. min]; - for i in 0..min { - dst[i] = src[i]; - } + let min = cmp::min($size, src.len()); + self.0[..min].copy_from_slice(&src[..min]); min } @@ -151,7 +146,7 @@ macro_rules! impl_hash { } fn copy_to(&self, dest: &mut[u8]) { - let min = ::std::cmp::min($size, dest.len()); + let min = cmp::min($size, dest.len()); dest[..min].copy_from_slice(&self.0[..min]); } diff --git a/util/src/rlp/bytes.rs b/util/src/rlp/bytes.rs index 1145ba27e..d252af828 100644 --- a/util/src/rlp/bytes.rs +++ b/util/src/rlp/bytes.rs @@ -258,14 +258,7 @@ impl FromBytes for T where T: FixedHash { Ordering::Equal => () }; - unsafe { - use std::{mem, ptr}; - - let mut res: T = mem::uninitialized(); - ptr::copy(bytes.as_ptr(), res.as_slice_mut().as_mut_ptr(), T::len()); - - Ok(res) - } + Ok(T::from_slice(bytes)) } } From af891f65a77971e00eab3071a1613899cde430ab Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Tue, 28 Jun 2016 13:23:15 +0200 Subject: [PATCH 12/12] verifier is no longer a template type of client (#1467) * verifier is no longer a template type of client * added missing , --- ethcore/src/client/client.rs | 31 +++++++++------------- ethcore/src/client/config.rs | 3 +++ ethcore/src/verification/canon_verifier.rs | 4 +-- ethcore/src/verification/mod.rs | 25 +++++++++++++++-- ethcore/src/verification/noop_verifier.rs | 4 +-- ethcore/src/verification/verifier.rs | 4 +-- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index b65bdfd4d..1a437bfac 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -16,7 +16,6 @@ //! Blockchain database client. -use std::marker::PhantomData; use std::path::PathBuf; use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}; use util::*; @@ -30,7 +29,8 @@ use engine::Engine; use views::HeaderView; use service::{NetSyncMessage, SyncMessage}; use env_info::LastHashes; -use verification::*; +use verification; +use verification::{PreverifiedBlock, Verifier}; use block::*; use transaction::{LocalizedTransaction, SignedTransaction, Action}; use blockchain::extras::TransactionAddress; @@ -83,7 +83,7 @@ impl ClientReport { /// Blockchain database client backed by a persistent database. Owns and manages a blockchain and a block queue. /// Call `import_block()` to import a block asynchronously; `flush_queue()` flushes the queue. -pub struct Client where V: Verifier { +pub struct Client { chain: Arc, tracedb: Arc>, engine: Arc>, @@ -92,7 +92,7 @@ pub struct Client where V: Verifier { report: RwLock, import_lock: Mutex<()>, panic_handler: Arc, - verifier: PhantomData, + verifier: Box, vm_factory: Arc, miner: Arc, io_channel: IoChannel, @@ -107,13 +107,6 @@ const HISTORY: u64 = 1200; // of which you actually want force an upgrade. const CLIENT_DB_VER_STR: &'static str = "5.3"; -impl Client { - /// Create a new client with given spec and DB path. - pub fn new(config: ClientConfig, spec: Spec, path: &Path, miner: Arc, message_channel: IoChannel ) -> Result, ClientError> { - Client::::new_with_verifier(config, spec, path, miner, message_channel) - } -} - /// Get the path for the databases given the root path and information on the databases. pub fn get_db_path(path: &Path, pruning: journaldb::Algorithm, genesis_hash: H256) -> PathBuf { let mut dir = path.to_path_buf(); @@ -131,15 +124,15 @@ pub fn append_path(path: &Path, item: &str) -> String { p.to_str().unwrap().to_owned() } -impl Client where V: Verifier { +impl Client { /// Create a new client with given spec and DB path and custom verifier. - pub fn new_with_verifier( + pub fn new( config: ClientConfig, spec: Spec, path: &Path, miner: Arc, message_channel: IoChannel) - -> Result>, ClientError> + -> Result, ClientError> { let path = get_db_path(path, config.pruning, spec.genesis_header().hash()); let gb = spec.genesis_block(); @@ -180,7 +173,7 @@ impl Client where V: Verifier { report: RwLock::new(Default::default()), import_lock: Mutex::new(()), panic_handler: panic_handler, - verifier: PhantomData, + verifier: verification::new(config.verifier_type), vm_factory: Arc::new(EvmFactory::new(config.vm_type)), miner: miner, io_channel: message_channel, @@ -222,7 +215,7 @@ impl Client where V: Verifier { } // Verify Block Family - let verify_family_result = V::verify_block_family(&header, &block.bytes, engine, self.chain.deref()); + let verify_family_result = self.verifier.verify_block_family(&header, &block.bytes, engine, self.chain.deref()); if let Err(e) = verify_family_result { warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); return Err(()); @@ -248,7 +241,7 @@ impl Client where V: Verifier { // Final Verification let locked_block = enact_result.unwrap(); - if let Err(e) = V::verify_block_final(&header, locked_block.block().header()) { + if let Err(e) = self.verifier.verify_block_final(&header, locked_block.block().header()) { warn!(target: "client", "Stage 4 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); return Err(()); } @@ -483,7 +476,7 @@ impl Client where V: Verifier { } } -impl BlockChainClient for Client where V: Verifier { +impl BlockChainClient for Client { fn call(&self, t: &SignedTransaction, analytics: CallAnalytics) -> Result { let header = self.block_header(BlockID::Latest).unwrap(); let view = HeaderView::new(&header); @@ -806,7 +799,7 @@ impl BlockChainClient for Client where V: Verifier { } } -impl MiningBlockChainClient for Client where V: Verifier { +impl MiningBlockChainClient for Client { fn prepare_open_block(&self, author: Address, gas_range_target: (U256, U256), extra_data: Bytes) -> OpenBlock { let engine = self.engine.deref().deref(); let h = self.chain.best_block_hash(); diff --git a/ethcore/src/client/config.rs b/ethcore/src/client/config.rs index 7d7f8e524..52a875a2f 100644 --- a/ethcore/src/client/config.rs +++ b/ethcore/src/client/config.rs @@ -18,6 +18,7 @@ pub use block_queue::BlockQueueConfig; pub use blockchain::Config as BlockChainConfig; pub use trace::{Config as TraceConfig, Switch}; pub use evm::VMType; +pub use verification::VerifierType; use util::journaldb; /// Client state db compaction profile @@ -52,4 +53,6 @@ pub struct ClientConfig { pub db_cache_size: Option, /// State db compaction profile pub db_compaction: DatabaseCompactionProfile, + /// Type of block verifier used by client. + pub verifier_type: VerifierType, } diff --git a/ethcore/src/verification/canon_verifier.rs b/ethcore/src/verification/canon_verifier.rs index 30e368f1b..e0ebf1b7c 100644 --- a/ethcore/src/verification/canon_verifier.rs +++ b/ethcore/src/verification/canon_verifier.rs @@ -24,11 +24,11 @@ use super::verification; pub struct CanonVerifier; impl Verifier for CanonVerifier { - fn verify_block_family(header: &Header, bytes: &[u8], engine: &Engine, bc: &BlockProvider) -> Result<(), Error> { + fn verify_block_family(&self, header: &Header, bytes: &[u8], engine: &Engine, bc: &BlockProvider) -> Result<(), Error> { verification::verify_block_family(header, bytes, engine, bc) } - fn verify_block_final(expected: &Header, got: &Header) -> Result<(), Error> { + fn verify_block_final(&self, expected: &Header, got: &Header) -> Result<(), Error> { verification::verify_block_final(expected, got) } } diff --git a/ethcore/src/verification/mod.rs b/ethcore/src/verification/mod.rs index fe1f406cc..10aee21f4 100644 --- a/ethcore/src/verification/mod.rs +++ b/ethcore/src/verification/mod.rs @@ -17,11 +17,32 @@ pub mod verification; pub mod verifier; mod canon_verifier; -#[cfg(test)] mod noop_verifier; pub use self::verification::*; pub use self::verifier::Verifier; pub use self::canon_verifier::CanonVerifier; -#[cfg(test)] pub use self::noop_verifier::NoopVerifier; + +/// Verifier type. +#[derive(Debug)] +pub enum VerifierType { + /// Verifies block normally. + Canon, + /// Does not verify block at all. + /// Used in tests. + Noop, +} + +impl Default for VerifierType { + fn default() -> Self { + VerifierType::Canon + } +} + +pub fn new(v: VerifierType) -> Box { + match v { + VerifierType::Canon => Box::new(CanonVerifier), + VerifierType::Noop => Box::new(NoopVerifier), + } +} diff --git a/ethcore/src/verification/noop_verifier.rs b/ethcore/src/verification/noop_verifier.rs index 20c15c3f1..99d1d594c 100644 --- a/ethcore/src/verification/noop_verifier.rs +++ b/ethcore/src/verification/noop_verifier.rs @@ -24,11 +24,11 @@ use super::Verifier; pub struct NoopVerifier; impl Verifier for NoopVerifier { - fn verify_block_family(_header: &Header, _bytes: &[u8], _engine: &Engine, _bc: &BlockProvider) -> Result<(), Error> { + fn verify_block_family(&self, _header: &Header, _bytes: &[u8], _engine: &Engine, _bc: &BlockProvider) -> Result<(), Error> { Ok(()) } - fn verify_block_final(_expected: &Header, _got: &Header) -> Result<(), Error> { + fn verify_block_final(&self, _expected: &Header, _got: &Header) -> Result<(), Error> { Ok(()) } } diff --git a/ethcore/src/verification/verifier.rs b/ethcore/src/verification/verifier.rs index cc5edce29..5db81a4eb 100644 --- a/ethcore/src/verification/verifier.rs +++ b/ethcore/src/verification/verifier.rs @@ -21,6 +21,6 @@ use header::Header; /// Should be used to verify blocks. pub trait Verifier: Send + Sync { - fn verify_block_family(header: &Header, bytes: &[u8], engine: &Engine, bc: &BlockProvider) -> Result<(), Error>; - fn verify_block_final(expected: &Header, got: &Header) -> Result<(), Error>; + fn verify_block_family(&self, header: &Header, bytes: &[u8], engine: &Engine, bc: &BlockProvider) -> Result<(), Error>; + fn verify_block_final(&self, expected: &Header, got: &Header) -> Result<(), Error>; }