[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
| 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.
Typos in filenames: "corss-chain" should be "cross-chain" and "omnichian" should be "omnichain"
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.
| * - every chain include a startBlock, | |
| * - every chain includes a startBlock, |
Grammar error in comment: "every chain include a startBlock" should use singular verb "includes"
| ]); | ||
|
|
||
| // TODO: make it a cached call, so the RPC requests are performed just once, at the application startup | ||
| const chainsBlockRefs: Map<number, ChainBlockRefs> = await buildCachedChainsBlockRefs( |
There was a problem hiding this comment.
| const chainsBlockRefs: Map<number, ChainBlockRefs> = await buildCachedChainsBlockRefs( | |
| const chainsBlockRefs: Map<ChainId, ChainBlockRefs> = await buildCachedChainsBlockRefs( |
Type mismatch for chainsBlockRefs variable - declared as Map<number, ChainBlockRefs> but function returns Map<ChainId, ChainBlockRefs>
| 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
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)