Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Market-orientated transaction pricing #1963

Merged
merged 7 commits into from
Aug 23, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 54 additions & 5 deletions ethcore/src/miner/transaction_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@

use std::cmp::Ordering;
use std::cmp;
use std::collections::{HashMap, BTreeSet};
use std::collections::{HashSet, HashMap, BTreeSet, BTreeMap};
use util::{Address, H256, Uint, U256};
use util::table::Table;
use transaction::*;
Expand Down Expand Up @@ -226,23 +226,27 @@ impl VerifiedTransaction {
struct TransactionSet {
by_priority: BTreeSet<TransactionOrder>,
by_address: Table<Address, U256, TransactionOrder>,
by_gas_price: BTreeMap<U256, HashSet<H256>>,
limit: usize,
}

impl TransactionSet {
/// Inserts `TransactionOrder` to this set
fn insert(&mut self, sender: Address, nonce: U256, order: TransactionOrder) -> Option<TransactionOrder> {
self.by_priority.insert(order.clone());
assert!(Self::insert_item(&mut self.by_gas_price, order.gas_price, order.hash.clone()), "transaction of same hash cannot already exist in queue; qed");
let r = self.by_address.insert(sender, nonce, order);
// If transaction was replaced remove it from priority queue
if let Some(ref old_order) = r {
self.by_priority.remove(old_order);
assert!(Self::remove_item(&mut self.by_gas_price, &old_order.gas_price, &old_order.hash),
"hash is in `by_address`; all transactions' gas_prices in `by_address` must be in `by_gas_limit`; qed");
}
assert_eq!(self.by_priority.len(), self.by_address.len());
r
}

/// Remove low priority transactions if there is more then specified by given `limit`.
/// Remove low priority transactions if there is more than specified by given `limit`.
///
/// It drops transactions from this set but also removes associated `VerifiedTransaction`.
/// Returns addresses and lowest nonces of transactions removed because of limit.
Expand All @@ -267,7 +271,7 @@ impl TransactionSet {
.expect("Transaction has just been found in `by_priority`; so it is in `by_address` also.");

by_hash.remove(&order.hash)
.expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`");
.expect("hash is in `by_priorty`; all hashes in `by_priority` must be in `by_hash`; qed");

let min = removed.get(&sender).map_or(nonce, |val| cmp::min(*val, nonce));
removed.insert(sender, min);
Expand All @@ -278,6 +282,8 @@ impl TransactionSet {
/// Drop transaction from this set (remove from `by_priority` and `by_address`)
fn drop(&mut self, sender: &Address, nonce: &U256) -> Option<TransactionOrder> {
if let Some(tx_order) = self.by_address.remove(sender, nonce) {
assert!(Self::remove_item(&mut self.by_gas_price, &tx_order.gas_price, &tx_order.hash),
"hash is in `by_address`; all transactions' gas_prices in `by_address` must be in `by_gas_limit`; qed");
self.by_priority.remove(&tx_order);
assert_eq!(self.by_priority.len(), self.by_address.len());
return Some(tx_order);
Expand All @@ -290,13 +296,49 @@ impl TransactionSet {
fn clear(&mut self) {
self.by_priority.clear();
self.by_address.clear();
self.by_gas_price.clear();
}

/// Sets new limit for number of transactions in this `TransactionSet`.
/// Note the limit is not applied (no transactions are removed) by calling this method.
fn set_limit(&mut self, limit: usize) {
self.limit = limit;
}

/// Get the minimum gas price that we can accept into this queue that wouldn't cause the transaction to
/// immediately be dropped. 0 if the queue isn't at capacity; 1 plus the lowest if it is.
fn gas_price_entry_limit(&self) -> U256 {
match self.by_gas_price.keys().next() {
Some(k) if self.by_priority.len() >= self.limit => *k + 1.into(),
_ => U256::default(),
}
}

/// Insert an item into a BTreeMap/HashSet "multimap".
fn insert_item(into: &mut BTreeMap<U256, HashSet<H256>>, gas_price: U256, hash: H256) -> bool {
into.entry(gas_price).or_insert_with(Default::default).insert(hash)
}

/// Remove an item from a BTreeMap/HashSet "multimap".
/// Returns true if the item was removed successfully.
fn remove_item(from: &mut BTreeMap<U256, HashSet<H256>>, gas_price: &U256, hash: &H256) -> bool {
if let Some(mut hashes) = from.get_mut(gas_price) {
let only_one_left = hashes.len() == 1;
if !only_one_left {
// Operation may be ok: only if hash is in gas-price's Set.
return hashes.remove(hash);
}
if hashes.iter().next().unwrap() != hash {
// Operation failed: hash not the single item in gas-price's Set.
return false;
}
} else {
// Operation failed: gas-price not found in Map.
return false;
}
// Operation maybe ok: only if hash not found in gas-price Set.
from.remove(gas_price).is_some()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this to line #334 - cause it will happen only in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would like to, but hashes has captured an &mut of into in this scope so it has to escape before it can be removed.

Copy link
Collaborator

@tomusdrw tomusdrw Aug 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explicit drop(hashes) would work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not until non-lexical lifetimes are designed and implemented in rust.

}
}

#[derive(Debug)]
Expand All @@ -316,7 +358,6 @@ pub struct AccountDetails {
pub balance: U256,
}


/// Transactions with `gas > (gas_limit + gas_limit * Factor(in percents))` are not imported to the queue.
const GAS_LIMIT_HYSTERESIS: usize = 10; // %

Expand Down Expand Up @@ -355,12 +396,14 @@ impl TransactionQueue {
let current = TransactionSet {
by_priority: BTreeSet::new(),
by_address: Table::new(),
by_gas_price: Default::default(),
limit: limit,
};

let future = TransactionSet {
by_priority: BTreeSet::new(),
by_address: Table::new(),
by_gas_price: Default::default(),
limit: limit,
};

Expand Down Expand Up @@ -400,6 +443,12 @@ impl TransactionQueue {
self.minimal_gas_price = min_gas_price;
}

/// Get one more than the lowest gas price in the queue iff the pool is
/// full, otherwise 0.
pub fn effective_minimum_gas_price(&self) -> U256 {
self.current.gas_price_entry_limit()
}

/// Sets new gas limit. Transactions with gas slightly (`GAS_LIMIT_HYSTERESIS`) above the limit won't be imported.
/// Any transaction already imported to the queue is not affected.
pub fn set_gas_limit(&mut self, gas_limit: U256) {
Expand Down Expand Up @@ -431,7 +480,7 @@ impl TransactionQueue {

trace!(target: "txqueue", "Importing: {:?}", tx.hash());

if tx.gas_price < self.minimal_gas_price && origin != TransactionOrigin::Local {
if (tx.gas_price < self.minimal_gas_price || tx.gas_price < self.effective_minimum_gas_price()) && origin != TransactionOrigin::Local {
trace!(target: "txqueue",
"Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})",
tx.hash(),
Expand Down
2 changes: 1 addition & 1 deletion parity/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ Sealing/Mining Options:
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
[default: 0]. The minimum gas price is set
accordingly.
--usd-per-eth SOURCE USD value of a single ETH. SOURCE may be either an
amount in USD, a web service or 'auto' to use each
Expand Down