txpool: invalid interop tx conditionally included

[pcw109550]

When interop invalid exec msg tx is received, op-geth filters these tx via ingress filter:

op-geth/core/txpool/ingress_filters.go

Lines 17 to 19 in 47ccb23

	type IngressFilter interface {
	FilterTx(ctx context.Context, tx *types.Transaction) bool
	}

and never be included in the txpool. However, while running acceptance test we observed that some invalid exec msg is sometimes persist in the mempool.

Monorepo Issue:

• op-acceptance-tests: interop tx with invalid attribute not evicted from the mempool optimism#16559

Reproduction

• [DO NOT MERGE] op-acceptance-tests: interop invalid attribute exec msg tx mempool evict experiment optimism#16569

Because of #629, after 12 seconds the invalid tx will eventually be evicted from the mempool.

However this is a temporal fix, and there are still time span which the initially invalid txs enter the mempool.

Note that this is non-deterministic, and may be caused due to some race condition.

Find out why this happens and fix.

[axelKingsley]

I have been trying to dig deeper into this issue, and I cannot figure out how this bug might be possible.

Reproduction: I have been running the test in question from @pcw109550 's branch on repeat, with a filter to show the mempool contents on a separte screen. I'm running a count of 4, and requesting to run in parallel (though I think it's still serial). In my experience, running many tests in parallel is a good way to reproduce race conditions.

However, I have not had a single instance of the test include an invalid transaction in the mempool.

Analysis: I traced through geth, and at no point did I find any possible branching logic which could add to the mempool in any other way from the public Add function. The flow looks like:

• sendRawTransaction
• SubmitTransaction
• backend.SendTransaction
• internal backend sendTransaction
• pool.Add
• subpool.Add
• addTransactionsLocked

Prior to a recent change, the mempool performed ingress filtering at the subpool.Add level, but now it is at the addTransactionsLocked level. In either case, any transaction submitted would have gone through the filter, unavoidably. There are a couple notable branches, but those turned out not to matter either:

• The backend forwards Tx to the sequencer, but this is through sendRawTx anyway
• Some transactions can be added to the Local Tracker, but this feeds transactions back through pool.Add anyway

I am running the test on a loop in the background while I do other work, but am at a dead-end. @pcw109550 do you have any tips fro reproduction?

[pcw109550]

@axelKingsley I experienced this behavior at the op-acceptance-tests CI at ethereum-optimism/optimism#16569.
If you see the CI logs at https://app.circleci.com/pipelines/github/ethereum-optimism/optimism?branch=pcw109550%2Facceptance-test-interop-mempool-evict-experiment, you can see that I attempted to run the acceptance-tests-pr pipeline 9 times. 8 of them were green, and the last trial gave me the error case where the invalid tx was conditionally included.

There are some artifacts at https://app.circleci.com/pipelines/github/ethereum-optimism/optimism/94456/workflows/5e423a63-cea0-47cd-b777-51b7002daab6/jobs/3669140/artifacts when the flakiness occurred.

https://output.circle-artifacts.com/output/job/60e6f41e-fc0d-4d0a-adf9-02b46f876dc4/artifacts/0/op-acceptance-tests/logs/testrun-c79fd1f4-1d6c-444c-aafa-cb4991bb6b3d/passed/interop_message_TestExecMessageInvalidAttributes.log
this is the most interesting one since at least two invalid txs were inserted to the mempool, but one got evicted, the other got stuck.