Pre-checker tx filtering (using estimate gas call) by MishkaRogachev · Pull Request #4583 · OffchainLabs/nitro

MishkaRogachev · 2026-03-30T16:08:43Z

fixes NIT-4344
Pulls in OffchainLabs/go-ethereum#643

Add address filtering to TxPreChecker via gas estimation dry-run. Pre-checker now runs transactions through gasestimator.Run to detect filtered addresses before forwarding, catching sender/recipient, aliased, retryable, and event-based address matches including cascading redeems.

….PostTxFilter

…n-api

…ictnessNone

…n-api

…n-api-2

… gas_estimate

codecov · 2026-03-31T15:35:21Z

Codecov Report

❌ Patch coverage is 27.55102% with 71 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.98%. Comparing base (192393a) to head (0c9ab80).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4583      +/-   ##
==========================================
- Coverage   34.20%   33.98%   -0.23%     
==========================================
  Files         494      495       +1     
  Lines       58926    58972      +46     
==========================================
- Hits        20156    20041     -115     
- Misses      35230    35391     +161     
  Partials     3540     3540

github-actions · 2026-03-31T16:29:07Z

❌ 9 Tests Failed:

Tests completed	Failed	Passed	Skipped
2396	9	2387	0

View the top 3 failed tests by shortest run time

TestPruningDBSizeReduction

Stack Traces | 0.000s run time

=== RUN   TestPruningDBSizeReduction
--- FAIL: TestPruningDBSizeReduction (0.00s)

TestAliasingFlaky

Stack Traces | -0.000s run time

=== RUN   TestAliasingFlaky
=== PAUSE TestAliasingFlaky
=== CONT  TestAliasingFlaky
    common_test.go:777: BuildL1 deployConfig: DeployBold=true, DeployReferenceDAContracts=false
INFO [04-02|20:13:22.944] Started log indexer
WARN [04-02|20:13:22.944] Getting file info                        dir= error="stat : no such file or directory"

TestBatchPosterL1SurplusMatchesBatchGasFlaky

Stack Traces | 0.570s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
panic: runtime error: invalid memory address or nil pointer dereference [recovered, repanicked]
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x207f932]

goroutine 48 [running]:
testing.tRunner.func1.2({0x37ed480, 0x620c9b0})
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1872 +0x237
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1875 +0x35b
panic({0x37ed480?, 0x620c9b0?})
	/opt/hostedtoolcache/go/1.25.8/x64/src/runtime/panic.go:783 +0x132
github.com/offchainlabs/nitro/arbnode.(*InboxTracker).GetBatchCount(0x1ac87900?)
	/home/runner/work/nitro/nitro/arbnode/inbox_tracker.go:210 +0x12
github.com/offchainlabs/nitro/arbnode.(*InboxTracker).FindInboxBatchContainingMessage(0x0, 0x8)
	/home/runner/work/nitro/nitro/arbnode/inbox_tracker.go:225 +0x2f
github.com/offchainlabs/nitro/system_tests.TestBatchPosterL1SurplusMatchesBatchGasFlaky(0xc0002c36c0)
	/home/runner/work/nitro/nitro/system_tests/batch_poster_test.go:839 +0x725
testing.tRunner(0xc0002c36c0, 0x41c0bb0)
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1934 +0xea
created by testing.(*T).Run in goroutine 1
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1997 +0x465

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

Copilot

Pull request overview

This PR extends transaction address-filtering to the forwarder path by having TxPreChecker run a gas-estimation dry-run (via gasestimator.Run) to detect filtered addresses before forwarding transactions, covering direct sender/recipient touches, retryables, and event-based matches (including cascading redeems).

Changes:

