ENS: add coinType param to reverse resolver#8735
Conversation
Update generated UniversalResolver reverse read to include a coinType (uint256) parameter and new function selector; adjust FN_INPUTS/FN_OUTPUTS and encode/read params accordingly. Update resolve-name to call the new reverse signature (pass the raw address as reverseName and coinType: 60n), remove packetToBytes/toHex usage and unused imports, simplify error handling, and return the resolved name or null.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 05b9f6c The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe ENS reverse resolver in the thirdweb package was updated to pass Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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)
Comment |
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8735 +/- ##
==========================================
- Coverage 52.72% 52.71% -0.02%
==========================================
Files 934 934
Lines 62968 62958 -10
Branches 4134 4132 -2
==========================================
- Hits 33199 33187 -12
- Misses 29671 29673 +2
Partials 98 98
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/thirdweb/src/extensions/ens/resolve-name.ts (1)
34-34: Add explicit return type.The function lacks an explicit return type. As per coding guidelines, "Write idiomatic TypeScript with explicit function declarations and return types."
Proposed fix
-export async function resolveName(options: ResolveNameOptions) { +export async function resolveName(options: ResolveNameOptions): Promise<string | null> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/thirdweb/src/extensions/ens/resolve-name.ts` at line 34, The resolveName function is missing an explicit return type; update the function signature for resolveName(options: ResolveNameOptions) to include the concrete return type that matches its implementation (for example Promise<string | null> or the actual resolved type used in the body), and ensure ResolveNameOptions and any callers remain type-compatible after adding the explicit return type.
🤖 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/thirdweb/src/extensions/ens/resolve-name.ts`:
- Around line 45-53: The current catch-all on the reverse(...) call masks
verification failures; update the code around the reverse(...) invocation (the
reverse function call that returns [name]) to only swallow expected "no
resolver/no name" errors and let verification errors surface: replace the
.catch(() => [null]) with a try/catch (or inspect the thrown error), return
[null] for resolver-not-found / no-name cases, but re-throw if the error is a
ReverseAddressMismatch (or any other verification-related error) so callers can
detect data integrity issues; reference the reverse(...) call, the
ReverseAddressMismatch error type, and the address/coinType handling when making
this change.
---
Nitpick comments:
In `@packages/thirdweb/src/extensions/ens/resolve-name.ts`:
- Line 34: The resolveName function is missing an explicit return type; update
the function signature for resolveName(options: ResolveNameOptions) to include
the concrete return type that matches its implementation (for example
Promise<string | null> or the actual resolved type used in the body), and ensure
ResolveNameOptions and any callers remain type-compatible after adding the
explicit return type.
🪄 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: CHILL
Plan: Pro
Run ID: bf76f25f-bae0-45b9-a028-746d6c8308d3
⛔ Files ignored due to path filters (1)
packages/thirdweb/src/extensions/ens/__generated__/UniversalResolver/read/reverse.tsis excluded by!**/__generated__/**
📒 Files selected for processing (2)
.changeset/every-donuts-stop.mdpackages/thirdweb/src/extensions/ens/resolve-name.ts
Give resolveName an explicit Promise<string | null> return type and enhance its error handling: re-throw errors that include a data field starting with the ReverseAddressMismatch selector (0xef9c03ce) so callers can surface verification/data-integrity issues, while swallowing expected "no resolver"/"no name" errors. Also minor parameter formatting adjustments.
Update generated UniversalResolver reverse read to include a coinType (uint256) parameter and new function selector; adjust FN_INPUTS/FN_OUTPUTS and encode/read params accordingly. Update resolve-name to call the new reverse signature (pass the raw address as reverseName and coinType: 60n), remove packetToBytes/toHex usage and unused imports, simplify error handling, and return the resolved name or null.
PR-Codex overview
This PR introduces a
coinTypeparameter to the reverse resolver in the ENS extension of thethirdwebSDK, updating function signatures and logic to accommodate this new parameter.Detailed summary
coinTypeparameter toReverseParamsandFN_INPUTS.FN_SELECTORfrom"0xec11c823"to"0x5d78a217".encodeReverseParamsto includeoptions.coinType.reversefunction to accept and passoptions.coinType.resolveNameto use a defaultcoinTypeof60n.Summary by CodeRabbit
Bug Fixes
Chores