Fix text record resolution and enhance CCIP-Read handling by Douglasacost · Pull Request #111 · NodleCode/rollup

Douglasacost · 2026-04-13T21:34:22Z

This pull request introduces a new signed-gateway UniversalResolver integration for ENS offchain resolution, adding a /resolve endpoint that supports CCIP-Read (ERC-3668) with EIP-712 signatures, and refactors deployment scripts and configuration to support the new model. The changes include new resolver logic, EIP-712 signing, configuration wiring, and a simplified deployment script.

New ENS Gateway Resolution Functionality:

Added /resolve route that implements the CCIP-Read (ERC-3668) callback endpoint, decoding ENS resolution requests, routing them to the correct L2 NameService, and returning EIP-712 signed responses for use with the L1 UniversalResolver. [1] [2] [3]
Implemented resolveFromL2 utility to parse DNS-encoded ENS names, support addr, addr-multichain, and text lookups, and return ABI-encoded results.
Added signResolutionResponse for EIP-712 signing of resolution results, matching the L1 UniversalResolver contract.

Configuration and Setup Updates:

Extended setup.ts to wire up the new resolver signer, L1 resolver address, and signature TTL, and export them for use in the new route. [1] [2] [3] [4]
Updated the ENS NameService interface to include the getTextRecord function needed by the gateway.

Deployment Script Changes:

Refactored the L1 deployment script to support the new signed-gateway UniversalResolver, removing the StorageProofVerifier/SparseMerkleTree logic and adding support for setting the ENS resolver in a single broadcast if desired. [1] [2] [3]

These changes collectively enable secure, offchain ENS resolution via a gateway that signs responses for L1 verification, modernizing the resolver infrastructure and deployment flow.

Text records returned empty because NAME_SERVICE_INTERFACE was missing the getTextRecord read fragment — ethers treated the method as non-existent and the resolver's catch-all swallowed the error as "no record". CCIP-Read POSTs from the ENS app were also rejected because they send Content-Type: text/plain to skip CORS preflight, which express.json() ignored by default.

…d gateway

…ndow

…olver This commit introduces a comprehensive RFC-style documentation for the Signed-Gateway UniversalResolver, detailing its architecture, interfaces, and EIP-712 payload. The document outlines the resolver's functionality, including its integration with the L2 NameService and the trusted-gateway signature model, replacing the previous zkSync storage proof design.

github-actions · 2026-04-13T21:38:50Z

LCOV of commit `ad92b93` during checks #658

Summary coverage rate:
  lines......: 31.8% (771 of 2426 lines)
  functions..: 27.8% (105 of 378 functions)
  branches...: 37.2% (140 of 376 branches)

Files changed coverage rate:
                                                  |Lines       |Functions  |Branches    
  Filename                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================
  script/DeployL1Ens.s.sol                        | 0.0%     21| 0.0%     1| 0.0%      3
  src/nameservice/UniversalResolver.sol           | 0.0%     79| 0.0%     9| 0.0%     17

Copilot

Pull request overview

This PR migrates ENS offchain resolution to a signed-gateway CCIP-Read (ERC-3668) model: the L1 UniversalResolver reverts with OffchainLookup, the gateway resolves against L2 NameService, then returns an EIP-712 signed payload that the L1 resolver verifies.

Changes:

Replaced storage-proof based verification in UniversalResolver with EIP-712 signature verification and trusted signer rotation.
Added a new gateway /resolve endpoint plus L2 resolution + signing utilities to support addr, addr-multichain, and text.
Added protocol documentation and a Foundry test suite covering core success/failure paths and signer rotation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
`src/nameservice/UniversalResolver.sol`	Switches resolver to CCIP-Read + EIP-712 signed responses; adds trusted signers and URL rotation.
`clk-gateway/src/routes/resolve.ts`	Implements the CCIP-Read callback endpoint (`/resolve`) that decodes `(name,data)`, resolves via L2, and signs responses.
`clk-gateway/src/resolver/resolveFromL2.ts`	Adds L2 resolution dispatcher for `addr`, `addr-multichain`, and `text` and returns ABI-encoded results.
`clk-gateway/src/resolver/signResolution.ts`	Adds EIP-712 signing for `Resolution(name,data,result,expiresAt)` and ABI-encodes the callback response payload.
`clk-gateway/src/setup.ts`	Wires env/config for resolver signer, L1 resolver address, chainId, and signature TTL.
`clk-gateway/src/index.ts`	Extends JSON parsing to accept `text/plain` and registers the new `/resolve` router.
`clk-gateway/src/interfaces.ts`	Extends NameService interface with `getTextRecord`.
`script/DeployL1Ens.s.sol`	Refactors deployment for signed-gateway resolver model and adds optional ENS `setResolver` step.
`test/nameservice/UniversalResolver.t.sol`	Adds Foundry tests for signature verification, TTL bounds, signer rotation, and interface support.
`src/nameservice/doc/signed-resolver-protocol.md`	Adds RFC-style protocol specification for the signed-gateway resolver model.

