-
Notifications
You must be signed in to change notification settings - Fork 17
Split ENSApi and ENSIndexer #1716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
b3ac81b
63a6f30
09c7ae4
2abc626
39eb1ec
705ceb8
b0edcb7
e6eedab
6b6f23c
3ad279d
3784c8a
2ab4196
eb329fc
a8d11e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,16 @@ ENSINDEXER_URL=http://localhost:42069 | |
| # NOTE that ENSApi does NOT need to define DATABASE_SCHEMA, as it is inferred from the connected ENSIndexer's Config. | ||
| DATABASE_URL=postgresql://dbuser:abcd1234@localhost:5432/my_database | ||
|
|
||
| # ENSDb: schema name | ||
|
tk-o marked this conversation as resolved.
|
||
| # Required. This is a namespace for the tables that the ENSDbClient for ENSApi will apply to read ENSDb data. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest avoiding the use of the word "namespace" here -- it's overloading terminology too much with ENS namespace. |
||
| # Must be set to an existing namespace in the connected ENSDb at DATABASE_URL. | ||
| DATABASE_SCHEMA=public | ||
|
tk-o marked this conversation as resolved.
Outdated
tk-o marked this conversation as resolved.
Outdated
|
||
|
|
||
| # ENS Namespace Configuration | ||
| # Required. Must be an ENS namespace's Identifier such as mainnet, sepolia, or ens-test-env. | ||
| # (see `@ensnode/datasources` for available options). | ||
| NAMESPACE=mainnet | ||
|
|
||
| # ENSApi: RPC Configuration | ||
| # Required. ENSApi requires an HTTP RPC to the connected ENSIndexer's ENS Root Chain, which depends | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please invest time in carefully crafting a refinement to how this idea is documented. We need our language here to be precisely aligned with the new architecture refinements. |
||
| # on ENSIndexer's NAMESPACE (ex: mainnet, sepolia, ens-test-env). This ENS Root Chain RPC | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,23 @@ | ||
| import packageJson from "@/../package.json" with { type: "json" }; | ||
|
|
||
| import pRetry from "p-retry"; | ||
| import { parse as parseConnectionString } from "pg-connection-string"; | ||
| import { version } from "pino"; | ||
|
github-code-quality[bot] marked this conversation as resolved.
Fixed
tk-o marked this conversation as resolved.
Outdated
|
||
| import { prettifyError, ZodError, z } from "zod/v4"; | ||
|
|
||
| import type { EnsApiPublicConfig } from "@ensnode/ensnode-sdk"; | ||
| import { | ||
| buildRpcConfigsFromEnv, | ||
| canFallbackToTheGraph, | ||
| DatabaseSchemaNameSchema, | ||
| ENSNamespaceSchema, | ||
| EnsIndexerUrlSchema, | ||
| invariant_rpcConfigsSpecifiedForRootChain, | ||
| makeENSIndexerPublicConfigSchema, | ||
| makeEnsApiVersionSchema, | ||
| OptionalPortNumberSchema, | ||
| RpcConfigsSchema, | ||
| TheGraphApiKeySchema, | ||
| } from "@ensnode/ensnode-sdk/internal"; | ||
|
|
||
| import { ENSApi_DEFAULT_PORT } from "@/config/defaults"; | ||
| import type { EnsApiEnvironment } from "@/config/environment"; | ||
| import { invariant_ensIndexerPublicConfigVersionInfo } from "@/config/validations"; | ||
| import { fetchENSIndexerConfig } from "@/lib/fetch-ensindexer-config"; | ||
| import logger from "@/lib/logger"; | ||
|
|
||
| export const DatabaseUrlSchema = z.string().refine( | ||
|
|
@@ -60,20 +56,21 @@ const CustomReferralProgramEditionConfigSetUrlSchema = z | |
| }) | ||
| .optional(); | ||
|
|
||
| const EnsApiVersionSchema = makeEnsApiVersionSchema(); | ||
|
|
||
| const EnsApiConfigSchema = z | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Over the last month we've been solving some details with how the ENSRainbow config logic would work. One of the design choices we made is to try to make it more clear exactly which config is coming from environment variables vs other sources. Please review how configs are named and built in ENSRainbow. Applying some of those ideas here, it would mean we should do things including:
|
||
| .object({ | ||
| version: EnsApiVersionSchema, | ||
| port: OptionalPortNumberSchema.default(ENSApi_DEFAULT_PORT), | ||
| databaseUrl: DatabaseUrlSchema, | ||
| databaseSchemaName: DatabaseSchemaNameSchema, | ||
| ensIndexerUrl: EnsIndexerUrlSchema, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove this now? |
||
| theGraphApiKey: TheGraphApiKeySchema, | ||
| namespace: ENSNamespaceSchema, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will be great if we can remove this and instead load it dynamically from ENSDb.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll look into how we can replace all reads of |
||
| rpcConfigs: RpcConfigsSchema, | ||
| ensIndexerPublicConfig: makeENSIndexerPublicConfigSchema("ensIndexerPublicConfig"), | ||
| customReferralProgramEditionConfigSetUrl: CustomReferralProgramEditionConfigSetUrlSchema, | ||
| }) | ||
| .check(invariant_rpcConfigsSpecifiedForRootChain) | ||
| .check(invariant_ensIndexerPublicConfigVersionInfo); | ||
| .check(invariant_rpcConfigsSpecifiedForRootChain); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I note how invariants like this couldn't be checked on startup just from environment variables if we remove namespace as an environment variable. However we could move this check into the logic that validates if ENSApi supports a given config it loads from ENSDb. |
||
|
|
||
|
Comment on lines
+58
to
73
|
||
| export type EnsApiConfig = z.infer<typeof EnsApiConfigSchema>; | ||
|
|
||
|
|
@@ -83,29 +80,19 @@ export type EnsApiConfig = z.infer<typeof EnsApiConfigSchema>; | |
| * @returns A validated EnsApiConfig object | ||
|
tk-o marked this conversation as resolved.
|
||
| * @throws Error with formatted validation messages if environment parsing fails | ||
| */ | ||
| export async function buildConfigFromEnvironment(env: EnsApiEnvironment): Promise<EnsApiConfig> { | ||
| export function buildConfigFromEnvironment(env: EnsApiEnvironment): EnsApiConfig { | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| try { | ||
| const ensIndexerUrl = EnsIndexerUrlSchema.parse(env.ENSINDEXER_URL); | ||
|
|
||
| const ensIndexerPublicConfig = await pRetry(() => fetchENSIndexerConfig(ensIndexerUrl), { | ||
| retries: 3, | ||
| onFailedAttempt: ({ error, attemptNumber, retriesLeft }) => { | ||
| logger.info( | ||
| `ENSIndexer Config fetch attempt ${attemptNumber} failed (${error.message}). ${retriesLeft} retries left.`, | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| const rpcConfigs = buildRpcConfigsFromEnv(env, ensIndexerPublicConfig.namespace); | ||
| const namespace = ENSNamespaceSchema.parse(env.NAMESPACE); | ||
| const rpcConfigs = buildRpcConfigsFromEnv(env, namespace); | ||
|
tk-o marked this conversation as resolved.
|
||
|
|
||
| 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, | ||
| ensIndexerPublicConfig, | ||
| namespace: ensIndexerPublicConfig.namespace, | ||
| databaseSchemaName: ensIndexerPublicConfig.databaseSchemaName, | ||
| namespace: env.NAMESPACE, | ||
| rpcConfigs, | ||
|
tk-o marked this conversation as resolved.
|
||
| customReferralProgramEditionConfigSetUrl: env.CUSTOM_REFERRAL_PROGRAM_EDITIONS, | ||
| }); | ||
|
|
@@ -121,24 +108,3 @@ export async function buildConfigFromEnvironment(env: EnsApiEnvironment): Promis | |
| process.exit(1); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Builds the ENSApi public configuration from an EnsApiConfig object. | ||
| * | ||
| * @param config - The validated EnsApiConfig object | ||
| * @returns A complete ENSApiPublicConfig object | ||
| */ | ||
| export function buildEnsApiPublicConfig(config: EnsApiConfig): EnsApiPublicConfig { | ||
| return { | ||
| version: packageJson.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, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I missed it but I didn't see where we retained this logic? Please ignore if it's already retained somewhere. |
||
| isSubgraphCompatible: config.ensIndexerPublicConfig.isSubgraphCompatible, | ||
| }), | ||
| ensIndexerPublicConfig: config.ensIndexerPublicConfig, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,8 +15,9 @@ import type { | |
| * their state in `process.env`. This interface is intended to be the source type which then gets | ||
| * mapped/parsed into a structured configuration object like `EnsApiConfig`. | ||
| */ | ||
| export type EnsApiEnvironment = Omit<DatabaseEnvironment, "DATABASE_SCHEMA"> & | ||
| EnsIndexerUrlEnvironment & | ||
| export type EnsApiEnvironment = DatabaseEnvironment & { | ||
| NAMESPACE?: string; | ||
|
vercel[bot] marked this conversation as resolved.
tk-o marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be awesome if we could remove the need for namespace as an env variable and instead make ENSApi fully dynamic based on the config it loads from the database.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will look into achieving that goal 👍 |
||
| } & EnsIndexerUrlEnvironment & | ||
| RpcEnvironment & | ||
| PortEnvironment & | ||
| LogLevelEnvironment & | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,12 +4,12 @@ import { | |||||||||||||||||||||||||||
| EnsApiIndexingStatusResponseCodes, | ||||||||||||||||||||||||||||
| type EnsApiIndexingStatusResponseError, | ||||||||||||||||||||||||||||
| type EnsApiIndexingStatusResponseOk, | ||||||||||||||||||||||||||||
| serializeENSApiPublicConfig, | ||||||||||||||||||||||||||||
| serializeEnsApiIndexingStatusResponse, | ||||||||||||||||||||||||||||
| serializeEnsApiPublicConfig, | ||||||||||||||||||||||||||||
| } from "@ensnode/ensnode-sdk"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import { buildEnsApiPublicConfig } from "@/config/config.schema"; | ||||||||||||||||||||||||||||
| import { createApp } from "@/lib/hono-factory"; | ||||||||||||||||||||||||||||
| import { publicConfigBuilder } from "@/lib/public-config-builder/singleton"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import { getConfigRoute, getIndexingStatusRoute } from "./ensnode-api.routes"; | ||||||||||||||||||||||||||||
| import ensnodeGraphQLApi from "./ensnode-graphql-api"; | ||||||||||||||||||||||||||||
|
|
@@ -20,8 +20,8 @@ import resolutionApi from "./resolution-api"; | |||||||||||||||||||||||||||
| const app = createApp(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| app.openapi(getConfigRoute, async (c) => { | ||||||||||||||||||||||||||||
| const ensApiPublicConfig = buildEnsApiPublicConfig(config); | ||||||||||||||||||||||||||||
| return c.json(serializeENSApiPublicConfig(ensApiPublicConfig)); | ||||||||||||||||||||||||||||
| const ensApiPublicConfig = await publicConfigBuilder.getPublicConfig(); | ||||||||||||||||||||||||||||
| return c.json(serializeEnsApiPublicConfig(ensApiPublicConfig)); | ||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+22
|
||||||||||||||||||||||||||||
| 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, | |
| ); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my other related feedback for moving the public config into middleware so that no async operation like this is needed.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // This file is based on `packages/ponder-subgraph/src/drizzle.ts` file. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please create a follow-up issue for this? For example we need to work towards refactoring all of the ENS Referrals-related APIs out of ENSApi and into a separate distinct app that would also build on top of ENSDb the same way ENSApi does. We want to make it easy for us and others to build apps on top of ENSDb.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logged an issue here: |
||
| // We currently duplicate the makeDrizzle function, as we don't have | ||
| // a shared package for backend code yet. When we do, we can move | ||
| // this function to the shared package and import it in both places. | ||
| import { setDatabaseSchema } from "@ponder/client"; | ||
| import { drizzle } from "drizzle-orm/node-postgres"; | ||
|
|
||
| type Schema = { [name: string]: unknown }; | ||
|
|
||
| /** | ||
| * Makes a Drizzle DB object. | ||
| */ | ||
| export const makeDrizzle = <SCHEMA extends Schema>({ | ||
| schema, | ||
| databaseUrl, | ||
| databaseSchema, | ||
| }: { | ||
| schema: SCHEMA; | ||
| databaseUrl: string; | ||
| databaseSchema: string; | ||
| }) => { | ||
| // monkeypatch schema onto tables | ||
| setDatabaseSchema(schema, databaseSchema); | ||
|
|
||
| return drizzle(databaseUrl, { schema, casing: "snake_case" }); | ||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.