tests: add seed script to improve resolution tests#1994
tests: add seed script to improve resolution tests#1994
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 30 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThe PR refactors devnet account exports into separate address/data modules, changes the devnet chain id to Changes
Sequence DiagramsequenceDiagram
participant Orchestrator
participant DevNetContainer
participant SeedDevnet
participant SeedPrimary
participant SeedResolver
participant SmartContracts
Orchestrator->>DevNetContainer: wait for container ready
DevNetContainer-->>Orchestrator: ready (port mapped)
Orchestrator->>SeedDevnet: seedDevnet(rpcUrl)
SeedDevnet->>SeedDevnet: create Viem wallet clients (deployer, owner, user, user2)
SeedDevnet->>SeedPrimary: seedPrimaryNameRecords(clients)
SeedPrimary->>SmartContracts: writeContract(setName("test.eth"))
SmartContracts-->>SeedPrimary: tx hash
SeedPrimary-->>SeedDevnet: complete
SeedDevnet->>SeedResolver: seedResolverRecords(clients)
SeedResolver->>SmartContracts: findResolver("test.eth")
SmartContracts-->>SeedResolver: resolver address
SeedResolver->>SmartContracts: writeContract(setText/setAddr/setContenthash/setPubkey/setABI/setInterface)
SmartContracts-->>SeedResolver: tx hashes
SeedResolver-->>SeedDevnet: complete
SeedDevnet-->>Orchestrator: complete
Orchestrator->>Orchestrator: continue startup/tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Greptile SummaryThis PR introduces a devnet seed script that populates resolver records (text, multi-coin addresses, contenthash, pubkey, ABI, interface) and a primary name for
Confidence Score: 3/5Not safe to merge without confirming the chain ID change matches the actual devnet Anvil configuration. One P1 finding: DEVNET_DEFAULT_CHAIN_ID changed from 0xeeeeed to 1 without any visible accompanying change to the devnet startup configuration. If Anvil is not running with --chain-id 1, all seed transactions will fail and every downstream integration test will fail with confusing assertion mismatches. packages/datasources/src/lib/chains.ts — chain ID change needs verification against the actual Anvil devnet startup flags. Important Files Changed
Sequence DiagramsequenceDiagram
participant Orch as orchestrator.ts
participant Seed as seedDevnet()
participant PN as seedPrimaryNameRecords()
participant RR as seedResolverRecords()
participant Anvil as Devnet (Anvil RPC)
participant ENSNode as ENSNode Indexer
Orch->>Anvil: resolve devnet port
Orch->>Seed: seedDevnet(devnetRpcUrl)
Seed->>PN: seedPrimaryNameRecords(clients)
PN->>Anvil: writeContract(ethReverseRegistrar.setName("test.eth"))
Anvil-->>PN: txHash (not awaited)
Seed->>RR: seedResolverRecords(clients)
RR->>Anvil: readContract(universalResolverV2.findResolver)
Anvil-->>RR: activeResolver
RR->>Anvil: writeContract(permissionedResolver.setText×6)
RR->>Anvil: writeContract(permissionedResolver.setAddr×2)
RR->>Anvil: writeContract(permissionedResolver.setContenthash)
RR->>Anvil: writeContract(permissionedResolver.setPubkey)
RR->>Anvil: writeContract(permissionedResolver.setABI)
RR->>Anvil: writeContract(permissionedResolver.setInterface)
Anvil-->>RR: txHashes (not awaited)
Seed-->>Orch: done
Orch->>ENSNode: start indexer (Phase 2+)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const hash = await walletClient.writeContract({ | ||
| address: RESOLVER, | ||
| abi: publicResolverAbi, | ||
| functionName: "setText", | ||
| args: [node, key, value], | ||
| }); | ||
| console.log(`[seed] setText("${key}", "${value}") tx: ${hash}`); | ||
| } |
There was a problem hiding this comment.
Every writeContract call captures a hash but never calls waitForTransactionReceipt. While Anvil's default auto-mine mode makes this effectively safe, a silently-reverted transaction (e.g. the owner lacks permission on the permissionedResolver) would only show up as a console-logged hash with no error thrown. If devnet mining mode ever changes, or a contract call fails, seedDevnet will return successfully and the integration tests will fail with confusing assertion mismatches rather than a meaningful seed error.
| @@ -0,0 +1,97 @@ | |||
| /** | |||
There was a problem hiding this comment.
decided to keep all addresses from pnpm devnet logs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/ensapi/src/handlers/api/resolution/resolve-primary-names.integration.test.ts`:
- Around line 16-22: Update the test description string in the
resolve-primary-names integration test to reflect that a primary name is now
seeded for DEVNET_ACCOUNTS.owner on chain 1; locate the test case that currently
has description "resolves primary names for owner address on chain 1 (no primary
name set in devnet)" and change it to mention that test.eth is the seeded
primary (e.g., "resolves primary name test.eth for owner address on chain 1") so
the description matches expectedBody.names["1"] === "test.eth".
In `@packages/ensnode-sdk/src/shared/devnet/accounts.ts`:
- Line 26: The entry named one in DEVNET_ACCOUNTS is an undocumented hardcoded
address (toNormalizedAddress(`0x${"1".repeat(40)}`)) and is inconsistent with
sibling entries derived from DEVNET_MNEMONIC; either add a comment explaining
its purpose and why it’s included alongside mnemonic-derived signers (e.g.,
"Placeholder address used for X — not a signer") or remove/move it out of
DEVNET_ACCOUNTS into a clearer constant (e.g., DEVNET_BYTES or
DEVNET_PLACEHOLDER_ADDRESSES) and update any consumers to reference the new
constant so DEVNET_ACCOUNTS only contains mnemonic-derived signers. Ensure
references to DEVNET_MNEMONIC and toNormalizedAddress remain correct if you keep
it.
In `@packages/ensnode-sdk/src/shared/devnet/data.ts`:
- Around line 3-11: The DEVNET_BYTES object is currently annotated as
Record<string, Hex>, which widens and loses its literal keys; replace the
explicit type annotation with a satisfies clause so keys are preserved while
values remain constrained to Hex. Concretely, remove ": Record<string, Hex>" and
export the object as a constant using the satisfies operator, e.g. declare
DEVNET_BYTES with "export const DEVNET_BYTES = { ... } as const satisfies
Record<string, Hex>;" (or at minimum "export const DEVNET_BYTES = { ... }
satisfies Record<string, Hex>;") so IDE autocomplete and key-level type
narrowing for DEVNET_BYTES is retained.
In `@packages/integration-test-env/src/seed/index.ts`:
- Around line 19-24: Update the stale inline comments on the DevnetWalletClients
type so they no longer reference legacy constants
DEVNET_OWNER/DEVNET_USER/DEVNET_USER2; instead, describe each wallet's
role/purpose consistent with DEVNET_ACCOUNTS and the comments in
packages/ensnode-sdk/src/shared/devnet/accounts.ts (e.g., deployer — index 0 has
REGISTRAR role on ETHRegistry; owner — index 1 owns test.eth; user — index 2
general test user; user2 — index 3 secondary test user). Locate these comments
next to the DevnetWalletClients type and replace the old constant names with the
role/purpose descriptions.
In `@packages/integration-test-env/src/seed/primary-names.ts`:
- Around line 10-18: The setPrimaryNameRecord function (and all set*Record
functions in resolver-records.ts such as setTextRecord, setMulticoinAddress,
setContenthash, setPubkey, setAbi, setInterfaceImplementer) currently returns
immediately after walletClient.writeContract (which only submits to mempool);
modify each seeding function to await transaction finality by calling
publicClient.waitForTransactionReceipt({ hash }) after writeContract returns its
hash and before returning (use the returned hash from walletClient.writeContract
and ensure the wait call is awaited so seedPrimaryNameRecords and seedDevnet
only proceed after the tx is mined).
In `@packages/integration-test-env/src/seed/resolver-records.ts`:
- Around line 144-152: Remove the commented-out dead block for
clearResolverRecords (the async function referencing PUBLIC_RESOLVER) since it
uses an undefined symbol and clutters the file; either delete the entire
commented section or restore it as a working helper by updating the symbol to
the file's RESOLVER constant, import/define any missing types/abi
(DevnetWalletClient, Hex, publicResolverAbi) and adjust the writeContract call
to use RESOLVER and valid args so it compiles and works for local debugging.
- Around line 12-40: seedResolverRecords currently fires many writeContract
calls (via helpers like setTextRecord, setMulticoinAddress, setContenthash,
setPubkey, setAbi, setInterfaceImplementer) which return RPC tx hashes but do
not wait for mining; collect the returned tx hashes (or receipts) from those
helper calls and then use the public client's waitForTransactionReceipt (e.g.,
clients.owner.publicClient.waitForTransactionReceipt) to await the last tx (or
await each hash) before returning from seedResolverRecords so indexing cannot
start before transactions are mined and reverted txs surface.
- Line 14: The call to assertTestEthResolver is reaching into viem internals via
clients.owner.transport.url; instead, add an explicit rpcUrl parameter to
seedResolverRecords and assertTestEthResolver, replace uses of
clients.owner.transport.url with the rpcUrl argument inside seedResolverRecords
and assertTestEthResolver, and update the caller (from seedDevnet/seed/index.ts)
to pass the existing rpcUrl value through to seedResolverRecords so no transport
internals are accessed.
🪄 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: 96a22959-433e-410a-95be-35ae2754d0b0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
apps/ensapi/src/handlers/api/resolution/resolve-primary-name.integration.test.tsapps/ensapi/src/handlers/api/resolution/resolve-primary-names.integration.test.tsapps/ensapi/src/handlers/api/resolution/resolve-records.integration.test.tsapps/ensapi/src/omnigraph-api/schema/account.integration.test.tspackages/datasources/src/lib/chains.tspackages/ensnode-sdk/src/internal.tspackages/ensnode-sdk/src/omnigraph-api/example-queries.tspackages/ensnode-sdk/src/shared/devnet-accounts.tspackages/ensnode-sdk/src/shared/devnet/accounts.tspackages/ensnode-sdk/src/shared/devnet/addresses.tspackages/ensnode-sdk/src/shared/devnet/data.tspackages/integration-test-env/package.jsonpackages/integration-test-env/src/orchestrator.tspackages/integration-test-env/src/seed/abi.tspackages/integration-test-env/src/seed/index.tspackages/integration-test-env/src/seed/primary-names.tspackages/integration-test-env/src/seed/resolver-records.ts
💤 Files with no reviewable changes (1)
- packages/ensnode-sdk/src/shared/devnet-accounts.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
shrugs
left a comment
shrugs
left a comment
There was a problem hiding this comment.
LOVE the direction, way better than forking; i'd forgotten that we just know the test mnemonic and can hook in like that. definitely the correct direction, thanks for prototyping this
| import { mnemonicToAccount } from "viem/accounts"; | ||
|
|
||
| import { ensTestEnvChain } from "@ensnode/datasources"; | ||
| import { DEVNET_MNEMONIC } from "@ensnode/ensnode-sdk/internal"; |
There was a problem hiding this comment.
imo move this to the integration test env, no need for it to be shared, right?
| ]); | ||
|
|
||
| // https://github.com/ensdomains/contracts-v2/blob/42f2016e7ba87eb3854afe51ad55186a16b32a74/contracts/test/utils/resolutions.ts#L28 | ||
| export const publicResolverAbi = parseAbi([ |
There was a problem hiding this comment.
all of this should be available through @ensnode/datasources — ResolverABI, UniversalResolverABI — maybe not the reverse registrar abi, but perhaps just export it if not — let's keep the abi defs as centralized as possible.
| import { publicResolverAbi, universalResolverV2Abi } from "./abi"; | ||
| import type { DevnetWalletClient, DevnetWalletClients } from "./index"; | ||
|
|
||
| const RESOLVER = DEVNET_CONTRACTS.permissionedResolver; |
There was a problem hiding this comment.
the permissionedResolver address exported by the devnet may be a factory not a specific impl; each address gets their own PermRes in ensv2, so it'd be best to make sure we're acting on an instance of a PermRes that's tied to a specific address
There was a problem hiding this comment.
this permissionedResolver is a resolver for all names in devnet 👀
|
i didn't see the devnet commit version be updated in this pr, make sure that's done in the docker-compose so that the orchestrator is definitely using the correct version and the chain id update is locked in. all devnet-specific stuff must be pinned to a specific commit or we're going to have a really annoying time when things change behind the scenes (as they do a LOT) |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/integration-test-env/src/orchestrator.ts (1)
7-13:⚠️ Potential issue | 🟡 MinorFile-level phase docstring is out of sync with code.
The header still lists 5 phases, but the implementation now has 7 phases (Phase 2 = seed devnet, Phase 5 = poll indexing status, Phase 6 = ENSApi, Phase 7 = tests). New readers will miss that seeding is part of the orchestration.
📝 Proposed update
* Phases: * 1. Postgres + devnet via docker-compose (testcontainers DockerComposeEnvironment) - * 2. Download pre-built ENSRainbow LevelDB, extract, start ENSRainbow from source - * 3. Start ENSIndexer, wait for omnichain-following / omnichain-completed - * 4. Start ENSApi - * 5. Run `pnpm test:integration` at the monorepo root + * 2. Seed devnet with primary names and resolver records + * 3. Download pre-built ENSRainbow LevelDB, extract, start ENSRainbow from source + * 4. Start ENSIndexer + * 5. Wait for omnichain-following / omnichain-completed + * 6. Start ENSApi + * 7. Run `pnpm test:integration` at the monorepo root🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integration-test-env/src/orchestrator.ts` around lines 7 - 13, Update the top-of-file phase docstring in orchestrator.ts to match the current implementation: replace the old 5-phase list with the accurate 7-phase sequence and explicitly include the new "seed devnet" and "poll indexing status" steps (which correspond to the code paths that perform devnet seeding and polling for omnichain-following/omnichain-completed before starting ENSApi and running tests); reference the existing phase logic implemented in the orchestration functions (the devnet seed routine, the indexing poll routine, the ENSApi start routine, and the final test-run invocation) and reorder the docstring so each implemented phase is described in the same order the code executes.
♻️ Duplicate comments (1)
packages/ensnode-sdk/src/shared/devnet/addresses.ts (1)
13-100: 🧹 Nitpick | 🔵 Trivial
DEVNET_CONTRACTSvalues are not normalized (lowercase) addresses.These are checksummed
Addressliterals, notNormalizedAddress. Consumers likeseedResolverRecordsForNamealready work around this withactualResolver.toLowerCase() !== resolver.toLowerCase()(resolver-records.ts line 29), but any future strict equality / set membership comparison against anotherNormalizedAddresswill silently mismatch. Recommend normalizing once at definition (and ideally typing viatoNormalizedAddressso the type alias enforces the invariant).♻️ Proposed change
+import { type NormalizedAddress, toNormalizedAddress } from "enssdk"; + +const n = toNormalizedAddress; + -export const DEVNET_CONTRACTS = { +export const DEVNET_CONTRACTS = { // -- DNS -- - dnssecGatewayProvider: "0x5FbDB2315678afecb367f032d93F642f64180aa3", + dnssecGatewayProvider: n("0x5FbDB2315678afecb367f032d93F642f64180aa3"), // …apply to each entry… -} as const; +} as const satisfies Record<string, NormalizedAddress>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-sdk/src/shared/devnet/addresses.ts` around lines 13 - 100, DEVNET_CONTRACTS currently contains checksummed Address literals which can mismatch NormalizedAddress comparisons (see seedResolverRecordsForName’s toLowerCase workaround); update the DEVNET_CONTRACTS object so every contract address value is normalized (lowercase) at definition and typed as a NormalizedAddress (use the existing toNormalizedAddress helper or equivalent) so the invariant is enforced (refer to DEVNET_CONTRACTS, seedResolverRecordsForName, and toNormalizedAddress for locating spots to change).
🤖 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/datasources/src/lib/chains.ts`:
- Around line 3-7: Update the JSDoc comment in
packages/datasources/src/lib/chains.ts to remove the trailing colon and clearly
state that the ens-test-env intentionally uses chain id 1 to mirror mainnet (so
callers should not treat id==1 as a sentinel for real mainnet), and mention the
potential for mainnet-collision to discourage future sentinel-style checks
against the value; update the comment text above the file/module (where the
current /** The ens-test-env chain id is 1: */ block appears) accordingly.
- Around line 8-13: The current gate that disables Ponder's cache uses a numeric
comparison against ensTestEnvChain.id (e.g., `chainId === 31337 || chainId ===
1337 || chainId === ensTestEnvChain.id`) which collides with mainnet id; change
that logic to use the namespace instead — replace the `... || chainId ===
ensTestEnvChain.id` check with a namespace check such as `config.namespace ===
ENSNamespaceIds.EnsTestEnv` (use the existing ENSNamespaceIds enum/const and the
`config.namespace` value) so local-dev detection relies on `config.namespace`
rather than the numeric `ensTestEnvChain.id`, leaving other chainId checks
intact.
In `@packages/ensnode-sdk/src/shared/devnet/addresses.ts`:
- Around line 107-114: The JSDoc describing "Named signer accounts from the
ens-test-env devnet…" is incorrectly attached to the helper
deriveNormalizedAccount; move that doc block to sit directly above the exported
DEVNET_ACCOUNTS constant so the comment documents the export it describes, and
either remove or replace the JSDoc on deriveNormalizedAccount (or add a brief
helper-specific comment) so deriveNormalizedAccount, DEVNET_ACCOUNTS and
DEVNET_MNEMONIC are correctly and clearly documented.
---
Outside diff comments:
In `@packages/integration-test-env/src/orchestrator.ts`:
- Around line 7-13: Update the top-of-file phase docstring in orchestrator.ts to
match the current implementation: replace the old 5-phase list with the accurate
7-phase sequence and explicitly include the new "seed devnet" and "poll indexing
status" steps (which correspond to the code paths that perform devnet seeding
and polling for omnichain-following/omnichain-completed before starting ENSApi
and running tests); reference the existing phase logic implemented in the
orchestration functions (the devnet seed routine, the indexing poll routine, the
ENSApi start routine, and the final test-run invocation) and reorder the
docstring so each implemented phase is described in the same order the code
executes.
---
Duplicate comments:
In `@packages/ensnode-sdk/src/shared/devnet/addresses.ts`:
- Around line 13-100: DEVNET_CONTRACTS currently contains checksummed Address
literals which can mismatch NormalizedAddress comparisons (see
seedResolverRecordsForName’s toLowerCase workaround); update the
DEVNET_CONTRACTS object so every contract address value is normalized
(lowercase) at definition and typed as a NormalizedAddress (use the existing
toNormalizedAddress helper or equivalent) so the invariant is enforced (refer to
DEVNET_CONTRACTS, seedResolverRecordsForName, and toNormalizedAddress for
locating spots to change).
🪄 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: ba17a835-903e-45c1-8fbf-d54b284898b5
📒 Files selected for processing (13)
apps/ensapi/src/handlers/api/resolution/resolve-primary-name.integration.test.tsapps/ensapi/src/handlers/api/resolution/resolve-primary-names.integration.test.tsapps/ensapi/src/handlers/api/resolution/resolve-records.integration.test.tsapps/ensapi/src/omnigraph-api/schema/account.integration.test.tsdocker/services/devnet.ymlpackages/datasources/src/lib/chains.tspackages/ensnode-sdk/src/internal.tspackages/ensnode-sdk/src/omnigraph-api/example-queries.tspackages/ensnode-sdk/src/shared/devnet/addresses.tspackages/ensnode-sdk/src/shared/devnet/data.tspackages/integration-test-env/src/orchestrator.tspackages/integration-test-env/src/seed/index.tspackages/integration-test-env/src/seed/resolver-records.ts
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)
apps/ensindexer/src/lib/ponder-helpers.ts (1)
96-121: 🧹 Nitpick | 🔵 TrivialConsider making
namespacerequired to prevent silent cache-disable regressions.
namespace?: ENSNamespaceIdis optional. Any future caller that forgets to pass it on theEnsTestEnvnamespace will silently re-enable the cache for devnet (since thenamespace === ENSNamespaceIds.EnsTestEnvpredicate evaluatesfalseforundefined), which would corrupt integration test runs. Since the only existing in-tree callers (tokenscope/plugin.tsandchainsConnectionConfigForDatasources) already pass it, making it required has no migration cost and locks in the invariant.♻️ Proposed change
export function chainsConnectionConfig( rpcConfigs: ENSIndexerConfig["rpcConfigs"], chainId: ChainId, - namespace?: ENSNamespaceId, + namespace: ENSNamespaceId, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensindexer/src/lib/ponder-helpers.ts` around lines 96 - 121, Make the namespace parameter required on chainsConnectionConfig to avoid silent cache-disable regressions: change the function signature from namespace?: ENSNamespaceId to namespace: ENSNamespaceId in chainsConnectionConfig, update the disableCache predicate that checks namespace === ENSNamespaceIds.EnsTestEnv accordingly, and adjust all callers (e.g., tokenscope/plugin.ts and chainsConnectionConfigForDatasources) to pass an explicit ENSNamespaceId so the ENS test-env cache behavior remains correct.
♻️ Duplicate comments (1)
packages/ensnode-sdk/src/shared/devnet/addresses.ts (1)
13-100: 🧹 Nitpick | 🔵 TrivialConsider lowercasing
DEVNET_CONTRACTSaddresses for consistency.
DEVNET_ACCOUNTSandDEVNET_ADDRESSES.oneare funneled throughtoNormalizedAddress(lowercased), butDEVNET_CONTRACTSretains EIP-55 checksum casing. Consumers that compare these against on-chain query results (e.g. theactualResolver.toLowerCase() !== resolver.toLowerCase()check inresolver-records.ts) must remember to normalize each side. Lowercasing the entireDEVNET_CONTRACTSmap (or wrapping each value withtoNormalizedAddress) would let consumers drop the per-call.toLowerCase()and align this constant with the rest of the SDK'sNormalizedAddressconvention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-sdk/src/shared/devnet/addresses.ts` around lines 13 - 100, DEVNET_CONTRACTS contains mixed-case EIP-55 addresses while other constants (DEVNET_ACCOUNTS, DEVNET_ADDRESSES.one) use normalized lowercased addresses; update DEVNET_CONTRACTS so each address is normalized—either by lowercasing all string literals or by passing each contract value through the existing toNormalizedAddress helper—so consumers can rely on NormalizedAddress semantics (refer to DEVNET_CONTRACTS and the toNormalizedAddress usage in DEVNET_ACCOUNTS/DEVNET_ADDRESSES).
🤖 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 `@apps/ensindexer/src/lib/ponder-helpers.ts`:
- Around line 96-121: Make the namespace parameter required on
chainsConnectionConfig to avoid silent cache-disable regressions: change the
function signature from namespace?: ENSNamespaceId to namespace: ENSNamespaceId
in chainsConnectionConfig, update the disableCache predicate that checks
namespace === ENSNamespaceIds.EnsTestEnv accordingly, and adjust all callers
(e.g., tokenscope/plugin.ts and chainsConnectionConfigForDatasources) to pass an
explicit ENSNamespaceId so the ENS test-env cache behavior remains correct.
---
Duplicate comments:
In `@packages/ensnode-sdk/src/shared/devnet/addresses.ts`:
- Around line 13-100: DEVNET_CONTRACTS contains mixed-case EIP-55 addresses
while other constants (DEVNET_ACCOUNTS, DEVNET_ADDRESSES.one) use normalized
lowercased addresses; update DEVNET_CONTRACTS so each address is
normalized—either by lowercasing all string literals or by passing each contract
value through the existing toNormalizedAddress helper—so consumers can rely on
NormalizedAddress semantics (refer to DEVNET_CONTRACTS and the
toNormalizedAddress usage in DEVNET_ACCOUNTS/DEVNET_ADDRESSES).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec3d6001-59c1-4355-864d-0359d9ee3440
📒 Files selected for processing (8)
apps/ensindexer/src/lib/ponder-helpers.tsapps/ensindexer/src/plugins/tokenscope/plugin.tspackages/datasources/src/lib/chains.tspackages/ensnode-sdk/src/shared/config/rpc-configs-from-env.tspackages/ensnode-sdk/src/shared/devnet/addresses.tspackages/integration-test-env/src/seed/index.tspackages/integration-test-env/src/seed/primary-names.tspackages/integration-test-env/src/seed/resolver-records.ts
2 participants
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
Why
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)