Conversation
This builder reads ENSIndexer Public Config object from ENSDb and build `EnsApiPublicConfig` object that is later available for all modules of ENSApi.
`EnsIndexerPublicConfig` is managed by `PublicConfigBuilder` now. The `EnsApiConfig` has been updated to a) not include `ensIndexerPublicConfig` field anymore; b) include `databaseSchemaName` and `namespace` fields. The newly added fields are required to kick-off ENSApi app, while the `ensIndxerPublicConfig` can be fetched async.
…rrent `CrossChainIndexingStatus` object
…on ENSApi instances
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: a8d11e0 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 |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughReads public config and indexing status from ENSDb: adds EnsDbClient, a Drizzle helper and singleton, introduces PublicConfigBuilder (cached, validated public config), adds DATABASE_SCHEMA and NAMESPACE env vars, converts handlers/middleware to fetch runtime public config, and switches indexing-status retrieval to ENSDb snapshots. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Handler as ENSApi Handler
participant PCB as PublicConfigBuilder
participant ENSDb as ENSDbClient
participant DB as Database
Client->>Handler: Request needing public config
Handler->>PCB: getPublicConfig()
alt cache miss
PCB->>ENSDb: getEnsIndexerPublicConfig()
ENSDb->>DB: query ens_node_metadata (key: "ens-indexer-public-config")
DB-->>ENSDb: serialized config
ENSDb-->>PCB: deserialized ENSIndexerPublicConfig
PCB->>PCB: compute theGraphFallback, validate (validateEnsApiPublicConfig)
PCB-->>Handler: EnsApiPublicConfig (cached)
else cache hit
PCB-->>Handler: EnsApiPublicConfig (cached)
end
Handler-->>Client: Response using public config
sequenceDiagram
participant Client as Client
participant Cache as IndexingStatus Cache
participant ENSDb as ENSDbClient
participant DB as Database
Client->>Cache: getIndexingStatus()
Cache->>ENSDb: getIndexingStatusSnapshot()
ENSDb->>DB: query cross_chain_indexing_status_snapshot
DB-->>ENSDb: serialized snapshot
ENSDb-->>Cache: deserialized snapshot
Cache-->>Client: snapshot (or error if undefined)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR advances the “Split ENSApi and ENSIndexer” effort by decoupling ENSApi from ENSIndexer’s HTTP config/status endpoints and instead sourcing ENSIndexer public config + indexing status via ENSDb (plus adding SDK validation helpers to support the new config flow).
Changes:
- Add an ENSApi-side ENSDb client + public config builder and migrate middlewares/handlers to use it.
- Introduce ENSApi public-config validation in
@ensnode/ensnode-sdkand reuse a sharedversionschema. - Update env/terraform wiring to pass
DATABASE_SCHEMAandNAMESPACEexplicitly for the split.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| terraform/modules/ensindexer/main.tf | Moves DATABASE_SCHEMA/NAMESPACE into common env vars for the service. |
| packages/ensnode-sdk/src/ensapi/config/zod-schemas.ts | Extracts makeEnsApiVersionSchema() and reuses it in public-config schemas. |
| packages/ensnode-sdk/src/ensapi/config/validate/ensapi-public-config.ts | Adds validateEnsApiPublicConfig() with Zod-based validation + prettified errors. |
| packages/ensnode-sdk/src/ensapi/config/index.ts | Re-exports the new validateEnsApiPublicConfig() API. |
| apps/ensapi/src/middleware/thegraph-fallback.middleware.ts | Reads isSubgraphCompatible via ENSDb-backed public config instead of local config copy. |
| apps/ensapi/src/middleware/subgraph-meta.middleware.ts | Awaits async subgraph meta conversion. |
| apps/ensapi/src/middleware/require-core-plugin.middleware.ts | Uses ENSDb-backed public config to check plugin support. |
| apps/ensapi/src/middleware/registrar-actions.middleware.ts | Uses ENSDb-backed public config for registrar-actions prerequisites. |
| apps/ensapi/src/middleware/name-tokens.middleware.ts | Uses ENSDb-backed public config for name-tokens prerequisites. |
| apps/ensapi/src/middleware/can-accelerate.middleware.ts | Uses ENSDb-backed public config for plugin checks related to acceleration. |
| apps/ensapi/src/lib/subgraph/indexing-status-to-subgraph-meta.ts | Makes conversion async and pulls deployment/version from ENSDb-backed config. |
| apps/ensapi/src/lib/resolution/forward-resolution.ts | Uses ENSDb-backed public config for ENSv2 bailout check. |
| apps/ensapi/src/lib/public-config-builder/singleton.ts | Adds a singleton PublicConfigBuilder wired to the ENSDb client. |
| apps/ensapi/src/lib/public-config-builder/public-config-builder.ts | Implements immutable, validated ENSApi public-config construction from ENSDb + env. |
| apps/ensapi/src/lib/public-config-builder/public-config-builder.test.ts | Adds unit tests for caching, masking API key, and error propagation. |
| apps/ensapi/src/lib/public-config-builder/index.ts | Barrel export for the public-config-builder module. |
| apps/ensapi/src/lib/ensdb-client/singleton.ts | Adds ENSApi-local singleton ENSDb client instance. |
| apps/ensapi/src/lib/ensdb-client/ensdb-client.ts | Adds ENSApi-local ENSDb query client using Drizzle + ensnode_metadata. |
| apps/ensapi/src/lib/ensdb-client/ensdb-client.test.ts | Adds unit tests for ENSDb client query behavior. |
| apps/ensapi/src/lib/ensdb-client/ensdb-client.mock.ts | Provides fixtures for ENSDb client tests. |
| apps/ensapi/src/lib/ensdb-client/drizzle.ts | Duplicates makeDrizzle helper (currently) for ENSApi DB access. |
| apps/ensapi/src/handlers/name-tokens-api.ts | Defers subregistry computation until request-time using ENSDb-backed config. |
| apps/ensapi/src/handlers/ensnode-api.ts | Serves ENSApi public config from PublicConfigBuilder instead of constructing locally. |
| apps/ensapi/src/config/environment.ts | Updates env typing to include DATABASE_SCHEMA and NAMESPACE in ENSApi env surface. |
| apps/ensapi/src/config/config.schema.ts | Removes ENSIndexer fetch at startup; parses env for namespace/schema and adds config version. |
| apps/ensapi/src/config/config.schema.test.ts | Removes tests for deleted buildEnsApiPublicConfig(). |
| apps/ensapi/src/cache/indexing-status.cache.ts | Switches indexing status source from ENSNodeClient HTTP call to ENSDb snapshot reads. |
| apps/ensapi/.env.local.example | Documents required DATABASE_SCHEMA and NAMESPACE env vars for ENSApi. |
Comments suppressed due to low confidence (1)
apps/ensapi/src/lib/subgraph/indexing-status-to-subgraph-meta.ts:26
indexingContextToSubgraphMetareturnsnullin several branches, but the signature isPromise<SubgraphMeta>. This is a type mismatch that should fail typechecking; update the return type toPromise<SubgraphMeta | null>(and ideally align the JSDoc accordingly).
export async function indexingContextToSubgraphMeta(
indexingStatus: IndexingStatusMiddlewareVariables["indexingStatus"],
): Promise<SubgraphMeta> {
// indexing status middleware has never successfully fetched (and cached) an indexing status snapshot
// for the lifetime of this service instance.
if (indexingStatus instanceof Error) return null;
💡 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/ensapi/src/lib/public-config-builder/public-config-builder.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/lib/public-config-builder/public-config-builder.test.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ensapi/src/config/config.schema.test.ts (1)
26-177:⚠️ Potential issue | 🔴 CriticalRemove
.resolvesand.await—buildConfigFromEnvironmentis now synchronous, and add missing required environment variables.The test suite still uses pre-refactor patterns: (1)
buildConfigFromEnvironmentis now a synchronous function that parses environment variables directly, not an async function that fetches public config; (2)BASE_ENVis missing requiredNAMESPACEandDATABASE_SCHEMAkeys that the implementation expects; (3) the returned config shape no longer includesensIndexerPublicConfig,namespaceanddatabaseSchemaNamenow come from the environment, not from a fetched response.Update the test as follows:
- Add
NAMESPACE: "mainnet"andDATABASE_SCHEMA: "ensapi"toBASE_ENV- Remove the global
fetchmock andmockFetchsetup—the function does not make HTTP calls- Remove
awaitand.resolvesfrom the test assertion (line 67)- Update expected
namespaceanddatabaseSchemaNameto referenceBASE_ENVvalues instead ofENSINDEXER_PUBLIC_CONFIG- Remove the
ensIndexerPublicConfigfield from the expected config object🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/config/config.schema.test.ts` around lines 26 - 177, The tests still treat buildConfigFromEnvironment as async and rely on fetched ENS indexer data; update tests to reflect the new synchronous behavior by: add NAMESPACE ("mainnet") and DATABASE_SCHEMA ("ensapi") to BASE_ENV, remove mockFetch and the global fetch stub and any fetch-related setup/teardown, remove await and .resolves usage when calling buildConfigFromEnvironment (call it synchronously), remove the ensIndexerPublicConfig expectation and change expected namespace and databaseSchemaName to BASE_ENV.NAMESPACE and BASE_ENV.DATABASE_SCHEMA, and update/clean related tests (e.g., those referencing ENSINDEXER_PUBLIC_CONFIG) so they no longer depend on serialized remote config.apps/ensapi/src/config/config.schema.ts (1)
77-82: 🛠️ Refactor suggestion | 🟠 MajorJSDoc is stale and has redundant
@returnsThe summary still says this function fetches ENSIndexer public config, which is no longer true, and the
@returnstag is redundant.As per coding guidelines, "Do not add JSDoc `@returns` tags that merely restate the method summary; remove such redundancy during PR review".🧹 Proposed JSDoc fix
/** - * Builds the EnsApiConfig from an EnsApiEnvironment object, fetching the EnsIndexerPublicConfig. - * - * `@returns` A validated EnsApiConfig object + * Builds the EnsApiConfig from an EnsApiEnvironment object. * `@throws` Error with formatted validation messages if environment parsing fails */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/config/config.schema.ts` around lines 77 - 82, Update the JSDoc for the function that builds the EnsApiConfig from an EnsApiEnvironment (the block referencing EnsApiConfig and EnsApiEnvironment) to remove the stale claim about fetching the EnsIndexerPublicConfig and delete the redundant `@returns` tag; keep a concise summary that accurately reflects the function's behavior and preserve the `@throws` note for validation errors if present.
🤖 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/ensapi/.env.local.example`:
- Around line 15-21: The README/env example contains contradictory guidance
about DATABASE_SCHEMA: remove or reconcile the conflicting lines so it's clear
whether ENSApi requires DATABASE_SCHEMA; update the environment example and
comments to state one consistent rule (e.g., "ENSApi does not need to set
DATABASE_SCHEMA; it is inferred from the connected ENSIndexer's Config" OR "If
reading directly from ENSDb, set DATABASE_SCHEMA to an existing namespace in
DATABASE_URL"), and update references to DATABASE_SCHEMA, ENSApi, ENSDbClient
and ENSIndexer's Config so the example and comments align and cannot cause
startup misconfiguration.
In `@apps/ensapi/src/config/config.schema.ts`:
- Around line 85-96: Replace the raw env.NAMESPACE with the already-validated
namespace variable when constructing the config: use the parsed namespace (from
ENSNamespaceSchema.parse assigned to namespace) in the object passed to
EnsApiConfigSchema.parse and anywhere else the code currently uses env.NAMESPACE
(e.g., when calling buildRpcConfigsFromEnv and building the returned config) to
ensure the validated value is the single source of truth.
- Line 83: You changed buildConfigFromEnvironment to return EnsApiConfig
synchronously, so update all call sites and tests that expect a Promise (e.g.,
usages that use .resolves or await on a promise) to the new sync contract:
replace assertions using .resolves with direct assertions (e.g., await
expect(...).resolves -> expect(buildConfigFromEnvironment(...)).toEqual(...)),
remove unnecessary await/then chains or wrap calls appropriately, and update any
helper functions that treated buildConfigFromEnvironment as async; search for
buildConfigFromEnvironment and update tests and code to call it synchronously
and assert its returned EnsApiConfig directly.
In `@apps/ensapi/src/lib/ensdb-client/ensdb-client.ts`:
- Around line 118-124: Remove the redundant JSDoc `@returns` line from the
documentation block that begins with "Get ENSNode metadata record" in
ensdb-client.ts; leave the summary and the `@throws` tag intact, deleting only the
`@returns` tag that repeats the method summary so the JSDoc is concise and follows
the guideline.
In `@apps/ensapi/src/lib/ensdb-client/singleton.ts`:
- Around line 5-7: The docstring for the EnsDbClient singleton incorrectly
references "ENSIndexer"; update the comment to reference the correct service
(ENSApi / ENS API) so it accurately documents the singleton instance of
EnsDbClient used by the ENSApi service; locate the comment above the EnsDbClient
singleton in singleton.ts and replace "ENSIndexer" with "ENSApi" (or "ENS API")
to remove the confusion.
In `@apps/ensapi/src/lib/public-config-builder/public-config-builder.test.ts`:
- Around line 135-138: The test creates two PublicConfigBuilder instances
unnecessarily; replace the two constructions with a single instance and call
getPublicConfig() on that instance (e.g., instantiate PublicConfigBuilder once
into a variable and then await variable.getPublicConfig()), keeping the rest of
the test assertions intact and referencing PublicConfigBuilder and
getPublicConfig to locate the change.
In `@apps/ensapi/src/lib/public-config-builder/public-config-builder.ts`:
- Around line 12-16: Update the incorrect doc comment that reads "ENSRainbow
Client" to accurately say "ENSDb Client" in the top-of-file
header/documentation; search for the exact string "ENSRainbow Client" in
public-config-builder.ts and replace it with "ENSDb Client" (and update any
adjacent descriptive lines mentioning ENSRainbow to refer to ENSDb Client or
ENSDb Public Config) so the comment matches the actual dependency.
---
Outside diff comments:
In `@apps/ensapi/src/config/config.schema.test.ts`:
- Around line 26-177: The tests still treat buildConfigFromEnvironment as async
and rely on fetched ENS indexer data; update tests to reflect the new
synchronous behavior by: add NAMESPACE ("mainnet") and DATABASE_SCHEMA
("ensapi") to BASE_ENV, remove mockFetch and the global fetch stub and any
fetch-related setup/teardown, remove await and .resolves usage when calling
buildConfigFromEnvironment (call it synchronously), remove the
ensIndexerPublicConfig expectation and change expected namespace and
databaseSchemaName to BASE_ENV.NAMESPACE and BASE_ENV.DATABASE_SCHEMA, and
update/clean related tests (e.g., those referencing ENSINDEXER_PUBLIC_CONFIG) so
they no longer depend on serialized remote config.
In `@apps/ensapi/src/config/config.schema.ts`:
- Around line 77-82: Update the JSDoc for the function that builds the
EnsApiConfig from an EnsApiEnvironment (the block referencing EnsApiConfig and
EnsApiEnvironment) to remove the stale claim about fetching the
EnsIndexerPublicConfig and delete the redundant `@returns` tag; keep a concise
summary that accurately reflects the function's behavior and preserve the
`@throws` note for validation errors if present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1312259c-4986-4138-b39c-514d847555cb
📒 Files selected for processing (28)
apps/ensapi/.env.local.exampleapps/ensapi/src/cache/indexing-status.cache.tsapps/ensapi/src/config/config.schema.test.tsapps/ensapi/src/config/config.schema.tsapps/ensapi/src/config/environment.tsapps/ensapi/src/handlers/ensnode-api.tsapps/ensapi/src/handlers/name-tokens-api.tsapps/ensapi/src/lib/ensdb-client/drizzle.tsapps/ensapi/src/lib/ensdb-client/ensdb-client.mock.tsapps/ensapi/src/lib/ensdb-client/ensdb-client.test.tsapps/ensapi/src/lib/ensdb-client/ensdb-client.tsapps/ensapi/src/lib/ensdb-client/singleton.tsapps/ensapi/src/lib/public-config-builder/index.tsapps/ensapi/src/lib/public-config-builder/public-config-builder.test.tsapps/ensapi/src/lib/public-config-builder/public-config-builder.tsapps/ensapi/src/lib/public-config-builder/singleton.tsapps/ensapi/src/lib/resolution/forward-resolution.tsapps/ensapi/src/lib/subgraph/indexing-status-to-subgraph-meta.tsapps/ensapi/src/middleware/can-accelerate.middleware.tsapps/ensapi/src/middleware/name-tokens.middleware.tsapps/ensapi/src/middleware/registrar-actions.middleware.tsapps/ensapi/src/middleware/require-core-plugin.middleware.tsapps/ensapi/src/middleware/subgraph-meta.middleware.tsapps/ensapi/src/middleware/thegraph-fallback.middleware.tspackages/ensnode-sdk/src/ensapi/config/index.tspackages/ensnode-sdk/src/ensapi/config/validate/ensapi-public-config.tspackages/ensnode-sdk/src/ensapi/config/zod-schemas.tsterraform/modules/ensindexer/main.tf
apps/ensapi/src/lib/public-config-builder/public-config-builder.ts
Outdated
Show resolved
Hide resolved
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/ensapi/src/config/config.schema.ts (1)
76-81: 🛠️ Refactor suggestion | 🟠 MajorClean up stale/redundant JSDoc on
buildConfigFromEnvironmentThe docblock still mentions “fetching the EnsIndexerPublicConfig” (no longer true) and the
@returnsline is redundant. Please update/remove this block to match current behavior.As per coding guidelines, "Do not add JSDoc
@returnstags that merely restate the method summary; remove such redundancy during PR review".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/config/config.schema.ts` around lines 76 - 81, Update the JSDoc for buildConfigFromEnvironment to reflect current behavior: remove the stale phrase “fetching the EnsIndexerPublicConfig” and delete the redundant `@returns` tag that merely restates the summary; instead provide a concise one-line summary such as “Builds an EnsApiConfig from an EnsApiEnvironment” and keep or adjust any `@throws` description if it adds useful detail. Ensure the docblock above buildConfigFromEnvironment references only accurate responsibilities and validation/throw behavior, and remove any mention of fetching external configs.
♻️ Duplicate comments (2)
apps/ensapi/src/config/config.schema.ts (1)
84-95:⚠️ Potential issue | 🟡 MinorUse the parsed
namespacevalue when constructing configLine 84 parses
env.NAMESPACE, but Line 94 uses rawenv.NAMESPACEagain. Use the parsednamespacevariable as the single source of truth.♻️ Proposed fix
return EnsApiConfigSchema.parse({ version: packageJson.version, port: env.PORT, databaseUrl: env.DATABASE_URL, databaseSchemaName: env.DATABASE_SCHEMA, ensIndexerUrl: env.ENSINDEXER_URL, theGraphApiKey: env.THEGRAPH_API_KEY, - namespace: env.NAMESPACE, + namespace, rpcConfigs, customReferralProgramEditionConfigSetUrl: env.CUSTOM_REFERRAL_PROGRAM_EDITIONS, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/config/config.schema.ts` around lines 84 - 95, The code parses env.NAMESPACE into the variable namespace using ENSNamespaceSchema.parse but then passes the raw env.NAMESPACE into EnsApiConfigSchema.parse; update the config construction to use the parsed namespace variable instead of env.NAMESPACE. Locate the block where namespace is set (namespace = ENSNamespaceSchema.parse(env.NAMESPACE)) and where EnsApiConfigSchema.parse is called (the object with properties like version, port, rpcConfigs), and replace the namespace property value to the parsed namespace variable so buildRpcConfigsFromEnv and EnsApiConfigSchema.parse use the same validated value.apps/ensapi/src/lib/ensdb-client/ensdb-client.ts (1)
115-121:⚠️ Potential issue | 🟡 MinorRemove redundant
@returnsJSDoc lineThe
@returnsline repeats the summary and should be removed for consistency with the TS doc guideline.As per coding guidelines, "Do not add JSDoc `@returns` tags that merely restate the method summary; remove such redundancy during PR review".Proposed cleanup
/** * Get ENSNode metadata record * - * `@returns` selected record in ENSDb. * `@throws` when more than one matching metadata record is found * (should be impossible given the PK constraint on 'key') */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/lib/ensdb-client/ensdb-client.ts` around lines 115 - 121, Remove the redundant JSDoc `@returns` tag in the "Get ENSNode metadata record" comment block (the JSDoc immediately above the function that retrieves the ENS node metadata record) since it only restates the summary; leave the summary and the `@throws` line intact and ensure no other JSDoc tags are duplicated.
🤖 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/ensapi/src/lib/ensdb-client/ensdb-client.ts`:
- Around line 57-62: The constructor in ensdb-client.ts passes the wrong option
key to makeDrizzle; change the argument from databaseSchema to
databaseSchemaName so the call in the ENSDB client constructor (the makeDrizzle
invocation inside constructor(databaseUrl, databaseSchemaName)) uses the
expected key name (databaseSchemaName) to restore correct typing and schema
binding.
In `@apps/ensapi/src/middleware/registrar-actions.middleware.ts`:
- Around line 37-39: Wrap the await publicConfigBuilder.getPublicConfig() call
in a try/catch inside the registrar-actions middleware so failures don't bubble
out; if getPublicConfig throws, log the error and treat it as "registrar actions
unsupported" by setting the result used by hasRegistrarActionsConfigSupport to a
safe default (or set configSupport = false), then return the existing
backward-compatible serialized response using serializeRegistrarActionsResponse
with the 503 responseCode path so the middleware preserves the "Registrar
Actions unavailable" contract; reference publicConfigBuilder.getPublicConfig,
hasRegistrarActionsConfigSupport, serializeRegistrarActionsResponse and
responseCode when making the change.
---
Outside diff comments:
In `@apps/ensapi/src/config/config.schema.ts`:
- Around line 76-81: Update the JSDoc for buildConfigFromEnvironment to reflect
current behavior: remove the stale phrase “fetching the EnsIndexerPublicConfig”
and delete the redundant `@returns` tag that merely restates the summary; instead
provide a concise one-line summary such as “Builds an EnsApiConfig from an
EnsApiEnvironment” and keep or adjust any `@throws` description if it adds useful
detail. Ensure the docblock above buildConfigFromEnvironment references only
accurate responsibilities and validation/throw behavior, and remove any mention
of fetching external configs.
---
Duplicate comments:
In `@apps/ensapi/src/config/config.schema.ts`:
- Around line 84-95: The code parses env.NAMESPACE into the variable namespace
using ENSNamespaceSchema.parse but then passes the raw env.NAMESPACE into
EnsApiConfigSchema.parse; update the config construction to use the parsed
namespace variable instead of env.NAMESPACE. Locate the block where namespace is
set (namespace = ENSNamespaceSchema.parse(env.NAMESPACE)) and where
EnsApiConfigSchema.parse is called (the object with properties like version,
port, rpcConfigs), and replace the namespace property value to the parsed
namespace variable so buildRpcConfigsFromEnv and EnsApiConfigSchema.parse use
the same validated value.
In `@apps/ensapi/src/lib/ensdb-client/ensdb-client.ts`:
- Around line 115-121: Remove the redundant JSDoc `@returns` tag in the "Get
ENSNode metadata record" comment block (the JSDoc immediately above the function
that retrieves the ENS node metadata record) since it only restates the summary;
leave the summary and the `@throws` line intact and ensure no other JSDoc tags are
duplicated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fb5c75ec-02c1-4ab0-b8e8-912a934dc20c
📒 Files selected for processing (10)
apps/ensapi/src/config/config.schema.test.tsapps/ensapi/src/config/config.schema.tsapps/ensapi/src/handlers/ensnode-api.tsapps/ensapi/src/lib/ensdb-client/ensdb-client.test.tsapps/ensapi/src/lib/ensdb-client/ensdb-client.tsapps/ensapi/src/middleware/can-accelerate.middleware.tsapps/ensapi/src/middleware/name-tokens.middleware.tsapps/ensapi/src/middleware/registrar-actions.middleware.tsapps/ensindexer/src/lib/version-info.tspackages/ensnode-sdk/src/ensapi/config/conversions.test.ts
💤 Files with no reviewable changes (1)
- apps/ensindexer/src/lib/version-info.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 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.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
… Config API to rely solely on ENSDb Client for data.
…ulicClient` object
tk-o
force-pushed
the
feat/detach-ensapi-from-ensindexer
branch
from
ac27630 to
a8d11e0
Compare
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 6 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.
| const EnsApiVersionSchema = makeEnsApiVersionSchema(); | ||
|
|
||
| const EnsApiConfigSchema = z | ||
| .object({ | ||
| version: EnsApiVersionSchema, | ||
| port: OptionalPortNumberSchema.default(ENSApi_DEFAULT_PORT), | ||
| databaseUrl: DatabaseUrlSchema, | ||
| databaseSchemaName: DatabaseSchemaNameSchema, | ||
| ensIndexerUrl: EnsIndexerUrlSchema, | ||
| theGraphApiKey: TheGraphApiKeySchema, | ||
| namespace: ENSNamespaceSchema, | ||
| rpcConfigs: RpcConfigsSchema, | ||
| ensIndexerPublicConfig: makeENSIndexerPublicConfigSchema("ensIndexerPublicConfig"), | ||
| customReferralProgramEditionConfigSetUrl: CustomReferralProgramEditionConfigSetUrlSchema, | ||
| }) | ||
| .check(invariant_rpcConfigsSpecifiedForRootChain) | ||
| .check(invariant_ensIndexerPublicConfigVersionInfo); | ||
| .check(invariant_rpcConfigsSpecifiedForRootChain); | ||
|
|
There was a problem hiding this comment.
The previous startup-time version compatibility check between ENSApi and the ENSIndexer/ENSDb versions (via invariant_ensIndexerPublicConfigVersionInfo) has been removed, but there doesn’t appear to be an equivalent validation when reading config from ENSDb. This means ENSApi may now run against incompatible ENSDb/ENSIndexer/ENSRainbow versions without failing fast. Consider reintroducing an explicit version-mismatch guard in PublicConfigBuilder.getPublicConfig() (or as a Zod .check in validateEnsApiPublicConfig) to preserve the earlier safety behavior.
| @@ -35,8 +36,9 @@ export function indexingContextToSubgraphMeta( | |||
| case ChainIndexingStatusIds.Completed: | |||
| case ChainIndexingStatusIds.Backfill: | |||
| case ChainIndexingStatusIds.Following: { | |||
| const { ensIndexerPublicConfig } = await publicConfigBuilder.getPublicConfig(); | |||
| return { | |||
| deployment: config.ensIndexerPublicConfig.versionInfo.ensIndexer, | |||
| deployment: ensIndexerPublicConfig.versionInfo.ensIndexer, | |||
There was a problem hiding this comment.
indexingContextToSubgraphMeta determines the ENS root chain ID using config.namespace, while other data in this function (indexing status snapshot + deployment version) is now sourced from ENSDb / ENSIndexer public config. If config.namespace and ensIndexerPublicConfig.namespace ever diverge, _meta will be computed against the wrong root chain (likely returning null). Consider deriving the namespace from publicConfigBuilder.getPublicConfig().ensIndexerPublicConfig.namespace (or asserting the two match) to keep the computation consistent with ENSDb-sourced state.
| // will automatically 503 if config is not available due to ensIndexerPublicConfigMiddleware | ||
| app.get("/health", async (c) => { | ||
| return c.json({ message: "fallback ok" }); | ||
| try { | ||
| // ENSApi is healthy when its Public Config can be successfully built. | ||
| await publicConfigBuilder.getPublicConfig(); |
There was a problem hiding this comment.
The comment above /health still references ensIndexerPublicConfigMiddleware, but the endpoint now checks health by calling publicConfigBuilder.getPublicConfig(). Updating/removing this comment will avoid confusion during ops/debugging.
| const ensApiPublicConfig = await publicConfigBuilder.getPublicConfig(); | ||
| return c.json(serializeEnsApiPublicConfig(ensApiPublicConfig)); |
There was a problem hiding this comment.
/api/config now awaits publicConfigBuilder.getPublicConfig(), which can throw (e.g., ENSDb not reachable / missing ENSIndexer public config). Right now that exception will bubble to the global onError handler and return a 500. Consider catching expected “not ready/unavailable” failures here and returning a 503 (similar to the /health endpoint) so clients get a more accurate status code.
| const ensApiPublicConfig = await publicConfigBuilder.getPublicConfig(); | |
| return c.json(serializeEnsApiPublicConfig(ensApiPublicConfig)); | |
| try { | |
| const ensApiPublicConfig = await publicConfigBuilder.getPublicConfig(); | |
| return c.json(serializeEnsApiPublicConfig(ensApiPublicConfig)); | |
| } catch (error) { | |
| return c.json( | |
| { | |
| error: "ENS API public config is not available", | |
| }, | |
| 503, | |
| ); | |
| } |
| @@ -20,7 +23,7 @@ export function makeEnsApiPublicConfigSchema(valueLabel?: string) { | |||
| const label = valueLabel ?? "ENSApiPublicConfig"; | |||
|
|
|||
| return z.object({ | |||
| version: z.string().min(1, `${label}.version must be a non-empty string`), | |||
| version: makeEnsApiVersionSchema(), | |||
| theGraphFallback: TheGraphFallbackSchema, | |||
There was a problem hiding this comment.
makeEnsApiVersionSchema() uses a fixed error message, so makeEnsApiPublicConfigSchema(valueLabel) no longer includes the caller-provided valueLabel context for version validation errors (it used to include ${label}.version). Consider accepting a valueLabel/path argument in makeEnsApiVersionSchema (or restoring the ${label}.version message at the call site) so custom labels remain useful and consistent with other schema builders.
| this.immutablePublicConfig = validateEnsApiPublicConfig({ | ||
| version: config.version, | ||
| theGraphFallback: canFallbackToTheGraph({ | ||
| namespace: config.namespace, | ||
| // NOTE: very important here that we replace the actual server-side api key with a placeholder | ||
| // so that it's not sent to clients as part of the `theGraphFallback.url`. The placeholder must | ||
| // pass validation, of course, but the only validation necessary is that it is a string. | ||
| theGraphApiKey: config.theGraphApiKey ? "<API_KEY>" : undefined, | ||
| isSubgraphCompatible: ensIndexerPublicConfig.isSubgraphCompatible, | ||
| }), |
There was a problem hiding this comment.
getPublicConfig() uses config.namespace when building theGraphFallback, even though the authoritative namespace for this ENSApi instance is now coming from ENSDb (ensIndexerPublicConfig.namespace). If these ever diverge due to misconfiguration, the public config could become internally inconsistent. Consider using ensIndexerPublicConfig.namespace here (and/or asserting it matches config.namespace and throwing a clear error if it doesn’t).
Greptile SummaryThis PR architecturally decouples ENSApi from ENSIndexer by replacing the direct ENSIndexer HTTP client dependency with a shared ENSDb (PostgreSQL) client. ENSApi now reads Key changes:
Notable findings:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as HTTP Client
participant ENSApi as ENSApi
participant PCB as PublicConfigBuilder
participant DbClient as EnsDbClient
participant DB as PostgreSQL ENSDb
participant Indexer as ENSIndexer
Note over Indexer,DB: ENSIndexer writes metadata to ENSDb on startup
Indexer->>DB: Store EnsIndexerPublicConfig
Indexer->>DB: Store IndexingStatus
Note over ENSApi,DB: ENSApi startup self health-check
ENSApi->>PCB: getPublicConfig()
PCB->>DbClient: getEnsIndexerPublicConfig()
DbClient->>DB: Query ensnode_metadata by public_config
DB-->>DbClient: EnsIndexerPublicConfig data
DbClient-->>PCB: Deserialized EnsIndexerPublicConfig
PCB->>PCB: validateEnsApiPublicConfig
PCB->>PCB: Assign immutablePublicConfig
PCB-->>ENSApi: EnsApiPublicConfig cached
Note over Client,ENSApi: Subsequent requests use cached config
Client->>ENSApi: GET /api/config
ENSApi->>PCB: getPublicConfig()
PCB-->>ENSApi: EnsApiPublicConfig from cache
ENSApi-->>Client: 200 OK
Note over Client,ENSApi: Indexing status always read from ENSDb
Client->>ENSApi: GET /api/indexing-status
ENSApi->>DbClient: getIndexingStatusSnapshot()
DbClient->>DB: Query ensnode_metadata by indexing_status
DB-->>DbClient: IndexingStatus data
DbClient-->>ENSApi: CrossChainIndexingStatusSnapshot
ENSApi-->>Client: 200 OK
Last reviewed commit: a8d11e0 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -84,7 +85,17 @@ app.get("/openapi.json", (c) => { | |||
|
|
|||
| // will automatically 503 if config is not available due to ensIndexerPublicConfigMiddleware | |||
There was a problem hiding this comment.
Stale comment references a non-existent middleware
The comment // will automatically 503 if config is not available due to ensIndexerPublicConfigMiddleware refers to a middleware that no longer exists in the codebase after this refactor. The 503 is now returned by the try/catch block inside the handler itself, not by a middleware.
| // will automatically 503 if config is not available due to ensIndexerPublicConfigMiddleware | |
| // will automatically 503 if config is not available (see try/catch below) |
Additional Comments (1)
The existing test suite sidesteps this entirely by mocking
This is a pre-existing pattern, but as this file is newly introduced in ENSApi it is worth documenting clearly. |
|
@greptile review |
|
|
||
| return z.object({ | ||
| version: z.string().min(1, `${label}.version must be a non-empty string`), | ||
| version: makeEnsApiVersionSchema(), |
There was a problem hiding this comment.
Version error message no longer contextualizes the valueLabel
makeEnsApiVersionSchema() emits a hard-coded error message ("ENSApi version must be a non-empty string") regardless of which valueLabel is passed to the parent schema builders. Before this PR, makeEnsApiPublicConfigSchema("testConfig") would produce "testConfig.version must be a non-empty string", allowing callers to trace a validation failure back to the exact path.
Now, when deserializeEnsApiPublicConfig(config, "testConfig") fails on the version field, the error message will say "ENSApi version must be a non-empty string" with no mention of the testConfig. prefix, making nested validation errors harder to diagnose.
The regression is masked by the corresponding test change in conversions.test.ts that switches from testing version: "" (which would expose this) to testing ensIndexerPublicConfig: {} instead:
// Before:
version: z.string().min(1, `${label}.version must be a non-empty string`)
// After (no longer label-aware):
version: makeEnsApiVersionSchema()Consider passing label into makeEnsApiVersionSchema:
export const makeEnsApiVersionSchema = (label?: string) =>
z.string().min(1, `${label ? label + "." : ""}version must be a non-empty string`);And update the callers:
return z.object({
version: makeEnsApiVersionSchema(label),
...| //////////////////////////// | ||
| /// Temporary ENSv2 Bailout | ||
| //////////////////////////// | ||
| // TODO: re-enable acceleration for ensv2 once implemented |
There was a problem hiding this comment.
Unhandled getPublicConfig() throws yield 500 instead of 503
When publicConfigBuilder.getPublicConfig() throws (e.g., ENSIndexer hasn't written its config to ENSDb yet during startup, or the DB connection is temporarily unavailable), the error propagates uncaught through the middleware chain and reaches Hono's onError handler, which returns 500 Internal Server Error.
Contrast this with the /health endpoint which wraps the same call in try/catch and returns a proper 503 Service Unavailable. API consumers would benefit from the same informative status code in regular request paths.
The same pattern applies to all other middlewares that were updated in this PR:
apps/ensapi/src/middleware/name-tokens.middleware.tsapps/ensapi/src/middleware/registrar-actions.middleware.tsapps/ensapi/src/middleware/require-core-plugin.middleware.tsapps/ensapi/src/middleware/thegraph-fallback.middleware.tsapps/ensapi/src/lib/subgraph/indexing-status-to-subgraph-meta.tsapps/ensapi/src/lib/resolution/forward-resolution.ts
Consider adding a centralized guard, or at minimum wrapping getPublicConfig() in each middleware with a try/catch that maps the error to a 503.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
PublicConfigBuilderclass to cover all abstractions required for buildingEnsApiPublicConfigobject within ENSIndexer runtime.buildConfigFromEnvironmentfunction had on ENSIndexer Client. This dependency has been replaced with ENSDb Client call fromPublicConfigBuilder.getPublicConfig()method leve.EnsDbWriterWorkerto build ENSIndexer Public Config object withPublicConfigBuilder, instead of reading the config fromEnsIndexerClient.EnsIndexerPublicConfigobject withconfig.ensIndexerPublicConfigreference has been updated. Now, the ENSApi modules can getEnsIndexerPublicConfigobject withconst { ensIndexerPublicConfig } = await publicConfigBuilder.getPublicConfig();call.Why
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)
Resolves #1328