Add pre-checker filtering using a gas-estimation dry-run and a new txFilterer implementation for the backend.
Refactor transaction-filtering configuration wiring so filters/services are initialized at the execution-node level (and adjust tests accordingly).
Add system tests validating prechecker-based filtering in a two-node (sequencer + forwarder) setup, plus a Solidity helper for retryable redemption.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
system_tests/tx_address_filter_test.go	Updates filtering-readiness test to inject a filter service directly.
system_tests/retryable_tickets_filtering_test.go	Forces manual gas limits to bypass `eth_estimateGas` pre-rejection in filtering tests.
system_tests/prechecker_filter_test.go	New end-to-end system tests for prechecker filtering via forwarder dry-runs.
system_tests/delayed_message_filter_test.go	Updates config field paths for transaction filterer RPC client / delayed sequencing filter toggle.
system_tests/common_test.go	Updates helper to set event filter rules via the new config location.
execution/gethexec/tx_pre_checker.go	Adds backend wiring + dry-run filtering check before publishing/forwarding.
execution/gethexec/tx_filterer.go	New tx filterer hook used by gas estimation to detect filtered touches.
execution/gethexec/sequencer.go	Moves filtering/service setup out of `SequencerConfig`; adjusts constructor and test helper.
execution/gethexec/node.go	Introduces top-level `TransactionFiltering` config; initializes services and wires backend + prechecker.
execution/gethexec/executionengine.go	Refactors address-touching helper to be reusable by the new tx filterer.
contracts-local/src/mocks/AddressFilterTest.sol	Adds a wrapper method to trigger retryable redemption from a contract.
changelog/mrogachev-nit-4344.md	Adds changelog entry for prechecker filtering behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T17:22:14Z

execution/gethexec/sequencer.go

-		t.Fatal("addressFilterService is nil")
-	}
-	s.addressFilterService.GetHashStore().Store(salt, hashes, digest)
+	s.addressFilterService = service


SetAddressFilterServiceForTest only swaps the addressFilterService pointer; it doesn't mirror the production wiring done in ExecutionNode.Start (starting the service and setting ExecEngine's address checker). This can leave tests in an inconsistent state where FilteringReady() reflects the injected service but actual filtering behavior still uses the previous checker (or none). Consider also setting s.execEngine.SetAddressChecker(service.GetAddressChecker()) (and optionally starting/stopping the service), or rename/document the helper to clarify it only affects readiness checks.

Suggested change

s.addressFilterService = service

s.addressFilterService = service

if s.execEngine != nil && service != nil {

s.execEngine.SetAddressChecker(service.GetAddressChecker())

}

pre-existing, i believe it should be handled separately with some tests refactor

system_tests/retryable_tickets_filtering_test.go

execution/gethexec/tx_pre_checker.go

execution/gethexec/node.go

diegoximenes

There should also be tests covering eth_estimateGas filtering.

execution/gethexec/node.go

execution/gethexec/tx_filterer.go

diegoximenes · 2026-04-01T13:23:39Z

execution/gethexec/tx_filterer.go

