feat: new sepolia-v2 namespace for temp ensv2 deployment #1516
Conversation
🦋 Changeset detectedLatest commit: 4e557d7 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.
|
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Thanks for updates. Seeking your advice, thanks 👍
shrugs
force-pushed
the
feat/ensv2-sepolia
branch
from
42a3165 to
dd271d1
Compare
shrugs
changed the title
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds an experimental "sepolia-v2" ENS namespace and datasource, updates SDK/UI mappings and admin defaults to include SepoliaV2, removes ENSv2 contract entries from existing Sepolia/Mainnet datasources, and refactors API/indexer/plugin logic to use dynamic contract lookups and additional guards for ENSv2 bridging. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as GraphQL API
participant DS as DatasourceLayer
participant Registry as ENSRoot ETHRegistry
participant Namechain as Namechain ETHRegistry
participant Resolver as Resolver/ResolverLogic
Client->>API: getDomainByFqdn(fqdn)
API->>DS: maybeGetDatasourceContract(ENSRoot)
DS-->>API: ENSRoot ETHRegistry (addr/abi) or null
API->>DS: maybeGetDatasourceContract(Namechain)
DS-->>API: Namechain ETHRegistry (addr/abi) or null
alt ENSRoot registry missing and Namechain missing
API-->>Client: return null / early exit
else registries available
API->>Registry: validate leaf.registryId == ENS_ROOT_V2_ETH_REGISTRY_ID
alt leaf registered on ENSRoot
API->>Resolver: v2_getDomainIdByFqdn (validate depth, .eth path, leaf)
Resolver-->>API: domainId or defer to v1
else not leaf on ENSRoot
API->>Namechain: attempt bridging (compute 2LD node, check active v1)
alt active ENSv1 registration exists
API->>API: call v1_getDomainIdByFqdn
API-->>Client: v1 result
else
API->>Namechain: v2_getDomainIdByFqdn (shortened name)
Namechain-->>API: domainId or null
API-->>Client: result or null
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 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 unit tests (beta)
Comment |
…ended for testing of ephemeral ENSv2 deployments to the Sepolia chain. This feature is intended for developers of the ENS protocol, and should be considered highly experimental and unstable.
There was a problem hiding this comment.
Pull request overview
This PR adds a new temporary sepolia-v2 namespace to support ENSv2 test deployments on the Sepolia testnet. This creates a separate namespace from the original Sepolia ENS deployment, allowing both ENSv1 and ENSv2 contracts to coexist for testing purposes.
Changes:
- Created a new
sepolia-v2namespace that extends the base Sepolia namespace with ENSv2 contracts - Removed ENSv2 contract stubs from the original Sepolia and Mainnet namespaces
- Updated all namespace-specific utility functions to handle SepoliaV2 consistently with Sepolia
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/datasources/src/sepolia-v2.ts | New file defining the SepoliaV2 namespace with ENSv2 contracts on ENSRoot and a new Namechain datasource |
| packages/datasources/src/namespaces.ts | Registers the sepolia-v2 namespace in the namespace map |
| packages/datasources/src/lib/types.ts | Adds SepoliaV2 to ENSNamespaceIds enum and removes unused MergeNamespaces type |
| packages/datasources/src/sepolia.ts | Removes ENSv2 contract stubs and commented-out Namechain configuration |
| packages/datasources/src/mainnet.ts | Removes ENSv2 contract stubs and commented-out Namechain configuration |
| packages/namehash-ui/src/utils/ensMetadata.ts | Adds SepoliaV2 case to use Sepolia metadata service URL |
| packages/namehash-ui/src/utils/ensManager.ts | Adds SepoliaV2 case to use Sepolia ENS manager URL |
| packages/ensnode-sdk/src/shared/thegraph.ts | Adds SepoliaV2 case to use Sepolia subgraph URL |
| packages/ensnode-sdk/src/registrars/*.ts | Adds SepoliaV2 cases to return appropriate managed names for subregistries |
| apps/ensindexer/src/plugins/protocol-acceleration/plugin.ts | Adds conditional check for Registry contract existence for backwards compatibility |
| apps/ensindexer/src/plugins/ensv2/plugin.ts | Adds conditional checks for ENSv2 contract existence and refactors reduce logic |
| apps/ensindexer/src/lib/ponder-helpers.ts | Refactors disableCache logic into a variable for better readability |
| apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts | Refactors ENSv2 contract lookups to use maybeGetDatasourceContract for backwards compatibility |
| apps/ensadmin/src/lib/default-records-selection.ts | Adds default records selection for SepoliaV2 namespace |
💡 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: 1
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts`:
- Around line 168-172: The redundant defensive check for namechain after calling
maybeGetDatasourceContract should be resolved: either remove the if (!namechain)
return null; since maybeGetDatasourceContract already handles missing
datasources (locate the call to maybeGetDatasourceContract and the subsequent
namechain usage), or if namechain can carry extra state beyond existence, add a
brief inline comment above the check explaining why it’s needed and what state
it guards, and keep the logger.debug({ name, rows }) as-is; update code around
the namechain variable in get-domain-by-fqdn.ts accordingly.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts`:
- Around line 201-209: The fallback call to v2_getDomainIdByFqdn is not
receiving the caller's custom now value, causing inconsistent expiry checks;
update the call in get-domain-by-fqdn so it forwards the now parameter (e.g.,
v2_getDomainIdByFqdn(NAMECHAIN_V2_ETH_REGISTRY_ID, nameWithoutTld, now)) and
ensure any intermediary helpers (interpretedLabelsToInterpretedName /
interpretedNameToInterpretedLabels usage) remain unchanged; if
v2_getDomainIdByFqdn signature lacks a now param, add it and thread the now
argument through the recursive ENSv2 lookup path.
- Around line 133-199: The early Namechain lookup (maybeGetDatasourceContract ->
NAMECHAIN_V2_ETH_REGISTRY) and its null-return guard should be deferred so ENSv1
fallback can run; remove the early `if (!NAMECHAIN_V2_ETH_REGISTRY) return
null;` and move the Namechain datasource lookup and construction of
NAMECHAIN_V2_ETH_REGISTRY_ID to just before the code that actually uses it
(after the ENSv1 check that calls makeENSv1DomainId, getLatestRegistration,
isRegistrationFullyExpired and v1_getDomainIdByFqdn); ensure the
ENS_ROOT_V2_ETH_REGISTRY lookup and its null guard remain as-is and keep the
label/path invariants unchanged.
In `@package.json`:
- Around line 43-45: Replace the unbounded greater-than ranges in the overrides
for tar and lodash with caret ranges to prevent accidental major upgrades:
update the override entries "tar@<=7.5.3" from ">=7.5.4" to use "^7.5.4" and
update "lodash@<=4.17.22" and "lodash-es@<=4.17.22" from ">=4.17.23" to use
"^4.17.23" so the intent is expressed as compatible-semver rather than an
open-ended >= constraint.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Docstrings generation was requested by @shrugs. * #1516 (comment) The following files were modified: * `apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts` * `apps/ensindexer/src/lib/ponder-helpers.ts` * `packages/ensnode-sdk/src/registrars/basenames-subregistry.ts` * `packages/ensnode-sdk/src/registrars/ethnames-subregistry.ts` * `packages/ensnode-sdk/src/registrars/lineanames-subregistry.ts` * `packages/namehash-ui/src/utils/ensManager.ts` * `packages/namehash-ui/src/utils/ensMetadata.ts`
|
Caution The CodeRabbit agent's plans did not produce any file changes. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/ensindexer/src/plugins/protocol-acceleration/plugin.ts:159
- The code should check if "Registry" exists in namechain.contracts before accessing it, similar to how it's checked for ensroot.contracts on line 148. This would make the code more defensive and consistent with the approach in the ensv2 plugin. Consider adding the check:
...(namechain && "Registry" in namechain.contracts &&
...(namechain &&
chainConfigForContract(
config.globalBlockrange,
namechain.chain.id,
namechain.contracts.Registry,
)),
💡 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}
| import logger from "@/lib/logger"; | ||
|
|
||
| const ETH_LABELHASH = labelhashLiteralLabel("eth" as LiteralLabel); | ||
|
|
There was a problem hiding this comment.
The module initialization will crash when using the old sepolia namespace because it tries to access the RootRegistry contract which no longer exists in that namespace.
View Details
📝 Patch Details
diff --git a/apps/ensapi/src/graphql-api/lib/get-canonical-path.ts b/apps/ensapi/src/graphql-api/lib/get-canonical-path.ts
index 87428b37..254bab84 100644
--- a/apps/ensapi/src/graphql-api/lib/get-canonical-path.ts
+++ b/apps/ensapi/src/graphql-api/lib/get-canonical-path.ts
@@ -6,23 +6,27 @@ import * as schema from "@ensnode/ensnode-schema";
import {
type CanonicalPath,
type DomainId,
- getENSv2RootRegistryId,
+ maybeGetENSv2RootRegistryId,
type RegistryId,
} from "@ensnode/ensnode-sdk";
import { db } from "@/lib/db";
const MAX_DEPTH = 16;
-const ROOT_REGISTRY_ID = getENSv2RootRegistryId(config.namespace);
+const ROOT_REGISTRY_ID = maybeGetENSv2RootRegistryId(config.namespace);
/**
* Provide the canonical parents from the Root Registry to `domainId`.
* i.e. reverse traversal of the namegraph
*
+ * Returns null if the ENSv2 Root Registry is not available in the configured namespace.
+ *
* TODO: this implementation has undefined canonical name behavior, need to updated based on proposed
* reverse mapping
*/
export async function getCanonicalPath(domainId: DomainId): Promise<CanonicalPath | null> {
+ // If v2 Root Registry is not available in this namespace, no canonical path can be computed
+ if (!ROOT_REGISTRY_ID) return null;
const result = await db.execute(sql`
WITH RECURSIVE upward AS (
-- Base case: start from the target domain
diff --git a/apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts b/apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts
index 8db970a0..af9db068 100644
--- a/apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts
+++ b/apps/ensapi/src/graphql-api/lib/get-domain-by-fqdn.ts
@@ -11,6 +11,7 @@ import {
type ENSv2DomainId,
ETH_NODE,
getENSv2RootRegistryId,
+ maybeGetENSv2RootRegistryId,
type InterpretedName,
interpretedLabelsToInterpretedName,
interpretedNameToInterpretedLabels,
@@ -31,7 +32,7 @@ import { db } from "@/lib/db";
import logger from "@/lib/logger";
const ETH_LABELHASH = labelhashLiteralLabel("eth" as LiteralLabel);
-const ROOT_REGISTRY_ID = getENSv2RootRegistryId(config.namespace);
+const ROOT_REGISTRY_ID = maybeGetENSv2RootRegistryId(config.namespace);
/**
* Gets the DomainId of the Domain addressed by `name`.
@@ -42,7 +43,7 @@ export async function getDomainIdByInterpretedName(
// Domains addressable in v2 are preferred, but v1 lookups are cheap, so just do them both ahead of time
const [v1DomainId, v2DomainId] = await Promise.all([
v1_getDomainIdByFqdn(name),
- v2_getDomainIdByFqdn(ROOT_REGISTRY_ID, name),
+ ROOT_REGISTRY_ID ? v2_getDomainIdByFqdn(ROOT_REGISTRY_ID, name) : Promise.resolve(null),
]);
// prefer v2DomainId
diff --git a/packages/ensnode-sdk/src/shared/root-registry.ts b/packages/ensnode-sdk/src/shared/root-registry.ts
index 02cc0243..bab8e0e3 100644
--- a/packages/ensnode-sdk/src/shared/root-registry.ts
+++ b/packages/ensnode-sdk/src/shared/root-registry.ts
@@ -4,6 +4,7 @@ import {
accountIdEqual,
getDatasourceContract,
makeRegistryId,
+ maybeGetDatasourceContract,
} from "@ensnode/ensnode-sdk";
//////////////
@@ -43,3 +44,19 @@ export const getENSv2RootRegistryId = (namespace: ENSNamespaceId) =>
*/
export const isENSv2RootRegistry = (namespace: ENSNamespaceId, contract: AccountId) =>
accountIdEqual(getENSv2RootRegistry(namespace), contract);
+
+/**
+ * Gets the AccountId representing the ENSv2 Root Registry in the selected `namespace`,
+ * or undefined if it is not defined.
+ */
+export const maybeGetENSv2RootRegistry = (namespace: ENSNamespaceId) =>
+ maybeGetDatasourceContract(namespace, DatasourceNames.ENSRoot, "RootRegistry");
+
+/**
+ * Gets the RegistryId representing the ENSv2 Root Registry in the selected `namespace`,
+ * or undefined if it is not defined.
+ */
+export const maybeGetENSv2RootRegistryId = (namespace: ENSNamespaceId) => {
+ const registry = maybeGetENSv2RootRegistry(namespace);
+ return registry ? makeRegistryId(registry) : undefined;
+};
Analysis
Module initialization crashes when using old sepolia namespace due to missing RootRegistry contract
What fails: The GraphQL API fails to initialize when config.namespace = "sepolia" because get-domain-by-fqdn.ts and get-canonical-path.ts try to obtain the ENSv2 RootRegistry contract at module load time via getENSv2RootRegistryId(config.namespace), but the old sepolia namespace no longer has this contract (it was removed as part of the ENSv2 Sepolia namespace refactoring).
How to reproduce:
- Start the ensapi service with
config.namespace = "sepolia"(old namespace) - Send any GraphQL request to
/api/graphql - On the first GraphQL request, the schema initialization occurs, triggering the import chain
Result: The application throws Error: Expected contract not found for sepolia ENSRoot RootRegistry and the GraphQL endpoint becomes unavailable.
Expected behavior: The application should gracefully handle the absence of the RootRegistry contract in the old sepolia namespace by:
- Only attempting v2 domain resolution when the RootRegistry contract is available
- Falling back to v1-only resolution when v2 is not available
- Computing canonical paths only when the RootRegistry is available
Root cause: The modules get-domain-by-fqdn.ts and get-canonical-path.ts called the throwing function getENSv2RootRegistryId() at module load time unconditionally. The new sepolia-v2 namespace includes the RootRegistry contract, but the old sepolia namespace does not. This breaks the stated intent: "Implemented as independent Sepolia namespace to avoid interrupting mainstream usage of the existing Sepolia namespace."
Fix: Added maybeGetENSv2RootRegistryId() function to safely retrieve the RootRegistry contract and made v2-specific logic conditional on its availability. The modules now initialize successfully regardless of whether RootRegistry is available in the configured namespace.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Looks great. Thanks! Shared a few small feedback items. Please take the lead to merge when ready 👍
| return new URL(`https://app.ens.domains/`); | ||
| case ENSNamespaceIds.Sepolia: | ||
| case ENSNamespaceIds.SepoliaV2: | ||
| return new URL(`https://sepolia.app.ens.domains/`); |
There was a problem hiding this comment.
Perhaps for SepoliaV2 better to return null?
| return new URL(name, `https://metadata.ens.domains/mainnet/avatar/`); | ||
| case ENSNamespaceIds.Sepolia: | ||
| case ENSNamespaceIds.SepoliaV2: | ||
| return new URL(name, `https://metadata.ens.domains/sepolia/avatar/`); |
There was a problem hiding this comment.
Perhaps for SepoliaV2 better to return null?
| }, | ||
|
|
||
| // add a namechain | ||
| [DatasourceNames.Namechain]: { |
There was a problem hiding this comment.
Appreciate your advice on what name we give to this namespace. If we call it "Namechain" then it's being named after a specific chain rather than a data source. I think this may be a source of some future confusion or difficulty in our language.
What if we renamed this data source to something like ENSV2Root or something like that?
makes a new temporary namespace for the test deployments of ensv2 contracts on sepolia.
this pr represents the temporary ad hoc deployment of ensv2 to the sepolia chain as an independent sepolia namespace to avoid interrupting mainstream usage of the existing sepolia namespace
confirmed indexed to latest