Add reorg threshold entry tolerance for multichain indexers - #1460
Merged
Conversation
Reorg-threshold entry is a one-time whole-indexer transition that requires every chain to satisfy isReadyToEnterReorgThreshold at the same batch check (BatchProcessing / ChainFetching Array.every). The per-chain predicate was instantaneous and non-monotonic: a chain that reached its lagged head with a drained buffer was retracted the moment its head advanced by a block. On a live multichain indexer some chain's head is always advancing, so the conjunction is never observed for all chains at once and the indexer stays below the threshold forever. The same race hits a single busy chain with many partitions, where the slowest partition holds the frontier back past each batch boundary. Latch the per-chain readiness on ChainState (reachedReorgThresholdEdge): once a chain's frontier momentarily reaches its lagged head with an empty buffer, it stays ready even after its head moves on. Entry then converges monotonically as each chain latches. Safe because nothing above head - maxReorgDepth is processed before the threshold, so a latched chain that later trails the head has still not touched an un-reorg-safe block. Add EnterReorgThreshold_test.res reproducing the multichain race through the real indexer loop: it fails (envio_reorg_threshold stuck at 0) before the fix and passes after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
Covers the single-chain manifestation of the entry bug at the unit level: a chain that reaches its lagged head with a drained buffer must stay ready after its head advances by a block. Without the latch the readiness predicate is retracted on the head bump (true -> false); with it, readiness is monotonic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
Even with the latch, a chain must reach its lagged head exactly once for the readiness predicate to fire; a head that advances between a chain's catch-up query being sized and its response landing can keep deferring that instant. Add a tolerance (in blocks) below the lagged head within which a chain still counts as ready, absorbing that gap. Applied to the head comparison only, not to endBlock (an exact target). Threaded as a Config.t field defaulting to 100 in production and stored on ChainState, so the latch reads it per chain. MockIndexer defaults it to 0 so the small-scale test fixtures (head 300, maxReorgDepth 200) don't enter the threshold before fetching; tests exercising the tolerance pass an explicit value. Adds unit coverage (FetchState predicate, ChainState latch beyond the tolerance) and an e2e test entering within the tolerance below the lagged head. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
The ready tolerance already keeps a chain ready across the head advancing between catch-up and the entry check, so the monotonic latch is unnecessary; readiness goes back to the plain tolerant predicate. Also remove the reorg-threshold entry check from finishWaitingForNewBlock: it already calls scheduleProcessing, and startProcessing always runs at least one processNextBatch (even with no items or progress), which owns the entry decision via the same predicate. The duplicated check added nothing. Tests updated: the ChainState readiness test now asserts the tolerance boundary instead of the latch; the multichain repro sets an explicit tolerance (it no longer has the latch to absorb the head advance during the handoff). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
Make FetchState.isReadyToEnterReorgThreshold's ~tolerance a required argument instead of defaulting to 0, so callers state the tolerance explicitly. Remove ChainState.isReadyToEnterReorgThreshold: after entry moved entirely into processNextBatch (via isReadyToEnterReorgThresholdAfterBatch), the plain per-chain accessor had no production caller and existed only for a unit test. The tolerance is covered by FetchState predicate tests and the end-to-end EnterReorgThreshold test, so the ChainState-level test and its helper are removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
The readiness check required the whole merged buffer to be empty. With many partitions tracking a moving head, faster partitions leave items stuck above bufferBlockNumber (the slowest partition's frontier) behind the gap, so the buffer is never empty and the chain never enters the threshold. Require bufferReadyCount == 0 instead: only processable items (at or below the frontier) defer entry. Items stuck above the frontier are reorg-safe (all <= head - blockLag before the threshold) and are processed in-threshold once their partition catches up. Single-partition behaviour is unchanged, since with no gap every buffered item is processable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReorg-threshold readiness now supports configurable block tolerance, evaluates the ready frontier, and is decided during processing. Fetch completion always schedules fetching and processing. New multichain and single-chain tests validate threshold entry. ChangesReorg threshold readiness
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ChainFetching
participant ChainState
participant FetchState
ChainFetching->>ChainState: scheduleProcessing()
ChainState->>FetchState: isReadyToEnterReorgThreshold(tolerance)
FetchState-->>ChainState: readiness result
ChainState->>ChainFetching: enter reorg threshold when ready
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/envio/src/FetchState.res`:
- Around line 2572-2575: Update the endBlock completion switch in
packages/envio/src/FetchState.res at lines 2572-2575 so Some(endBlock) returns
only bufferBlockNumber >= endBlock, while the head-tolerance calculation applies
only for None; update scenarios/test_codegen/test/lib_tests/FetchState_test.res
at lines 3444-3464 to expect false before endBlock and add a regression case
with nonzero tolerance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca8fa1b2-3232-4e62-8e43-e37b4cae6b3f
📒 Files selected for processing (8)
packages/envio/src/ChainFetching.respackages/envio/src/ChainState.respackages/envio/src/ChainState.resipackages/envio/src/Config.respackages/envio/src/FetchState.resscenarios/test_codegen/test/EnterReorgThreshold_test.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/FetchState_test.res
…rg-threshold-ykabjx # Conflicts: # packages/envio/src/ChainState.res
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe15c1308a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| switch endBlock { | ||
| | Some(endBlock) if bufferBlockNumber >= endBlock => true | ||
| | _ => bufferBlockNumber >= knownHeight - blockLag | ||
| | _ => bufferBlockNumber >= knownHeight - blockLag - tolerance |
There was a problem hiding this comment.
When a chain has an endBlock below the current lagged head, this fallback can return true while bufferBlockNumber is still below that endBlock as long as it is within the new tolerance of knownHeight - blockLag. In that finite-backfill case, BatchProcessing enters the reorg threshold early and subsequent blocks up to the configured end block are processed with rollback-history/checkpoint behavior even though they are still below the reorg threshold, changing storage/history semantics and doing unnecessary work for the last tolerated range before endBlock.
Useful? React with 👍 / 👎.
The tolerance leaked into the endBlock branch: a chain whose endBlock sat within `tolerance` of the lagged head entered the reorg threshold before reaching the endBlock, processing the last blocks with rollback history unnecessarily. Apply the tolerance only when the moving head is the target. A finite endBlock at or below the lagged head is an exact target (no moving head to absorb), so it is reached without tolerance; an endBlock beyond the lagged head still tracks the head like a chain with no endBlock, so it isn't regressed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
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
Adds a configurable tolerance to the reorg threshold entry check, allowing multichain indexers to enter the threshold even when a chain's head advances slightly between catch-up and the entry decision. This solves the deadlock where a multichain indexer never enters because at least one chain's head is always advancing during the cross-chain handoff.
Key Changes
FetchState.isReadyToEnterReorgThreshold: Now accepts a
~toleranceparameter (in blocks) that allows a chain to be considered ready within that distance below the lagged head, rather than requiring exact alignment. Also changed the readiness check from requiring an empty buffer to checkingbufferReadyCount == 0, which correctly ignores items stuck above the frontier behind a lagging partition's gap.ChainState: Added
reorgThresholdReadyTolerancefield to carry the tolerance value through the chain state.Config: Added
reorgThresholdReadyTolerancefield (defaults to 100 blocks in production) as a configurable parameter.ChainFetching.finishWaitingForNewBlock: Removed the reorg threshold entry check from this location since
scheduleProcessingalways runs at least one batch and owns the entry decision.BatchProcessing: Updated to pass the tolerance when calling
isReadyToEnterReorgThresholdduring the whole-indexer entry check.Test coverage: Added comprehensive test
EnterReorgThreshold_test.resdemonstrating the multichain scenario and tolerance behavior, plus updated existing FetchState tests to pass the tolerance parameter.Implementation Details
The tolerance is applied only to the lagged-head readiness check (
bufferBlockNumber >= knownHeight - blockLag - tolerance), not to endBlock targets where reaching the exact end is required. This allows chains to enter the threshold while still within the tolerance window of their lagged head, absorbing the head advances that occur during the cross-chain handoff in multichain indexers.The shift from checking buffer emptiness to checking
bufferReadyCount == 0is critical for many-partition chains: items stuck above the frontier (behind a lagging partition's gap) are reorg-safe and must not defer entry.https://claude.ai/code/session_01TTuugjVrBRsj5Kb7EFRyVv
Summary by CodeRabbit
reorgThresholdReadyTolerance(default:100blocks) to control how early a chain is considered ready for reorg-threshold entry.