feat(mempool): remove transaction for locally mined blocks that fail validation

[sdbondi]

Description

On block validation failure submitted locally (i.e. via grpc), clear all associated transactions.

Motivation and Context

This is a failsafe that prevents transactions that passes transaction validation but fails block validation, preventing a miner from submitting a block until the mempool is manually cleared.

This should be rarely applied and be a last line of defense/failsafe against incorrect transaction validation and block template generation.

How Has This Been Tested?

[SWvheerden]

Why are even getting this?
Why would a transaction fail transaction validation but not block validation?
Surely the problem lies in the disconnect between the two?

[sdbondi]

@SWvheerden this change only applies to transactions already in the mempool (transactions validation passes) but block validation of a locally mined block fails. A failsafe, which kicks in to prevent the chain from stalling likely from some validation edge case that is not handled. In general, this should never happen (failsafe).

[SWvheerden]

Well, that's exactly kinda the point it's suppressing a potential vulnerability in the validation.
I would rather stop the testnet chain, than let an unknown validation bug go through to mainnet.

If a transaction fails block validation but passes mempool validation, we have a bug in validation or block construction. It's very important we fix those rather, especially edge cases.

[sdbondi]

Of course, you are right about fixing the issue. It's not "suppressing" anything though, just handing the case locally for the miner, instead of falling over and over - a miner may choose not to include transactions if it so wishes.

For sure, alarm bells should go off in this case, and nothing prevents you/the system/monitoring from recognising this case and raising them. In the meantime, the chain can progress.