Make ENSDb Writer Worker to use builders for ENSIndexer Public Config and Indexing Status objects#1715
Merged
lightwalker-eth merged 13 commits intomainfrom Mar 4, 2026
Conversation
…EnsIndexerVersionInfo` functions.
…SIndexer Public Config in `EnsDbWriterWorker`
…nly data provider that is needed.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
🦋 Changeset detectedLatest commit: 5fa7ba0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces ad-hoc public-config construction with a cached PublicConfigBuilder, shifts EnsDbWriterWorker to depend on that builder, adds SDK validators validateEnsIndexerPublicConfig and validateEnsIndexerVersionInfo, and adds tests, mocks, singletons, and ENSDB/API handler wiring to consume stored config and snapshots. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Worker as EnsDbWriterWorker
participant Builder as PublicConfigBuilder
participant ENSRainbow as ENSRainbowApiClient
participant Validator as SDKValidators
participant EnsDB as EnsDbClient
Worker->>Builder: getPublicConfig()
Builder->>ENSRainbow: config() (fetch ENSRainbowPublicConfig)
ENSRainbow-->>Builder: ENSRainbowPublicConfig
Builder->>Validator: validateEnsIndexerVersionInfo(...)
Validator-->>Builder: ValidatedVersionInfo
Builder->>Validator: validateEnsIndexerPublicConfig(assembledConfig)
Validator-->>Builder: ValidatedPublicConfig
Builder-->>Worker: EnsIndexerPublicConfig (cached)
Worker->>EnsDB: upsertEnsIndexerPublicConfig / upsertIndexingStatusSnapshot(...)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
Contributor
Greptile SummaryThis PR successfully decouples ENSIndexer's HTTP handlers from config-building by introducing a Key changes:
The core logic — moving config reads to ENSDb and writes to the worker — is correct and well-tested. Error handling is appropriate for each endpoint. Confidence Score: 5/5
Sequence DiagramsequenceDiagram
participant App as ENSIndexer App
participant Worker as EnsDbWriterWorker
participant PCB as PublicConfigBuilder
participant ENSRainbow as ENSRainbow Client
participant ENSDb as ENSDb Client
participant HTTP as HTTP Handler (ensnode-api)
App->>Worker: startEnsDbWriterWorker()
note over App,Worker: run() is NOT awaited
par Concurrent fetch during startup
Worker->>PCB: getPublicConfig() [with pRetry]
PCB->>ENSRainbow: config()
ENSRainbow-->>PCB: EnsRainbowPublicConfig
PCB->>PCB: validateEnsIndexerPublicConfig(...)
PCB-->>Worker: EnsIndexerPublicConfig (cached)
and
Worker->>ENSDb: getEnsIndexerPublicConfig()
ENSDb-->>Worker: storedConfig (or undefined)
end
opt storedConfig exists
Worker->>Worker: validateEnsIndexerPublicConfigCompatibility(stored, inMemory)
end
Worker->>ENSDb: upsertEnsDbVersion(versionInfo.ensDb)
Worker->>ENSDb: upsertEnsIndexerPublicConfig(inMemoryConfig)
loop Every 1 second
Worker->>Worker: getValidatedIndexingStatusSnapshot()
Worker->>ENSDb: upsertIndexingStatusSnapshot(crossChainSnapshot)
end
HTTP->>ENSDb: GET /config → getEnsIndexerPublicConfig()
ENSDb-->>HTTP: EnsIndexerPublicConfig
HTTP-->>HTTP: serializeENSIndexerPublicConfig(config)
HTTP->>ENSDb: GET /indexing-status → getIndexingStatusSnapshot()
ENSDb-->>HTTP: CrossChainIndexingStatusSnapshot
HTTP-->>HTTP: createRealtimeIndexingStatusProjection(snapshot, now)
Last reviewed commit: 8e53487 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Additional Comments (2)
Now that Consider removing
If the mock is ever removed or the real validator is invoked, this would produce a confusing type mismatch. It appears the intent was: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the ENSIndexer/ENSApi decoupling by making ENSDb the single source of truth for ENSIndexer’s public config and indexing status in HTTP handlers, and by moving public-config construction into a dedicated builder used during startup/worker execution.
Changes:
- Added SDK validation helpers for
EnsIndexerPublicConfigandEnsIndexerVersionInfoand exported them from@ensnode/ensnode-sdk. - Introduced
PublicConfigBuilderin ENSIndexer to build/cache the public config (incl. ENSRainbow config + runtime version info) for ENSDb writes. - Updated ENSDb writer worker and HTTP endpoints to read public config and indexing status from
EnsDbClientinstead of building them per-request.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts | New validator for EnsIndexerVersionInfo. |
| packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-public-config.ts | New validator for EnsIndexerPublicConfig. |
| packages/ensnode-sdk/src/ensindexer/config/index.ts | Re-exports the new validation helpers. |
| apps/ensindexer/src/lib/version-info.ts | Adds helpers to get ENSIndexer + Node.js versions for building version info. |
| apps/ensindexer/src/lib/public-config-builder/public-config-builder.ts | New builder responsible for building/caching validated public config (incl. ENSRainbow fetch). |
| apps/ensindexer/src/lib/public-config-builder/singleton.ts | Singleton wiring for PublicConfigBuilder. |
| apps/ensindexer/src/lib/public-config-builder/public-config-builder.test.ts | Unit tests for builder caching/error behavior. |
| apps/ensindexer/src/lib/public-config-builder/index.ts | Barrel export for builder. |
| apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.ts | Worker now uses PublicConfigBuilder instead of EnsIndexerClient for config. |
| apps/ensindexer/src/lib/ensdb-writer-worker/singleton.ts | Updates singleton wiring for new worker constructor signature. |
| apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts | Updates worker tests for the new builder dependency. |
| apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.mock.ts | Adds mock helpers for the updated worker tests. |
| apps/ensindexer/src/config/public.ts | Removes the old per-request public-config building helper. |
| apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts | HTTP handlers now read public config and indexing status snapshot from ENSDb. |
| .changeset/violet-tires-sell.md | Changeset for ensindexer release. |
| .changeset/flat-flowers-shave.md | Changeset for @ensnode/ensnode-sdk release. |
Comments suppressed due to low confidence (1)
apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.ts:160
pRetrycallback is returning thegetPublicConfigfunction instead of invoking it. This will causepRetryto resolve to a function value, andinMemoryConfigwon’t be anEnsIndexerPublicConfig(and will likely break when accessingversionInfo). CallgetPublicConfig()inside the retry callback.
const inMemoryConfigPromise = pRetry(() => this.publicConfigBuilder.getPublicConfig(), {
retries: 3,
onFailedAttempt: ({ error, attemptNumber, retriesLeft }) => {
console.warn(
`ENSIndexer Config fetch attempt ${attemptNumber} failed (${error.message}). ${retriesLeft} retries left.`,
);
},
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
apps/ensindexer/src/lib/public-config-builder/public-config-builder.test.ts
Outdated
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-public-config.ts
Outdated
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts
Outdated
Show resolved
Hide resolved
| // Invariant: the public config is guaranteed to be available in ENSDb after | ||
| // application startup. | ||
| if (typeof publicConfig === "undefined") { | ||
| throw new Error("Unreachable: ENSIndexer Public Config is not available in ENSDb"); |
There was a problem hiding this comment.
The "Unreachable" branch is reachable: startEnsDbWriterWorker() is kicked off without awaiting .run(), so /config can be called before the worker has upserted the public config to ENSDb. Instead of throwing (which becomes a 500), return a controlled 503/500 response indicating config isn’t ready yet, or await worker initialization before serving routes.
Suggested change
| throw new Error("Unreachable: ENSIndexer Public Config is not available in ENSDb"); | |
| // During startup there is a window where the public config may not yet | |
| // be available in ENSDb. In that case, return a controlled error rather | |
| // than throwing and causing an unstructured 500 response. | |
| return c.json( | |
| { | |
| error: "ENSIndexer Public Config is not yet available in ENSDb", | |
| }, | |
| 503, | |
| ); |
Contributor
Author
There was a problem hiding this comment.
In practice, it won't be rechable. ENSIndexer might not need any HTTP API whatsoever.
apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts
Outdated
Show resolved
Hide resolved
![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: 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 @.changeset/violet-tires-sell.md:
- Line 5: The changeset description line "Improved HTTP handlers with ENSDb
Client being the only data provider that is needed." is a bit verbose—update the
sentence to a clearer, more concise wording such as "Refactored HTTP handlers to
rely solely on ENSDb Client for data" (edit the content of
.changeset/violet-tires-sell.md to replace the existing line), leaving the
frontmatter intact and ignoring MD041 as noted.
In `@apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts`:
- Around line 159-180: The test assigns storedConfig to the module namespace
ensDbClientMock instead of the actual stored config object, letting an invalid
shape slip through; change storedConfig to use the real public config (e.g.,
storedConfig = ensDbClientMock.publicConfig) and pass that into
createMockEnsDbClient's getEnsIndexerPublicConfig mock so the worker.run()
concurrency test uses a properly shaped stored config (references: storedConfig
variable, ensDbClientMock, createMockEnsDbClient, and
getEnsIndexerPublicConfig).
In `@apps/ensindexer/src/lib/public-config-builder/public-config-builder.test.ts`:
- Around line 131-168: Add a new test that verifies concurrent-call caching by
starting two (or more) getPublicConfig() calls before the first one resolves and
asserting that only a single fetch/build occurs; to implement, mock
ensRainbowClientMock.config to return a controllable (deferred) Promise so you
can call builder.getPublicConfig() twice, then resolve the deferred promise and
await both results, and assert ensRainbowClientMock.config was called once,
getEnsIndexerVersion/getNodeJsVersion/getPackageVersion/validateEnsIndexerVersionInfo/validateEnsIndexerPublicConfig
were each called only once (or expected counts), and both returned values
strictly equal the same cached mockPublicConfig; use
PublicConfigBuilder.getPublicConfig and ensRainbowClientMock.config as the
unique symbols to locate where to modify/add the test.
In `@apps/ensindexer/src/lib/public-config-builder/public-config-builder.ts`:
- Around line 28-29: Concurrent callers of getPublicConfig() can race and build
multiple times; introduce an in-flight promise to dedupe builds by adding a
private field (e.g., buildingPromise: Promise<EnsIndexerPublicConfig> | null)
and update getPublicConfig() to: return immutablePublicConfig if set; otherwise
if buildingPromise exists await and return it; otherwise set buildingPromise to
the async build/validate flow that sets immutablePublicConfig on success and
clears buildingPromise in a finally block (propagate errors so callers see
failures and ensure buildingPromise is cleared on error).
In `@apps/ensindexer/src/lib/version-info.ts`:
- Around line 22-24: getNodeJsVersion() currently returns process.versions.node
(e.g., "20.0.0") but tests/mocks expect a "v" prefixed string; update
getNodeJsVersion() to return `v` + process.versions.node (ensuring no double
prefix if already present) and also update getENSIndexerVersionInfo() where it
directly uses process.versions.node to use getNodeJsVersion() (or prepend "v"
similarly) so the Node version in the version info object matches the "vX.Y.Z"
format used in tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd5e06cc-e156-4d3b-a6f0-b048975a4cfe
📒 Files selected for processing (16)
.changeset/flat-flowers-shave.md.changeset/violet-tires-sell.mdapps/ensindexer/ponder/src/api/handlers/ensnode-api.tsapps/ensindexer/src/config/public.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.mock.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsapps/ensindexer/src/lib/ensdb-writer-worker/singleton.tsapps/ensindexer/src/lib/public-config-builder/index.tsapps/ensindexer/src/lib/public-config-builder/public-config-builder.test.tsapps/ensindexer/src/lib/public-config-builder/public-config-builder.tsapps/ensindexer/src/lib/public-config-builder/singleton.tsapps/ensindexer/src/lib/version-info.tspackages/ensnode-sdk/src/ensindexer/config/index.tspackages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-public-config.tspackages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts
💤 Files with no reviewable changes (1)
- apps/ensindexer/src/config/public.ts
apps/ensindexer/src/lib/public-config-builder/public-config-builder.test.ts
Outdated
Show resolved
Hide resolved
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@greptile review |
apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts (1)
157-165:⚠️ Potential issue | 🟡 MinorUse the actual stored config object in this concurrency test.
storedConfigis currently assigned to the module namespace, not anEnsIndexerPublicConfigobject, so the test can pass with an invalid shape.✅ Suggested fix
- const storedConfig = ensDbClientMock; + const storedConfig = ensDbClientMock.publicConfig; const inMemoryConfig = ensDbClientMock.publicConfig; @@ expect(ensDbClient.getEnsIndexerPublicConfig).toHaveBeenCalledTimes(1); expect(publicConfigBuilder.getPublicConfig).toHaveBeenCalledTimes(1); + expect(validateEnsIndexerPublicConfigCompatibility).toHaveBeenCalledWith( + storedConfig, + inMemoryConfig, + );🤖 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 157 - 165, The test assigns storedConfig to the module namespace (ensDbClientMock) instead of a real EnsIndexerPublicConfig, allowing an invalid shape to slip through; update storedConfig to use the actual public config object (e.g., ensDbClientMock.publicConfig or an explicit EnsIndexerPublicConfig fixture) and ensure the mocked getEnsIndexerPublicConfig returns that real config; check references in the test to storedConfig, inMemoryConfig, createMockEnsDbClient, getEnsIndexerPublicConfig, and validateEnsIndexerPublicConfigCompatibility to confirm the shape matches the expected EnsIndexerPublicConfig.apps/ensindexer/src/lib/public-config-builder/public-config-builder.test.ts (1)
141-172: 🧹 Nitpick | 🔵 TrivialAdd an overlapping-call cache test for
getPublicConfig().This currently validates sequential caching only; it doesn’t verify that concurrent callers are deduplicated into a single fetch/build path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/lib/public-config-builder/public-config-builder.test.ts` around lines 141 - 172, Add a new test that verifies getPublicConfig() deduplicates concurrent calls: mock ensRainbowClientMock.config to return a single deferred Promise (e.g., create a manual resolver or use a delay) so multiple simultaneous calls can be issued before it resolves, instantiate PublicConfigBuilder, call Promise.all on three concurrent builder.getPublicConfig() invocations, then resolve the deferred response and await the Promise.all result; assert ensRainbowClientMock.config was called exactly once, validateEnsIndexerPublicConfig and other validators were called once as in the existing test, and assert all three results are strictly the same object (===) to confirm deduplication. Reference PublicConfigBuilder, getPublicConfig, ensRainbowClientMock.config, and validateEnsIndexerPublicConfig when implementing the test.
🤖 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/handlers/ensnode-api.ts`:
- Around line 21-25: The current guard in the /config handler (where
getEnsIndexerPublicConfig() result is stored in publicConfig) treats undefined
as unreachable and throws, causing startup race 500s; change this to handle the
legitimate undefined case by returning a transient response (e.g., 503 Service
Unavailable or an empty/fallback config) instead of throwing. Locate the check
around publicConfig in ensnode-api.ts (the code that calls
getEnsIndexerPublicConfig()), remove the throw, and implement a graceful early
return that sets appropriate HTTP status and message or a safe default payload
so callers don’t get a hard 500 during initial metadata writes.
In
`@packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-public-config.ts`:
- Around line 18-22: Replace the non-fail-fast safeParse usage with zod's parse
so validation throws immediately: change schema.safeParse(unvalidatedConfig) +
the result.success branch to simply schema.parse(unvalidatedConfig) and remove
the manual throw/pretty-error branch; apply the same change in the other
occurrence in ensindexer-version-info.ts (the schema.safeParse call at line 18)
so both validators use schema.parse() for immediate failure handling.
In
`@packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts`:
- Around line 18-22: Replace the safeParse-based validation with zod.parse() so
validation is fail-fast: call schema.parse(unvalidatedVersionInfo) inside a
try/catch, and in the catch detect ZodError (imported from zod) and rethrow a
formatted error like new Error(`Invalid EnsIndexerVersionInfo:
${prettifyError(e)}`); rethrow non-Zod errors unchanged; update the code around
schema.safeParse/unvalidatedVersionInfo to use schema.parse and reference
prettifyError and ZodError accordingly.
---
Duplicate comments:
In `@apps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.ts`:
- Around line 157-165: The test assigns storedConfig to the module namespace
(ensDbClientMock) instead of a real EnsIndexerPublicConfig, allowing an invalid
shape to slip through; update storedConfig to use the actual public config
object (e.g., ensDbClientMock.publicConfig or an explicit EnsIndexerPublicConfig
fixture) and ensure the mocked getEnsIndexerPublicConfig returns that real
config; check references in the test to storedConfig, inMemoryConfig,
createMockEnsDbClient, getEnsIndexerPublicConfig, and
validateEnsIndexerPublicConfigCompatibility to confirm the shape matches the
expected EnsIndexerPublicConfig.
In `@apps/ensindexer/src/lib/public-config-builder/public-config-builder.test.ts`:
- Around line 141-172: Add a new test that verifies getPublicConfig()
deduplicates concurrent calls: mock ensRainbowClientMock.config to return a
single deferred Promise (e.g., create a manual resolver or use a delay) so
multiple simultaneous calls can be issued before it resolves, instantiate
PublicConfigBuilder, call Promise.all on three concurrent
builder.getPublicConfig() invocations, then resolve the deferred response and
await the Promise.all result; assert ensRainbowClientMock.config was called
exactly once, validateEnsIndexerPublicConfig and other validators were called
once as in the existing test, and assert all three results are strictly the same
object (===) to confirm deduplication. Reference PublicConfigBuilder,
getPublicConfig, ensRainbowClientMock.config, and validateEnsIndexerPublicConfig
when implementing the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ec57794-adac-4670-8b79-fee8903d2010
📒 Files selected for processing (7)
.changeset/violet-tires-sell.mdapps/ensindexer/ponder/src/api/handlers/ensnode-api.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.test.tsapps/ensindexer/src/lib/ensdb-writer-worker/ensdb-writer-worker.tsapps/ensindexer/src/lib/public-config-builder/public-config-builder.test.tspackages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-public-config.tspackages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts
Comment on lines
+21
to
+25
| // Invariant: the public config is guaranteed to be available in ENSDb after | ||
| // application startup. | ||
| if (typeof publicConfig === "undefined") { | ||
| throw new Error("Unreachable: ENSIndexer Public Config is not available in ENSDb"); | ||
| } |
Contributor
There was a problem hiding this comment.
/config can hit a real startup race; this guard is reachable.
getEnsIndexerPublicConfig() legitimately returns undefined before metadata is written. Treating it as unreachable will produce transient 500s right after startup.
✅ Suggested handling
if (typeof publicConfig === "undefined") {
- throw new Error("Unreachable: ENSIndexer Public Config is not available in ENSDb");
+ return c.json(
+ { message: "ENSIndexer Public Config is not available yet." },
+ 503,
+ );
}📝 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.
Suggested change
| // Invariant: the public config is guaranteed to be available in ENSDb after | |
| // application startup. | |
| if (typeof publicConfig === "undefined") { | |
| throw new Error("Unreachable: ENSIndexer Public Config is not available in ENSDb"); | |
| } | |
| // Invariant: the public config is guaranteed to be available in ENSDb after | |
| // application startup. | |
| if (typeof publicConfig === "undefined") { | |
| return c.json( | |
| { message: "ENSIndexer Public Config is not available yet." }, | |
| 503, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts` around lines 21 - 25,
The current guard in the /config handler (where getEnsIndexerPublicConfig()
result is stored in publicConfig) treats undefined as unreachable and throws,
causing startup race 500s; change this to handle the legitimate undefined case
by returning a transient response (e.g., 503 Service Unavailable or an
empty/fallback config) instead of throwing. Locate the check around publicConfig
in ensnode-api.ts (the code that calls getEnsIndexerPublicConfig()), remove the
throw, and implement a graceful early return that sets appropriate HTTP status
and message or a safe default payload so callers don’t get a hard 500 during
initial metadata writes.
Contributor
Author
There was a problem hiding this comment.
In practice, it won't be rechable. ENSIndexer might not need any HTTP API whatsoever.
Contributor
There was a problem hiding this comment.
@tk-o, understood — the EnsDbWriterWorker populates the public config at startup (fail-fast), so by the time the HTTP layer accepts requests, the config is guaranteed to be present in ENSDb. My concern about the startup race was incorrect. The throw pattern is consistent with the existing design philosophy.
I'll record this for future reviews.
✏️ Learnings added
Learnt from: tk-o
Repo: namehash/ensnode PR: 1715
File: apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts:21-25
Timestamp: 2026-03-04T11:40:29.081Z
Learning: In `apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts`, the guard `if (typeof publicConfig === "undefined") { throw new Error("Unreachable: ...") }` in the `/config` handler is intentionally unreachable. The `EnsDbWriterWorker` populates the ENSIndexer public config into ENSDb at startup (fail-fast), so the HTTP layer is guaranteed to only serve requests after the config is available. The `throw` is a defensive invariant, not a real error path.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:28:59.644Z
Learning: Applies to apps/ensindexer/**/*.ts : Use Ponder as the indexer framework for blockchain indexing
Learnt from: tk-o
Repo: namehash/ensnode PR: 1639
File: packages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.ts:21-76
Timestamp: 2026-02-16T17:53:46.139Z
Learning: In the ENSNode SDK (`packages/ensnode-sdk`), schema builder functions exported from `zod-schemas.ts` files (e.g., `makeEnsApiIndexingStatusResponseSchema`) are considered internal API, not public API. These can have breaking changes without requiring deprecated aliases, even when exported via the `internal` entry point.
Learnt from: tk-o
Repo: namehash/ensnode PR: 1705
File: apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts:25-36
Timestamp: 2026-03-02T20:10:05.060Z
Learning: Ensure the ENS label set validation (validateSupportedLabelSetAndVersion) is performed at startup by the ENSDb Writer Worker during application startup for ENSIndexer. If validation fails, the worker should crash the process (fail-fast), so that runtime /config endpoints do not need to raise or return error responses. This enforces configuration correctness at deploy/startup time rather than at runtime for the file apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts.
Learnt from: tk-o
Repo: namehash/ensnode PR: 1705
File: apps/ensapi/src/config/validations.ts:37-43
Timestamp: 2026-03-02T19:03:50.730Z
Learning: In `apps/ensapi/src/config/validations.ts`, the ENSRainbow version equality invariant (ensIndexerPublicConfig.versionInfo.ensRainbowPublicConfig.version === packageJson.version) is intentionally kept. Even though ensRainbowPublicConfig represents a connected ENSRainbow service, strict version parity with ENSApi is enforced as a deployment requirement.
Learnt from: notrab
Repo: namehash/ensnode PR: 1631
File: apps/ensapi/src/handlers/ensnode-api.ts:23-27
Timestamp: 2026-02-18T16:11:09.421Z
Learning: In the ensapi application, dynamic `import("@/config")` inside request handlers is an acceptable pattern because Node.js caches modules after the first import, making subsequent calls resolve from cache with negligible overhead (just promise resolution).
Learnt from: tk-o
Repo: namehash/ensnode PR: 1614
File: apps/ensindexer/src/lib/ponder-api-client.ts:7-7
Timestamp: 2026-02-18T15:26:09.067Z
Learning: In apps/ensindexer/src/lib/ponder-api-client.ts, the localPonderClientPromise is intentionally left as a permanently rejected promise when LocalPonderClient.init fails after retries. This is expected behavior because process.exitCode = 1 signals the process should terminate, and if it continues running, all subsequent calls should fail immediately with the cached rejection rather than retrying initialization.
Learnt from: tk-o
Repo: namehash/ensnode PR: 1615
File: packages/ensnode-sdk/src/api/indexing-status/deserialize.ts:38-40
Timestamp: 2026-02-07T12:22:32.900Z
Learning: In `packages/ensnode-sdk/src/api/indexing-status/deserialize.ts`, the pattern of using `z.preprocess()` with `buildUnvalidatedIndexingStatusResponse` (which returns `unknown`) is intentional. This enforces a "parse serialized → preprocess to unvalidated → validate final schema" flow, where `z.preprocess` is semantically correct because it runs before final validation. Using `.transform()` would be incorrect here as it runs after parsing and receives a typed input.
Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:28:59.644Z
Learning: Applies to apps/ensapi/src/**/*.ts : Use the shared `errorResponse` helper from `apps/ensapi/src/lib/handlers/error-response.ts` for all error responses in ENSApi
Learnt from: tk-o
Repo: namehash/ensnode PR: 1615
File: packages/ensnode-sdk/src/ensindexer/indexing-status/chain-indexing-status-snapshot.ts:314-322
Timestamp: 2026-02-07T11:54:52.607Z
Learning: In the ENSNode SDK, `ChainIndexingStatusSnapshot[]` parameters in functions like `getTimestampForLowestOmnichainStartBlock` are guaranteed not to be empty arrays by design.
Learnt from: tk-o
Repo: namehash/ensnode PR: 1617
File: packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts:9-12
Timestamp: 2026-02-09T10:19:29.575Z
Learning: In ensnode-sdk validation functions (e.g., `validateChainIndexingStatusSnapshot` in `packages/ensnode-sdk/src/ensindexer/indexing-status/validate/chain-indexing-status-snapshot.ts`), the pattern of using `ChainIndexingStatusSnapshot | unknown` (even though it collapses to `unknown` in TypeScript) is intentionally kept for semantic clarity and documentation purposes.
Learnt from: Goader
Repo: namehash/ensnode PR: 1663
File: packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts:74-96
Timestamp: 2026-02-24T15:53:06.633Z
Learning: In TypeScript code reviews, prefer placing invariants on type aliases only when the invariant is context-independent or reused across multiple fields. If an invariant depends on surrounding rules or object semantics (e.g., field-specific metrics), keep the invariant as a field JSDoc instead. This guideline applies to TS files broadly (e.g., the repo's v1/award-models and similar modules).
packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-public-config.ts
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts
Show resolved
Hide resolved
Contributor
Author
|
@greptile review |
tk-o
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-public-config.ts
Show resolved
Hide resolved
packages/ensnode-sdk/src/ensindexer/config/validate/ensindexer-version-info.ts
Show resolved
Hide resolved
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
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.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
PublicConfigBuilderclass to cover all abstractions required for buildingEnsIndexerPublicConfigobject within ENSIndexer runtime.EnsDbWriterWorkerto build ENSIndexer Public Config object withPublicConfigBuilder, instead of reading the config fromEnsIndexerClient.Why
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)