Skip to content

arbitrum: Return filtered address records from TxFilterer.CheckFiltered#657

Open
MishkaRogachev wants to merge 3 commits intomasterfrom
prechecker-calls-cmdfiltering-report
Open

arbitrum: Return filtered address records from TxFilterer.CheckFiltered#657
MishkaRogachev wants to merge 3 commits intomasterfrom
prechecker-calls-cmdfiltering-report

Conversation

@MishkaRogachev
Copy link
Copy Markdown
Contributor

@MishkaRogachev MishkaRogachev commented Apr 17, 2026

Part of NIT-4645
Pulled in by OffchainLabs/nitro#4653

Return filtered address records from TxFilterer.CheckFiltered so nitro callers (TxPreChecker) can build a filtered-tx report without re-querying the statedb. gasestimator.Run propagates records only on the ErrArbTxFilter branch.

@MishkaRogachev MishkaRogachev changed the title Return filtered address records from TxFilterer.CheckFiltered arbitrum: Return filtered address records from TxFilterer.CheckFiltered Apr 17, 2026
@MishkaRogachev MishkaRogachev requested a review from Copilot April 17, 2026 15:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends Arbitrum transaction address-filtering plumbing so callers can obtain the set of filtered address records directly from gas estimation execution, avoiding re-querying the StateDB when state.ErrArbTxFilter is raised (per NIT-4645 / nitro#4653 context).

Changes:

  • Change core.TxFilterer.CheckFiltered to return ([]filter.FilteredAddressRecord, error) instead of just error.
  • Update eth/gasestimator.Run / internal run to return filtered address records when the failure is state.ErrArbTxFilter, while keeping existing behavior for other errors.
  • Adjust internal call sites (e.g., execute) to accommodate the new run return signature.

Reviewed changes

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

File Description
eth/gasestimator/gasestimator.go Propagates filtered address records alongside state.ErrArbTxFilter from run/Run; updates execute to ignore the new return value.
core/arbitrum_hooks.go Updates the TxFilterer interface so CheckFiltered returns filtered address records.

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

Comment thread core/arbitrum_hooks.go Outdated
Comment thread eth/gasestimator/gasestimator.go Outdated
Comment on lines +244 to +246
// Public API for gas estimation. Separated to simplify upstream merges.
func Run(ctx context.Context, call *core.Message, opts *Options) (*core.ExecutionResult, error) {
// When address filtering trips, the returned records describe the filtered addresses.
func Run(ctx context.Context, call *core.Message, opts *Options) (*core.ExecutionResult, []filter.FilteredAddressRecord, error) {
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exported Run function’s doc comment doesn’t start with the function name, which breaks standard Go doc conventions and can cause lint/doc tooling to miss it. Consider rewriting the comment so the first sentence begins with "Run" and still mentions that filtered address records are returned when filtering trips.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignored

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread eth/gasestimator/gasestimator.go Outdated
Copy link
Copy Markdown
Contributor

@diegoximenes diegoximenes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assigning back to @MishkaRogachev while the related nitro PR is not approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants