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

Transaction queue limited by gas #2528

Merged
merged 1 commit into from
Oct 7, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion ethcore/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl 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 txq = Arc::new(Mutex::new(TransactionQueue::with_limits(
options.tx_queue_strategy, options.tx_queue_size, options.tx_gas_limit
options.tx_queue_strategy, options.tx_queue_size, !U256::zero(), options.tx_gas_limit
)));
Arc::new(Miner {
transaction_queue: txq,
Expand Down Expand Up @@ -401,6 +401,8 @@ impl Miner {
let gas_limit = HeaderView::new(&chain.best_block_header()).gas_limit();
let mut queue = self.transaction_queue.lock();
queue.set_gas_limit(gas_limit);
// Set total qx queue gas limit to be 2x the block gas limit.
queue.set_total_gas_limit(gas_limit << 1);
}

/// Returns true if we had to prepare new pending block
Expand Down
62 changes: 49 additions & 13 deletions ethcore/src/miner/transaction_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ struct TransactionSet {
by_priority: BTreeSet<TransactionOrder>,
by_address: Table<Address, U256, TransactionOrder>,
limit: usize,
gas_limit: U256,
}

impl TransactionSet {
Expand All @@ -299,14 +300,18 @@ impl TransactionSet {
/// It drops transactions from this set but also removes associated `VerifiedTransaction`.
/// Returns addresses and lowest nonces of transactions removed because of limit.
fn enforce_limit(&mut self, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> Option<HashMap<Address, U256>> {
let len = self.by_priority.len();
if len <= self.limit {
return None;
}
let mut count = 0;
let mut gas: U256 = 0.into();
let to_drop : Vec<(Address, U256)> = {
self.by_priority
.iter()
.skip(self.limit)
.skip_while(|order| {
count = count + 1;
let r = gas.overflowing_add(order.gas);
if r.1 { return false }
gas = r.0;
count <= self.limit && gas <= self.gas_limit
})
.map(|order| by_hash.get(&order.hash)
.expect("All transactions in `self.by_priority` and `self.by_address` are kept in sync with `by_hash`."))
.map(|tx| (tx.sender(), tx.nonce()))
Expand Down Expand Up @@ -423,21 +428,23 @@ impl Default for TransactionQueue {
impl TransactionQueue {
/// Creates new instance of this Queue
pub fn new(strategy: PrioritizationStrategy) -> Self {
Self::with_limits(strategy, 1024, !U256::zero())
Self::with_limits(strategy, 1024, !U256::zero(), !U256::zero())
}

/// Create new instance of this Queue with specified limits
pub fn with_limits(strategy: PrioritizationStrategy, limit: usize, tx_gas_limit: U256) -> Self {
pub fn with_limits(strategy: PrioritizationStrategy, limit: usize, gas_limit: U256, tx_gas_limit: U256) -> Self {
let current = TransactionSet {
by_priority: BTreeSet::new(),
by_address: Table::new(),
limit: limit,
gas_limit: gas_limit,
};

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

TransactionQueue {
Expand Down Expand Up @@ -488,6 +495,13 @@ impl TransactionQueue {
};
}

/// Sets new total gas limit.
pub fn set_total_gas_limit(&mut self, gas_limit: U256) {
self.future.gas_limit = gas_limit;
self.current.gas_limit = gas_limit;
self.future.enforce_limit(&mut self.by_hash);
}

/// 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: U256) {
Expand Down Expand Up @@ -797,6 +811,16 @@ impl TransactionQueue {
let nonce = tx.nonce();
let hash = tx.hash();

{
// Rough size sanity check
let gas = &tx.transaction.gas;
if U256::from(tx.transaction.data.len()) > *gas {
// Droping transaction
trace!(target: "txqueue", "Dropping oversized transaction: {:?} (gas: {} < size {})", hash, gas, tx.transaction.data.len());
return Err(TransactionError::LimitReached);
}
}

// The transaction might be old, let's check that.
// This has to be the first test, otherwise calculating
// nonce height would result in overflow.
Expand Down Expand Up @@ -1049,7 +1073,8 @@ mod test {
let mut set = TransactionSet {
by_priority: BTreeSet::new(),
by_address: Table::new(),
limit: 1
limit: 1,
gas_limit: !U256::zero(),
};
let (tx1, tx2) = new_txs(U256::from(1));
let tx1 = VerifiedTransaction::new(tx1, TransactionOrigin::External).unwrap();
Expand Down Expand Up @@ -1088,7 +1113,8 @@ mod test {
let mut set = TransactionSet {
by_priority: BTreeSet::new(),
by_address: Table::new(),
limit: 1
limit: 1,
gas_limit: !U256::zero(),
};
// Create two transactions with same nonce
// (same hash)
Expand Down Expand Up @@ -1668,7 +1694,7 @@ mod test {
#[test]
fn should_drop_old_transactions_when_hitting_the_limit() {
// given
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero());
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero(), !U256::zero());
let (tx, tx2) = new_txs(U256::one());
let sender = tx.sender().unwrap();
let nonce = tx.nonce;
Expand All @@ -1690,7 +1716,7 @@ mod test {
#[test]
fn should_return_correct_nonces_when_dropped_because_of_limit() {
// given
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 2, !U256::zero());
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 2, !U256::zero(), !U256::zero());
let tx = new_tx();
let (tx1, tx2) = new_txs(U256::one());
let sender = tx1.sender().unwrap();
Expand All @@ -1711,7 +1737,7 @@ mod test {

#[test]
fn should_limit_future_transactions() {
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero());
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero(), !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));
Expand All @@ -1728,6 +1754,16 @@ mod test {
assert_eq!(txq.status().future, 1);
}

#[test]
fn should_limit_by_gas() {
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 100, default_gas_val() * U256::from(2), !U256::zero());
let (tx1, _) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1));
let (tx3, _) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2));
txq.add(tx1.clone(), &default_nonce, TransactionOrigin::External).unwrap();
txq.add(tx3.clone(), &default_nonce, TransactionOrigin::External).unwrap();
assert_eq!(txq.status().pending, 2);
}

#[test]
fn should_drop_transactions_with_old_nonces() {
let mut txq = TransactionQueue::default();
Expand Down Expand Up @@ -1971,7 +2007,7 @@ mod test {
#[test]
fn should_keep_right_order_in_future() {
// given
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero());
let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 1, !U256::zero(), !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 };
Expand Down
8 changes: 7 additions & 1 deletion sync/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ const MAX_ROUND_PARENTS: usize = 32;
const MAX_NEW_HASHES: usize = 64;
const MAX_TX_TO_IMPORT: usize = 512;
const MAX_NEW_BLOCK_AGE: BlockNumber = 20;
const MAX_TRANSACTION_SIZE: usize = 300*1024;

const STATUS_PACKET: u8 = 0x00;
const NEW_BLOCK_HASHES_PACKET: u8 = 0x01;
Expand Down Expand Up @@ -1079,7 +1080,12 @@ impl ChainSync {
item_count = min(item_count, MAX_TX_TO_IMPORT);
let mut transactions = Vec::with_capacity(item_count);
for i in 0 .. item_count {
let tx = try!(r.at(i)).as_raw().to_vec();
let rlp = try!(r.at(i));
if rlp.as_raw().len() > MAX_TRANSACTION_SIZE {
debug!("Skipped oversized transaction of {} bytes", rlp.as_raw().len());
continue;
}
let tx = rlp.as_raw().to_vec();
transactions.push(tx);
}
io.chain().queue_transactions(transactions);
Expand Down