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

txpool api: remove invalid call improved #6661

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

michalkucharczyk
Copy link
Contributor

@michalkucharczyk michalkucharczyk commented Nov 26, 2024

Currently the transaction which is reported as invalid by a block builder (or removed_invalid by other components) is silently skipped.

This PR improves this behavior. The transaction pool report_invalid function now accepts optional error associated with every reported transaction, and also the optional block hash which provides hints how reported transaction shall be handled. The following API change is proposed:

/// Reports invalid transactions to the transaction pool.
///
/// This function accepts an array of tuples, each containing a transaction hash and an
/// optional error encountered during the transaction execution at a specific (also optional)
/// block.
///
/// The transaction pool implementation decides which transactions to remove. Transactions
/// dependent on invalid ones will also be removed.
///
/// If the tuple's error is None, the transaction will be forcibly removed from the pool.
///
/// The optional `at` parameter provides additional context regarding the block where the error
/// occurred.
fn report_invalid(
&self,
at: Option<<Self::Block as BlockT>::Hash>,
invalid_tx_errors: &[(TxHash<Self>, Option<sp_blockchain::Error>)],
) -> Vec<Arc<Self::InPoolTransaction>>;

Depending on error, the transaction pool can decide if transaction shall be removed from the view only or entirely from the pool. Invalid event will be dispatched if required.

fixes: #6008

&self,
at: Option<<Self::Block as BlockT>::Hash>,
invalid_tx_errors: &[(TxHash<Self>, Option<sp_blockchain::Error>)],
) -> Vec<Arc<Self::InPoolTransaction>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also keep old function remove_invalid and add new one, with no optional error/block parameters. Really no strong opinion here.

I also think that triggering invalid event in case of removal the transaction via RPC call is (maybe) undesirable - so maybe we should have pure remove function (which would trigger Dropped, IMO it makes more sense). Any thoughts?

@michalkucharczyk michalkucharczyk changed the title txpool api: remove invalid call improved txpool api: remove invalid call improved Nov 26, 2024
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/12036823135
Failed job name: test-linux-stable-no-try-runtime

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.

fatxpool: transaction reported as invalid during block building shall be removed from the pool
1 participant