Ensure valid ENSRainbow connection before indexing starts#1850
Ensure valid ENSRainbow connection before indexing starts#1850
Conversation
This function will enbale ENSIndexer modules to wait for when the ENSRainbow instance is ready to serve traffic.
Allows to wait with indexing onchain events until ENSRainbow instance is ready.
…ndler-preconditions
Co-authored-by: lightwalker.eth <[email protected]>
…` object This enable storing the public config of the connected ENSRainbow instance in ENSDb. The ENSDb record will be used to read the public config for ENSRainbow instance from other ENSNode apps.
🦋 Changeset detectedLatest commit: 3d7bfb3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughExtended ENSDb SDK with ENSRainbow Public Config storage and retrieval methods. Updated ENSIndexer to validate ENSRainbow configuration compatibility before indexing initialization, including stored config comparison, omnichain status checks, and label set version validation. Changes
Sequence DiagramsequenceDiagram
actor Indexer as ENSIndexer<br/>(Activation)
participant ENSDb as ENSDb Client
participant Retry as Retry Engine<br/>(p-retry)
participant Status as Indexing Status<br/>Builder
participant RBow as ENSRainbow<br/>Service
Indexer->>ENSDb: getEnsRainbowPublicConfig()
alt Stored Config Exists
ENSDb-->>Indexer: return stored config
Indexer->>RBow: waitForEnsRainbowToBeReady()
RBow-->>Indexer: ready
Indexer->>RBow: config()
RBow-->>Indexer: return live config
Indexer->>Indexer: validate labelSetId match
Indexer->>Indexer: validate highestLabelSetVersion ≥ stored
Indexer->>ENSDb: upsertEnsRainbowPublicConfig(validated)
ENSDb-->>Indexer: ✓ upserted
else No Stored Config
ENSDb-->>Indexer: undefined
Indexer->>Retry: retry getIndexingStatusSnapshot()
Retry->>Status: getIndexingStatusSnapshot()
Status-->>Retry: snapshot
Retry-->>Indexer: snapshot
Indexer->>Indexer: verify status === Unstarted
Indexer->>RBow: waitForEnsRainbowToBeReady()
RBow-->>Indexer: ready
Indexer->>RBow: config()
RBow-->>Indexer: return config
Indexer->>ENSDb: upsertEnsRainbowPublicConfig(config)
ENSDb-->>Indexer: ✓ upserted
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR strengthens ENSIndexer startup/indexing preconditions around ENSRainbow by persisting ENSRainbow’s public config in ENSDb metadata and enforcing a “valid ENSRainbow connection + compatible config” gate before onchain event handlers run.
Changes:
- ENSDb SDK: add ENSNode metadata key/type + reader/writer helpers for
EnsRainbowPublicConfig. - ENSIndexer: introduce
ensureValidEnsRainbowConnection()precondition that loads/validates/upserts ENSRainbow public config and enforces “Unstarted” status on first-ever config write. - ENSDb writer worker: allow persisting
Unstartedindexing status snapshots.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensdb-sdk/src/client/serialize/ensnode-metadata.ts | Adds serialized union support for ENSRainbow public config metadata. |
| packages/ensdb-sdk/src/client/ensnode-metadata.ts | Introduces ensrainbow_public_config metadata key + typed variant. |
| packages/ensdb-sdk/src/client/ensdb-writer.ts | Adds upsertEnsRainbowPublicConfig() writer API. |
| packages/ensdb-sdk/src/client/ensdb-writer.test.ts | Tests ENSRainbow public config upsert writes correct metadata. |
| packages/ensdb-sdk/src/client/ensdb-reader.ts | Adds getEnsRainbowPublicConfig() reader API. |
| packages/ensdb-sdk/src/client/ensdb-reader.test.ts | Tests ENSRainbow public config read behavior (undefined vs stored). |
| apps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.ts | New precondition implementing ENSRainbow readiness + config validation/upsert logic. |
| apps/ensindexer/src/lib/indexing-engines/ponder.ts | Switches onchain precondition from ENSRainbow readiness to full ensureValidEnsRainbowConnection(). |
| apps/ensindexer/src/lib/indexing-engines/ponder.test.ts | Expands tests to cover new ENSRainbow config gating behaviors. |
| apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.ts | Removes prior guard preventing Unstarted snapshot upserts. |
| apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts | Updates snapshot upsert test to expect writes across Unstarted and started statuses. |
| .changeset/whole-lines-smoke.md | Publishes a minor bump for @ensnode/ensdb-sdk due to new metadata support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.ts
Outdated
Show resolved
Hide resolved
apps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.ts
Outdated
Show resolved
Hide resolved
Greptile SummaryThis PR introduces a startup precondition gate ( Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant P as Ponder (onchain event)
participant IAP as indexingActivationPromise
participant VERC as ensureValidEnsRainbowConnection
participant ENSDb as ENSDb (ensDbClient)
participant ERW as ENSRainbow (singleton)
P->>IAP: await eventHandlerPreconditions(Onchain)
note over IAP: First call only — promise cached thereafter
IAP->>VERC: initializeIndexingActivation()
VERC->>ENSDb: getEnsRainbowPublicConfig()
ENSDb-->>VERC: storedConfig (or undefined)
alt Cold start (no storedConfig)
loop pRetry (max 3)
VERC->>ENSDb: getIndexingStatusSnapshot()
ENSDb-->>VERC: snapshot (or undefined → retry)
end
VERC->>VERC: invariant_indexingStatusUnstarted(snapshot)
end
VERC->>ERW: waitForEnsRainbowToBeReady()
note over ERW: pRetry health check, up to ~1 hour
ERW-->>VERC: ready
VERC->>ERW: ensRainbowClient.config()
ERW-->>VERC: fetchedConfig
alt Warm start (storedConfig exists)
VERC->>VERC: invariant_labelSetIdCompatibility
VERC->>VERC: invariant_highestLabelSetVersionCompatibility
end
VERC->>ENSDb: upsertEnsRainbowPublicConfig(fetchedConfig)
ENSDb-->>VERC: ok
VERC-->>IAP: resolved
IAP-->>P: preconditions met — handler executes
Reviews (2): Last reviewed commit: "Apply PR feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
apps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.ts
Outdated
Show resolved
Hide resolved
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
apps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.ts
Outdated
Show resolved
Hide resolved
Also, allow for async preconditions for the indexing setup events. This will be handy for ensuring certain pg extensions were enabled before indexing starts.
tk-o
force-pushed
the
feat/ensure-valid-ensrainbow-connection
branch
from
d498f8d to
4e2454b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Thanks for updates. Shared some small feedback 👍
| * @returns Validated Omnichain Indexing Status Snapshot. | ||
| * @throws Error if the Omnichain Indexing Status is not in expected status yet. | ||
| */ | ||
| private async getValidatedIndexingStatusSnapshot(): Promise<OmnichainIndexingStatusSnapshot> { |
There was a problem hiding this comment.
I'm not confident it's right to completely remove this logic.
My worry is: how do we tell the difference between the following situations:
- Omnichain status is Unstarted because no indexing has started in ENSDb yet.
- Indexing has started in ENSDb with an earlier instance of ENSIndexer, but now ENSIndexer is being restarted and it's still working to discover it's true omnichain indexing status from the state in ENSDb. During this case I'm worried that ENSIndexer also thinks it's "Unstarted"?
There was a problem hiding this comment.
Omnichain status is "unstarted" if an only if no indexing has started. This is related to the fact that the omnichain status can only be "unstarted" if and only if all chains are queued. And a chain is queued if and only if its config.startBlock is equal to its checkpointBlock (checkpointBlock is sourced from Ponder Indexing Status, which is sourced from _ponder_checkpoint table in the ENSIndexer Schema).
If indexing has started and ENSIndexer has written any indexed data into the ENSIndexer Schema, the checkpointBlock for some indexed chain has also been stored in the _ponder_checkpoint table in the ENSIndexer Schema in ENSDb. Therefore, if ENSIndexer instance restarts, some of the indexed chains will not be "queued" anyomore as checkpointBlock will be ahead of config.startBlock for that chain. That leads us to the fact that the omnichain status cannot be "unstarted" anymore, and goes straight to "backfill".
| } | ||
|
|
||
| /** | ||
| * Ensure that we have a valid connection to ENSRainbow and |
There was a problem hiding this comment.
We need to refine our terminology here.
Why are we using the word "valid"? I assume the goal is to describe something about "compatible" instead? And assuming the key idea here is "compatible", can you please help to make it more explicit exactly what "compatible" means?
There was a problem hiding this comment.
I was thinking that the connection is valid after it was validated that the connected ENSRainbow instance can be used to serve the ENSIndexer instance. The "compatibility" term was suggested to reference a relation between two EnsRainbowPublicConfig objects, where two of such objects are "compatbile if, and only if, both conditions are true:
- The label set ID is the same between these two objects.
- The highest label set version of the
EnsRainbowPublicConfigobject fetched from ENSRainbow instance is greater than or equal to the highest label set version of theEnsRainbowPublicConfigobject stored in ENSDb.
The compatibility of EnsRainbowPublicConfig objects is enforced in the form of invariants. And when both invariants are OK, we can say that the ENSIndexer has a valid connection to ENSRainbow instance. In other words, there's a valid connection to ENSRainbow.
Does that make sense?
shrugs
left a comment
shrugs
left a comment
There was a problem hiding this comment.
I can't comment on the indexing status terminology or logic, but otherwise looks good to me
|
@greptile review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts (1)
283-315:⚠️ Potential issue | 🟡 MinorAssert the builder inputs, not only the mocked outputs.
Because
buildCrossChainIndexingStatusSnapshotOmnichain(...)is preprogrammed with sequential return values, this test still passes if the worker calls the builder with the wrong omnichain snapshot on either tick. AddtoHaveBeenNthCalledWith(...)assertions on the builder so the regression coverage is tied to the newUnstartedflow, not just to mocked return order.🧪 Suggested assertion tightening
vi.mocked(buildCrossChainIndexingStatusSnapshotOmnichain) .mockReturnValueOnce(unstartedCrossChainSnapshot) .mockReturnValueOnce(validCrossChainSnapshot); @@ // assert expect(indexingStatusBuilder.getOmnichainIndexingStatusSnapshot).toHaveBeenCalledTimes(2); + expect(buildCrossChainIndexingStatusSnapshotOmnichain).toHaveBeenNthCalledWith( + 1, + unstartedSnapshot, + expect.any(Number), + ); + expect(buildCrossChainIndexingStatusSnapshotOmnichain).toHaveBeenNthCalledWith( + 2, + validSnapshot, + expect.any(Number), + ); expect(ensDbClient.upsertIndexingStatusSnapshot).toHaveBeenCalledTimes(2); expect(ensDbClient.upsertIndexingStatusSnapshot).toHaveBeenNthCalledWith( 1, unstartedCrossChainSnapshot, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts` around lines 283 - 315, The test currently only asserts the mocked outputs from buildCrossChainIndexingStatusSnapshotOmnichain and ensDbClient.upsertIndexingStatusSnapshot, so add assertions that verify the worker actually called the builder with the expected omnichain snapshots on each tick: use expect(buildCrossChainIndexingStatusSnapshotOmnichain).toHaveBeenNthCalledWith(1, unstartedSnapshot) and .toHaveBeenNthCalledWith(2, validSnapshot) (or, if the builder method under test is indexingStatusBuilder.getOmnichainIndexingStatusSnapshot, assert that one instead) to ensure the worker passed the correct input snapshots to buildCrossChainIndexingStatusSnapshotOmnichain/indexingStatusBuilder.getOmnichainIndexingStatusSnapshot on the first and second interval ticks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/src/lib/indexing-engines/ponder.test.ts`:
- Around line 321-336: The test currently accepts any error (/.*/), which is too
broad; update the assertion in the it("throws when indexing status snapshot is
missing") case to match the specific error thrown when
mockGetIndexingStatusSnapshot resolves to undefined—capture the actual error
message produced by the indexing code and replace the generic regex with a
targeted one (for example matching "indexing status snapshot" or the exact
phrase thrown) in the expectHandlerThrows call that wraps
getRegisteredCallback(); ensure you still set
mockGetIndexingStatusSnapshot.mockResolvedValue(undefined) and call
addOnchainEventListener/getRegisteredCallback as before so the test validates
the correct failure path.
In
`@apps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.ts`:
- Around line 146-166: The pRetry call that loads the indexing status snapshot
(wrapping ensDbClient.getIndexingStatusSnapshot) only sets retries and relies on
default exponential backoff; update that pRetry invocation to include explicit
timing options (e.g., minTimeout and maxTimeout or a timeout and factor) to make
retry timing deterministic, matching the backoff configuration used by
waitForEnsRainbowToBeReady(); adjust the options object passed to pRetry for the
indexingStatusSnapshot block so it includes the explicit timing fields alongside
retries and onFailedAttempt.
---
Outside diff comments:
In `@apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts`:
- Around line 283-315: The test currently only asserts the mocked outputs from
buildCrossChainIndexingStatusSnapshotOmnichain and
ensDbClient.upsertIndexingStatusSnapshot, so add assertions that verify the
worker actually called the builder with the expected omnichain snapshots on each
tick: use
expect(buildCrossChainIndexingStatusSnapshotOmnichain).toHaveBeenNthCalledWith(1,
unstartedSnapshot) and .toHaveBeenNthCalledWith(2, validSnapshot) (or, if the
builder method under test is
indexingStatusBuilder.getOmnichainIndexingStatusSnapshot, assert that one
instead) to ensure the worker passed the correct input snapshots to
buildCrossChainIndexingStatusSnapshotOmnichain/indexingStatusBuilder.getOmnichainIndexingStatusSnapshot
on the first and second interval ticks.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 3b2e6925-5d95-4fec-9821-1feaa55211c3
📒 Files selected for processing (13)
.changeset/whole-lines-smoke.mdapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsapps/ensindexer/src/lib/ensrainbow/singleton.tsapps/ensindexer/src/lib/indexing-engines/ponder.test.tsapps/ensindexer/src/lib/indexing-engines/ponder.tsapps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.tspackages/ensdb-sdk/src/client/ensdb-reader.test.tspackages/ensdb-sdk/src/client/ensdb-reader.tspackages/ensdb-sdk/src/client/ensdb-writer.test.tspackages/ensdb-sdk/src/client/ensdb-writer.tspackages/ensdb-sdk/src/client/ensnode-metadata.tspackages/ensdb-sdk/src/client/serialize/ensnode-metadata.ts
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
EnsRainbowPublicConfigobject to/from ENSNode Metadata table.packages/ensdb-sdk/src/client/*apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsensureValidEnsRainbowConnectionfunction to cover all business logic required as precondition for starting indexing.apps/ensindexer/src/lib/indexing-engines/preconditions/valid-ensrainbow-connection.tsandapps/ensindexer/src/lib/indexing-engines/ponder.tsWhy
Testing
Scenario 1: cold start
Simulated the scenarion when ENSIndexer was running for the first time and ENSRainbow instance was temporarily unavailable.
Result: ENSIndexer was able to:
Details
Scenario 2: warm start
Simulated the scenario when ENSIndexer was restarted and ENSRainbow instance was temporarily unavailable.
Result: ENSIndexer was able to
Details
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)
This PR does not introduce significant changes and is low-risk to review quickly.