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

Adds rpc method for anvil to drop all pending transactions #7643

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

mw2000
Copy link
Contributor

@mw2000 mw2000 commented Apr 12, 2024

Motivation

Fixes #7249

Solution

Added a new rpc method, and added the corresponding implementation in the mod.rs file for pool.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

supportive

smol suggestion

Comment on lines 171 to 176
let mut transactions_iterator = pool.pending_transactions.transactions();

while let Some(pool_transaction) = transactions_iterator.next() {
let tx_hash = pool_transaction.as_ref().hash();
self.drop_transaction(tx_hash);
}
Copy link
Member

Choose a reason for hiding this comment

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

hmm, I think we want a fn clear on both tx sets

ready_transactions: ReadyTransactions,
pending_transactions: PendingTransactions,

that clear all maps

@mw2000
Copy link
Contributor Author

mw2000 commented Apr 12, 2024

@mattsse thank you for the quick review! if I understand you correctly, we want to clear both pending and ready transactions in the drop all transactions method?

@mw2000 mw2000 requested a review from mattsse April 12, 2024 16:47
@mw2000
Copy link
Contributor Author

mw2000 commented Apr 12, 2024

Now iterating over the ready transactions map and dropping them too.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

style suggestion

Comment on lines 169 to 184
pub fn drop_all_transactions(&self) {
let pool = self.inner.write();

let pending_transactions = pool.pending_transactions.transactions();
let ready_transactions = pool.ready_transactions.get_transactions();

pending_transactions.for_each(|pool_transaction| {
let tx_hash = pool_transaction.as_ref().hash();
self.drop_transaction(tx_hash);
});

ready_transactions.for_each(|pool_transaction| {
let tx_hash = pool_transaction.as_ref().hash();
self.drop_transaction(tx_hash);
});
}
Copy link
Member

Choose a reason for hiding this comment

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

while this works, I want this to be

fn clear(&self) {
let pool = self.inner.write();
pool.clear(); // this clears both tx sets, calls clear on all their internals
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, and this should be more performant too

…c to clear transaction internals + be more performant
@mw2000
Copy link
Contributor Author

mw2000 commented Apr 12, 2024

Moved to a clear function as requested @mattsse

@mw2000 mw2000 requested a review from mattsse April 12, 2024 19:25
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

clean

@mattsse mattsse merged commit 89f0fb9 into foundry-rs:master Apr 12, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a method to remove all pending txns from the txpool
2 participants