Skip to content
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions apps/ensapi/src/lib/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ import config from "@/config";

import * as schema from "@ensnode/ensnode-schema";

import { makeDrizzle } from "@/lib/handlers/drizzle";
import { makeReadOnlyDrizzle } from "@/lib/handlers/drizzle";

export const db = makeDrizzle({
/**
* Read-only Drizzle instance for ENSDb queries to ENSIndexer Schema
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Read-only Drizzle instance for ENSDb queries to ENSIndexer Schema
* Read-only Drizzle instance for ENSApi's queries to the ENSIndexer Schema in ENSDb

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we think about which db schemas ENSApi reads from? My understanding is it should read from both the ENSNode and ENSIndexer schemas. But here it's only reading from ENSIndexer. Why is that?

Assuming ENSApi really does read from multiple schemas than that would suggest it's wrong to call this variable something generic such as db because this db is only for the ENSIndexer schema specifically.

Please invest more effort into naming and communicating ideas.

*/
export const db = makeReadOnlyDrizzle({
databaseUrl: config.databaseUrl,
Copy link
Copy Markdown
Member

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 comment about renaming things in a follow-up issue.

I'd also like to see databaseUrl be renamed to ensDbUrl. This idea should be applied everywhere across our monorepo.

Eager to make all our terminology use be 100% precise, consistent, and aligned everywhere.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

databaseSchema: config.databaseSchemaName,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a follow-up issue for us to rename all the environment variable names / other variable names to be more precise and consistent in how they relate to our improved architecture.

For example: We should stop using the generic databaseSchema terminology. This begs the question: "database schema for what exactly???". The solution is we should use more precise terminology such as ensindexerSchema or ENSINDEXER_SCHEMA.

Suggest to create a follow-up issue for this and action it in a separate PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema,
Expand Down
11 changes: 8 additions & 3 deletions apps/ensapi/src/lib/handlers/drizzle.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { setDatabaseSchema } from "@ponder/client";
import { drizzle } from "drizzle-orm/node-postgres";
import { parseIntoClientConfig } from "pg-connection-string";

Comment thread
tk-o marked this conversation as resolved.
import { makeLogger } from "@/lib/logger";

Expand All @@ -8,9 +9,9 @@ type Schema = { [name: string]: unknown };
const logger = makeLogger("drizzle");

/**
* Makes a Drizzle DB object.
* Makes a read-only Drizzle DB object.
*/
export const makeDrizzle = <SCHEMA extends Schema>({
export const makeReadOnlyDrizzle = <SCHEMA extends Schema>({
schema,
databaseUrl,
databaseSchema,
Expand All @@ -22,7 +23,11 @@ export const makeDrizzle = <SCHEMA extends Schema>({
// monkeypatch schema onto tables
setDatabaseSchema(schema, databaseSchema);

return drizzle(databaseUrl, {
return drizzle({
connection: {
...parseIntoClientConfig(databaseUrl),
options: "-c default_transaction_read_only=on",
Comment thread
tk-o marked this conversation as resolved.
Outdated
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
tk-o marked this conversation as resolved.
Comment thread
tk-o marked this conversation as resolved.
schema,
casing: "snake_case",
logger: {
Expand Down
11 changes: 7 additions & 4 deletions apps/ensindexer/src/lib/ensdb-client/ensdb-client.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { beforeEach, describe, expect, it, vi } from "vitest";

import { ensNodeMetadata } from "@ensnode/ensnode-schema";
import * as ensNodeSchema from "@ensnode/ensnode-schema/ensnode";
import {
deserializeCrossChainIndexingStatusSnapshot,
EnsNodeMetadataKeys,
Expand Down Expand Up @@ -59,7 +59,7 @@ describe("EnsDbClient", () => {
await expect(client.getEnsDbVersion()).resolves.toBeUndefined();

expect(selectMock).toHaveBeenCalledTimes(1);
expect(fromMock).toHaveBeenCalledWith(ensNodeMetadata);
expect(fromMock).toHaveBeenCalledWith(ensNodeSchema.ensNodeMetadata);
});

it("returns value when one record exists", async () => {
Expand Down Expand Up @@ -150,13 +150,14 @@ describe("EnsDbClient", () => {
await client.upsertEnsDbVersion("0.2.0");

// assert
expect(insertMock).toHaveBeenCalledWith(ensNodeMetadata);
expect(insertMock).toHaveBeenCalledWith(ensNodeSchema.ensNodeMetadata);
expect(valuesMock).toHaveBeenCalledWith({
ensIndexerRef: ensDbClientMock.databaseSchemaName,
key: EnsNodeMetadataKeys.EnsDbVersion,
value: "0.2.0",
});
expect(onConflictDoUpdateMock).toHaveBeenCalledWith({
target: ensNodeMetadata.key,
target: [ensNodeSchema.ensNodeMetadata.ensIndexerRef, ensNodeSchema.ensNodeMetadata.key],
set: { value: "0.2.0" },
});
});
Expand All @@ -176,6 +177,7 @@ describe("EnsDbClient", () => {

// assert
expect(valuesMock).toHaveBeenCalledWith({
ensIndexerRef: ensDbClientMock.databaseSchemaName,
key: EnsNodeMetadataKeys.EnsIndexerPublicConfig,
value: expectedValue,
});
Expand All @@ -199,6 +201,7 @@ describe("EnsDbClient", () => {

// assert
expect(valuesMock).toHaveBeenCalledWith({
ensIndexerRef: ensDbClientMock.databaseSchemaName,
key: EnsNodeMetadataKeys.EnsIndexerIndexingStatus,
value: expectedValue,
});
Expand Down
44 changes: 24 additions & 20 deletions apps/ensindexer/src/lib/ensdb-client/ensdb-client.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { NodePgDatabase } from "drizzle-orm/node-postgres";
import { eq, sql } from "drizzle-orm/sql";
import { and, eq, sql } from "drizzle-orm/sql";
Comment thread
tk-o marked this conversation as resolved.
Dismissed
Comment thread
tk-o marked this conversation as resolved.
Comment thread
tk-o marked this conversation as resolved.

import { ensNodeMetadata } from "@ensnode/ensnode-schema";
import * as ensNodeSchema from "@ensnode/ensnode-schema/ensnode";
import {
type CrossChainIndexingStatusSnapshot,
deserializeCrossChainIndexingStatusSnapshot,
Expand All @@ -20,21 +20,12 @@ import {

import { makeDrizzle } from "./drizzle";

/**
* ENSDb Client Schema
*
* Includes schema definitions for {@link EnsDbClient} queries and mutations.
*/
const schema = {
ensNodeMetadata,
};

/**
* Drizzle database
*
* Allows interacting with Postgres database for ENSDb, using Drizzle ORM.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it confusing exactly what this class has responsibility for.

Is it for everything in an ENSDb, or is it only for the ENSNode schema in an ENSDb?

Please make all the naming and use of language 100% precise and clear.

*/
interface DrizzleDb extends NodePgDatabase<typeof schema> {}
interface DrizzleDb extends NodePgDatabase<typeof ensNodeSchema> {}

/**
* ENSDb Client
Expand All @@ -53,16 +44,23 @@ export class EnsDbClient implements EnsDbClientQuery, EnsDbClientMutation {
*/
private db: DrizzleDb;

/**
* ENSIndexer reference string for multi-tenancy in ENSDb.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be more precise. Multi-tenancy of what exactly?

Always write for an assumed audience that isn't familiar with all the context yet.

*/
private ensIndexerRef: string;

/**
* @param databaseUrl connection string for ENSDb Postgres database
* @param databaseSchemaName Postgres schema name for ENSDb tables
* @param ensIndexerRef reference string for ENSIndexer instance (used for multi-tenancy in ENSDb)
*/
constructor(databaseUrl: string, databaseSchemaName: string) {
constructor(databaseUrl: string, ensIndexerRef: string) {
this.db = makeDrizzle({
databaseSchema: databaseSchemaName,
databaseSchema: ensNodeSchema.ENSNODE_SCHEMA_NAME,
Comment thread
tk-o marked this conversation as resolved.
Outdated
databaseUrl,
schema,
schema: ensNodeSchema,
});
Comment thread
tk-o marked this conversation as resolved.
Comment on lines +56 to 60
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Ship the ensnode schema/PK migration before rolling this out.

This client now hard-depends on the ensnode schema and on ON CONFLICT (ensIndexerRef, key), but the PR notes say migration management for that schema is deferred to a follow-up PR and local testing required manual schema creation. On any existing ENSDb that has not been migrated yet, the first read/write here will fail at runtime. Please land the schema + composite-PK migration in the same release, or explicitly block deployment of this client until the database is migrated.

Also applies to: 182-190

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/ensindexer/src/lib/ensdb-client/ensdb-client.ts` around lines 56 - 60,
The code in the constructor uses makeDrizzle with ensNodeSchema and the runtime
depends on the composite primary key (ON CONFLICT (ensIndexerRef, key)), so
ensure the database migration that creates the ensnode table and adds the
composite PK is shipped together with this client; add or reference the
migration that creates the ensnode schema and the composite primary key
(ensIndexerRef,key) before enabling/rolling out this code, or add an explicit
startup check in the constructor (or in the init path used by constructor) that
verifies the ensnode table and composite index exist and fails fast with a clear
error if not, preventing deployment against unmigrated ENSDb (also apply the
same fix to the other usages referenced around the code block at symbols
handling reads/writes in lines ~182-190).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the plan, PRs #1753 and #1754 will address missing pieces.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tk-o, understood — thanks for the context! Tracking the follow-up in #1753 and #1754 sounds like a solid plan. I'll keep an eye on those PRs to confirm the ensnode schema creation and the composite PK migration land before this client is deployed against production ENSDb instances.


🧠 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: 1715
File: apps/ensindexer/ponder/src/api/handlers/ensnode-api.ts:21-25
Timestamp: 2026-03-04T11:40:35.846Z
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.

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: 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).

Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T05:28:59.644Z
Learning: Applies to **/*.ts : Use Drizzle as the ORM for database interactions


this.ensIndexerRef = ensIndexerRef;
Comment thread
tk-o marked this conversation as resolved.
Comment thread
vercel[bot] marked this conversation as resolved.
}

/**
Expand Down Expand Up @@ -154,8 +152,13 @@ export class EnsDbClient implements EnsDbClientQuery, EnsDbClientMutation {
): Promise<EnsNodeMetadataType["value"] | undefined> {
const result = await this.db
.select()
.from(ensNodeMetadata)
.where(eq(ensNodeMetadata.key, metadata.key));
.from(ensNodeSchema.ensNodeMetadata)
.where(
Comment thread
tk-o marked this conversation as resolved.
and(
eq(ensNodeSchema.ensNodeMetadata.ensIndexerRef, this.ensIndexerRef),
eq(ensNodeSchema.ensNodeMetadata.key, metadata.key),
),
);

if (result.length === 0) {
return undefined;
Expand Down Expand Up @@ -186,13 +189,14 @@ export class EnsDbClient implements EnsDbClientQuery, EnsDbClientMutation {
);

await tx
.insert(ensNodeMetadata)
.insert(ensNodeSchema.ensNodeMetadata)
.values({
ensIndexerRef: this.ensIndexerRef,
key: metadata.key,
value: metadata.value,
})
.onConflictDoUpdate({
target: ensNodeMetadata.key,
target: [ensNodeSchema.ensNodeMetadata.ensIndexerRef, ensNodeSchema.ensNodeMetadata.key],
set: { value: metadata.value },
});
});
Expand Down
44 changes: 35 additions & 9 deletions packages/ensnode-schema/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,34 +16,60 @@
"Ponder"
],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the description for this package several lines above.

"exports": {
".": "./src/ponder.schema.ts"
".": "./src/index.ts",
"./ponder": "./src/ponder-schema/index.ts",
"./ensindexer": "./src/ensindexer-schema/index.ts",
"./ensnode": "./src/ensnode-schema/index.ts"
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
"files": [
"dist"
],
"publishConfig": {
"access": "public",
"exports": {
"types": "./dist/ponder.schema.d.ts",
"default": "./dist/ponder.schema.js"
},
"main": "./dist/ponder.schema.js",
"module": "./dist/ponder.schema.mjs",
"types": "./dist/ponder.schema.d.ts"
".": {
"import": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
},
"./ponder": {
"import": {
"types": "./dist/ponder-schema.d.ts",
"default": "./dist/ponder-schema.js"
}
},
"./ensindexer": {
"import": {
"types": "./dist/ensindexer-schema.d.ts",
"default": "./dist/ensindexer-schema.js"
}
},
"./ensnode": {
"import": {
"types": "./dist/ensnode-schema.d.ts",
"default": "./dist/ensnode-schema.js"
Comment thread
tk-o marked this conversation as resolved.
Outdated
}
}
}
},
"scripts": {
"prepublish": "tsup",
"lint": "biome check --write .",
"lint:ci": "biome ci"
},
"dependencies": {
"peerDependencies": {
"drizzle-orm": "catalog:",
"ponder": "catalog:",
"viem": "catalog:"
},
"devDependencies": {
"@ensnode/ensnode-sdk": "workspace:",
"@ensnode/shared-configs": "workspace:*",
"drizzle-orm": "catalog:",
"ponder": "catalog:",
"tsup": "catalog:",
"typescript": "catalog:"
"typescript": "catalog:",
"viem": "catalog:"
}
}
28 changes: 28 additions & 0 deletions packages/ensnode-schema/src/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# ENSDb Schemas
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest we rename this package to ensdb-sdk. Open to creating a separate follow-up issue for this.

It's strange to have "schema" in the package name and then nested within it more "...schema".

Additionally I foresee us wanting to move a number of utility functions into this package in the future for operations on ENSDb.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each schema, please also document all helpful ideas about the writers and readers.


Package defining database schemas used in ENSDb.

## Ponder Schema
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be documenting the idea here how this always has a fixed name.


- Owned by Ponder app.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Owned by Ponder app.
- Owned by the Ponder package running inside ENSIndexer.

- Shared across ENSIndexer instances.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Shared across ENSIndexer instances.
- Shared across all ENSIndexer instances that connect to the same ENSDb.

- Includes responses to RPC requests made by cached public clients.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Includes responses to RPC requests made by cached public clients.
- Accelerates reindexing by storing cached responses to RPC requests made by ENSIndexer.

- Usually large in size, depending on selected ENS Namespace.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider more concise wording.

The phrase "large in size" could be simplified to "large" for better readability.

✍️ Proposed style improvement
-- Usually large in size, depending on selected ENS Namespace.
+- Usually large, depending on selected ENS Namespace.
📝 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
- Usually large in size, depending on selected ENS Namespace.
- Usually large, depending on selected ENS Namespace.
🧰 Tools
🪛 LanguageTool

[style] ~10-~10: This wording could be more concise.
Context: ...ade by cached public clients. - Usually large in size, depending on selected ENS Namespace. -...

(ADJECTIVE_IN_ATTRIBUTE)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ensnode-schema/src/README.md` at line 10, Replace the phrase
"Usually large in size, depending on selected ENS Namespace." with a more
concise wording such as "Usually large, depending on selected ENS Namespace." —
update the README sentence to remove the redundant "in size" for improved
readability.

- Takes a long time to build.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this idea is being communicated doesn't sound right.

- Backups highly recommended for sharing RPC cache across different ENSNode environments.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Backups highly recommended for sharing RPC cache across different ENSNode environments.
- Portable, in that a Ponder Schema can be copied from one ENSDb instance to another to provide an already-warm RPC cache to a new ENSNode deployment.

- For example, pulling production backup into local environment in order to test production workflows in isolation.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider more concise wording.

The phrase "in order to test" could be simplified to "to test" for better readability.

✍️ Proposed style improvement
-  - For example, pulling production backup into local environment in order to test production workflows in isolation.
+  - For example, pulling production backup into local environment to test production workflows in isolation.
📝 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
- For example, pulling production backup into local environment in order to test production workflows in isolation.
- For example, pulling production backup into local environment to test production workflows in isolation.
🧰 Tools
🪛 LanguageTool

[style] ~13-~13: Consider a more concise word here.
Context: ...roduction backup into local environment in order to test production workflows in isolation....

(IN_ORDER_TO_PREMIUM)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ensnode-schema/src/README.md` at line 13, The sentence "For example,
pulling production backup into local environment in order to test production
workflows in isolation." uses the verbose phrase "in order to test"; update that
sentence to use the concise wording "to test" (i.e., change "in order to test"
-> "to test") so it reads "For example, pulling production backup into local
environment to test production workflows in isolation." Ensure only that phrase
is changed and preserve the rest of the sentence.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- For example, pulling production backup into local environment in order to test production workflows in isolation.
- For example, copying an already-warm Ponder Schema into a developer's local environment accelerate local development work.


## ENSIndexer Schema
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be documenting the idea here how the ENSIndexer schema does NOT have a fixed name. Instead, each ENSIndexer instance must configure its own ENSIndexer schema name.


- Owned by an ENSIndexer instance and also influenced by Ponder implementation details, as ENSIndexer is a Ponder app.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Clarify what "influenced by Ponder implementation details" means.

The phrase "influenced by Ponder implementation details" is vague. Consider being more specific about what aspects are influenced (e.g., schema structure, table naming conventions, indexing metadata).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ensnode-schema/src/README.md` at line 17, Replace the vague phrase
"influenced by Ponder implementation details" with a concise list of the
specific aspects Ponder affects (for example: "schema structure, table naming
conventions, indexing metadata, relation mappings, and migration behavior") and
update the sentence mentioning ENSIndexer so it reads something like: "Owned by
an ENSIndexer instance and influenced by Ponder implementation details (schema
structure, table naming conventions, indexing metadata, relation mappings,
migration behavior)." Refer to the term ENSIndexer and Ponder in the README to
ensure clarity.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should explain here more specifically how it is influenced by Ponder implementation details.

- Isolated for specific ENSIndexer instance.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Isolated for specific ENSIndexer instance.
- Each ENSIndexer instance requires its own distinct ENSIndexer schema.

- Includes indexed data based on event handlers logic from active ENSIndexer plugins.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Includes indexed data based on event handlers logic from active ENSIndexer plugins.
- Stores indexed data based on indexing event handlers in active ENSIndexer plugins.

- May be large in size, depending on selected ENS Namespace and active plugins.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider more concise wording.

The phrase "large in size" could be simplified to "large" for consistency with the suggested improvements in other sections.

✍️ Proposed style improvement
-- May be large in size, depending on selected ENS Namespace and active plugins.
+- May be large, depending on selected ENS Namespace and active plugins.
📝 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
- May be large in size, depending on selected ENS Namespace and active plugins.
- May be large, depending on selected ENS Namespace and active plugins.
🧰 Tools
🪛 LanguageTool

[style] ~20-~20: To form a complete sentence, be sure to include a subject.
Context: ...logic from active ENSIndexer plugins. - May be large in size, depending on selected...

(MISSING_IT_THERE)


[style] ~20-~20: This wording could be more concise.
Context: ...rom active ENSIndexer plugins. - May be large in size, depending on selected ENS Namespace an...

(ADJECTIVE_IN_ATTRIBUTE)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ensnode-schema/src/README.md` at line 20, Replace the wording "May
be large in size, depending on selected ENS Namespace and active plugins." in
the README with the more concise "May be large, depending on selected ENS
Namespace and active plugins." to match the simpler phrasing used elsewhere and
improve consistency.

- May take a long time to build. Must be re-built from scratch in case of indexing logic changes (i.e. event handler code change, active plugins change).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Describe this idea better with more precision and excellence.

  • Describe the concept of a Ponder Build Id.
  • Explain how certain ENSIndexer environment variables may also change the Ponder Build Id.
  • Instead of saying framing this in terms of "must be re-built from scratch.." explain how Ponder enforces that the Ponder Build Id stored in an ENSIndexer schema must match the Ponder Build Id calculated by Ponder at runtime when an ENSIndexer instance starts up. Then link to other related docs that we have and that Ponder has in relation to these topics.


Comment on lines +20 to +22
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding backup recommendation for consistency.

The Ponder Schema section recommends backups (line 12), but ENSIndexer Schema doesn't mention backups despite also being large and time-consuming to build. Consider adding backup guidance, especially since line 21 notes it "must be re-built from scratch" on logic changes—backups before such changes could be valuable.

🧰 Tools
🪛 LanguageTool

[style] ~20-~20: To form a complete sentence, be sure to include a subject.
Context: ...logic from active ENSIndexer plugins. - May be large in size, depending on selected...

(MISSING_IT_THERE)


[style] ~20-~20: This wording could be more concise.
Context: ...rom active ENSIndexer plugins. - May be large in size, depending on selected ENS Namespace an...

(ADJECTIVE_IN_ATTRIBUTE)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ensnode-schema/src/README.md` around lines 20 - 22, The README's
"ENSIndexer Schema" section currently describes size and rebuild cost but omits
backup guidance; update the ENSIndexer Schema section to mirror the Ponder
Schema backup recommendation by adding a short backup/restore recommendation
(e.g., take periodic snapshots or export indexes before changing indexing logic
or plugins) and explicitly advise creating backups prior to rebuild-triggering
changes such as event handler or plugin modifications so users can restore
instead of rebuilding from scratch.

## ENSNode Schema
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be documenting the idea here how this always has a fixed name.


- Owned by ENSNode services.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be more clear: Exactly what does "own" mean here?

- Includes metadata describing configuration and state of various ENSNode services.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be more precise in your writing.

You should communicate in terms of an "ENSNode instance".

Make it more clear how everything relates. You should assume that your audience doesn't understand all these things yet. Your writing should make the answer clear.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lightwalker-eth What is "ENSNode instance"? It feels too generic.

In my mind, it's clearer to tell about "ENSNode services" than "ENSNode instance". ENSNode is an abstract term refering to a suite of services.

- Tiny in size.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider more concise wording.

The phrase "Tiny in size" could be simplified to "Tiny" for consistency with the suggested improvements in other sections.

✍️ Proposed style improvement
-- Tiny in size.
+- Tiny.
📝 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
- Tiny in size.
- Tiny.
🧰 Tools
🪛 LanguageTool

[style] ~27-~27: This wording could be more concise.
Context: ...nd state of various ENSNode services. - Tiny in size. - Takes virtually no time to be built....

(ADJECTIVE_IN_ATTRIBUTE)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ensnode-schema/src/README.md` at line 27, Replace the phrase "Tiny
in size." in the README with the more concise "Tiny" to match wording
consistency across sections; locate the text "Tiny in size." and update it to
"Tiny" so phrasing aligns with other short descriptors.

- Takes virtually no time to be built.
9 changes: 9 additions & 0 deletions packages/ensnode-schema/src/ensindexer-schema/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Merge the various sub-schemas into ENSIndexer Schema.
*/

export * from "./ensv2.subschema";
export * from "./protocol-acceleration.subschema";
export * from "./registrars.subschema";
export * from "./subgraph.subschema";
export * from "./tokenscope.subschema";
66 changes: 66 additions & 0 deletions packages/ensnode-schema/src/ensnode-schema/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* ENSNode Schema
*
* Defines the database objects describing the ENSNode services state.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Defines the database objects describing the ENSNode services state.
* Defines the database objects that hold the state of services in an ENSNode instance.

*/

import { primaryKey } from "drizzle-orm/pg-core";

import { ENSNODE_SCHEMA } from "./schema";

export { ENSNODE_SCHEMA_NAME } from "./schema";

/**
* ENSNode Metadata
*
* Possible key value pairs are defined by 'EnsNodeMetadata' type:
* - `EnsNodeMetadataEnsDbVersion`
* - `EnsNodeMetadataEnsIndexerPublicConfig`
* - `EnsNodeMetadataEnsIndexerIndexingStatus`
*/
export const ensNodeMetadata = ENSNODE_SCHEMA.table(
"ensnode_metadata",
(t) => ({
/**
* ENSIndexer Reference
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this introducing another terminology? That sounds unnecessary and creates space for confusion.

Why not call this the "ensIndexerSchemaName"?

Why not reuse our existing terminology?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lightwalker-eth Thanks for providing alternative option, this really makes feedback item actionable.

I appreciate that you saw many opportunities for naming improvements, while sharing less clear feedback, for example, "bad naming". It's great for me to know what requires improvements, but it's even better to learn examples of acceptable altenatives.

*
* References the ENSIndexer instance by a unique ENSIndexer schema name
* that a metadata record is associated with. This allows us to support
* multiple ENSIndexer instances using the same database, while ensuring
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* multiple ENSIndexer instances using the same database, while ensuring
* multiple ENSIndexer instances using the same ENSDb, while ensuring

* that their metadata records do not conflict with each other.
*/
ensIndexerRef: t.text().notNull(),

/**
* Key
*
* Allowed keys:
* - `EnsNodeMetadataEnsDbVersion['key']`
* - `EnsNodeMetadataEnsIndexerPublicConfig['key']`
* - `EnsNodeMetadataEnsIndexerIndexingStatus['key']`
*/
key: t.text().notNull(),

/**
* Value
*
* Allowed values:
* - `EnsNodeMetadataEnsDbVersion['value']`
* - `EnsNodeMetadataEnsIndexerPublicConfig['value']`
* - `EnsNodeMetadataEnsIndexerIndexingStatus['value']`
*
* Guaranteed to be a serialized representation of JSON object.
*/
value: t.jsonb().notNull(),
}),
(table) => [
/**
* Primary key constraint on 'ensIndexerRef' and 'key' columns,
* to ensure that there is only one record for each key per ENSIndexer instance.
*/
primaryKey({
name: "ensnode_metadata_pkey",
columns: [table.ensIndexerRef, table.key],
}),
],
);
5 changes: 5 additions & 0 deletions packages/ensnode-schema/src/ensnode-schema/schema.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { pgSchema } from "drizzle-orm/pg-core";

export const ENSNODE_SCHEMA_NAME = "ensnode";

export const ENSNODE_SCHEMA = pgSchema(ENSNODE_SCHEMA_NAME);
2 changes: 2 additions & 0 deletions packages/ensnode-schema/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Re-export relevant schema definitions for backward compatibility.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which backwards compatibility are we working to preserve? My understanding is no one is using ensnode-schema outside our monorepo.

export * from "./ensindexer-schema";
8 changes: 8 additions & 0 deletions packages/ensnode-schema/src/ponder-schema/index.ts
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is modified in a later commit to only include reference to Ponder code repository. Feel free to skip reviewing it.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* Ponder Schema
*
* Definition of the Ponder Schema can be found in the Ponder repository.
* @see https://github.com/ponder-sh/ponder/blob/ponder%400.16.3/packages/core/src/sync-store/schema.ts
*/

export const PONDER_SCHEMA_NAME = "ponder_sync";
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping this one to signal the name of Ponder Schema in database.

10 changes: 0 additions & 10 deletions packages/ensnode-schema/src/ponder.schema.ts

This file was deleted.

Loading
Loading