grassrootseconomics/openethereum
7
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

CLI to specify queue ordering strategy (#2494)

Browse Source 
* Alter gas priorities to include gas_price also

* CLI options and tests

* Adding ordering by gas

* whitespace

[ci:skip]
This commit is contained in:
Tomasz Drwięga 2016-10-07 09:04:13 +02:00 committed by Arkadiy Paronyan
parent 9b398421ce
commit 9cf777510f
5 changed files with 202 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
ethcore/src/miner/miner.rs
View File

@ -30,7 +30,7 @@ use transaction::{Action, SignedTransaction};
use receipt::{Receipt, RichReceipt}; use receipt::{Receipt, RichReceipt};
use spec::Spec; use spec::Spec;
use engines::Engine; use engines::Engine;
use miner::{MinerService, MinerStatus, TransactionQueue, AccountDetails, TransactionOrigin}; use miner::{MinerService, MinerStatus, TransactionQueue, PrioritizationStrategy, AccountDetails, TransactionOrigin};
use miner::work_notify::WorkPoster; use miner::work_notify::WorkPoster;
use client::TransactionImportResult; use client::TransactionImportResult;
use miner::price_info::PriceInfo; use miner::price_info::PriceInfo;
@ -64,6 +64,8 @@ pub struct MinerOptions {
pub tx_gas_limit: U256, pub tx_gas_limit: U256,
/// Maximum size of the transaction queue. /// Maximum size of the transaction queue.
pub tx_queue_size: usize, pub tx_queue_size: usize,
/// Strategy to use for prioritizing transactions in the queue.
pub tx_queue_strategy: PrioritizationStrategy,
/// Whether we should fallback to providing all the queue's transactions or just pending. /// Whether we should fallback to providing all the queue's transactions or just pending.
pub pending_set: PendingSet, pub pending_set: PendingSet,
/// How many historical work packages can we store before running out? /// How many historical work packages can we store before running out?
@ -81,6 +83,7 @@ impl Default for MinerOptions {
reseal_on_own_tx: true, reseal_on_own_tx: true,
tx_gas_limit: !U256::zero(), tx_gas_limit: !U256::zero(),
tx_queue_size: 1024, tx_queue_size: 1024,
tx_queue_strategy: PrioritizationStrategy::GasFactorAndGasPrice,
pending_set: PendingSet::AlwaysQueue, pending_set: PendingSet::AlwaysQueue,
reseal_min_period: Duration::from_secs(2), reseal_min_period: Duration::from_secs(2),
work_queue_size: 20, work_queue_size: 20,
@ -188,7 +191,7 @@ impl Miner {
/// Creates new instance of miner without accounts, but with given spec. /// Creates new instance of miner without accounts, but with given spec.
pub fn with_spec(spec: &Spec) -> Miner { pub fn with_spec(spec: &Spec) -> Miner {
Miner { Miner {
transaction_queue: Arc::new(Mutex::new(TransactionQueue::new())), transaction_queue: Arc::new(Mutex::new(TransactionQueue::default())),
options: Default::default(), options: Default::default(),
next_allowed_reseal: Mutex::new(Instant::now()), next_allowed_reseal: Mutex::new(Instant::now()),
sealing_block_last_request: Mutex::new(0), sealing_block_last_request: Mutex::new(0),
@ -206,7 +209,9 @@ impl Miner {
/// Creates new instance of miner /// Creates new instance of miner
pub fn new(options: MinerOptions, gas_pricer: GasPricer, spec: &Spec, accounts: Option<Arc<AccountProvider>>) -> Arc<Miner> { pub fn new(options: MinerOptions, gas_pricer: GasPricer, spec: &Spec, accounts: Option<Arc<AccountProvider>>) -> Arc<Miner> {
let work_poster = if !options.new_work_notify.is_empty() { Some(WorkPoster::new(&options.new_work_notify)) } else { None }; let work_poster = if !options.new_work_notify.is_empty() { Some(WorkPoster::new(&options.new_work_notify)) } else { None };
let txq = Arc::new(Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, options.tx_gas_limit))); let txq = Arc::new(Mutex::new(TransactionQueue::with_limits(
options.tx_queue_strategy, options.tx_queue_size, options.tx_gas_limit
)));
Arc::new(Miner { Arc::new(Miner {
transaction_queue: txq, transaction_queue: txq,
next_allowed_reseal: Mutex::new(Instant::now()), next_allowed_reseal: Mutex::new(Instant::now()),
@ -918,7 +923,7 @@ impl MinerService for Miner {
mod tests { mod tests {
use std::time::Duration; use std::time::Duration;
use super::super::MinerService; use super::super::{MinerService, PrioritizationStrategy};
use super::*; use super::*;
use util::*; use util::*;
use client::{TestBlockChainClient, EachBlockWith}; use client::{TestBlockChainClient, EachBlockWith};
@ -969,6 +974,7 @@ mod tests {
reseal_min_period: Duration::from_secs(5), reseal_min_period: Duration::from_secs(5),
tx_gas_limit: !U256::zero(), tx_gas_limit: !U256::zero(),
tx_queue_size: 1024, tx_queue_size: 1024,
tx_queue_strategy: PrioritizationStrategy::GasFactorAndGasPrice,
pending_set: PendingSet::AlwaysSealing, pending_set: PendingSet::AlwaysSealing,
work_queue_size: 5, work_queue_size: 5,
enable_resubmission: true, enable_resubmission: true,

2
ethcore/src/miner/mod.rs
View File

@ -47,7 +47,7 @@ mod transaction_queue;
mod work_notify; mod work_notify;
mod price_info; mod price_info;
pub use self::transaction_queue::{TransactionQueue, AccountDetails, TransactionOrigin}; pub use self::transaction_queue::{TransactionQueue, PrioritizationStrategy, AccountDetails, TransactionOrigin};
pub use self::miner::{Miner, MinerOptions, PendingSet, GasPricer, GasPriceCalibratorOptions}; pub use self::miner::{Miner, MinerOptions, PendingSet, GasPricer, GasPriceCalibratorOptions};
pub use self::external::{ExternalMiner, ExternalMinerService}; pub use self::external::{ExternalMiner, ExternalMinerService};
pub use client::TransactionImportResult; pub use client::TransactionImportResult;

213
ethcore/src/miner/transaction_queue.rs
View File

@ -49,7 +49,7 @@
//! balance: U256::from(1_000_000), //! balance: U256::from(1_000_000),
//! }; //! };
//! //!
//! let mut txq = TransactionQueue::new(); //! let mut txq = TransactionQueue::default();
//! txq.add(st2.clone(), &default_nonce, TransactionOrigin::External).unwrap(); //! txq.add(st2.clone(), &default_nonce, TransactionOrigin::External).unwrap();
//! txq.add(st1.clone(), &default_nonce, TransactionOrigin::External).unwrap(); //! txq.add(st1.clone(), &default_nonce, TransactionOrigin::External).unwrap();
//! //!
@ -134,9 +134,17 @@ struct TransactionOrder {
/// Gas Price of the transaction. /// Gas Price of the transaction.
/// Low gas price = Low priority (processed later) /// Low gas price = Low priority (processed later)
gas_price: U256, gas_price: U256,
/// Gas (limit) of the transaction. /// Gas usage priority factor. Usage depends on strategy.
/// Represents the linear increment in required gas price for heavy transactions.
///
/// High gas limit + Low gas price = Low priority
/// High gas limit + High gas price = High priority
gas_factor: U256,
/// Gas (limit) of the transaction. Usage depends on strategy.
/// Low gas limit = High priority (processed earlier) /// Low gas limit = High priority (processed earlier)
gas: U256, gas: U256,
/// Transaction ordering strategy
strategy: PrioritizationStrategy,
/// Hash to identify associated transaction /// Hash to identify associated transaction
hash: H256, hash: H256,
/// Origin of the transaction /// Origin of the transaction
@ -147,11 +155,15 @@ struct TransactionOrder {
impl TransactionOrder { impl TransactionOrder {
fn for_transaction(tx: &VerifiedTransaction, base_nonce: U256) -> Self {
fn for_transaction(tx: &VerifiedTransaction, base_nonce: U256, min_gas_price: U256, strategy: PrioritizationStrategy) -> Self {
let factor = (tx.transaction.gas >> 15) * min_gas_price;
TransactionOrder { TransactionOrder {
nonce_height: tx.nonce() - base_nonce, nonce_height: tx.nonce() - base_nonce,
gas_price: tx.transaction.gas_price, gas_price: tx.transaction.gas_price,
gas: tx.transaction.gas, gas: tx.transaction.gas,
gas_factor: factor,
strategy: strategy,
hash: tx.hash(), hash: tx.hash(),
origin: tx.origin, origin: tx.origin,
penalties: 0, penalties: 0,
@ -199,18 +211,28 @@ impl Ord for TransactionOrder {
return self.origin.cmp(&b.origin); return self.origin.cmp(&b.origin);
} }
// Then compare gas usage match self.strategy {
let a_gas = self.gas; PrioritizationStrategy::GasAndGasPrice => {
let b_gas = b.gas; if self.gas != b.gas {
if a_gas != b_gas { return self.gas.cmp(&b.gas);
return a_gas.cmp(&b_gas); }
},
PrioritizationStrategy::GasFactorAndGasPrice => {
// avoiding overflows
// (gp1 - g1) > (gp2 - g2) <=>
// (gp1 + g2) > (gp2 + g1)
let f_a = self.gas_price + b.gas_factor;
let f_b = b.gas_price + self.gas_factor;
if f_a != f_b {
return f_b.cmp(&f_a);
}
},
PrioritizationStrategy::GasPriceOnly => {},
} }
// Then compare gas_prices // Then compare gas_prices
let a_gas_price = self.gas_price; if self.gas_price != b.gas_price {
let b_gas_price = b.gas_price; return b.gas_price.cmp(&self.gas_price);
if a_gas_price != b_gas_price {
return b_gas_price.cmp(&a_gas_price);
} }
// Compare hashes // Compare hashes
@ -350,8 +372,32 @@ pub struct AccountDetails {
/// Transactions with `gas > (gas_limit + gas_limit * Factor(in percents))` are not imported to the queue. /// Transactions with `gas > (gas_limit + gas_limit * Factor(in percents))` are not imported to the queue.
const GAS_LIMIT_HYSTERESIS: usize = 10; // (100/GAS_LIMIT_HYSTERESIS) % const GAS_LIMIT_HYSTERESIS: usize = 10; // (100/GAS_LIMIT_HYSTERESIS) %
/// Describes the strategy used to prioritize transactions in the queue.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum PrioritizationStrategy {
/// Use only gas price. Disregards the actual computation cost of the transaction.
/// i.e. Higher gas price = Higher priority
GasPriceOnly,
/// Use gas limit and then gas price.
/// i.e. Higher gas limit = Lower priority
GasAndGasPrice,
/// Calculate and use priority based on gas and gas price.
/// PRIORITY = GAS_PRICE - GAS/2^15 * MIN_GAS_PRICE
///
/// Rationale:
/// Heavy transactions are paying linear cost (GAS * GAS_PRICE)
/// while the computation might be more expensive.
///
/// i.e.
/// 1M gas tx with `gas_price=30*min` has the same priority
/// as 32k gas tx with `gas_price=min`
GasFactorAndGasPrice,
}
/// `TransactionQueue` implementation /// `TransactionQueue` implementation
pub struct TransactionQueue { pub struct TransactionQueue {
/// Prioritization strategy for this queue
strategy: PrioritizationStrategy,
/// Gas Price threshold for transactions that can be imported to this queue (defaults to 0) /// Gas Price threshold for transactions that can be imported to this queue (defaults to 0)
minimal_gas_price: U256, minimal_gas_price: U256,
/// The maximum amount of gas any individual transaction may use. /// The maximum amount of gas any individual transaction may use.
@ -370,18 +416,18 @@ pub struct TransactionQueue {
impl Default for TransactionQueue { impl Default for TransactionQueue {
fn default() -> Self { fn default() -> Self {
TransactionQueue::new() TransactionQueue::new(PrioritizationStrategy::GasPriceOnly)
} }
} }
impl TransactionQueue { impl TransactionQueue {
/// Creates new instance of this Queue /// Creates new instance of this Queue
pub fn new() -> Self { pub fn new(strategy: PrioritizationStrategy) -> Self {
Self::with_limits(1024, !U256::zero()) Self::with_limits(strategy, 1024, !U256::zero())
} }
/// Create new instance of this Queue with specified limits /// Create new instance of this Queue with specified limits
pub fn with_limits(limit: usize, tx_gas_limit: U256) -> Self { pub fn with_limits(strategy: PrioritizationStrategy, limit: usize, tx_gas_limit: U256) -> Self {
let current = TransactionSet { let current = TransactionSet {
by_priority: BTreeSet::new(), by_priority: BTreeSet::new(),
by_address: Table::new(), by_address: Table::new(),
@ -395,6 +441,7 @@ impl TransactionQueue {
}; };
TransactionQueue { TransactionQueue {
strategy: strategy,
minimal_gas_price: U256::zero(), minimal_gas_price: U256::zero(),
tx_gas_limit: tx_gas_limit, tx_gas_limit: tx_gas_limit,
gas_limit: !U256::zero(), gas_limit: !U256::zero(),
@ -745,6 +792,7 @@ impl TransactionQueue {
return Err(TransactionError::AlreadyImported); return Err(TransactionError::AlreadyImported);
} }
let min_gas_price = (self.minimal_gas_price, self.strategy);
let address = tx.sender(); let address = tx.sender();
let nonce = tx.nonce(); let nonce = tx.nonce();
let hash = tx.hash(); let hash = tx.hash();
@ -772,7 +820,7 @@ impl TransactionQueue {
if nonce > next_nonce { if nonce > next_nonce {
// We have a gap - put to future. // We have a gap - put to future.
// Insert transaction (or replace old one with lower gas price) // Insert transaction (or replace old one with lower gas price)
try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, &mut self.future, &mut self.by_hash))); try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, min_gas_price, &mut self.future, &mut self.by_hash)));
// Enforce limit in Future // Enforce limit in Future
let removed = self.future.enforce_limit(&mut self.by_hash); let removed = self.future.enforce_limit(&mut self.by_hash);
// Return an error if this transaction was not imported because of limit. // Return an error if this transaction was not imported because of limit.
@ -788,7 +836,7 @@ impl TransactionQueue {
self.move_matching_future_to_current(address, nonce + U256::one(), state_nonce); self.move_matching_future_to_current(address, nonce + U256::one(), state_nonce);
// Replace transaction if any // Replace transaction if any
try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash))); try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, min_gas_price, &mut self.current, &mut self.by_hash)));
// Keep track of highest nonce stored in current // Keep track of highest nonce stored in current
let new_max = self.last_nonces.get(&address).map_or(nonce, |n| cmp::max(nonce, *n)); let new_max = self.last_nonces.get(&address).map_or(nonce, |n| cmp::max(nonce, *n));
self.last_nonces.insert(address, new_max); self.last_nonces.insert(address, new_max);
@ -824,8 +872,8 @@ impl TransactionQueue {
/// ///
/// Returns `true` if transaction actually got to the queue (`false` if there was already a transaction with higher /// Returns `true` if transaction actually got to the queue (`false` if there was already a transaction with higher
/// gas_price) /// gas_price)
fn replace_transaction(tx: VerifiedTransaction, base_nonce: U256, set: &mut TransactionSet, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> bool { fn replace_transaction(tx: VerifiedTransaction, base_nonce: U256, min_gas_price: (U256, PrioritizationStrategy), set: &mut TransactionSet, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> bool {
let order = TransactionOrder::for_transaction(&tx, base_nonce); let order = TransactionOrder::for_transaction(&tx, base_nonce, min_gas_price.0, min_gas_price.1);
let hash = tx.hash(); let hash = tx.hash();
let address = tx.sender(); let address = tx.sender();
let nonce = tx.nonce(); let nonce = tx.nonce();
@ -991,6 +1039,10 @@ mod test {
assert_eq!(TransactionOrigin::External.cmp(&TransactionOrigin::RetractedBlock), Ordering::Greater); assert_eq!(TransactionOrigin::External.cmp(&TransactionOrigin::RetractedBlock), Ordering::Greater);
} }
fn transaction_order(tx: &VerifiedTransaction, nonce: U256) -> TransactionOrder {
TransactionOrder::for_transaction(tx, nonce, 0.into(), PrioritizationStrategy::GasPriceOnly)
}
#[test] #[test]
fn should_create_transaction_set() { fn should_create_transaction_set() {
// given // given
@ -1011,9 +1063,9 @@ mod test {
x x
}; };
// Insert both transactions // Insert both transactions
let order1 = TransactionOrder::for_transaction(&tx1, U256::zero()); let order1 = transaction_order(&tx1, U256::zero());
set.insert(tx1.sender(), tx1.nonce(), order1.clone()); set.insert(tx1.sender(), tx1.nonce(), order1.clone());
let order2 = TransactionOrder::for_transaction(&tx2, U256::zero()); let order2 = transaction_order(&tx2, U256::zero());
set.insert(tx2.sender(), tx2.nonce(), order2.clone()); set.insert(tx2.sender(), tx2.nonce(), order2.clone());
assert_eq!(set.by_priority.len(), 2); assert_eq!(set.by_priority.len(), 2);
assert_eq!(set.by_address.len(), 2); assert_eq!(set.by_address.len(), 2);
@ -1052,12 +1104,12 @@ mod test {
x x
}; };
// Insert both transactions // Insert both transactions
let order1 = TransactionOrder::for_transaction(&tx1, U256::zero()); let order1 = transaction_order(&tx1, U256::zero());
set.insert(tx1.sender(), tx1.nonce(), order1.clone()); set.insert(tx1.sender(), tx1.nonce(), order1.clone());
assert_eq!(set.by_priority.len(), 1); assert_eq!(set.by_priority.len(), 1);
assert_eq!(set.by_address.len(), 1); assert_eq!(set.by_address.len(), 1);
// Two different orders (imagine nonce changed in the meantime) // Two different orders (imagine nonce changed in the meantime)
let order2 = TransactionOrder::for_transaction(&tx2, U256::one()); let order2 = transaction_order(&tx2, U256::one());
set.insert(tx2.sender(), tx2.nonce(), order2.clone()); set.insert(tx2.sender(), tx2.nonce(), order2.clone());
assert_eq!(set.by_priority.len(), 1); assert_eq!(set.by_priority.len(), 1);
assert_eq!(set.by_address.len(), 1); assert_eq!(set.by_address.len(), 1);
@ -1072,7 +1124,7 @@ mod test {
#[test] #[test]
fn should_handle_same_transaction_imported_twice_with_different_state_nonces() { fn should_handle_same_transaction_imported_twice_with_different_state_nonces() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_similar_txs(); let (tx, tx2) = new_similar_txs();
let prev_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance: let prev_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance:
!U256::zero() }; !U256::zero() };
@ -1097,7 +1149,7 @@ mod test {
#[test] #[test]
fn should_move_all_transactions_from_future() { fn should_move_all_transactions_from_future() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs_with_gas_price_diff(1.into(), 1.into()); let (tx, tx2) = new_txs_with_gas_price_diff(1.into(), 1.into());
let prev_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance: let prev_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance:
!U256::zero() }; !U256::zero() };
@ -1123,7 +1175,7 @@ mod test {
#[test] #[test]
fn should_import_tx() { fn should_import_tx() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
// when // when
@ -1138,7 +1190,7 @@ mod test {
#[test] #[test]
fn should_order_by_gas() { fn should_order_by_gas() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::new(PrioritizationStrategy::GasAndGasPrice);
let tx1 = new_tx_with_gas(50000.into(), 40.into()); let tx1 = new_tx_with_gas(50000.into(), 40.into());
let tx2 = new_tx_with_gas(40000.into(), 30.into()); let tx2 = new_tx_with_gas(40000.into(), 30.into());
let tx3 = new_tx_with_gas(30000.into(), 10.into()); let tx3 = new_tx_with_gas(30000.into(), 10.into());
@ -1168,10 +1220,45 @@ mod test {
assert_eq!(txq.top_transactions()[2].gas_price, 20.into()); assert_eq!(txq.top_transactions()[2].gas_price, 20.into());
} }
#[test]
fn should_order_by_gas_factor() {
// given
let mut txq = TransactionQueue::new(PrioritizationStrategy::GasFactorAndGasPrice);
let tx1 = new_tx_with_gas(150_000.into(), 40.into());
let tx2 = new_tx_with_gas(40_000.into(), 16.into());
let tx3 = new_tx_with_gas(30_000.into(), 15.into());
let tx4 = new_tx_with_gas(150_000.into(), 62.into());
txq.set_minimal_gas_price(15.into());
// when
let res1 = txq.add(tx1, &default_nonce, TransactionOrigin::External);
let res2 = txq.add(tx2, &default_nonce, TransactionOrigin::External);
let res3 = txq.add(tx3, &default_nonce, TransactionOrigin::External);
let res4 = txq.add(tx4, &default_nonce, TransactionOrigin::External);
// then
assert_eq!(res1.unwrap(), TransactionImportResult::Current);
assert_eq!(res2.unwrap(), TransactionImportResult::Current);
assert_eq!(res3.unwrap(), TransactionImportResult::Current);
assert_eq!(res4.unwrap(), TransactionImportResult::Current);
let stats = txq.status();
assert_eq!(stats.pending, 4);
println!("{:?}", txq.top_transactions());
assert_eq!(txq.top_transactions()[0].gas, 30_000.into());
assert_eq!(txq.top_transactions()[1].gas, 150_000.into());
assert_eq!(txq.top_transactions()[2].gas, 40_000.into());
assert_eq!(txq.top_transactions()[3].gas, 150_000.into());
assert_eq!(txq.top_transactions()[0].gas_price, 15.into());
assert_eq!(txq.top_transactions()[1].gas_price, 62.into());
assert_eq!(txq.top_transactions()[2].gas_price, 16.into());
assert_eq!(txq.top_transactions()[3].gas_price, 40.into());
}
#[test] #[test]
fn gas_limit_should_never_overflow() { fn gas_limit_should_never_overflow() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
txq.set_gas_limit(U256::zero()); txq.set_gas_limit(U256::zero());
assert_eq!(txq.gas_limit, U256::zero()); assert_eq!(txq.gas_limit, U256::zero());
@ -1185,7 +1272,7 @@ mod test {
#[test] #[test]
fn should_not_import_transaction_above_gas_limit() { fn should_not_import_transaction_above_gas_limit() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
let gas = tx.gas; let gas = tx.gas;
let limit = gas / U256::from(2); let limit = gas / U256::from(2);
@ -1207,7 +1294,7 @@ mod test {
#[test] #[test]
fn should_drop_transactions_from_senders_without_balance() { fn should_drop_transactions_from_senders_without_balance() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
let account = |a: &Address| AccountDetails { let account = |a: &Address| AccountDetails {
nonce: default_nonce(a).nonce, nonce: default_nonce(a).nonce,
@ -1230,7 +1317,7 @@ mod test {
#[test] #[test]
fn should_not_import_transaction_below_min_gas_price_threshold_if_external() { fn should_not_import_transaction_below_min_gas_price_threshold_if_external() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
txq.set_minimal_gas_price(tx.gas_price + U256::one()); txq.set_minimal_gas_price(tx.gas_price + U256::one());
@ -1250,7 +1337,7 @@ mod test {
#[test] #[test]
fn should_import_transaction_below_min_gas_price_threshold_if_local() { fn should_import_transaction_below_min_gas_price_threshold_if_local() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
txq.set_minimal_gas_price(tx.gas_price + U256::one()); txq.set_minimal_gas_price(tx.gas_price + U256::one());
@ -1267,7 +1354,7 @@ mod test {
#[test] #[test]
fn should_reject_incorrectly_signed_transaction() { fn should_reject_incorrectly_signed_transaction() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_unsigned_tx(U256::from(123)); let tx = new_unsigned_tx(U256::from(123));
let stx = { let stx = {
let mut s = RlpStream::new_list(9); let mut s = RlpStream::new_list(9);
@ -1292,7 +1379,7 @@ mod test {
#[test] #[test]
fn should_import_txs_from_same_sender() { fn should_import_txs_from_same_sender() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(1)); let (tx, tx2) = new_txs(U256::from(1));
@ -1310,7 +1397,7 @@ mod test {
#[test] #[test]
fn should_prioritize_local_transactions_within_same_nonce_height() { fn should_prioritize_local_transactions_within_same_nonce_height() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
// the second one has same nonce but higher `gas_price` // the second one has same nonce but higher `gas_price`
let (_, tx2) = new_similar_txs(); let (_, tx2) = new_similar_txs();
@ -1331,7 +1418,7 @@ mod test {
#[test] #[test]
fn should_prioritize_reimported_transactions_within_same_nonce_height() { fn should_prioritize_reimported_transactions_within_same_nonce_height() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
// the second one has same nonce but higher `gas_price` // the second one has same nonce but higher `gas_price`
let (_, tx2) = new_similar_txs(); let (_, tx2) = new_similar_txs();
@ -1352,7 +1439,7 @@ mod test {
#[test] #[test]
fn should_not_prioritize_local_transactions_with_different_nonce_height() { fn should_not_prioritize_local_transactions_with_different_nonce_height() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(1)); let (tx, tx2) = new_txs(U256::from(1));
// when // when
@ -1399,7 +1486,7 @@ mod test {
#[test] #[test]
fn should_penalize_transactions_from_sender() { fn should_penalize_transactions_from_sender() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
// txa, txb - slightly bigger gas price to have consistent ordering // txa, txb - slightly bigger gas price to have consistent ordering
let (txa, txb) = new_txs(U256::from(1)); let (txa, txb) = new_txs(U256::from(1));
let (tx1, tx2) = new_txs_with_higher_gas_price(U256::from(3)); let (tx1, tx2) = new_txs_with_higher_gas_price(U256::from(3));
@ -1432,7 +1519,7 @@ mod test {
#[test] #[test]
fn should_return_pending_hashes() { fn should_return_pending_hashes() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(1)); let (tx, tx2) = new_txs(U256::from(1));
@ -1450,7 +1537,7 @@ mod test {
#[test] #[test]
fn should_put_transaction_to_futures_if_gap_detected() { fn should_put_transaction_to_futures_if_gap_detected() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(2)); let (tx, tx2) = new_txs(U256::from(2));
@ -1476,7 +1563,7 @@ mod test {
!U256::zero() }; !U256::zero() };
let next2_nonce = default_nonce_val() + U256::from(3); let next2_nonce = default_nonce_val() + U256::from(3);
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(1)); let (tx, tx2) = new_txs(U256::from(1));
txq.add(tx.clone(), &prev_nonce, TransactionOrigin::External).unwrap(); txq.add(tx.clone(), &prev_nonce, TransactionOrigin::External).unwrap();
@ -1495,7 +1582,7 @@ mod test {
#[test] #[test]
fn should_move_transactions_if_gap_filled() { fn should_move_transactions_if_gap_filled() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let kp = KeyPair::create().unwrap(); let kp = KeyPair::create().unwrap();
let secret = kp.secret(); let secret = kp.secret();
let tx = new_unsigned_tx(U256::from(123)).sign(secret); let tx = new_unsigned_tx(U256::from(123)).sign(secret);
@ -1519,7 +1606,7 @@ mod test {
#[test] #[test]
fn should_remove_transaction() { fn should_remove_transaction() {
// given // given
let mut txq2 = TransactionQueue::new(); let mut txq2 = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(3)); let (tx, tx2) = new_txs(U256::from(3));
txq2.add(tx.clone(), &default_nonce, TransactionOrigin::External).unwrap(); txq2.add(tx.clone(), &default_nonce, TransactionOrigin::External).unwrap();
txq2.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap(); txq2.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap();
@ -1540,7 +1627,7 @@ mod test {
#[test] #[test]
fn should_move_transactions_to_future_if_gap_introduced() { fn should_move_transactions_to_future_if_gap_introduced() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(1)); let (tx, tx2) = new_txs(U256::from(1));
let tx3 = new_tx(); let tx3 = new_tx();
txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap(); txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap();
@ -1561,7 +1648,7 @@ mod test {
#[test] #[test]
fn should_clear_queue() { fn should_clear_queue() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::one()); let (tx, tx2) = new_txs(U256::one());
// add // add
@ -1581,7 +1668,7 @@ mod test {
#[test] #[test]
fn should_drop_old_transactions_when_hitting_the_limit() { fn should_drop_old_transactions_when_hitting_the_limit() {
// given // given
let mut txq = TransactionQueue::with_limits(1, !U256::zero()); let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero());
let (tx, tx2) = new_txs(U256::one()); let (tx, tx2) = new_txs(U256::one());
let sender = tx.sender().unwrap(); let sender = tx.sender().unwrap();
let nonce = tx.nonce; let nonce = tx.nonce;
@ -1603,7 +1690,7 @@ mod test {
#[test] #[test]
fn should_return_correct_nonces_when_dropped_because_of_limit() { fn should_return_correct_nonces_when_dropped_because_of_limit() {
// given // given
let mut txq = TransactionQueue::with_limits(2, !U256::zero()); let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 2, !U256::zero());
let tx = new_tx(); let tx = new_tx();
let (tx1, tx2) = new_txs(U256::one()); let (tx1, tx2) = new_txs(U256::one());
let sender = tx1.sender().unwrap(); let sender = tx1.sender().unwrap();
@ -1624,7 +1711,7 @@ mod test {
#[test] #[test]
fn should_limit_future_transactions() { fn should_limit_future_transactions() {
let mut txq = TransactionQueue::with_limits(1, !U256::zero()); let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero());
txq.current.set_limit(10); txq.current.set_limit(10);
let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1)); 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)); let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2));
@ -1643,7 +1730,7 @@ mod test {
#[test] #[test]
fn should_drop_transactions_with_old_nonces() { fn should_drop_transactions_with_old_nonces() {
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
let last_nonce = tx.nonce + U256::one(); let last_nonce = tx.nonce + U256::one();
let fetch_last_nonce = |_a: &Address| AccountDetails{ nonce: last_nonce, balance: !U256::zero() }; let fetch_last_nonce = |_a: &Address| AccountDetails{ nonce: last_nonce, balance: !U256::zero() };
@ -1663,7 +1750,7 @@ mod test {
// given // given
let nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce + U256::one(), let nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce + U256::one(),
balance: !U256::zero() }; balance: !U256::zero() };
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (_tx1, tx2) = new_txs(U256::from(1)); let (_tx1, tx2) = new_txs(U256::from(1));
txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap(); txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap();
assert_eq!(txq.status().future, 1); assert_eq!(txq.status().future, 1);
@ -1682,7 +1769,7 @@ mod test {
#[test] #[test]
fn should_accept_same_transaction_twice_if_removed() { fn should_accept_same_transaction_twice_if_removed() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx1, tx2) = new_txs(U256::from(1)); let (tx1, tx2) = new_txs(U256::from(1));
txq.add(tx1.clone(), &default_nonce, TransactionOrigin::External).unwrap(); txq.add(tx1.clone(), &default_nonce, TransactionOrigin::External).unwrap();
txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap(); txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap();
@ -1703,7 +1790,7 @@ mod test {
#[test] #[test]
fn should_not_move_to_future_if_state_nonce_is_higher() { fn should_not_move_to_future_if_state_nonce_is_higher() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx, tx2) = new_txs(U256::from(1)); let (tx, tx2) = new_txs(U256::from(1));
let tx3 = new_tx(); let tx3 = new_tx();
txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap(); txq.add(tx2.clone(), &default_nonce, TransactionOrigin::External).unwrap();
@ -1726,7 +1813,7 @@ mod test {
fn should_replace_same_transaction_when_has_higher_fee() { fn should_replace_same_transaction_when_has_higher_fee() {
init_log(); init_log();
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let keypair = KeyPair::create().unwrap(); let keypair = KeyPair::create().unwrap();
let tx = new_unsigned_tx(U256::from(123)).sign(keypair.secret()); let tx = new_unsigned_tx(U256::from(123)).sign(keypair.secret());
let tx2 = { let tx2 = {
@ -1749,7 +1836,7 @@ mod test {
#[test] #[test]
fn should_replace_same_transaction_when_importing_to_futures() { fn should_replace_same_transaction_when_importing_to_futures() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let keypair = KeyPair::create().unwrap(); let keypair = KeyPair::create().unwrap();
let tx0 = new_unsigned_tx(U256::from(123)).sign(keypair.secret()); let tx0 = new_unsigned_tx(U256::from(123)).sign(keypair.secret());
let tx1 = { let tx1 = {
@ -1783,7 +1870,7 @@ mod test {
!U256::zero() }; !U256::zero() };
let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance: let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance:
!U256::zero() }; !U256::zero() };
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx1, tx2) = new_txs(U256::one()); let (tx1, tx2) = new_txs(U256::one());
txq.add(tx1.clone(), &previous_nonce, TransactionOrigin::External).unwrap(); txq.add(tx1.clone(), &previous_nonce, TransactionOrigin::External).unwrap();
txq.add(tx2, &previous_nonce, TransactionOrigin::External).unwrap(); txq.add(tx2, &previous_nonce, TransactionOrigin::External).unwrap();
@ -1801,7 +1888,7 @@ mod test {
#[test] #[test]
fn should_return_none_when_transaction_from_given_address_does_not_exist() { fn should_return_none_when_transaction_from_given_address_does_not_exist() {
// given // given
let txq = TransactionQueue::new(); let txq = TransactionQueue::default();
// then // then
assert_eq!(txq.last_nonce(&Address::default()), None); assert_eq!(txq.last_nonce(&Address::default()), None);
@ -1810,7 +1897,7 @@ mod test {
#[test] #[test]
fn should_return_correct_nonce_when_transactions_from_given_address_exist() { fn should_return_correct_nonce_when_transactions_from_given_address_exist() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let tx = new_tx(); let tx = new_tx();
let from = tx.sender().unwrap(); let from = tx.sender().unwrap();
let nonce = tx.nonce; let nonce = tx.nonce;
@ -1826,7 +1913,7 @@ mod test {
#[test] #[test]
fn should_remove_old_transaction_even_if_newer_transaction_was_not_known() { fn should_remove_old_transaction_even_if_newer_transaction_was_not_known() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx1, tx2) = new_txs(U256::one()); let (tx1, tx2) = new_txs(U256::one());
let (nonce1, nonce2) = (tx1.nonce, tx2.nonce); let (nonce1, nonce2) = (tx1.nonce, tx2.nonce);
let details1 = |_a: &Address| AccountDetails { nonce: nonce1, balance: !U256::zero() }; let details1 = |_a: &Address| AccountDetails { nonce: nonce1, balance: !U256::zero() };
@ -1844,7 +1931,7 @@ mod test {
#[test] #[test]
fn should_return_valid_last_nonce_after_remove_all() { fn should_return_valid_last_nonce_after_remove_all() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx1, tx2) = new_txs(U256::from(4)); let (tx1, tx2) = new_txs(U256::from(4));
let sender = tx1.sender().unwrap(); let sender = tx1.sender().unwrap();
let (nonce1, nonce2) = (tx1.nonce, tx2.nonce); let (nonce1, nonce2) = (tx1.nonce, tx2.nonce);
@ -1868,7 +1955,7 @@ mod test {
#[test] #[test]
fn should_return_true_if_there_is_local_transaction_pending() { fn should_return_true_if_there_is_local_transaction_pending() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx1, tx2) = new_txs(U256::from(1)); let (tx1, tx2) = new_txs(U256::from(1));
assert_eq!(txq.has_local_pending_transactions(), false); assert_eq!(txq.has_local_pending_transactions(), false);
@ -1884,7 +1971,7 @@ mod test {
#[test] #[test]
fn should_keep_right_order_in_future() { fn should_keep_right_order_in_future() {
// given // given
let mut txq = TransactionQueue::with_limits(1, !U256::zero()); let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero());
let (tx1, tx2) = new_txs(U256::from(1)); let (tx1, tx2) = new_txs(U256::from(1));
let prev_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce - U256::one(), balance: let prev_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce - U256::one(), balance:
default_nonce(a).balance }; default_nonce(a).balance };
@ -1901,7 +1988,7 @@ mod test {
#[test] #[test]
fn should_return_correct_last_nonce() { fn should_return_correct_last_nonce() {
// given // given
let mut txq = TransactionQueue::new(); let mut txq = TransactionQueue::default();
let (tx1, tx2, tx2_2, tx3) = { let (tx1, tx2, tx2_2, tx3) = {
let keypair = KeyPair::create().unwrap(); let keypair = KeyPair::create().unwrap();
let secret = &keypair.secret(); let secret = &keypair.secret();

7
parity/cli.rs
View File

@ -194,6 +194,12 @@ Sealing/Mining Options:
more than 32 characters. more than 32 characters.
--tx-queue-size LIMIT Maximum 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]. to be included in next block) [default: 1024].
--tx-queue-strategy S Prioritization strategy used to order transactions
in the queue. S may be:
gas - Prioritize txs with low gas limit;
gas_price - Prioritize txs with high gas price;
gas_factor - Prioritize txs using gas price
and gas limit ratio [default: gas_factor].
--remove-solved Move solved blocks from the work package queue --remove-solved Move solved blocks from the work package queue
instead of cloning them. This gives a slightly instead of cloning them. This gives a slightly
faster import speed, but means that extra solutions faster import speed, but means that extra solutions
@ -372,6 +378,7 @@ pub struct Args {
pub flag_gas_cap: String, pub flag_gas_cap: String,
pub flag_extra_data: Option<String>, pub flag_extra_data: Option<String>,
pub flag_tx_queue_size: usize, pub flag_tx_queue_size: usize,
pub flag_tx_queue_strategy: String,
pub flag_notify_work: Option<String>, pub flag_notify_work: Option<String>,
pub flag_logging: Option<String>, pub flag_logging: Option<String>,
pub flag_version: bool, pub flag_version: bool,

34
parity/configuration.rs
View File

@ -25,7 +25,7 @@ use util::{Hashable, U256, Uint, Bytes, version_data, Secret, Address};
use util::log::Colour; use util::log::Colour;
use ethsync::{NetworkConfiguration, is_valid_node_url}; use ethsync::{NetworkConfiguration, is_valid_node_url};
use ethcore::client::{VMType, Mode}; use ethcore::client::{VMType, Mode};
use ethcore::miner::MinerOptions; use ethcore::miner::{MinerOptions, PrioritizationStrategy};
use rpc::{IpcConfiguration, HttpConfiguration}; use rpc::{IpcConfiguration, HttpConfiguration};
use ethcore_rpc::NetworkSettings; use ethcore_rpc::NetworkSettings;
@ -319,6 +319,15 @@ impl Configuration {
Ok(cfg) Ok(cfg)
} }
fn transaction_queue_strategy(&self) -> Result<PrioritizationStrategy, String> {
match self.args.flag_tx_queue_strategy.as_str() {
"gas" => Ok(PrioritizationStrategy::GasAndGasPrice),
"gas_price" => Ok(PrioritizationStrategy::GasPriceOnly),
"gas_factor" => Ok(PrioritizationStrategy::GasFactorAndGasPrice),
_ => Err(format!("Invalid queue strategy: {}", self.args.flag_tx_queue_strategy)),
}
}
fn miner_options(&self) -> Result<MinerOptions, String> { fn miner_options(&self) -> Result<MinerOptions, String> {
let reseal = try!(self.args.flag_reseal_on_txs.parse::<ResealPolicy>()); let reseal = try!(self.args.flag_reseal_on_txs.parse::<ResealPolicy>());
@ -332,6 +341,7 @@ impl Configuration {
None => U256::max_value(), None => U256::max_value(),
}, },
tx_queue_size: self.args.flag_tx_queue_size, tx_queue_size: self.args.flag_tx_queue_size,
tx_queue_strategy: try!(self.transaction_queue_strategy()),
pending_set: try!(to_pending_set(&self.args.flag_relay_set)), pending_set: try!(to_pending_set(&self.args.flag_relay_set)),
reseal_min_period: Duration::from_millis(self.args.flag_reseal_min_period), reseal_min_period: Duration::from_millis(self.args.flag_reseal_min_period),
work_queue_size: self.args.flag_work_queue_size, work_queue_size: self.args.flag_work_queue_size,
@ -604,6 +614,7 @@ mod tests {
use docopt::Docopt; use docopt::Docopt;
use ethcore_rpc::NetworkSettings; use ethcore_rpc::NetworkSettings;
use ethcore::client::{VMType, BlockID}; use ethcore::client::{VMType, BlockID};
use ethcore::miner::{MinerOptions, PrioritizationStrategy};
use helpers::{replace_home, default_network_config}; use helpers::{replace_home, default_network_config};
use run::RunCmd; use run::RunCmd;
use blockchain::{BlockchainCmd, ImportBlockchain, ExportBlockchain, DataFormat}; use blockchain::{BlockchainCmd, ImportBlockchain, ExportBlockchain, DataFormat};
@ -777,6 +788,27 @@ mod tests {
})); }));
} }
#[test]
fn should_parse_mining_options() {
// given
let mut mining_options = MinerOptions::default();
// when
let conf0 = parse(&["parity"]);
let conf1 = parse(&["parity", "--tx-queue-strategy", "gas_factor"]);
let conf2 = parse(&["parity", "--tx-queue-strategy", "gas_price"]);
let conf3 = parse(&["parity", "--tx-queue-strategy", "gas"]);
// then
assert_eq!(conf0.miner_options().unwrap(), mining_options);
mining_options.tx_queue_strategy = PrioritizationStrategy::GasFactorAndGasPrice;
assert_eq!(conf1.miner_options().unwrap(), mining_options);
mining_options.tx_queue_strategy = PrioritizationStrategy::GasPriceOnly;
assert_eq!(conf2.miner_options().unwrap(), mining_options);
mining_options.tx_queue_strategy = PrioritizationStrategy::GasAndGasPrice;
assert_eq!(conf3.miner_options().unwrap(), mining_options);
}
#[test] #[test]
fn should_parse_network_settings() { fn should_parse_network_settings() {
// given // given