… panic

aliXsed

Consolidated inline review — 10 items across the changeset, ordered by priority.

aliXsed · 2026-04-17T03:49:39Z

 }

-contract UniversalResolver is IExtendedResolver, IERC165, Ownable {
+contract UniversalResolver is IExtendedResolver, IERC165, Ownable, EIP712 {


[1/10] Create a new contract — don't modify the existing UniversalResolver

The deployed contract's source should be preserved as a historical artifact. The new contract has a fundamentally different verification model (EIP-712 signatures vs. storage proofs). It should live in a separate file/contract. (Reiterating my earlier comment.)

aliXsed · 2026-04-17T03:49:40Z

 pragma solidity ^0.8.26;

 import {IERC165} from "lib/forge-std/src/interfaces/IERC165.sol";
 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";


[2/10] Ownable → Ownable2Step

Per the project's own standards: "When a contract requires an owner, prefer Ownable2Step over Ownable." This contract requires ownership for setUrl and setTrustedSigner. A single-step transferOwnership to a wrong address is irrecoverable and bricks signer management.

aliXsed · 2026-04-17T03:49:40Z

+    /// @notice Update the CCIP-Read gateway URL.
    function setUrl(string memory _url) external onlyOwner {
+        emit UrlUpdated(url, _url);
        url = _url;


[3/10] setUrl missing empty-URL validation

The constructor validates bytes(_url).length == 0 → revert EmptyUrl(), but setUrl does not. The owner can accidentally brick all subdomain resolution by setting an empty URL.

(Reiterating my earlier inline suggestion — add CEI ordering + empty check.)

Suggested change

url = _url;

function setUrl(string memory _url) external onlyOwner {

bytes memory b = bytes(_url);

if (b.length == 0) revert EmptyUrl();

string memory oldUrl = url;

url = _url;

emit UrlUpdated(oldUrl, url);

}

aliXsed · 2026-04-17T03:49:40Z

+    /// @notice Enable or disable a trusted gateway signer.
+    /// @dev Keeps `trustedSignerCount` in sync and enforces a floor of 1 so the
+    ///      owner cannot brick resolution by disabling the last signer.
+    function setTrustedSigner(address signer, bool trusted) external onlyOwner {


[4/10] Split setTrustedSigner into trustSigner / revokeSigner

setTrustedSigner(addr, false) is semantically ambiguous — "set trusted signer" that actually removes trust. Two distinct functions simplify access-control auditing and event decoding. (Reiterating my earlier comment.)

aliXsed · 2026-04-17T03:49:40Z

        }

+        // Dispatch only on supported selectors so the gateway is never asked for nonsense.
        bytes4 functionSelector = bytes4(_data[:4]);


[5/10] Add _data.length >= 4 check in resolve()

Short calldata causes a panic (out-of-bounds) rather than a clean CallDataTooShort revert. The custom error CallDataTooShort already exists but the guard is missing before this slice. (Copilot flagged this — verify it's addressed.)

Suggested change

bytes4 functionSelector = bytes4(_data[:4]);

if (_data.length < 4) revert CallDataTooShort();

bytes4 functionSelector = bytes4(_data[:4]);

aliXsed · 2026-04-17T03:49:40Z

+            if (functionSelector == _TEXT_SELECTOR) {
+                return abi.encode("");
+            }
            if (functionSelector == _ADDR_MULTICHAIN_SELECTOR) {


[6/10] Fix addr(bytes32,uint256) bare-domain return type

Bare-domain handler returns abi.encode(address(0)) for multichain, but ENSIP-11 defines addr(bytes32,uint256) as returning bytes. Should return abi.encode(bytes("")) for the multichain case. (Copilot flagged this — verify it's addressed.)

aliXsed · 2026-04-17T03:49:40Z

+  return parsed;
+}
+
+const resolutionSignatureTtlSeconds = parseResolutionSignatureTtl(


[7/10] Convert parseResolutionSignatureTtl to IIFE for consistency

Every other derived config in this file is an inline const expression. The 23-line named function stands out. The validation is necessary (prevents emitting signatures guaranteed to revert on-chain), but converting to an IIFE would match the file's style:

const resolutionSignatureTtlSeconds = (() => { const raw = process.env.RESOLUTION_SIGNATURE_TTL_SECONDS; if (raw === undefined || raw === "") return 60; const parsed = Number(raw); if (!Number.isFinite(parsed) || !Number.isInteger(parsed)) throw new Error(`Invalid RESOLUTION_SIGNATURE_TTL_SECONDS: "${raw}" is not a finite integer`); if (parsed <= 0) throw new Error(`RESOLUTION_SIGNATURE_TTL_SECONDS must be > 0, got ${parsed}`); if (parsed > MAX_RESOLUTION_SIGNATURE_TTL_SECONDS) throw new Error(`RESOLUTION_SIGNATURE_TTL_SECONDS must be <= ${MAX_RESOLUTION_SIGNATURE_TTL_SECONDS}, got ${parsed}`); return parsed; })();

aliXsed · 2026-04-17T03:49:40Z

@@ -0,0 +1,163 @@
+import { AbiCoder, dataSlice, getAddress, hexlify, isAddress, isHexString } from "ethers"


[8/10] Unused hexlify import

hexlify is imported here but never used anywhere in this file. Remove it.

aliXsed · 2026-04-17T03:49:40Z

+      decodedData = d
+    }
+
+    const parsed = parseDnsDomain(Buffer.from(decodedName.slice(2), "hex"))


[9/10] Consider validating parsed.tld against parentTLD

The gateway uses parsed.sub and parsed.domain for routing but never validates parsed.tld. A sanity check against the configured parentTLD would reject obviously malformed names early. Low priority since the L1 resolver already constrains which names reach the gateway.

aliXsed · 2026-04-17T03:49:40Z

+        assertTrue(resolver.isTrustedSigner(signer));
+        assertFalse(resolver.isTrustedSigner(backupSigner));
+    }
+}


[10/10] Missing fuzz tests for TTL boundaries

Per project standards (testFuzz_ required for arithmetic/functional logic): TTL boundary checks and DNS parsing have no fuzz coverage. At minimum testFuzz_ResolveWithSig_ExpiresAt for various timestamp values would strengthen confidence in the TTL/expiry arithmetic.

aliXsed · 2026-04-17T04:04:17Z

+    /// @param _data ABI-encoded ENS resolution call (addr / addr-multichain / text)
    function resolve(bytes calldata _name, bytes calldata _data) external view returns (bytes memory) {
-        (string memory sub, string memory dom,) = _parseDnsDomain(_name);
+        (string memory sub,,) = _parseDnsDomain(_name);


[Addendum] dom is discarded — consider using it for validation

resolve() extracts (sub,,) and discards dom entirely. The contract never validates that the domain portion of the DNS name matches its intended domain.

Impact: If the ENS registry mistakenly points a different domain at this resolver, it would happily trigger OffchainLookup for that unrelated domain with no guard. The gateway compensates (it routes by parsed.domain), but the contract itself is blind to it.

Suggested fix: At minimum, pass dom through to the gateway in the callData/extraData so it can be validated end-to-end. Better yet, store the expected domain in the contract and reject mismatches on-chain — the gas cost is trivial for a view function.

// Instead of: (string memory sub,,) = _parseDnsDomain(_name); // Consider: (string memory sub, string memory dom,) = _parseDnsDomain(_name); // ... use dom for validation or pass it through

aliXsed · 2026-04-17T04:19:35Z

Tracking issue for evaluating a trustless alternative to the signed-gateway interim solution: NodleCode/meta#169 — Evaluate Unruggable Gateways ZkSync support as replacement for trusted signed-gateway.

The signed-gateway model in this PR is standards-compliant (ERC-3668, ENSIP-10) and matches what Coinbase/Uniswap/ENS reference use in production. However, Unruggable Gateways already has ZkSync support (ZKSyncRollup + ZKSyncProver + ZKSyncVerifierHooks) and could potentially restore the trustless verification model we had before the ZkSync proof format break.

Douglasacost added 6 commits April 13, 2026 15:32

feat(resolver): replace storage-proof verification with EIP-712 signe…

9bca5e9

…d gateway

fix(resolver): block renounceOwnership to prevent bricking admin setters

019599c

feat(resolver): cap max signature TTL at 5 minutes to bound replay wi…

9723b9e

…ndow

feat(gateway): EIP-712 signed CCIP-Read endpoint for UniversalResolver

0dbeba5

Douglasacost requested a review from Copilot April 13, 2026 21:34

Copilot started reviewing on behalf of Douglasacost April 13, 2026 21:34 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Douglasacost added 10 commits April 13, 2026 17:41

chore(cspell): allow typehash and hexlify as domain terms

b1fe5ee

fix(resolver): encode addr-multichain as ENSIP-11 bytes, not address

1243a8c

fix(resolver): reject short calldata with CallDataTooShort instead of…

8de71d2

… panic

refactor(resolver): use OwnershipCannotBeRenounced custom error

1e654e9

fix(gateway): validate RESOLUTION_SIGNATURE_TTL_SECONDS at startup

eadbca2

fix(resolver): validate signer inputs and enforce trusted-signer floor

cffeb52

fix(gateway): validate sender matches configured L1 resolver address

8b2a8e1

test(resolver): cover multichain resolveWithSig empty-record case

f156a16

test(resolver): avoid cspell false positive in short-calldata test

b5ad8a3

chore(cspell): allow repoint, repointed, cutover

ad92b93

Douglasacost requested a review from aliXsed April 15, 2026 21:20

This comment was marked as outdated.

Sign in to view

aliXsed reviewed Apr 17, 2026

View reviewed changes

-        url = _url;
+    function setUrl(string memory _url) external onlyOwner {
+        bytes memory b = bytes(_url);
+        if (b.length == 0) revert EmptyUrl();
+        string memory oldUrl = url;
+        url = _url;
+        emit UrlUpdated(oldUrl, url);
+    }

	bytes4 functionSelector = bytes4(_data[:4]);
	if (_data.length < 4) revert CallDataTooShort();
	bytes4 functionSelector = bytes4(_data[:4]);

		@@ -0,0 +1,163 @@
		import { AbiCoder, dataSlice, getAddress, hexlify, isAddress, isHexString } from "ethers"

Conversation

Douglasacost commented Apr 13, 2026

Uh oh!

github-actions bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LCOV of commit ad92b93 during checks #658

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

aliXsed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

[1/10] Create a new contract — don't modify the existing UniversalResolver

Uh oh!

Choose a reason for hiding this comment

[2/10] Ownable → Ownable2Step

Uh oh!

Choose a reason for hiding this comment

[3/10] setUrl missing empty-URL validation

Uh oh!

Choose a reason for hiding this comment

[4/10] Split setTrustedSigner into trustSigner / revokeSigner

Uh oh!

Choose a reason for hiding this comment

[5/10] Add _data.length >= 4 check in resolve()

Uh oh!

Choose a reason for hiding this comment

[6/10] Fix addr(bytes32,uint256) bare-domain return type

Uh oh!

Choose a reason for hiding this comment

[7/10] Convert parseResolutionSignatureTtl to IIFE for consistency

Uh oh!

Choose a reason for hiding this comment

[8/10] Unused hexlify import

Uh oh!

Choose a reason for hiding this comment

[9/10] Consider validating parsed.tld against parentTLD

Uh oh!

Choose a reason for hiding this comment

[10/10] Missing fuzz tests for TTL boundaries

Uh oh!

Choose a reason for hiding this comment

[Addendum] dom is discarded — consider using it for validation

Uh oh!

aliXsed commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Apr 13, 2026 •

edited

Loading

LCOV of commit `ad92b93` during checks #658

[1/10] Create a new contract — don't modify the existing `UniversalResolver`

[2/10] `Ownable` → `Ownable2Step`

[3/10] `setUrl` missing empty-URL validation

[4/10] Split `setTrustedSigner` into `trustSigner` / `revokeSigner`

[5/10] Add `_data.length >= 4` check in `resolve()`

[6/10] Fix `addr(bytes32,uint256)` bare-domain return type

[7/10] Convert `parseResolutionSignatureTtl` to IIFE for consistency

[8/10] Unused `hexlify` import

[9/10] Consider validating `parsed.tld` against `parentTLD`

[Addendum] `dom` is discarded — consider using it for validation