update ensv2 indexing for latest devnet: reservations, event renames, resolver.dedicated removal#1845
update ensv2 indexing for latest devnet: reservations, event renames, resolver.dedicated removal#1845
Conversation
…or integration testing
…d` field in advance of PermissionedResolver integration.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: a9c73b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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 |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRemoves the Changes
Sequence DiagramsequenceDiagram
participant Blockchain
participant Handler
participant DB
participant GraphQL
participant Client
Blockchain->>Handler: LabelRegistered / LabelReserved event
Handler->>Handler: Validate label / canonicalId
alt owner present
Handler->>DB: Insert registration (type=ENSv2RegistryRegistration)
Handler->>DB: Upsert v2Domain owner/update, emit domain event
else owner absent
Handler->>DB: Insert registration (type=ENSv2RegistryReservation)
end
Blockchain->>Handler: LabelUnregistered event
Handler->>DB: Set registration.unregistrantId and expiry, emit domain event
Client->>GraphQL: Query registration
GraphQL->>DB: Fetch registration by id
alt type == ENSv2RegistryRegistration
GraphQL-->>Client: Return Registration shape
else type == ENSv2RegistryReservation
GraphQL-->>Client: Return Reservation shape
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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
Updates ENSv2 devnet indexing + schema/API to match the latest contracts, including the new reservation/unregistration event model, plus refactors the integration test environment to run Postgres+devnet via the root docker-compose.
Changes:
- Refactors ENSv2Registry event handling to unify LabelRegistered/LabelReserved logic and adds LabelUnregistered handling.
- Splits
RegistrationTypefor ENSv2 intoENSv2RegistryRegistrationvsENSv2RegistryReservation, updating indexer + GraphQL API types accordingly. - Updates devnet ABIs/addresses and moves integration-test-env orchestration to
DockerComposeEnvironment; removesResolver.dedicated/ dedicated resolver helpers.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Removes @testcontainers/postgresql, adds shared-configs workspace dep for integration-test-env. |
| packages/integration-test-env/tsconfig.json | Aligns integration-test-env TS config with shared monorepo config. |
| packages/integration-test-env/src/orchestrator.ts | Starts Postgres+devnet via docker-compose/testcontainers; keeps remaining services running from source. |
| packages/integration-test-env/package.json | Adds shared-configs dependency and a typecheck script. |
| packages/integration-test-env/README.md | Updates devnet pin/reference to docker-compose as source of truth. |
| packages/ensnode-sdk/src/rpc/is-dedicated-resolver.ts | Removes dedicated resolver interface helper. |
| packages/ensnode-sdk/src/rpc/index.ts | Stops exporting isDedicatedResolver. |
| packages/ensnode-sdk/src/graphql-api/example-queries.ts | Updates devnet test user address. |
| packages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.ts | Splits ENSv2 registry registration enum into Registration vs Reservation. |
| packages/datasources/src/ens-test-env.ts | Updates devnet contract addresses for latest deployment. |
| packages/datasources/src/abis/ensv2/UniversalResolverV2.ts | Updates UniversalResolverV2 ABI to latest devnet version. |
| packages/datasources/src/abis/ensv2/Registry.ts | Updates Registry ABI (event renames + new events). |
| packages/datasources/src/abis/ensv2/EnhancedAccessControl.ts | Updates EnhancedAccessControl ABI to latest devnet version. |
| packages/datasources/src/abis/ensv2/ETHRegistrar.ts | Updates ETHRegistrar ABI to latest devnet version. |
| package.json | Adds devnet helper script for docker compose. |
| docker-compose.yml | Adds devnet service + healthcheck; adjusts env wiring for schema names. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.ts | Updates invariants to the new ENSv2 registration type value. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.ts | Introduces unified registration/reservation handler + LabelUnregistered expiry behavior. |
| apps/ensindexer/src/lib/ensv2/registry-db-helpers.ts | Extracts ensureRegistry helper for registry upserts. |
| apps/ensapi/src/graphql-api/schema/resolver.ts | Removes Resolver.dedicated and dedicated resolver metadata types. |
| apps/ensapi/src/graphql-api/schema/registration.ts | Adds ENSv2RegistryReservation GraphQL object type + discriminator updates. |
| apps/ensapi/src/graphql-api/schema/account.integration.test.ts | Updates devnet owner address used by integration test. |
| AGENTS.md | Updates guidance to prefer workspace typecheck scripts. |
| .changeset/tricky-clouds-dress.md | Changeset documenting breaking removal of Resolver.dedicated. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/ensv2/ENSv2Registry.ts`:
- Around line 337-340: The TODO notes a temporary invariant that early-returns
non-Registry ERC1155 transfers; create a formal issue/ticket to track removing
this check and link it from the code by replacing the TODO with the issue ID (or
a brief "tracked in ISSUE-<id>") so future work isn't lost. Update the comment
near makeRegistryId / registryId / context.ensDb.find /
ensIndexerSchema.registry to reference that issue and include expected follow-up
steps (remove the guard and rely on only indexing Registry contracts), then push
that comment change with the issue created in your project tracker.
In `@docker-compose.yml`:
- Line 10: Update the example env so it matches the new schema name: in
apps/ensapi/.env.local.example replace the value currently set to "ensindexer_0"
(the entry around line 16) with "a_unique_schema_name" so it matches
DATABASE_SCHEMA in docker-compose.yml; also scan apps/ensapi/.env.local.example
for any other occurrences of "ensindexer_0" and update them to
"a_unique_schema_name" to keep examples consistent.
In `@packages/integration-test-env/src/orchestrator.ts`:
- Around line 241-250: The orchestrator is referencing auto-generated container
names ("postgres-1", "devnet-1") while docker-compose.yml sets explicit
container_name (e.g. "postgres"), causing getContainer to fail; update the
DockerComposeEnvironment usage to match the explicit names: change the
withWaitStrategy calls that reference "devnet-1" and "postgres-1" to "devnet"
and "postgres", change composeEnvironment.getContainer("postgres-1") to
composeEnvironment.getContainer("postgres"), and ensure any other references
(mapped port, DATABASE_URL construction) use the same explicit container/service
names so names align with docker-compose.yml.
- Line 56: The constant ENSINDEXER_SCHEMA_NAME in orchestrator.ts is set to
"ensindexer_0" but Docker Compose uses "a_unique_schema_name"; change the
ENSINDEXER_SCHEMA_NAME value to "a_unique_schema_name" so the spawned ensindexer
and ensapi processes use the same Postgres schema as the docker-compose service
(look for the ENSINDEXER_SCHEMA_NAME constant in orchestrator.ts and update its
string literal accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0bfcf860-2819-4647-ad6a-9050247a3540
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.changeset/tricky-clouds-dress.mdAGENTS.mdapps/ensapi/src/graphql-api/schema/account.integration.test.tsapps/ensapi/src/graphql-api/schema/registration.tsapps/ensapi/src/graphql-api/schema/resolver.tsapps/ensindexer/src/lib/ensv2/registry-db-helpers.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.tsdocker-compose.ymlpackage.jsonpackages/datasources/src/abis/ensv2/ETHRegistrar.tspackages/datasources/src/abis/ensv2/EnhancedAccessControl.tspackages/datasources/src/abis/ensv2/Registry.tspackages/datasources/src/abis/ensv2/UniversalResolverV2.tspackages/datasources/src/ens-test-env.tspackages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.tspackages/ensnode-sdk/src/graphql-api/example-queries.tspackages/ensnode-sdk/src/rpc/index.tspackages/ensnode-sdk/src/rpc/is-dedicated-resolver.tspackages/integration-test-env/README.mdpackages/integration-test-env/package.jsonpackages/integration-test-env/src/orchestrator.tspackages/integration-test-env/tsconfig.json
💤 Files with no reviewable changes (2)
- packages/ensnode-sdk/src/rpc/is-dedicated-resolver.ts
- packages/ensnode-sdk/src/rpc/index.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 106-120: Add an explicit container_name for the devnet service so
it matches the other services; inside the devnet service block (the service
named "devnet") add the key container_name: devnet at the same indentation level
as image/command/ports, ensuring no duplicate container name conflicts with
other services and updating any external references if necessary.
- Around line 114-115: The healthcheck for the devnet service is incorrect (it
probes http://localhost:8000) and will always fail because the image
ghcr.io/ensdomains/contracts-v2 (Anvil node) exposes a JSON-RPC on port 8545;
either remove the healthcheck block for the devnet service entirely, or change
the healthcheck test to target the JSON-RPC port (e.g., use healthcheck/test
pointing to http://localhost:8545 or a TCP check against port 8545) so the check
actually verifies the Anvil JSON-RPC is responsive.
In `@packages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.ts`:
- Around line 356-364: The schema adds an unregistrantId field but
registration_relations lacks a matching relation; update the
registration_relations definition to include an unregistrant relation mirroring
the existing registrant relation (use unregistrantId as the foreign key, point
to the same target entity/type used by registrant, and name the relation
"unregistrant") so relation-based queries can resolve unregistrant links (refer
to registration_relations, registrantId, and unregistrantId to implement).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0df35386-e07c-498c-95c1-820908d9d85b
📒 Files selected for processing (5)
apps/ensapi/src/graphql-api/schema/registration.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.tsdocker-compose.ymlpackages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.tspackages/integration-test-env/src/orchestrator.ts
Greptile SummaryThis PR updates ENSv2 indexing to align with the latest devnet contract changes: event renames ( Key changes:
Confidence Score: 5/5Safe to merge — all changes are scoped to devnet; production ENSv1 indexing is unaffected. All findings are P2 style or documentation issues. The core indexer logic (unified handler, invariant guards, schema split, API removal) is well-structured and internally consistent. No P0/P1 issues were found. The four P2 observations are: a variable name typo in a test, stale error message text in ETHRegistrar, an unconfirmed assumption about TransferSingle clearing ownerId on unregistration, and a removed build-command doc line. None of these block merge. ENSv2Registry.ts (LabelUnregistered ownerId gap) and ETHRegistrar.ts (stale error messages) are worth a second glance, but neither is blocking. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ENSv2 Registry Contract Event] --> B{Event Type}
B -->|LabelRegistered| C[handleRegistrationOrReservation
owner defined → isReservation=false]
B -->|LabelReserved| D[handleRegistrationOrReservation
owner undefined → isReservation=true]
B -->|LabelUnregistered| E[LabelUnregistered Handler]
B -->|ExpiryUpdated| F[ExpiryUpdated Handler]
C --> G{Existing Registration?}
D --> G
G -->|None or Expired| H[Upsert v2Domain
Insert Registration
type=ENSv2RegistryRegistration or ENSv2RegistryReservation]
G -->|Unexpired + not a Reservation| I[Invariant Error]
E --> J{Registration Exists
and Not Expired?}
J -->|Yes| K[Set expiry=block.timestamp
Set unregistrantId]
J -->|No| L[Invariant Error]
H --> M[ensureDomainEvent]
K --> M
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docker-compose.yml (1)
106-121:⚠️ Potential issue | 🟠 MajorHealthcheck targets incorrect port - devnet (Anvil) does not expose
/healthon port 8000.The
ghcr.io/ensdomains/contracts-v2image runs an Anvil blockchain node that exposes JSON-RPC on port 8545, not an HTTP health endpoint on port 8000. This healthcheck will fail indefinitely, causingWait.forHealthCheck()in the orchestrator to hang.Replace with a check against the JSON-RPC endpoint:
🔧 Proposed fix
healthcheck: - test: ["CMD", "curl", "--fail", "-s", "http://localhost:8000/health"] + test: ["CMD", "curl", "--fail", "-s", "-X", "POST", "-H", "Content-Type: application/json", "-d", "{\"jsonrpc\":\"2.0\",\"method\":\"eth_blockNumber\",\"params\":[],\"id\":1}", "http://localhost:8545"] interval: 10s timeout: 5s retries: 5 start_period: 30s start_interval: 1sAlternatively, if
curlis not available in the container, use a TCP check orwget:test: ["CMD-SHELL", "wget -q --spider http://localhost:8545 || exit 1"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 106 - 121, The devnet service healthcheck is targeting port 8000 which Anvil does not expose; update the devnet healthcheck (service name "devnet", healthcheck block) to probe the JSON-RPC endpoint on port 8545 instead (e.g., curl or wget against http://localhost:8545) or switch to a TCP check against port 8545 if curl/wget may be missing; ensure the test array and command form (e.g., CMD or CMD-SHELL) is adjusted accordingly so Wait.forHealthCheck() can succeed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/integration-test-env/README.md`:
- Around line 36-121: The README's "Start Postgres" subsection has a missing
blank line immediately before the fenced code block that contains the shell
command "docker compose up postgres"; add a single blank line between the
explanatory paragraph ("or with the local docker compose:") and the subsequent
```sh fenced code block so the Markdown renders correctly; locate the "Start
Postgres" heading and the code block with "docker compose up postgres" to make
this change.
---
Duplicate comments:
In `@docker-compose.yml`:
- Around line 106-121: The devnet service healthcheck is targeting port 8000
which Anvil does not expose; update the devnet healthcheck (service name
"devnet", healthcheck block) to probe the JSON-RPC endpoint on port 8545 instead
(e.g., curl or wget against http://localhost:8545) or switch to a TCP check
against port 8545 if curl/wget may be missing; ensure the test array and command
form (e.g., CMD or CMD-SHELL) is adjusted accordingly so Wait.forHealthCheck()
can succeed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1893894-89a5-4800-b73c-87e84d3a69e3
📒 Files selected for processing (4)
apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.tsdocker-compose.ymlpackages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.tspackages/integration-test-env/README.md
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Great work 🙌 Shared a few small suggestions. Please take the lead to merge when ready 👍
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (composeEnvironment) { | ||
| try { | ||
| await container.stop(); | ||
| await composeEnvironment.down(); |
There was a problem hiding this comment.
composeEnvironment.down() will stop containers but (like docker compose down) typically keeps named volumes (e.g. postgres_data in the root compose file). That can leak DB state between integration test runs (schemas/data persist), which may cause flakiness locally and unbounded disk growth. Consider removing volumes on teardown (if the testcontainers API supports it) or switching Postgres to an anonymous/ephemeral volume for orchestrated runs.
| await composeEnvironment.down(); | |
| await composeEnvironment.down({ removeVolumes: true }); |
| pnpm start | ||
| ``` | ||
|
|
||
| Works both in CI and locally — just make sure the required ports are available (8545, 8000, 3223, 42069, 4334). |
There was a problem hiding this comment.
The docs list required ports as (8545, 8000, 3223, 42069, 4334), but the current docker-compose config also publishes Postgres on 5432, and devnet's 8000 health port isn't published to the host. Please update this list to reflect the actual ports that must be free when running the orchestrator locally.
| Works both in CI and locally — just make sure the required ports are available (8545, 8000, 3223, 42069, 4334). | |
| Works both in CI and locally — just make sure the required ports are available (8545, 5432, 3223, 42069, 4334). |
| with environment variables: | ||
|
|
||
| ```env | ||
| DATABASE_URL=postgresql://ensnode:ensnode@localhost:5432/ensnode |
There was a problem hiding this comment.
The manual ENSApi env example uses DATABASE_URL=postgresql://ensnode:ensnode@localhost:5432/ensnode, which doesn't match the repo's docker-compose Postgres defaults (postgres:password / db postgres) nor the orchestrator's DATABASE_URL. Either update the example to match the compose/orchestrator defaults, or explicitly note that this is only an example and must be aligned with how Postgres is started.
| DATABASE_URL=postgresql://ensnode:ensnode@localhost:5432/ensnode | |
| DATABASE_URL=postgresql://postgres:password@localhost:5432/postgres |
| const DEVNET_DEPLOYER: Address = "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266"; | ||
| const DEFAULT_OWNER: Address = "0x70997970c51812dc3a010c7d01b50e0d17dc79c8"; | ||
| const NEW_OWNER: Address = "0x90f79bf6eb2c4f870365e785982e1f101e93b906"; | ||
| const NEW_OWNER_OWNER: Address = "0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC"; | ||
|
|
||
| describe("Account.domains", () => { |
There was a problem hiding this comment.
The constant name NEW_OWNER_OWNER reads like a typo and makes the test harder to follow. Rename it to something unambiguous (e.g. NEW_OWNER) and update the query variables accordingly.
Reviewer Focus (Read This First)
reviewers should focus on:
Problem & Motivation
What Changed (Concrete)
Design & Planning
Self-Review
Cross-Codebase Alignment
Downstream & Consumer Impact
Testing Evidence
Risk Analysis
Pre-Review Checklist (Blocking)