ENSv2: Index ENSv1 Manager and Surface via API#1733
Conversation
…ailable, indicating the owner of the Domain's node within the ENSv1 Registry contract.
🦋 Changeset detectedLatest commit: b3d2bfe 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a new nullable field Changes
Sequence Diagram(s)sequenceDiagram
participant Event as ENS Registry Event
participant Indexer as ENS Indexer (ENSv1Registry handler)
participant DB as Database (v1Domain.row)
participant Materializer as Domain Materializer
participant API as GraphQL API
Event->>Indexer: NewOwner / Transfer (node, owner)
Indexer->>DB: upsert v1Domain (id, parentId, labelHash)
Indexer->>DB: update rootRegistryOwnerId = owner (zeroAddress -> null)
Indexer->>Materializer: materialize domain owner
Materializer->>DB: update owner-related fields
API->>DB: query ENSv1Domain { rootRegistryOwner }
DB-->>API: return rootRegistryOwnerId -> resolve Account
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Adds first-class tracking of the ENSv1 Registry “manager” (the Registry owner() for a node) to the ENSv2 data model and exposes it via the GraphQL API, while correcting indexing behavior to not delete domains when Registry ownership is transferred to the zero address.
Changes:
- Added nullable
managerIdtov1Domainplus a Drizzlemanagerrelation toAccount. - Updated ENSv1 Registry event handlers to persist
managerIdand to stop deletingv1Domainrows on zero-address transfers. - Exposed
manageronENSv1Domainin the GraphQL schema and updated the SDK example query to include it.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/ensnode-sdk/src/graphql-api/example-queries.ts | Extends the “DomainByName” example query to request ENSv1Domain.manager. |
| packages/ensnode-schema/src/schemas/ensv2.schema.ts | Adds v1Domain.managerId and relations_v1Domain.manager -> account. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.ts | Persists managerId and stops deleting v1 domains on zero-address transfer; always re-materializes effective owner. |
| apps/ensapi/src/graphql-api/schema/domain.ts | Adds ENSv1Domain.manager: Account field resolved from managerId. |
💡 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ensnode-schema/src/schemas/ensv2.schema.ts (1)
182-186: 🧹 Nitpick | 🔵 TrivialConsider adding an index on
managerIdfor query performance.If the API will support filtering or ordering domains by manager, an index similar to
byOwnermay be beneficial:byParent: index().on(t.parentId), byOwner: index().on(t.ownerId), byLabelHash: index().on(t.labelHash), + byManager: index().on(t.managerId),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-schema/src/schemas/ensv2.schema.ts` around lines 182 - 186, Add an index for managerId to improve queries that filter/order by manager: inside the schema index block (the arrow function that returns byParent/byOwner/byLabelHash) add a new entry like byManager that uses index().on(t.managerId) so the schema includes a `byManager` index for the managerId field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/ensnode-schema/src/schemas/ensv2.schema.ts`:
- Around line 182-186: Add an index for managerId to improve queries that
filter/order by manager: inside the schema index block (the arrow function that
returns byParent/byOwner/byLabelHash) add a new entry like byManager that uses
index().on(t.managerId) so the schema includes a `byManager` index for the
managerId field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fcd2a6f7-2954-4f2e-b860-e92b950ea4f1
📒 Files selected for processing (4)
apps/ensapi/src/graphql-api/schema/domain.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.tspackages/ensnode-schema/src/schemas/ensv2.schema.tspackages/ensnode-sdk/src/graphql-api/example-queries.ts
Greptile SummaryThis PR adds first-class tracking of the ENSv1 Registry Key changes:
Confidence Score: 4/5
Sequence DiagramsequenceDiagram
participant ENSv1Registry as ENSv1 Registry Contract
participant Handler as ENSv1Registry Handler
participant DB as Ponder DB (v1_domains)
participant AccountDB as Ponder DB (accounts)
Note over ENSv1Registry,AccountDB: NewOwner Event (subdomain creation / re-assignment)
ENSv1Registry->>Handler: NewOwner(node, label, owner)
Handler-->>Handler: early-return if owner == zeroAddress
Handler->>DB: INSERT v1Domain (id, parentId, labelHash) ON CONFLICT DO NOTHING
Handler->>DB: UPDATE v1Domain SET managerId = interpretAddress(owner)
Handler->>AccountDB: ensureAccount(owner)
Handler->>DB: UPDATE v1Domain SET ownerId = interpretAddress(owner)
Note over ENSv1Registry,AccountDB: Transfer Event (ownership change, incl. zero address)
ENSv1Registry->>Handler: Transfer(node, owner)
Handler-->>Handler: early-return if node == ROOT_NODE
Handler->>DB: UPDATE v1Domain SET managerId = interpretAddress(owner)
Note right of DB: null if owner == zeroAddress
Handler->>AccountDB: ensureAccount(owner) — no-op for zeroAddress
Handler->>DB: UPDATE v1Domain SET ownerId = interpretAddress(owner)
Note right of DB: Domain persists (no longer deleted on zero transfer)
|
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Updates here look good to me 👍
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.ts (1)
82-93:⚠️ Potential issue | 🟠 MajorForeign key constraint may be violated:
ensureAccountshould be called before settingrootRegistryOwnerId.The
rootRegistryOwnerIdcolumn referencesaccount.id(per the relation inensv2.schema.ts). At lines 85 and 113,rootRegistryOwnerIdis set tointerpretAddress(owner), butensureAccountis only called later insidematerializeENSv1DomainEffectiveOwner.In
handleNewOwner, the early return at line 52 (if owner is zeroAddress) guarantees thatowneris non-zero when reaching line 85, sointerpretAddress(owner)will be a non-null address. If the account doesn't exist yet in the database, this will cause a foreign key violation.The same issue applies to
handleTransferat line 113, though the impact is less severe ifownercan be zeroAddress (which nullifies the FK constraint).🔧 Proposed fix: ensure account exists before setting rootRegistryOwnerId
Import
ensureAccountand call it before the update:+import { ensureAccount } from "@/lib/ensv2/account-db-helpers"; ... // upsert domain await context.db .insert(schema.v1Domain) .values({ id: domainId, parentId, labelHash }) .onConflictDoNothing(); + // ensure account exists before setting rootRegistryOwnerId + await ensureAccount(context, owner); + // update rootRegistryOwner await context.db .update(schema.v1Domain, { id: domainId }) .set({ rootRegistryOwnerId: interpretAddress(owner) }); // materialize domain owner ... await materializeENSv1DomainEffectiveOwner(context, domainId, owner);Apply the same fix to
handleTransfer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.ts` around lines 82 - 93, The update to rootRegistryOwnerId must ensure the referenced account exists first: in handleNewOwner (and likewise in handleTransfer) call ensureAccount(context, interpretAddress(owner)) before updating schema.v1Domain with rootRegistryOwnerId, then perform the await context.db.update(...).set({ rootRegistryOwnerId: interpretAddress(owner) });; you can still call materializeENSv1DomainEffectiveOwner afterwards, but ensureAccount is invoked prior to setting rootRegistryOwnerId to avoid foreign key violations (refer to ensureAccount, interpretAddress, handleNewOwner, handleTransfer, and materializeENSv1DomainEffectiveOwner).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.ts`:
- Around line 110-121: The update to rootRegistryOwnerId is happening before the
account is guaranteed to exist, which can violate the FK when
interpretAddress(owner) is non-null; move or change the logic so that
materializeENSv1DomainEffectiveOwner (which calls ensureAccount) runs before
updating rootRegistryOwnerId, or explicitly call ensureAccount for the owner
address prior to the context.db.update; reference the update call that sets
rootRegistryOwnerId and the function materializeENSv1DomainEffectiveOwner (and
ensureAccount) to locate where to reorder or add the ensureAccount call, and
preserve the current handling where interpretAddress(owner) may return null for
the zero address.
---
Outside diff comments:
In `@apps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.ts`:
- Around line 82-93: The update to rootRegistryOwnerId must ensure the
referenced account exists first: in handleNewOwner (and likewise in
handleTransfer) call ensureAccount(context, interpretAddress(owner)) before
updating schema.v1Domain with rootRegistryOwnerId, then perform the await
context.db.update(...).set({ rootRegistryOwnerId: interpretAddress(owner) });;
you can still call materializeENSv1DomainEffectiveOwner afterwards, but
ensureAccount is invoked prior to setting rootRegistryOwnerId to avoid foreign
key violations (refer to ensureAccount, interpretAddress, handleNewOwner,
handleTransfer, and materializeENSv1DomainEffectiveOwner).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 57d5638e-e95a-42df-8867-56442bce5950
📒 Files selected for processing (5)
.changeset/strong-baboons-help.mdapps/ensapi/src/graphql-api/schema/domain.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/ENSv1Registry.tspackages/ensnode-schema/src/schemas/ensv2.schema.tspackages/ensnode-sdk/src/graphql-api/example-queries.ts
closes #1731
Reviewer Focus (Read This First)
handleTransferinENSv1Registry.ts— previously deleted domains on transfer to zero address, which was not technically correct, since they do still existProblem & Motivation
owner()in the ENSv1 Registry), which was not being tracked as a distinct fieldhandleTransferimplementation deleted domains when transferred to zero address, which is incorrect — transferring registry ownership to zero doesn't mean the domain ceases to existWhat Changed (Concrete)
rootRegistryOwnerIdcolumn tov1Domainschema (ensv2.schema.ts)rootRegistryOwnerdrizzle relation onv1Domainpointing toAccounthandleNewOwnernow setsrootRegistryOwnerIdafter upserting the domainhandleTransferno longer deletes domains on zero-address transfer — instead updatesrootRegistryOwnerId(withinterpretAddressto normalize zero to null) and always materializes the effective ownerrootRegistryOwnerfield onENSv1DomainRefin the graphql api (domain.ts)rootRegistryOwnerinline fragment to example query inensnode-sdk_owner/ownerreassignment pattern inhandleTransferDesign & Planning
straightforward addition of a tracked field that was previously implicit in registry events
no upfront design doc needed — small, self-contained change
Planning artifacts: none
Reviewed / approved by: n/a
Self-Review
handleTransferinto a single update + materialize path_ownerrename pattern; removed the stub (second commit)Cross-Codebase Alignment
rootRegistryOwnerId,v1Domain,handleTransfer,handleNewOwnermaterializeENSv1DomainEffectiveOwner— confirmed it handles null correctlyDownstream & Consumer Impact
ENSv1Domainnow exposes a nullablerootRegistryOwnerfield via graphqlensnode-sdkrootRegistryOwneraligns with ENS terminology — the registryowner()is the "rootRegistryOwner" of a domain, distinct from the effective ownerTesting Evidence
Scope Reductions
Risk Analysis
Pre-Review Checklist (Blocking)