feat: handle reorgs in get_filter_changes with reorg watermark#98
feat: handle reorgs in get_filter_changes with reorg watermark#98prestwich wants to merge 1 commit intojames/eng-1970from
Conversation
Evalir
left a comment
Evalir
left a comment
There was a problem hiding this comment.
I think this is fine. It requires thinking quite a bit through the possible reorg cases but the flow is clear to me. I'm good with filter changes returning the empty output instead of querying an impossible range
| pub fn rewind_to(&self, ancestor: u64) { | ||
| self.latest.fetch_min(ancestor, Ordering::Release); | ||
| self.safe.fetch_min(ancestor, Ordering::Release); | ||
| self.finalized.fetch_min(ancestor, Ordering::Release); | ||
| } |
There was a problem hiding this comment.
this order should be fine, since finalized should actually not be possible to move (else things went very, very wrong (or you are unichain))
prestwich
force-pushed
the
james/eng-1971
branch
from
93f823c to
31e6420
Compare
Add a `reorg_watermark` field to `ActiveFilter` that records the common ancestor block when a chain reorganization occurs. `FilterManager` now subscribes to `ChainEvent::Reorg` broadcasts and eagerly propagates watermarks to all active filters. On the next poll, `get_filter_changes` rewinds `next_start_block` so re-fetched data reflects the new chain. An implicit reorg detection check (latest < start) provides a belt-and-suspenders fallback when the explicit watermark is missed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
prestwich
force-pushed
the
james/eng-1971
branch
from
31e6420 to
ac15316
Compare
| let ChainEvent::Reorg(reorg) = event else { continue }; | ||
|
|
||
| let Some(manager) = self.manager.upgrade() else { break }; | ||
| manager.set_reorg_watermark_all(reorg.common_ancestor); |
There was a problem hiding this comment.
hmm, this will set a watermark for all filters alive, but these filters weren't necessarily alive when the reorg event happened? due to scheduling, this task could run after a reorg happened, and then a filter was created. This would then set an invalid watermark for this new filter, no?
2 participants
Summary
reorg_watermark: Option<u64>toActiveFilterso filters can rewindnext_start_blockwhen a reorg invalidates their polling windowFilterReorgTaskthat subscribes toChainEvent::Reorgbroadcasts and eagerly propagates watermarks to all active filtersget_filter_changesto check the watermark before querying, plus an implicit fallback check (latest < start) for missed broadcastsCloses ENG-1971
Stack
This PR includes commits from #96 and #97. Review only the top commit.
BlockTags::rewind_tofor reorg tag updatesSubscriptionTaskreorg handlingget_filter_changesreorg watermarkTest plan
cargo clippy -p signet-rpc --all-features --all-targets— cleancargo +nightly fmt— cleancargo t -p signet-rpc— 35 tests + 4 doc-tests passFilterReorgTaskself-terminates whenFilterManageris dropped (usesWeak)🤖 Generated with Claude Code