Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evict transaction from transaction pool #2797

Merged
merged 2 commits into from
May 13, 2019
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
2 changes: 1 addition & 1 deletion pool/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ impl Pool {
/// containing the tx it depends on.
/// Sorting the buckets by fee_to_weight will therefore preserve dependency ordering,
/// maximizing both cut-through and overall fees.
fn bucket_transactions(&self, weighting: Weighting) -> Vec<Transaction> {
pub fn bucket_transactions(&self, weighting: Weighting) -> Vec<Transaction> {
let mut tx_buckets: Vec<Bucket> = Vec::new();
let mut output_commits = HashMap::new();
let mut rejected = HashSet::new();
Expand Down
44 changes: 38 additions & 6 deletions pool/src/transaction_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,13 @@ impl TransactionPool {
}

// Do we have the capacity to accept this transaction?
self.is_acceptable(&tx, stem)?;
let acceptability = self.is_acceptable(&tx, stem);
let mut evict = false;
if !stem && acceptability.as_ref().err() == Some(&PoolError::OverCapacity) {
evict = true;
} else if acceptability.is_err() {
return acceptability;
}

// Make sure the transaction is valid before anything else.
// Validate tx accounting for max tx weight.
Expand Down Expand Up @@ -171,9 +177,36 @@ impl TransactionPool {
self.adapter.tx_accepted(&entry.tx);
}

// Transaction passed all the checks but we have to make space for it
if evict {
self.evict_from_txpool();
}

Ok(())
}

// Remove the last transaction from the flattened bucket transactions.
// No other tx depends on it, it has low fee_to_weight and is unlikely to participate in any cut-through.
pub fn evict_from_txpool(&mut self) {
// Get bucket transactions
let bucket_transactions = self.txpool.bucket_transactions(Weighting::NoLimit);

// Get last transaction and remove it
match bucket_transactions.last() {
Some(evictable_transaction) => {
// Remove transaction
Copy link
Member

Choose a reason for hiding this comment

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

We should be able use entries.remove() here I think.

Copy link
Member Author

@quentinlesceller quentinlesceller May 3, 2019

Choose a reason for hiding this comment

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

I'm not sure this is doable since we are comparing a PoolEntry with a Transaction.
If both were PoolEntry a remove_item(...) would be possible
(or there is a cheap way to get the index of the PoolEntry that I am missing).

Copy link
Member

Choose a reason for hiding this comment

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

Ah good point.

self.txpool.entries = self
.txpool
.entries
.iter()
.filter(|x| x.tx != *evictable_transaction)
.map(|x| x.clone())
.collect::<Vec<_>>();
}
None => (),
}
}

// Old txs will "age out" after 30 mins.
pub fn truncate_reorg_cache(&mut self, cutoff: DateTime<Utc>) {
let mut cache = self.reorg_cache.write();
Expand Down Expand Up @@ -245,11 +278,10 @@ impl TransactionPool {
}

// Check that the stempool can accept this transaction
if stem {
if self.stempool.size() > self.config.max_stempool_size {
// TODO evict old/large transactions instead
return Err(PoolError::OverCapacity);
}
if stem && self.stempool.size() > self.config.max_stempool_size {
return Err(PoolError::OverCapacity);
} else if self.total_size() > self.config.max_pool_size {
return Err(PoolError::OverCapacity);
}

// for a basic transaction (1 input, 2 outputs) -
Expand Down
2 changes: 1 addition & 1 deletion pool/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ pub struct TxSource {
}

/// Possible errors when interacting with the transaction pool.
#[derive(Debug, Fail)]
#[derive(Debug, Fail, PartialEq)]
pub enum PoolError {
/// An invalid pool entry caused by underlying tx validation error
#[fail(display = "Invalid Tx {}", _0)]
Expand Down