core/txpool: move setcode tx validation into legacyPool

[rjl493456442]

In this PR, several improvements have been made:

Authorization-related validations have been moved to legacyPool.
Previously, these checks were part of the standard validation procedure,
which applies common validations across different pools. Since these
checks are specific to SetCode transactions, relocating them to legacyPool
is a more reasonable choice.

Additionally, authorization conflict checks are now performed regardless
of whether the transaction is a replacement or not.

[lightclient]

The changes look correct to me, but not sure I understand the motivation in the PR?

We have a mechanism for defining tx validation checks that is currently used by both pools. Not every method is implemented by both pools, e.g. FirstNonceGap is not implemented by the legacy pool. KnownConflicts is similar where it is optional for pools to implement and so only the legacy pool implements it.

If we go through this PR, we also need to change the comment for UsedAndLeftSlots because it is currently listed as "mandatory" for pools to implement.

authorization conflict checks are now performed regardless
of whether the transaction is a replacement or not.

could you point out this change for me also? having trouble seeing this in the diff

[lightclient]

Suggested change

	// Replace the existing inflight transaction for delegated accounts
	// Replace the existing in-flight transaction for delegated accounts

[lightclient]

Suggested change

	if !exists {
	exists = queue.Contains(tx.Nonce()) || exists

simpler ?

[rjl493456442]

The changes look correct to me, but not sure I understand the motivation in the PR?

The primary motivation is to keep common validation as generic as possible and not to
pile up additional checks endlessly. KnownConflict is very SetCodeTx specific and
won't be required for blobPool, do the validation inside of legacyPool is a better option
to me.

e.g. FirstNonceGap is not implemented by the legacy pool

It's true that legacyPool doesn't need to check the nonce gap. But it's the technical debt
and the legacypool might be refactored later to abstract the nonce checking in the common
validation.

Checking nonce is a very common requirement, while auth conflict is very specific to
certain types of transaction.

could you point out this change for me also? having trouble seeing this in the diff

This KnownConflict check is only performed the transaction with same nonce is not
existent. For transaction replacement, this check is skipped. I feel it could be an attack
vector to bypass the restriction.

[lightclient]

Okay I see what you mean where KnownConflicts only runs when the tx is new and not replacing another pending tx. Definitely want to resolve that. I feel indifferent about the more broad change of removing KnownConflicts and stopping the use of UsedAndLeftSlots. I think we are seeing this interface was trying to be generic, but turns out it is hard to define validation options across the pools that can be reused.

Since you have already implemented I suppose we can just accept this. Once we fix the nits on this PR it can be merged.

[rjl493456442]

Yep, we should definitely fix the KnownConflict validations for transaction replacement.

@fjl How do you feel about the changes of relocating the setcode-tx specific validations to the legacypool?
I feel the txpool.Validation should be the common validations across different pools and Matt thinks it's a
generic validation framework and different rules can be added.

Would be nice to have some inputs from your side

[lightclient]

LGTM - will give Felix a couple days in case he wants to weigh in

[MariusVanDerWijden]

LGTM, but I agree with @lightclient that we should probably get rid of the validationOptionsWithState in favor of some code duplication since it isn't a great abstraction