[WIP] Integration of Indexing Status Builder into ENSIndexer API #1614
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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.
| * Get a {@link Blockrange} for each indexed chain. | ||
| * | ||
| * Invariants: | ||
| * - every chain include a startBlock, |
There was a problem hiding this comment.
Grammar in the invariants list: “every chain include a startBlock” should be “every chain includes a startBlock”.
| * - every chain include a startBlock, | |
| * - every chain includes a startBlock, |
| /** | ||
| * Chain Name | ||
| * | ||
| * Often use as type for object keys expressing Ponder ideas, such as | ||
| * chain status, or chain metrics. | ||
| */ | ||
| export type ChainName = string; | ||
|
|
||
| /** | ||
| * Ponder config datasource with a flat `chain` value. | ||
| */ | ||
| export type PonderConfigDatasourceFlat = { | ||
| chain: ChainName; | ||
| } & AddressConfig & | ||
| Blockrange; | ||
|
|
||
| /** | ||
| * Ponder config datasource with a nested `chain` value. | ||
| */ | ||
| export type PonderConfigDatasourceNested = { | ||
| chain: Record<ChainName, AddressConfig & Blockrange>; | ||
| }; | ||
|
|
||
| /** | ||
| * Ponder config datasource | ||
| */ | ||
| export type PonderConfigDatasource = PonderConfigDatasourceFlat | PonderConfigDatasourceNested; | ||
|
|
||
| /** | ||
| * Ponder config datasource | ||
| */ |
There was a problem hiding this comment.
This file appears to duplicate the existing Ponder-config parsing logic in apps/ensindexer/src/lib/indexing-status/ponder-metadata/config.ts (including identical types and error messages). Duplicating this logic across two locations increases the risk of them drifting apart; consider re-exporting/reusing the existing implementation (or moving it to a shared module) instead of maintaining two copies.
| /** | |
| * Chain Name | |
| * | |
| * Often use as type for object keys expressing Ponder ideas, such as | |
| * chain status, or chain metrics. | |
| */ | |
| export type ChainName = string; | |
| /** | |
| * Ponder config datasource with a flat `chain` value. | |
| */ | |
| export type PonderConfigDatasourceFlat = { | |
| chain: ChainName; | |
| } & AddressConfig & | |
| Blockrange; | |
| /** | |
| * Ponder config datasource with a nested `chain` value. | |
| */ | |
| export type PonderConfigDatasourceNested = { | |
| chain: Record<ChainName, AddressConfig & Blockrange>; | |
| }; | |
| /** | |
| * Ponder config datasource | |
| */ | |
| export type PonderConfigDatasource = PonderConfigDatasourceFlat | PonderConfigDatasourceNested; | |
| /** | |
| * Ponder config datasource | |
| */ | |
| export type { | |
| ChainName, | |
| PonderConfigDatasourceFlat, | |
| PonderConfigDatasourceNested, | |
| PonderConfigDatasource, | |
| } from "../../../src/lib/indexing-status/ponder-metadata/config"; | |
| /** | |
| * Ponder config datasource | |
| */ |
| type ChainBlockRefs, | ||
| getChainsBlockRefs, | ||
| } from "@/lib/indexing-status-builder/chain-block-refs"; | ||
| import { buildCrossChainIndexingStatusSnapshotOmnichain } from "@/lib/indexing-status-builder/corss-chain-indexing-status-snapshot"; |
There was a problem hiding this comment.
Import path contains a typo (corss-chain-indexing-status-snapshot). Even if the underlying file currently uses that spelling, propagating the typo into new code makes it harder to discover/maintain. Consider renaming the module to cross-chain-indexing-status-snapshot (and updating imports) or adding a correctly spelled re-export to avoid baking in the misspelling.
| import { buildCrossChainIndexingStatusSnapshotOmnichain } from "@/lib/indexing-status-builder/corss-chain-indexing-status-snapshot"; | |
| import { buildCrossChainIndexingStatusSnapshotOmnichain } from "@/lib/indexing-status-builder/cross-chain-indexing-status-snapshot"; |
| const crossChainIndexingSnapshot = await fetchAndBuildCrossChainIndexingStatusSnapshot( | ||
| publicClients, | ||
| snapshotTime, | ||
| ); |
There was a problem hiding this comment.
/indexing-status no longer handles failures from fetchAndBuildCrossChainIndexingStatusSnapshot(). Any thrown error will be caught by the global app.onError handler and returned as { message: "Internal Server Error" }, which breaks the ENSNode API contract that callers can always discriminate on responseCode (should return IndexingStatusResponseCodes.Error with HTTP 500). Wrap this call in a try/catch and return a serialized error response on failure (optionally log the underlying error).
| // TODO: make it a cached call, so the RPC requests are performed just once, at the application startup | ||
| const chainsBlockRefs: Map<number, ChainBlockRefs> = await getChainsBlockRefs( | ||
| indexedChainIds, | ||
| chainsConfigBlockrange, | ||
| ponderIndexingMetrics.chains, | ||
| publicClients, | ||
| ); |
There was a problem hiding this comment.
This implementation performs RPC calls on every /indexing-status request via getChainsBlockRefs(...) (which fetches multiple block refs per chain). Previously this endpoint used a cached block-ref lookup; without caching this can add significant latency/load. Consider memoizing the computed chainsBlockRefs (and/or the chainsConfigBlockrange) similarly to the existing cached approach so the RPC calls happen once per process/startup, not per request.
| ]); | ||
|
|
||
| // TODO: make it a cached call, so the RPC requests are performed just once, at the application startup | ||
| const chainsBlockRefs: Map<number, ChainBlockRefs> = await getChainsBlockRefs( |
There was a problem hiding this comment.
chainsBlockRefs is typed as Map<number, ChainBlockRefs>, but getChainsBlockRefs() returns Map<ChainId, ChainBlockRefs>. Using number here weakens type safety and can hide key-type mismatches; prefer Map<ChainId, ChainBlockRefs> to keep the type consistent end-to-end.
| const chainsBlockRefs: Map<number, ChainBlockRefs> = await getChainsBlockRefs( | |
| const chainsBlockRefs: Map<ChainId, ChainBlockRefs> = await getChainsBlockRefs( |
| /** | ||
| * Chain Name | ||
| * | ||
| * Often use as type for object keys expressing Ponder ideas, such as | ||
| * chain status, or chain metrics. | ||
| */ | ||
| export type ChainName = string; | ||
|
|
There was a problem hiding this comment.
The code/documentation refers to ChainName, but in this codebase ponderConfig.chains keys are chain IDs stringified (see chainsConnectionConfig() using [chainId.toString()]). Using ChainName = string here is misleading and makes it easier to accidentally pass non-chain-id keys. Consider renaming this to something like ChainIdString (or reusing the SDK’s ChainIdString type) and updating the related comments accordingly.
tk-o
force-pushed
the
feat/indexing-status-builder-3
branch
from
17e224c to
8ac1c11
Compare
| * Build {@link Blockrange} for each indexed chain. | ||
| * | ||
| * Invariants: | ||
| * - every chain include a startBlock, |
There was a problem hiding this comment.
Grammar error in comment: "every chain include a startBlock" should use singular verb "includes"
apps/ensindexer/ponder/src/lib/omnichian-indexing-status-snapshot.ts
Outdated
Show resolved
Hide resolved
| deserializeChainId, | ||
| } from "@ensnode/ensnode-sdk"; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Code duplication in Ponder config parsing logic across two files creates maintenance burden and risk of divergence
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.
| import { deserializeChainId } from "@ensnode/ensnode-sdk"; | ||
| import { type ChainId, PonderClient } from "@ensnode/ponder-sdk"; | ||
|
|
||
| import type { ChainBlockRefs } from "@/lib/indexing-status/ponder-metadata"; |
There was a problem hiding this comment.
ChainBlockRefs is imported from @/lib/indexing-status/ponder-metadata, but fetchChainsBlockRefs returns ChainBlockRefs from @/lib/indexing-status-builder/chain-block-refs. These are different types (ensnode-sdk vs ponder-sdk BlockRef) and will likely make the returned Map<ChainId, ChainBlockRefs> not typecheck. Import ChainBlockRefs from the same module as fetchChainsBlockRefs (or re-export a single canonical type) so the map value type is consistent.
| import type { ChainBlockRefs } from "@/lib/indexing-status/ponder-metadata"; | |
| import type { ChainBlockRefs } from "@/lib/indexing-status-builder/chain-block-refs"; |
| // TODO: this operation may fail, so it should be wrapped in auto-retry logic. | ||
| // pRetry could be used, with a retry strategy that includes | ||
| // exponential backoff and jitter, to avoid overwhelming the RPC endpoints | ||
| // in case of transient errors or rate limits. | ||
| export const cachedChainsBlockRefs = await initializeCachedChainsBlockRefs(); | ||
|
|
||
| async function initializeCachedChainsBlockRefs(): Promise<Readonly<Map<ChainId, ChainBlockRefs>>> { |
There was a problem hiding this comment.
cachedChainsBlockRefs is initialized using a top-level await, which performs network requests (Ponder metrics + RPC block fetches) at module import time. If any of these calls fail, the whole module import fails and the API server may not start. Consider lazy initialization (e.g., an async getter with memoization) and/or moving initialization into the server startup sequence with explicit retry/backoff and a clear failure mode.
| throw new Error("Failed to fetch chainsBlockRefs: no block refs found"); | ||
| } | ||
|
|
||
| return Object.freeze(chainsBlockRefs); |
There was a problem hiding this comment.
Object.freeze(chainsBlockRefs) does not make a Map immutable (callers can still mutate via .set()/.delete()). If immutability is required, avoid exporting a mutable Map instance (e.g., return a ReadonlyMap reference that isn’t shared, or wrap access behind functions and keep the mutable map private).
| return Object.freeze(chainsBlockRefs); | |
| return chainsBlockRefs as Readonly<Map<ChainId, ChainBlockRefs>>; |
| // 3.a) The endBlock can only be set for a chain if and only if every | ||
| // ponderSource for that chain has its respective `endBlock` defined. | ||
| const isEndBlockForChainAllowed = chainEndBlocks.length === chainStartBlocks.length; | ||
|
|
There was a problem hiding this comment.
isEndBlockForChainAllowed is computed as chainEndBlocks.length === chainStartBlocks.length, but chainStartBlocks only counts sources where startBlock is a number. This can incorrectly allow an endBlock when some datasources for the chain use non-numeric startBlock (e.g. "latest") or otherwise weren’t counted. Track the total number of datasources that match the chain separately and only allow endBlock if all matching datasources provide a numeric endBlock (and consider failing fast if any matching datasource has a non-numeric startBlock, since it’s described as unsupported).
| // 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.
Typo in comment: endBLock should be endBlock.
| // 3.b) Get the highest endBLock for the chain. | |
| // 3.b) Get the highest endBlock for the chain. |
| export function buildChainsBlockrange(ponderConfig: PonderConfigType): Map<ChainId, Blockrange> { | ||
| const chainsBlockrange = new Map<ChainId, Blockrange>(); | ||
|
|
There was a problem hiding this comment.
buildChainsBlockrange introduces non-trivial parsing/invariant logic (flat vs nested datasources, min start block, conditional end block). There are existing Vitest suites for indexing-status/ponder-metadata; adding focused unit tests for this function (including mixed flat/nested configs and partial endBlock definitions) would help prevent regressions.
| */ | ||
| // TODO: this operation may fail, so it should be wrapped in auto-retry logic. | ||
| // pRetry could be used, with a retry strategy that includes | ||
| // exponential backoff and jitter, to avoid overwhelming the RPC endpoints |
There was a problem hiding this comment.
Top-level await on module-level exports causes entire API to fail if initialization fails, with no error handling or recovery mechanism
1 participant
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
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)