+}
+
+func (f *txFilterer) TouchAddresses(statedb *state.StateDB, tx *types.Transaction, sender common.Address) {
+	touchAddresses(statedb, nil, tx, sender)


Using nil here is weird.
touchAddresses implementation in this PR was originally used only for the post tx filter for the delayed sequencing.
This txFilterer.TouchAddresses function is used for a pre tx filter for estimateGas/prechecker.
For regular tx sequencing, sequencer applies other code for pre tx filtering, of only touching from/to.

Why in this case txFilterer use delayed sequencing pre tx filtering policy instead of regular sequencing pre tx filtering policy?

TouchAddresses is called only for scheduled txes (retryable redeems) from RunScheduledTxes: not for the main tx. These are retryables, so we need the full set: aliased sender and retryable-specific fields. The regular preTxFilter's from/to-only approach would miss those.

For the main tx, gasestimator touches only from/to directly, matching the regular preTxFilter policy. We can't reuse TouchAddresses there because gasestimator uses *core.Message not *types.Transaction.

So, the nil event filter is because event-based filtering happens once at the end in CheckFiltered, across all accumulated logs.

These are retryables, so we need the full set: aliased sender and retryable-specific fields. The regular preTxFilter's from/to-only approach would miss those.

Current scenario:

For delayed sequencing, the post tx filter calls touchRetryableAddresses, to handle ArbitrumSubmitRetryableTx in a special way. It also handles "TxTypeAlias" in a special way.

For regular transaction sequencing, touchRetryableAddresses is not called, ArbitrumSubmitRetryableTx is not handled in a special way in any step of the filtering process.

For pre-checker/eth_estimateGas we are enforcing to use the same policy as the delayed sequencing.

All those scenarios handle scheduled transactions.

At this point this is confusing to me.
I don't understand if regular transaction sequencing has an issue right now, and it should actually handle ArbitrumSubmitRetryableTx in a special way.
Or if we are enforcing this ArbitrumSubmitRetryableTx handling in eth_estimateGas unnecessarily, which doesn't seem create wrong results, but the whole inconsistency is causing me confusion.
I will let the way it is right now, but depending on the reason we should revisit it in another task to clean up this inconsistency.

@tsahee am I missing something?

So the event filter will be added in checkFiltered and de-facto will enforce the same policy as normal transactions. But it deserves more thought and possibly renaming

diegoximenes · 2026-04-01T13:31:59Z

execution/gethexec/tx_pre_checker.go

 	if err != nil {
 		return err
 	}
+	if err := c.checkFilteredAddresses(tx, block); err != nil {


This call can be made inside PreCheckTx, so you can avoid calling it twice, here and PublishExpressLaneTransaction.

I think we discussed it on previous attempt with dry-run, these two paths are not the same. PreCheckTx has an early return at Strictness < LikelyCompatible that exits before the end, while checkFilteredAddresses only needs Strictness >= AlwaysCompatible. Restructuring the early returns to accommodate this messes things up.

checkFilteredAddresses could be done in the beginning of PreCheckTx to avoid that.
But fully running current PreCheckTx can avoid unnecessary gas estimation anyway.

system_tests/retryable_tickets_filtering_test.go

execution/gethexec/tx_pre_checker.go

system_tests/prechecker_filter_test.go

diegoximenes · 2026-04-02T15:07:28Z

execution/gethexec/node.go


+type TransactionFilteringConfig struct {
+	DisableDelayedSequencingFilter bool                          `koanf:"disable-delayed-sequencing-filter"`
+	EnableRPCFilter                bool                          `koanf:"enable-rpc-filter"`


EnableRPCFilter is too generic to me, since publishing a transaction also happens through RPC.
I am inclined on using EnableNonMutatingRPCFilter, but I think Tsahi doesn't like it.
I will let @tsahee decide that.

Suggested change

EnableRPCFilter bool `koanf:"enable-rpc-filter"`

EnableNonMutatingRPCFilter bool `koanf:"enable-non-mutating-rpc-filter"`

I indeed don't like non-mutating. enable-ethcall-filter? I know it's not exact but much more self-explanatory.

It should be identical to: https://github.com/OffchainLabs/nitro/pull/4591/changes

diegoximenes · 2026-04-02T16:46:08Z

execution/gethexec/tx_pre_checker.go

 	if err != nil {
 		return err
 	}
+	if err := c.checkFilteredAddresses(tx, block); err != nil {


checkFilteredAddresses could be done in the beginning of PreCheckTx to avoid that.
But fully running current PreCheckTx can avoid unnecessary gas estimation anyway.

system_tests/retryable_tickets_filtering_test.go

system_tests/delayed_message_filter_test.go

diegoximenes · 2026-04-02T18:18:24Z

execution/gethexec/tx_filterer.go

+}
+
+func (f *txFilterer) TouchAddresses(statedb *state.StateDB, tx *types.Transaction, sender common.Address) {
+	touchAddresses(statedb, nil, tx, sender)


These are retryables, so we need the full set: aliased sender and retryable-specific fields. The regular preTxFilter's from/to-only approach would miss those.

Current scenario:

For delayed sequencing, the post tx filter calls touchRetryableAddresses, to handle ArbitrumSubmitRetryableTx in a special way. It also handles "TxTypeAlias" in a special way.

For regular transaction sequencing, touchRetryableAddresses is not called, ArbitrumSubmitRetryableTx is not handled in a special way in any step of the filtering process.

For pre-checker/eth_estimateGas we are enforcing to use the same policy as the delayed sequencing.

All those scenarios handle scheduled transactions.

At this point this is confusing to me.
I don't understand if regular transaction sequencing has an issue right now, and it should actually handle ArbitrumSubmitRetryableTx in a special way.
Or if we are enforcing this ArbitrumSubmitRetryableTx handling in eth_estimateGas unnecessarily, which doesn't seem create wrong results, but the whole inconsistency is causing me confusion.
I will let the way it is right now, but depending on the reason we should revisit it in another task to clean up this inconsistency.

@tsahee am I missing something?

tsahee · 2026-04-02T23:14:24Z

execution/gethexec/node.go

+	if n.AddressFilterService != nil {
+		n.AddressFilterService.Start(ctx)
+		checker := n.AddressFilterService.GetAddressChecker()
+		n.ExecEngine.SetAddressChecker(checker)


is this o.k. in this order?

I think we need to move get/set address checker to (n *ExecutionNode) Initialize, and then ExecEngine, when started, will know it has an addresschecker (so it could do things like refuse transactions while the filter is not initialized)

tsahee · 2026-04-02T23:33:09Z

execution/gethexec/tx_filterer.go

+}
+
+func (f *txFilterer) TouchAddresses(statedb *state.StateDB, tx *types.Transaction, sender common.Address) {
+	touchAddresses(statedb, nil, tx, sender)


So the event filter will be added in checkFiltered and de-facto will enforce the same policy as normal transactions. But it deserves more thought and possibly renaming

tsahee · 2026-04-03T23:31:07Z

execution/gethexec/executionengine.go

-	if tx.Type() == types.ArbitrumInternalTxType {
-		return nil
-	}
+func touchAddresses(db *state.StateDB, ef *eventfilter.EventFilter, tx *types.Transaction, sender common.Address) {


I think Diego's (justified) confusion comes from the fact that this function tries to do two things:
both "touch addresses" and apply event filter, while the function with the same name in core.TxFilterer only does one of those things, and eventFilter is applied later.

Either remove eventFilter from touchAddresses, or make the two funcs have different names.

tsahee · 2026-04-03T23:40:57Z

execution/gethexec/node.go

 	if config.Sequencer.Enable {
 		seqConfigFetcher := func() *SequencerConfig { return &configFetcher.Get().Sequencer }
-		sequencer, err = NewSequencer(execEngine, parentChainReader, seqConfigFetcher, seqParentChain)
+		if config.TransactionFiltering.TransactionFiltererRPCClient.URL != "" {


not critical for this PR - but reporting will also move and will also cover (at least) prechecker

tsahee · 2026-04-03T23:43:41Z

execution/gethexec/node.go

+	if n.AddressFilterService != nil {
+		n.AddressFilterService.Start(ctx)
+		checker := n.AddressFilterService.GetAddressChecker()
+		n.ExecEngine.SetAddressChecker(checker)


I think we need to move get/set address checker to (n *ExecutionNode) Initialize, and then ExecEngine, when started, will know it has an addresschecker (so it could do things like refuse transactions while the filter is not initialized)

MishkaRogachev and others added 23 commits March 2, 2026 15:58

Prechecker address filtering, mirrors DelayedFilteringSequencingHooks…

494c6ba

….PostTxFilter

Support redeems filtering in preCheckAddressFilter

a07f255

Refactor address touching in execution path to reuse in prechecker

27f5611

Merge remote-tracking branch 'origin/master' into prefilter-blockchai…

ae331b2

…n-api

Handle cascading redeems filtering on pre-checker path

0fa289f

Refactor duplication in preCheckAddressFilter

96dd07e

Merge branch 'master' into prefilter-blockchain-api

61e6bcb

Limit gas usage for pre-checker filtering and improve warnings

e6c5b4c

Move defaultSpeculativeFilterGasCap to config

36f4ab2

Remove artificial gas cap

a31a18c

Merge branch 'master' into prefilter-blockchain-api

c634be1

Improve TestPrecheckerFilterCascadingRedeemDepthN tests

844c9c7

Review fixes

b59cc31

Merge remote-tracking branch 'origin/master' into prefilter-blockchai…

bd4eeab

…n-api

Use real gas limit for filtering on the pre-checker

40ebc29

Move filtering config to gethexec and reuse it for prechecker

ada54be

Prechecker skips address filtering when strictness is TxPreCheckerStr…

8398ff4

…ictnessNone

Fix TestSyncBlockedUntilFilteringReady for gethexec config change

2f54a4f

Merge remote-tracking branch 'origin/master' into prefilter-blockchai…

7139535

…n-api

Merge remote-tracking branch 'origin/master' into prefilter-blockchai…

3765d49

…n-api

Make pre-checker use gasestimator.Run for filtering

b749e7d

Replace precheckerBackend with node backend

992c0d8

Simplify pre-checker wiring for tx filtering

78d9069

MishkaRogachev mentioned this pull request Mar 30, 2026

core: TxFilterer interface for address-based transaction filtering in gas estimation OffchainLabs/go-ethereum#643

Open

MishkaRogachev added 4 commits March 30, 2026 20:29

Remove tails from presvious impl

87d7d9c

Move tx filter to APIBackend

a4f6e19

Merge remote-tracking branch 'origin/master' into prefilter-blockchai…

ab5fae9

…n-api-2

Add GasLimit for retryable filtering tests to skip eraly filtering in…

2b91fd1

… gas_estimate

MishkaRogachev force-pushed the prefilter-blockchain-api-2 branch from 91af9cf to 2b91fd1 Compare March 31, 2026 15:18

MishkaRogachev requested a review from diegoximenes March 31, 2026 15:31

MishkaRogachev requested review from Tristan-Wilson and tsahee March 31, 2026 15:31

MishkaRogachev assigned diegoximenes Mar 31, 2026

MishkaRogachev changed the title ~~Pre-checker tx filtering (with estimate gas call)~~ Pre-checker tx filtering (using estimate gas call) Mar 31, 2026

MishkaRogachev marked this pull request as ready for review March 31, 2026 15:32

diegoximenes requested a review from Copilot March 31, 2026 17:14

Copilot started reviewing on behalf of diegoximenes March 31, 2026 17:15 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

diegoximenes requested changes Apr 1, 2026

View reviewed changes

diegoximenes assigned MishkaRogachev and unassigned diegoximenes Apr 1, 2026

MishkaRogachev added 2 commits April 1, 2026 20:26

Review fixes and polishing

d364b7e

Remove duplication in ptrchecker_filter_test

6974673

MishkaRogachev assigned diegoximenes and unassigned MishkaRogachev Apr 1, 2026

MishkaRogachev requested a review from diegoximenes April 1, 2026 18:49

diegoximenes added 3 commits April 2, 2026 14:02

Removes outdated comment.

29033fb

Disable RPC filter for all tests by default

0b66c8b

Adds estimate gas filter test

40bfadd

diegoximenes requested changes Apr 2, 2026

View reviewed changes

Merge branch 'master' into prefilter-blockchain-api-2

0c9ab80

diegoximenes approved these changes Apr 2, 2026

View reviewed changes

diegoximenes assigned tsahee and unassigned diegoximenes Apr 2, 2026

tsahee reviewed Apr 2, 2026

View reviewed changes

tsahee reviewed Apr 3, 2026

View reviewed changes

	EnableRPCFilter bool `koanf:"enable-rpc-filter"`
	EnableNonMutatingRPCFilter bool `koanf:"enable-non-mutating-rpc-filter"`

Conversation

MishkaRogachev commented Mar 30, 2026

Uh oh!

codecov bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 9 Tests Failed:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

diegoximenes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov bot commented Mar 31, 2026 •

edited

Loading

github-actions bot commented Mar 31, 2026 •

edited

Loading