Integration of Indexing Status Builder into ENSIndexer API#1614
Integration of Indexing Status Builder into ENSIndexer API#1614
Conversation
This module aims to host an abstraction layer that primarly relies on `viem` and `@ensnode/ponder-sdk` abstractions. It is still using `@ensnode/ensnode-sdk` for convenience, but leave the possiblity to iterate and remove `@ensnode/ensnode-sdk` dependency in the future.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
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:
📝 WalkthroughWalkthroughIntroduces a LocalPonderClient singleton with retry-based initialization; moves indexing-status handler to fetch per-chain metadata via that client; adds utilities for chain blockrange and block fetching; splits ChainIndexingMetadata into immutable and dynamic parts; and updates p-retry dependency to the workspace catalog. Changes
Sequence Diagram(s)sequenceDiagram
participant App as ENSIndexer App
participant Loader as getLocalPonderClient()
participant LocalClient as LocalPonderClient
participant PonderApp as Ponder App (HTTP)
participant PublicClient as Chain PublicClient
participant Handler as /indexing-status Handler
App->>Loader: request LocalPonderClient
Loader->>LocalClient: init(ponderAppUrl, indexedChainIds) [with p-retry]
LocalClient->>PonderApp: fetch ponderConfig, public clients, metrics/status
PonderApp-->>LocalClient: ponderConfig, ponderPublicClients, metrics/status
LocalClient->>PublicClient: build per-chain PublicClient map
LocalClient-->>Loader: initialized LocalPonderClient
Handler->>Loader: getLocalPonderClient()
Loader-->>Handler: LocalPonderClient instance
Handler->>LocalClient: chainsIndexingMetadata()
LocalClient->>PonderApp: fetch dynamic metrics/status
PonderApp-->>LocalClient: metrics/status
LocalClient->>PublicClient: fetch blocks (via fetchBlockRef) as needed
LocalClient-->>Handler: merged ChainIndexingMetadata per chain
Handler->>Handler: buildOmnichainIndexingStatusSnapshot()
Handler-->>Client: return snapshot
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 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 |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
apps/ensindexer/ponder/src/lib/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
apps/ensindexer/ponder/src/lib/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Integrates the new “Indexing Status Builder” layer into the ENSIndexer (Ponder) HTTP API by moving /indexing-status snapshot construction onto builder + PonderClient-fetched metadata.
Changes:
- Added
fetchAndBuildCrossChainIndexingStatusSnapshot()to fetch Ponder/metrics+/status, compute chain block refs, and build an omnichain cross-chain snapshot. - Added a Ponder-config parsing helper to derive per-chain blockranges from datasource start/end blocks.
- Updated
/indexing-statushandler to use the new snapshot builder flow.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| apps/ensindexer/ponder/src/lib/cross-chain-indexing-status-snapshot.ts | New builder integration: fetch Ponder metadata, compute block refs, build cross-chain snapshot. |
| apps/ensindexer/ponder/src/lib/chains-config-blockrange.ts | New helper to derive per-chain blockranges from ponder.config.ts datasources. |
| apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts | Switches /indexing-status to the new fetch+build snapshot function. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensindexer/ponder/src/lib/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
apps/ensindexer/ponder/src/lib/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
apps/ensindexer/ponder/src/lib/cross-chain-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
tk-o
force-pushed
the
feat/indexing-status-builder-3
branch
from
17e224c to
8ac1c11
Compare
apps/ensindexer/ponder/src/lib/omnichian-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
tk-o
force-pushed
the
feat/indexing-status-builder-3
branch
from
8ac1c11 to
6303636
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Simplify code, merge input params for `buildOmnichainIndexingStatusSnapshot` into a single input object with chain metadata.
tk-o
force-pushed
the
feat/indexing-status-builder-3
branch
from
6303636 to
f5ecab1
Compare
Additional Comments (1)
The old |
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Thanks. Shared a few questions appreciate your advice.
apps/ensindexer/tsconfig.json
Outdated
| "paths": { | ||
| "@/*": ["./src/*"] | ||
| "@/*": ["./src/*"], | ||
| "@/ponder/*": ["./ponder/src/*"] |
There was a problem hiding this comment.
@tk-o Appreciate background info that motivated this update. Thanks
There was a problem hiding this comment.
This update was required so we could use the following style of imports:
import { LocalPonderClient } from "@/ponder/api/lib/local-ponder-client";
import ponderConfig from "@/ponder/config";
Without this update, we'd need to use relative paths, which would also work. However, this @/ponder/* alias is quite nice to apply. Please feel free to share your thoughts on that motivation 👍
| import { createCrossChainIndexingStatusSnapshotOmnichain } from "@/lib/indexing-status/build-index-status"; | ||
| import { fetchAndBuildOmnichainIndexingStatusSnapshot } from "@/ponder/lib/omnichian-indexing-status-snapshot"; | ||
|
|
||
| const publicClients = new Map<ChainId, PublicClient>( |
There was a problem hiding this comment.
This will be a great utility function to move into the "local ponder client"
There was a problem hiding this comment.
I created publicClient(chainId: ChainId): PublicClient method on LocalPonderClient to allow access to Ponder cached RPC clients 👍
|
|
||
| // Ponder API dependencies: Ponder config, and PonderClient | ||
| // TODO: move into a separate file | ||
| const indexedChainIds = Object.keys(ponderConfig.chains).map((maybeChainId) => |
There was a problem hiding this comment.
Is there a special reason we read this from the ponder config and not the ENSIndexer config object?
There was a problem hiding this comment.
Great point, I think we could use just ENSIndexer config here 👍
| /** | ||
| * Build a list of indexed chain IDs. | ||
| */ | ||
| export function buildIndexedChainIds(publicClients: Map<ChainId, PublicClient>): ChainId[] { |
There was a problem hiding this comment.
Not clear on some things that are happening here.
I thought I saw some other code that was getting the set of indexed chain ids from the ponder config?
But now this is another function that seems to be getting the set of indexed chain ids from a different source: from a map of ChainId / PublicClient.
Are we doing things here right?
Is there potentially a mixup here caused by how there's a distinction in Ponder between:
- The set of RPCs (PublicClients) that are configured.
- The set of chains configured for indexing.
Where these two might not be exactly the same? If so we need more precision with our terminology and how we communicate these ideas.
It also seems strange to me to see this function all alone in this file operating on such a specific data structure. Maybe it should live somewhere else?
Appreciate your advice.
There was a problem hiding this comment.
I think you are right, we don't have to build a list of indexed chain IDs based on any Ponder API. All we need is to pass the set of indexed chain IDs from ENSIndexer config into the Local Ponder Client.
| * @throws Error if any of the required data cannot be fetched or is invalid, | ||
| * or if invariants are violated. | ||
| */ | ||
| async function buildChainsIndexingMetadataFixed( |
There was a problem hiding this comment.
What does "fixed" mean? Does it mean "immutable"?
There was a problem hiding this comment.
Yes, it means "immutable", which I think suits here better than "fixed". Will apply renames accordingly 👍
| * @returns The initialized LocalPonderClient instance. | ||
| * @throws Error if the client fails to initialize after the specified number of retries. | ||
| */ | ||
| localPonderClientPromise = pRetry(() => LocalPonderClient.init(config.ensIndexerUrl), { |
There was a problem hiding this comment.
Hmm. Why are we trying to force the local ponder client to init this way upon process startup?
I assume we're doing this because of a strategy to eagerly load some state into the local ponder client that requires network fetches? But why do we need such an eager strategy? Why wouldn't a lazy strategy work nice?
There was a problem hiding this comment.
As discussed, we'll change the approach and will rely on data fetching done by SWR caches so that the client initialization could remain sync. More in PR #1652.
Updates LocalPonderClient dependency to favour Ponder SDK types
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/ponder/src/api/lib/chains-config-blockrange.ts`:
- Around line 43-53: The JSDoc for PonderConfigDatasources duplicates the
singular comment used for PonderConfigDatasource; update the comment above the
type PonderConfigDatasources to a distinct, plural description (e.g., "Ponder
config datasources" or "Collection of Ponder config datasources") so it
accurately documents the map type instead of repeating the singular description
for PonderConfigDatasource.
- Line 152: Fix the typo in the inline comment that currently reads "Get the
highest endBLock for the chain." — change "endBLock" to "endBlock" so the
comment correctly references the endBlock concept used in the
chains-config-blockrange logic; ensure any other nearby occurrences of the same
misspelling in the same file are corrected to "endBlock".
- Around line 175-181: The second guard checking typeof blockrange.startBlock
=== "undefined" is unreachable; remove that throw and instead assert the type of
the returned value from deserializeBlockrange so TypeScript knows startBlock is
present. Replace the runtime guard with a type assertion/cast to
BlockrangeWithStartBlock (or add an inline assertion comment) when calling
chainsBlockrange.set(chainId, ...) after
deserializeBlockrange(chainLowestStartBlock, ...), referencing
deserializeBlockrange, blockrange, chainLowestStartBlock,
BlockrangeWithStartBlock, chainsBlockrange.set and serializedChainId to locate
the code.
- Around line 88-92: The type guard isPonderDatasourceNested incorrectly treats
null as an object; update it to explicitly exclude null (and arrays if
applicable) before returning true. Change the implementation to check
ponderDatasource.chain !== null && typeof ponderDatasource.chain === "object"
(and add && !Array.isArray(ponderDatasource.chain) if chain should be a plain
object) so downstream access like ponderSource.chain[serializedChainId] is safe.
In `@apps/ensindexer/src/lib/indexing-status-builder/chain-indexing-metadata.ts`:
- Around line 53-60: The JSDoc for the exported type ChainIndexingMetadata still
uses the outdated term "fixed metadata"; update that comment to say "immutable
metadata" to match the renamed type ChainIndexingMetadataImmutable and ensure
consistency — e.g., replace "fixed metadata (backfill scope and indexing
config)" with "immutable metadata (backfill scope and indexing config)" in the
block above the ChainIndexingMetadata declaration.
---
Duplicate comments:
In `@apps/ensindexer/ponder/src/api/lib/chains-config-blockrange.ts`:
- Around line 110-114: The inline cast on ponderSources hides type errors —
remove the blanket "as PonderConfigDatasource[]" and ensure each source is
correctly typed or filtered: either change
ponderConfig.accounts/blocks/contracts to have values of PonderConfigDatasource,
or create a type‑guard function (e.g. isPonderConfigDatasource) and build
ponderSources by concatenating Object.values(...) and filtering with that guard
so only valid PonderConfigDatasource items remain; reference symbols:
ponderSources, ponderConfig, PonderConfigDatasource, and the type‑guard you add.
In `@apps/ensindexer/src/lib/indexing-status-builder/chain-indexing-metadata.ts`:
- Around line 30-51: No changes required—the JSDoc title was corrected and the
interface ChainIndexingMetadataDynamic with non-optional fields indexingMetrics
and indexingStatus is correct; leave the interface and its doc-comment as-is.
| // ponderSource for that chain has its respective `endBlock` defined. | ||
| const isEndBlockForChainAllowed = chainEndBlocks.length === chainStartBlocks.length; | ||
|
|
||
| // 3.b) Get the highest endBLock for the chain. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Typo: "endBLock" → "endBlock".
- // 3.b) Get the highest endBLock for the chain.
+ // 3.b) Get the highest endBlock for the chain.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 3.b) Get the highest endBLock for the chain. | |
| // 3.b) Get the highest endBlock for the chain. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensindexer/ponder/src/api/lib/chains-config-blockrange.ts` at line 152,
Fix the typo in the inline comment that currently reads "Get the highest
endBLock for the chain." — change "endBLock" to "endBlock" so the comment
correctly references the endBlock concept used in the chains-config-blockrange
logic; ensure any other nearby occurrences of the same misspelling in the same
file are corrected to "endBlock".
| if (typeof blockrange.startBlock === "undefined") { | ||
| throw new Error( | ||
| `Invalid blockrange for chain '${serializedChainId}'. The blockrange must include a valid startBlock number.`, | ||
| ); | ||
| } | ||
|
|
||
| chainsBlockrange.set(chainId, blockrange as BlockrangeWithStartBlock); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
The second startBlock guard at lines 175–179 is unreachable at runtime.
After the throw at lines 161–165, chainLowestStartBlock is a valid BlockNumber, so deserializeBlockrange will either throw on schema failure or return with startBlock present. The if (typeof blockrange.startBlock === "undefined") branch can never execute.
The guard exists only to satisfy TypeScript's inferred return type of deserializeBlockrange (Blockrange with optional startBlock) before the cast on line 181. Consider using an assertion comment or a type assertion directly to make the intent clearer without the misleading dead throw:
♻️ Proposed simplification
- // Invariant: the blockrange must include a valid startBlock number.
- if (typeof blockrange.startBlock === "undefined") {
- throw new Error(
- `Invalid blockrange for chain '${serializedChainId}'. The blockrange must include a valid startBlock number.`,
- );
- }
-
- chainsBlockrange.set(chainId, blockrange as BlockrangeWithStartBlock);
+ // startBlock is guaranteed non-undefined: chainLowestStartBlock was validated above.
+ chainsBlockrange.set(chainId, blockrange as BlockrangeWithStartBlock);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof blockrange.startBlock === "undefined") { | |
| throw new Error( | |
| `Invalid blockrange for chain '${serializedChainId}'. The blockrange must include a valid startBlock number.`, | |
| ); | |
| } | |
| chainsBlockrange.set(chainId, blockrange as BlockrangeWithStartBlock); | |
| // startBlock is guaranteed non-undefined: chainLowestStartBlock was validated above. | |
| chainsBlockrange.set(chainId, blockrange as BlockrangeWithStartBlock); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensindexer/ponder/src/api/lib/chains-config-blockrange.ts` around lines
175 - 181, The second guard checking typeof blockrange.startBlock ===
"undefined" is unreachable; remove that throw and instead assert the type of the
returned value from deserializeBlockrange so TypeScript knows startBlock is
present. Replace the runtime guard with a type assertion/cast to
BlockrangeWithStartBlock (or add an inline assertion comment) when calling
chainsBlockrange.set(chainId, ...) after
deserializeBlockrange(chainLowestStartBlock, ...), referencing
deserializeBlockrange, blockrange, chainLowestStartBlock,
BlockrangeWithStartBlock, chainsBlockrange.set and serializedChainId to locate
the code.
apps/ensindexer/src/lib/indexing-status-builder/chain-indexing-metadata.ts
Show resolved
Hide resolved
Building a list of indexed chain IDs from Ponder object was unnecessary.
tk-o
force-pushed
the
feat/indexing-status-builder-3
branch
from
34db972 to
b8b2f19
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ponder/src/api/lib/local-ponder-client.ts`:
- Around line 195-211: The init path (LocalPonderClient.init) currently builds
publicClients (via buildPublicClientsMap) and chainIndexingMetadataImmutable
(via buildChainsIndexingMetadataImmutable) but does not validate that the
provided indexedChainIds are present in those maps, causing runtime errors in
chainsIndexingMetadata; update init to validate that every id in indexedChainIds
exists in the publicClients (or chainIndexingMetadataImmutable) keys and, if any
are missing, throw a clear startup error listing the missing ChainId(s) so the
app fails fast with an actionable message.
---
Duplicate comments:
In `@apps/ensindexer/ponder/src/api/lib/local-ponder-client.ts`:
- Around line 76-80: The current check in local-ponder-client.ts uses if
(!chainConfigBlockrange.startBlock) which erroneously treats a valid genesis
block 0 as missing; update the conditional in the function handling
chainConfigBlockrange (referencing chainConfigBlockrange.startBlock) to use an
explicit nullish check (e.g., if (chainConfigBlockrange.startBlock == null) or
=== undefined) so that 0 is accepted, and keep the same error throw when truly
null/undefined.
In `@apps/ensindexer/src/lib/indexing-status-builder/chain-indexing-metadata.ts`:
- Around line 53-60: Update the JSDoc for the ChainIndexingMetadata type to use
"immutable metadata" instead of "fixed metadata" to match the renamed interface;
edit the comment block above the exported type (referencing
ChainIndexingMetadata, ChainIndexingMetadataImmutable and
ChainIndexingMetadataDynamic) so the description reads "...Combines both the
immutable metadata (backfill scope and indexing config) and the dynamic
metadata..." for consistency.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (chainIndexingMetrics.state !== ChainIndexingStates.Historical) { | ||
| throw new Error( | ||
| `In order to build 'ChainsIndexingMetadataFixed', chain indexing state must be "historical" for indexed chain ID ${chainId}, but got "${chainIndexingMetrics.state}"`, |
There was a problem hiding this comment.
The error message references 'ChainsIndexingMetadataFixed', but the actual type name is 'ChainIndexingMetadataImmutable' (singular 'Chain' and 'Immutable' instead of 'Chains' and 'Fixed'). This inconsistency could confuse developers debugging issues. The error message should reference the correct type name.
| `In order to build 'ChainsIndexingMetadataFixed', chain indexing state must be "historical" for indexed chain ID ${chainId}, but got "${chainIndexingMetrics.state}"`, | |
| `In order to build 'ChainIndexingMetadataImmutable', chain indexing state must be "historical" for indexed chain ID ${chainId}, but got "${chainIndexingMetrics.state}"`, |
| /** | ||
| * Initialize the LocalPonderClient by connecting to the local Ponder app and | ||
| * fetching necessary data to build the client's state. This operation is | ||
| * retried up to 3 times in case of failure, with a warning logged on each | ||
| * failed attempt. | ||
| * | ||
| * @returns The initialized LocalPonderClient instance. | ||
| * @throws Error if the client fails to initialize after the specified number of retries. | ||
| */ |
There was a problem hiding this comment.
This JSDoc comment block (lines 30-38) is redundant and appears to be leftover documentation that should have been removed. It duplicates the function-level JSDoc comment above (lines 9-20) and is placed incorrectly within the function body rather than before a function declaration. The inline comments on lines 27-29 already explain what's happening at this point in the code.
| /** | |
| * Initialize the LocalPonderClient by connecting to the local Ponder app and | |
| * fetching necessary data to build the client's state. This operation is | |
| * retried up to 3 times in case of failure, with a warning logged on each | |
| * failed attempt. | |
| * | |
| * @returns The initialized LocalPonderClient instance. | |
| * @throws Error if the client fails to initialize after the specified number of retries. | |
| */ |
| // ponderSource for that chain has its respective `endBlock` defined. | ||
| const isEndBlockForChainAllowed = chainEndBlocks.length === chainStartBlocks.length; | ||
|
|
||
| // 3.b) Get the highest endBLock for the chain. |
There was a problem hiding this comment.
Spelling error: "endBLock" should be "endBlock" (lowercase 'l' in "Block").
| // 3.b) Get the highest endBLock for the chain. | |
| // 3.b) Get the highest endBlock for the chain. |
2 participants
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
Indexing Status Buildermodule #1613 with ENSIndexer APILocalPonderClientto ensure initialization was executed exactly once during application startup.LocalPonderClientmay require retiries, as Ponder HTTP endpoints may not be available when the first client initialization is attempted.Why
OmnichainIndexingStatusSnapshotbased on abstractions fromviemand@ensnode/ponder-sdk(and if really needed, for the time being, from@ensnode/ensnode-sdk).Testing
Notes for Reviewer (Optional)
Indexing Status Buildermodule #1613Pre-Review Checklist (Blocking)