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

Commit

Permalink
tx-pool: accept local tx with higher gas price when pool full (#10901)
Browse files Browse the repository at this point in the history
* tx-pool: accept local tx with higher gas price when pool full

* Revert "tx-pool: accept local tx with higher gas price when pool full"

This reverts commit 9d4adc5

* tx-pool: new tx with same nonce as existing is ready

* tx-pool: explicit check for replacement tx (same sender & nonce)

* Update comment

Co-Authored-By: Tomasz Drwięga <[email protected]>

* Replace `ReplaceOld` with `InsertNew` for replacement txs
  • Loading branch information
ascjones authored and s3krit committed Sep 10, 2019
1 parent 12c9169 commit c80f98d
Showing 1 changed file with 98 additions and 0 deletions.
98 changes: 98 additions & 0 deletions miner/src/pool/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ where
let old_score = (old.priority(), old.gas_price());
let new_score = (new.priority(), new.gas_price());
if new_score > old_score {
// Check if this is a replacement transaction.
//
// With replacement transactions we can safely return `InsertNew` here, because
// we don't need to remove `old` (worst transaction in the pool) since `new` will replace
// some other transaction in the pool so we will never go above limit anyway.
if let Some(txs) = new.pooled_by_sender {
if let Ok(index) = txs.binary_search_by(|old| self.scoring.compare(old, new)) {
return match self.scoring.choose(&txs[index], new) {
Choice::ReplaceOld => Choice::InsertNew,
choice => choice,
}
}
}

let state = &self.client;
// calculate readiness based on state nonce + pooled txs from same sender
let is_ready = |replace: &ReplaceTransaction<T>| {
Expand Down Expand Up @@ -412,4 +426,88 @@ mod tests {

assert_eq!(replace.should_replace(&old, &new), ReplaceOld);
}

#[test]
fn should_accept_local_tx_with_same_sender_and_nonce_with_better_gas_price() {
let scoring = NonceAndGasPrice(PrioritizationStrategy::GasPriceOnly);
let client = TestClient::new().with_nonce(1);
let replace = ReplaceByScoreAndReadiness::new(scoring, client);

// current transaction is ready
let old_tx = {
let tx = Tx {
nonce: 1,
gas_price: 1,
..Default::default()
};
tx.signed().verified()
};

let new_sender = Random.generate().unwrap();
let tx_new_ready_1 = local_tx_verified(Tx {
nonce: 1,
gas_price: 1,
..Default::default()
}, &new_sender);

let tx_new_ready_2 = local_tx_verified(Tx {
nonce: 1,
gas_price: 2, // same nonce, higher gas price
..Default::default()
}, &new_sender);

let old_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(old_tx) };

let new_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_2) };
let pooled_txs = [
txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_1) },
];

let old = ReplaceTransaction::new(&old_tx, None);
let new = ReplaceTransaction::new(&new_tx, Some(&pooled_txs));

assert_eq!(replace.should_replace(&old, &new), InsertNew);
}

#[test]
fn should_reject_local_tx_with_same_sender_and_nonce_with_worse_gas_price() {
let scoring = NonceAndGasPrice(PrioritizationStrategy::GasPriceOnly);
let client = TestClient::new().with_nonce(1);
let replace = ReplaceByScoreAndReadiness::new(scoring, client);

// current transaction is ready
let old_tx = {
let tx = Tx {
nonce: 1,
gas_price: 1,
..Default::default()
};
tx.signed().verified()
};

let new_sender = Random.generate().unwrap();
let tx_new_ready_1 = local_tx_verified(Tx {
nonce: 1,
gas_price: 2,
..Default::default()
}, &new_sender);

let tx_new_ready_2 = local_tx_verified(Tx {
nonce: 1,
gas_price: 1, // same nonce, lower gas price
..Default::default()
}, &new_sender);

let old_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(old_tx) };

let new_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_2) };
let pooled_txs = [
txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_1) },
];

let old = ReplaceTransaction::new(&old_tx, None);
let new = ReplaceTransaction::new(&new_tx, Some(&pooled_txs));

assert_eq!(replace.should_replace(&old, &new), RejectNew);
}
}

0 comments on commit c80f98d

Please sign in to comment.