Fix infinite reorg loop when reorg chain has no events processed - #1027
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an E2E rollback test, extends the test mock Indexer to accept an optional Changes
Sequence Diagram(s)sequenceDiagram
participant ReorgDetector as ReorgDetector
participant GlobalState as GlobalState
participant ChainFetcher as ChainFetcher
participant Processor as Processor
participant BlockSource as BlockSource
Note right of ReorgDetector: reorg detected
ReorgDetector->>GlobalState: notifyReorg(reorgChain, rollbackTarget)
GlobalState->>ChainFetcher: rollback(reorgChain, rollbackTarget) rgba(255,0,0,0.5)
ChainFetcher->>BlockSource: fetchBlocks(from=rollbackTarget+1) rgba(0,128,0,0.5)
BlockSource-->>ChainFetcher: blocks (may include zero-event blocks)
ChainFetcher->>Processor: deliverBlocks(blocks) rgba(0,0,255,0.5)
Processor-->>GlobalState: processingComplete(result)
alt no further reorg triggered
GlobalState-->>ReorgDetector: resumeNormalOperation
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/envio/src/GlobalState.res (1)
1078-1125:⚠️ Potential issue | 🔴 CriticalHandle the reorg chain in the
Nonerollback branch.When
getRollbackProgressDiffhas no row for the reorg chain, Line 1125 returnscfunchanged. That leaves stalereorgDetectiondata in memory and keepsfetchStateat the pre-rollback position, so the next fetch can continue from 102 instead of 101 and immediately re-detect the same reorg. The new regression test atscenarios/test_codegen/test/rollback/Rollback_test.resLines 3033-3177 is exercising exactly this path.💡 Proposed fix
let chainFetchers = state.chainManager.chainFetchers->ChainMap.mapWithKey((chain, cf) => { switch newProgressBlockNumberPerChain->Utils.Dict.dangerouslyGetByIntNonOption( chain->ChainMap.Chain.toChainId, ) { | Some(newProgressBlockNumber) => let fetchState = cf.fetchState->FetchState.rollback(~targetBlockNumber=newProgressBlockNumber) let newTotalEventsProcessed = cf.numEventsProcessed - eventsProcessedDiffByChain ->Utils.Dict.dangerouslyGetByIntNonOption(chain->ChainMap.Chain.toChainId) ->Option.getUnsafe @@ { ...cf, reorgDetection: chain == reorgChain ? cf.reorgDetection->ReorgDetection.rollbackToValidBlockNumber( ~blockNumber=rollbackTargetBlockNumber, ) : cf.reorgDetection, safeCheckpointTracking: switch cf.safeCheckpointTracking { | Some(safeCheckpointTracking) => Some( safeCheckpointTracking->SafeCheckpointTracking.rollback( ~targetBlockNumber=newProgressBlockNumber, ), ) | None => None }, fetchState, committedProgressBlockNumber: newProgressBlockNumber, numEventsProcessed: newTotalEventsProcessed, } - | None => cf + | None => + if chain == reorgChain { + { + ...cf, + reorgDetection: cf.reorgDetection->ReorgDetection.rollbackToValidBlockNumber( + ~blockNumber=rollbackTargetBlockNumber, + ), + fetchState: cf.fetchState->FetchState.rollback( + ~targetBlockNumber=rollbackTargetBlockNumber, + ), + } + } else { + cf + } } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/envio/src/GlobalState.res` around lines 1078 - 1125, The None branch currently returns cf unchanged which leaves stale reorgDetection and fetchState; update that branch so when chain == reorgChain you roll back reorgDetection and fetchState (and safeCheckpointTracking if present) to rollbackTargetBlockNumber: set reorgDetection = cf.reorgDetection->ReorgDetection.rollbackToValidBlockNumber(~blockNumber=rollbackTargetBlockNumber), set fetchState = cf.fetchState->FetchState.rollback(~targetBlockNumber=rollbackTargetBlockNumber), and if cf.safeCheckpointTracking is Some(...) call SafeCheckpointTracking.rollback(~targetBlockNumber=rollbackTargetBlockNumber); leave other fields as-is unless you need to adjust committedProgressBlockNumber/numEventsProcessed elsewhere.
🧹 Nitpick comments (1)
scenarios/test_codegen/test/rollback/Rollback_test.res (1)
3138-3141: Drop the temporary debug logging before merge.The assertion below already captures the payload state, so Lines 3140-3141 look like leftover debugging.
🧹 Proposed cleanup
- // DEBUG: check actual value let actualPayloads = sourceMock1.getItemsOrThrowCalls->Js.Array2.map(c => c.payload) - Js.log2("DEBUG actualPayloads:", actualPayloads) - Js.log2("DEBUG last:", actualPayloads->Utils.Array.last) t.expect(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scenarios/test_codegen/test/rollback/Rollback_test.res` around lines 3138 - 3141, Remove the temporary debug logging lines: the Js.log2 calls that print "DEBUG actualPayloads:" and "DEBUG last:" which use actualPayloads derived from sourceMock1.getItemsOrThrowCalls->Js.Array2.map and Utils.Array.last; the assertion already verifies payload state so delete those two debug log statements to clean up Rollback_test.res.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/envio/src/GlobalState.res`:
- Around line 1078-1125: The None branch currently returns cf unchanged which
leaves stale reorgDetection and fetchState; update that branch so when chain ==
reorgChain you roll back reorgDetection and fetchState (and
safeCheckpointTracking if present) to rollbackTargetBlockNumber: set
reorgDetection =
cf.reorgDetection->ReorgDetection.rollbackToValidBlockNumber(~blockNumber=rollbackTargetBlockNumber),
set fetchState =
cf.fetchState->FetchState.rollback(~targetBlockNumber=rollbackTargetBlockNumber),
and if cf.safeCheckpointTracking is Some(...) call
SafeCheckpointTracking.rollback(~targetBlockNumber=rollbackTargetBlockNumber);
leave other fields as-is unless you need to adjust
committedProgressBlockNumber/numEventsProcessed elsewhere.
---
Nitpick comments:
In `@scenarios/test_codegen/test/rollback/Rollback_test.res`:
- Around line 3138-3141: Remove the temporary debug logging lines: the Js.log2
calls that print "DEBUG actualPayloads:" and "DEBUG last:" which use
actualPayloads derived from sourceMock1.getItemsOrThrowCalls->Js.Array2.map and
Utils.Array.last; the assertion already verifies payload state so delete those
two debug log statements to clean up Rollback_test.res.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf3950d5-0827-4150-9f75-7b12b2ac0421
📒 Files selected for processing (3)
packages/envio/src/GlobalState.resscenarios/test_codegen/test/helpers/Mock.resscenarios/test_codegen/test/rollback/Rollback_test.res
DZakh
force-pushed
the
claude/fix-indexer-reorg-loop-7hffU
branch
from
March 9, 2026 14:59
3214890 to
e895670
Compare
When a reorg is detected on a chain that had no events in the current batch (e.g. another chain filled batchSize=1 first), the reorg chain has no progress diff entry. Without a fix, the fetchState and reorgDetection are not rolled back, causing stale block hashes to persist and the same reorg to be re-detected infinitely. This test currently fails, demonstrating the bug. https://claude.ai/code/session_01K8NhAjzDHp9H3sLJ7gHjQs
DZakh
force-pushed
the
claude/fix-indexer-reorg-loop-7hffU
branch
from
March 9, 2026 19:54
e895670 to
f61922f
Compare
When a reorg is detected on a chain that had no events in the current batch, getRollbackProgressDiff returns no entry for that chain. Roll back reorgDetection and fetchState for the reorg chain in the None branch to clear stale block hashes from dataByBlockNumber and prevent re-detecting the same reorg infinitely. https://claude.ai/code/session_01K8NhAjzDHp9H3sLJ7gHjQs
DZakh
enabled auto-merge (squash)
March 10, 2026 09:49
DZakh
pushed a commit
that referenced
this pull request
Mar 10, 2026
…essed (#1027) Cherry-pick from main. Fixes infinite reorg->rollback loop when a blockchain reorganization is detected on a chain with no events processed since the target checkpoint. Now properly rolls back reorgDetection and fetchState even when no progress diff exists. https://claude.ai/code/session_017jqSDS8homENyhbexMaoG4
DZakh
pushed a commit
that referenced
this pull request
Mar 10, 2026
The cherry-picked tests from PRs #1026/#1027 used `sourceConfig: Config.CustomSources(...)` which is the v3 API. This branch uses `sources: [...]` in Mock.Indexer.chainConfig. https://claude.ai/code/session_017jqSDS8homENyhbexMaoG4
DZakh
pushed a commit
that referenced
this pull request
Mar 10, 2026
The tests from PRs #1026/#1027 use Vitest APIs (t.expect().toEqual()), partition-aware Mock APIs (~resolveAt, call.resolve/payload), and metrics with "p" field that don't exist on this branch (pre-Vitest migration). The production code fixes are retained. https://claude.ai/code/session_017jqSDS8homENyhbexMaoG4
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
2 participants
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.
Summary
Fixed an infinite reorg→rollback loop that occurred when a chain experienced a reorganization but had no events processed since the target checkpoint. The issue was that rollback state wasn't being cleared for chains with no progress diff entries, causing the same reorg to be re-detected on subsequent fetches.
Key Changes
GlobalState.res: Modified the rollback logic to handle the case where a reorg chain has no progress diff entry. Now, even when
getRollbackProgressDiffreturnsNonefor the reorg chain, we still rollback itsreorgDetectionandfetchStateto prevent stale block hashes from triggering infinite reorg detection cycles.Rollback_test.res: Added comprehensive test case "Should not enter infinite reorg loop when reorg chain has no events processed since target checkpoint" that verifies:
Implementation Details
The fix specifically checks if the current chain being processed is the reorg chain. If so, it applies rollback operations to both
reorgDetectionandfetchStateregardless of whether a progress diff entry exists. This ensures that the stale block hash information stored inreorgDetection.dataByBlockNumberis cleared, preventing re-detection of the same reorg on the next fetch cycle.https://claude.ai/code/session_01K8NhAjzDHp9H3sLJ7gHjQs
Summary by CodeRabbit
Tests
New Features
Bug Fixes