Add FilterSetID to FilteredAddressRecord#4626
Open
MishkaRogachev wants to merge 11 commits intomasterfrom
Open
Add FilterSetID to FilteredAddressRecord#4626MishkaRogachev wants to merge 11 commits intomasterfrom
MishkaRogachev wants to merge 11 commits intomasterfrom
Conversation
…e HashStore.IsRestricted
MishkaRogachev
changed the base branch from
master
to
update-touchaddress-and-hashedaddresscheckerstateisfiltered
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4626 +/- ##
==========================================
+ Coverage 33.75% 34.20% +0.45%
==========================================
Files 501 501
Lines 60225 60224 -1
==========================================
+ Hits 20326 20601 +275
+ Misses 36336 36010 -326
- Partials 3563 3613 +50 |
Contributor
❌ 15 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends address-filtering telemetry by adding a FilterSetID to each FilteredAddressRecord, threading it through filtering/touch points, and removing an unused sender parameter from event-based address extraction.
Changes:
- Update event-filter address extraction to return
FilteredAddressWithReasonand drop the unused_senderparameter. - Extend
HashStore.IsRestrictedto return(restricted, filterSetID)from a single snapshot and addHashStore.ID(). - Update address-touch call sites and tests to use
FilteredAddressWithReason, and update filtered-tx report JSON expectations to includefilterSetId.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| execution/gethexec/sequencer.go | Updates event-log filtering touch flow to new AddressesForFiltering signature and TouchAddress argument type. |
| execution/gethexec/executionengine.go | Updates address-touching helpers and event filter application to use FilteredAddressWithReason. |
| execution/gethexec/eventfilter/eventfilter.go | Removes unused sender parameter; returns FilteredAddressWithReason values for matched rules. |
| execution/gethexec/eventfilter/eventfilter_test.go | Adapts tests to updated AddressesForFiltering signature. |
| execution/gethexec/addressfilter/service_test.go | Updates IsRestricted call sites for new (bool, uuid) return. |
| execution/gethexec/addressfilter/hash_store.go | Changes IsRestricted to return filter set ID atomically with the restriction result; adds ID(). |
| execution/gethexec/addressfilter/filter_report_test.go | Updates JSON tests to include filterSetId and populates FilterSetID in records. |
| execution/gethexec/addressfilter/address_checker.go | Threads filter set ID into records produced by hashed address checking; changes TouchAddress to take FilteredAddressWithReason. |
| execution/gethexec/addressfilter/address_checker_test.go | Updates checker tests for new touch type and validates recorded FilterSetID. |
| arbos/tx_processor.go | Updates contract/caller touch points to use FilteredAddressWithReason. |
| changelog/mrogachev-nit-4780.md | Adds changelog entry for FilterSetID propagation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
109
to
+116
| // If the checker is stopped, conservatively mark filtered | ||
| if s.checker.Stopped() { | ||
| s.report(nil, true) | ||
| record := filter.FilteredAddressRecord{ | ||
| FilterSetID: s.checker.FilterSetID(), | ||
| FilteredAddressWithReason: addr, | ||
| } | ||
| s.report(&record) | ||
| s.pending.Done() |
There was a problem hiding this comment.
In the "checker stopped" / "context done" fallback, this now appends a full FilteredAddressRecord even though the address was never checked against the HashStore. Previously this path marked the tx filtered without attributing specific addresses (since the result is unknown). Recording an address+reason here can mislead consumers into thinking the address was definitively restricted, and FilterSetID() may not match the snapshot that would have been used. Consider preserving the old behavior (set filtered=true but do not append a record), or introduce an explicit "unknown/unverified" marker in the record format if you need to surface touched addresses during shutdown.
Comment on lines
+134
to
+139
| // report records a filtered address. Must only be called for restricted addresses. | ||
| func (s *HashedAddressCheckerState) report(record *filter.FilteredAddressRecord) { | ||
| s.mu.Lock() | ||
| s.filtered = true | ||
| s.filteredAddresses = append(s.filteredAddresses, *record) | ||
| s.mu.Unlock() |
There was a problem hiding this comment.
The comment on report() says it must only be called for restricted addresses, but TouchAddress() calls report() in shutdown/stopped paths where the address restriction status is unknown. Either adjust the shutdown behavior so report() is only used for confirmed restricted addresses, or update the comment/API to reflect that report() can also record conservative/unknown filtering.
diegoximenes
requested changes
Apr 14, 2026
| type workItem struct { | ||
| record *filter.FilteredAddressRecord | ||
| state *HashedAddressCheckerState | ||
| addr filter.FilteredAddressWithReason |
Contributor
There was a problem hiding this comment.
Suggested change
| addr filter.FilteredAddressWithReason | |
| addr *filter.FilteredAddressWithReason |
…dresscheckerstateisfiltered' into add-filtersetid-to-filteredaddressrecord
diegoximenes
previously approved these changes
Apr 15, 2026
Contributor
|
LGTM, I will keep it assigned to me while the base branch is not merged |
Base automatically changed from
update-touchaddress-and-hashedaddresscheckerstateisfiltered
to
master
April 16, 2026 15:23
…filteredaddressrecord
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes NIT-4780
Waits #4557
Pulls in OffchainLabs/go-ethereum#648
filter set ID atomically with the restriction check.