txpool api: remove_invalid call improved

[michalkucharczyk]

Description

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:

polkadot-sdk/substrate/client/transaction-pool/api/src/lib.rs

Lines 293 to 314 in 5b70ad9

	// *** Block production
	/// 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
	/// removed from the pool will be notified with `TransactionStatus::Invalid` event (if
	/// `submit_and_watch` was used for submission).
	///
	/// 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.
	///
	/// Function returns the transactions actually removed from the pool.
	fn report_invalid(
	&self,
	at: Option<<Self::Block as BlockT>::Hash>,
	invalid_tx_errors: &[(TxHash<Self>, Option<TransactionValidityError>)],
	) -> 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.

Notes for reviewers

• Actual logic of removing invalid txs is impleneted in ViewStore::report_invalid. Method's doc explains the flow.
• This PR changes HashMap to IndexMap in revalidation logic. This is to preserve the original order of transactions (mainly for purposes of unit tests).
• This PR solves the problem mentioned in: fatxpool: add tests for rejecting invalid transactions #5477 (comment) (which can now be resolved). The invalid transactions found during mempool revalidation are now also removed from the view_store. No dangling invalid transaction shall be left in the pool. (bfec262)
• The support for dropping invalid transactions reported from the views was also added. This should never happen, but if for any case all views will report invalid transcation (which previously was valid) the transaction will be dropped from the pool (48214a3).

fixes: #6008, #5477

[michalkucharczyk]

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

Maybe 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? (This could also be done as follow-up).

[michalkucharczyk]

I'd like to force-push here to clean up commit history.
Is that fine for you (@skunert @iulianbarbu)?

[skunert]

Looks good.

[michalkucharczyk]

I decided not to force push to preserve comments which could be deleted.

I added fatp_invalid.rs file, which focuses on handling widely understood invalid transactions (submit / revalidate and report).

Some tests are new, but some where just moved (and slightly updated, with no logic changes) from the original fatp.rs file.

I also reviewed these tests having in mind #5477.

[paritytech-workflow-stopper]

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/13174136857
Failed job name: cargo-clippy