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

Unify txpool.remove function #6641

Merged
merged 6 commits into from Feb 20, 2024

Conversation

loocapro
Copy link
Contributor

Related to: #6617

Copy link
Collaborator

@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.

@Rjected

this removes the distinction between remove and prune, and fixes the bug if TransactionPool::remove_transaction is called manually, which we currently only do in auto-seal

/// Removes all transactions corresponding to the given hashes.
///
/// Also removes all _dependent_ transactions.
///
/// Consumer: Block production
fn remove_transactions(
&self,
hashes: Vec<TxHash>,

This isn't correct now, and I think we should change this to adhere to the docs?

now we're advancing the independent tx on remove
this does not affect how we remove mined transactions
and also does not affect how we discard txs because we remove all descendant txs there

pending @Rjected

Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

lgtm, yeah the trait docs should be updated

@Rjected Rjected added this pull request to the merge queue Feb 20, 2024
Merged via the queue into paradigmxyz:main with commit e676f8c Feb 20, 2024
29 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.

None yet

4 participants