Skip to content

test: integration tests for reorg tracking in RPC subscriptions and filters#99

Draft
prestwich wants to merge 3 commits intojames/eng-1971from
james/eng-1972
Draft

test: integration tests for reorg tracking in RPC subscriptions and filters#99
prestwich wants to merge 3 commits intojames/eng-1971from
james/eng-1972

Conversation

@prestwich
Copy link
Member

@prestwich prestwich commented Mar 10, 2026

Summary

  • Adds 7 integration tests in crates/node-tests/tests/reorg.rs verifying end-to-end reorg handling across the RPC layer
  • Tests cover block tag rewind, polling filter watermark reset, push subscription removed: true log emission, filter selectivity during reorgs, and a no-regression check
  • Uses host system transactions (simple_transact) to avoid tx pool conflicts on block rebuild after revert

Closes ENG-1972

Stack

This PR includes commits from #96, #97, and #98. Review only the top commit.

  1. feat: update BlockTags during reorgs to prevent stale tag window #96BlockTags::rewind_to for reorg tag updates
  2. feat: handle ChainEvent::Reorg in SubscriptionTask #97SubscriptionTask reorg handling
  3. feat: handle reorgs in get_filter_changes with reorg watermark #98get_filter_changes reorg watermark
  4. test: integration tests for reorg tracking in RPC subscriptions and filters #99 ← this PR — Integration tests

Test plan

  • cargo clippy -p signet-node-tests --all-features --all-targets — clean
  • cargo +nightly fmt — clean
  • cargo t -p signet-node-tests --test reorg — 7/7 pass
  • cargo t -p signet-node-tests — full suite passes, no regressions

🤖 Generated with Claude Code

Copy link
Contributor

@Fraser999 Fraser999 left a comment

Choose a reason for hiding this comment

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

LGTM (one non-blocking nitpick), but claude has made a couple of points:

  • No multi-block reorg test for filters/subscriptions:
    All filter and subscription reorg tests only revert a single block. test_block_tags_reorg does a 2-block revert, but without filters or subscriptions active. A test that reverts 2+ blocks while a log filter/subscription is active would exercise the watermark.min() path and deeper reorg recovery.

  • No test for multiple reorgs between polls:
    set_reorg_watermark uses min() to keep the deepest watermark when multiple reorgs arrive before a single poll. This behavior is untested.

I'm not convinced either of these are actually worthwhile actioning though, so approving as is, and you can decide whether to add these cases or not.

@prestwich
Copy link
Member Author

[Claude Code]

Addressed all review feedback in 8859bfe:

  1. assert_eq! nitpick — now fetches the actual block 3 hash and asserts equality instead of assert_ne!.

  2. Multi-block reorg tests — added test_multi_block_reorg_log_filter (reverts 3 blocks with active log filter) and test_multi_block_reorg_log_subscription (reverts 2 blocks with active log subscription, verifies removed: true emission per block).

  3. Multiple reorgs between polls — added test_multiple_reorgs_between_polls: two separate reorg-rebuild cycles without polling between them, where the second reorg is deeper. Validates that set_reorg_watermark's min() keeps the deeper watermark, causing correct re-delivery from block 4 onward.

All 10 reorg tests pass.

@prestwich prestwich changed the base branch from develop to james/eng-1971 March 11, 2026 13:13
prestwich and others added 2 commits March 15, 2026 09:13
…nd filters

Adds 7 integration tests verifying end-to-end reorg handling across the
RPC layer: block tag rewind, polling filter watermark reset, push
subscription removed-log emission, filter selectivity, and a
no-regression check for normal block progression.

Closes ENG-1972

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review feedback: strengthen reorg integration tests with
multi-block reverts and multiple-reorgs-between-polls scenarios.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ring buffer design now correctly returns removed logs on
early-return paths (start > latest) instead of silently discarding
them. Update test_multi_block_reorg_log_filter and
test_multiple_reorgs_between_polls to expect the removed logs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

2 participants