[SDK] Add signAuthorization support to 1193 provider #8029
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 4b6a41f The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
WalkthroughAdds a changeset for a patch release noting signAuthorization support. Implements Authorization signing in the injected wallet by importing Authorization utilities, defining AuthorizationRequest, and adding an Account.signAuthorization method that prepares a payload and signs via eth_sign using the EIP-1193 provider. Changes
Sequence Diagram(s)sequenceDiagram
participant D as Dapp
participant W as InjectedWallet (Account)
participant P as EIP-1193 Provider
participant U as User
D->>W: signAuthorization(authorization)
activate W
W->>W: build sign payload via Authorization.getSignPayload
W->>P: request({ method: "eth_sign", params: [address, payload] })
activate P
P->>U: Prompt to sign
U-->>P: Approve / Reject
P-->>W: signature or error
deactivate P
W-->>D: signature or error
deactivate W
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
joaquim-verges
changed the title
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8029 +/- ##
==========================================
- Coverage 56.62% 56.62% -0.01%
==========================================
Files 904 904
Lines 58677 58685 +8
Branches 4161 4162 +1
==========================================
+ Hits 33225 33229 +4
- Misses 25346 25351 +5
+ Partials 106 105 -1
🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (6)
.changeset/eighty-taxis-own.md (1)
5-6: Clarify the changeset entry.Call out the new
Account.signAuthorizationmethod and that it signs an EIP‑7702 Authorization viaeth_sign. Helps consumers understand impact.packages/thirdweb/src/wallets/injected/index.ts (5)
270-276: Add return type, guard, and cast for the new method.Make the method consistent with others (
signMessage), and type its return. Also cast the RPC result toHex.- async signAuthorization(authorization: AuthorizationRequest) { - const payload = ox__Authorization.getSignPayload(authorization); - return await provider.request({ - method: "eth_sign", - params: [getAddress(account.address), payload], - }); - }, + async signAuthorization(authorization: AuthorizationRequest): Promise<Hex> { + if (!provider || !account.address) { + throw new Error("Provider not setup"); + } + const payload = ox__Authorization.getSignPayload(authorization); + const signature = (await provider.request({ + method: "eth_sign", + params: [getAddress(account.address), payload], + })) as Hex; + return signature; + },
1-1: Lazy‑load ox/Authorization to keep the injected entry lean.
ox/Authorizationmay be sizable; import it on demand inside the method.-import * as ox__Authorization from "ox/Authorization"; +// lazy-load inside signAuthorization to reduce initial bundleAnd update the method:
- const payload = ox__Authorization.getSignPayload(authorization); + const { getSignPayload } = await import("ox/Authorization"); + const payload = getSignPayload(authorization);Also applies to: 270-276
270-276: Validate signer consistency with the authorization payload.Avoid signing authorizations for a different address by mistake.
async signAuthorization(authorization: AuthorizationRequest): Promise<Hex> { + // Optional: enforce signer consistency if the request carries an address + const signer = getAddress(account.address); + // @ts-expect-error - runtime check based on optional shape + if (authorization.address && getAddress(authorization.address) !== signer) { + throw new Error("Authorization signer does not match connected account"); + } + // @ts-expect-error - alt field name some callers may use + // if (authorization.from && getAddress(authorization.from) !== signer) { + // throw new Error("Authorization 'from' does not match connected account"); + // } … - const signature = (await provider.request({ + const signature = (await provider.request({ method: "eth_sign", - params: [getAddress(account.address), payload], + params: [signer, payload], })) as Hex; return signature; },
18-18: Type import looks correct. Document the new API surface.If
signAuthorizationis added to the publicAccounttype, add TSDoc (with@beta/@experimentaland an@example) where the type is declared.
270-276: Wrapeth_signin a try/catch and fallback topersonal_sign
eth_signcorrectly uses[address, data], but MetaMask disables it by default and Rabby’s support is inconsistent. Catch any rejection from theeth_signcall and retry withpersonal_sign(which expects[data, address]) to ensure broad compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/eighty-taxis-own.md(1 hunks)packages/thirdweb/src/wallets/injected/index.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/*.md: Each change inpackages/*must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API
Files:
.changeset/eighty-taxis-own.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/wallets/injected/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/wallets/injected/index.ts
packages/thirdweb/src/wallets/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/wallets/**: UnifiedWalletandAccountinterfaces in wallet architecture
Support for in-app wallets (social/email login)
Smart wallets with account abstraction
EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Files:
packages/thirdweb/src/wallets/injected/index.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/wallets/injected/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/thirdweb/src/wallets/injected/index.ts (1)
270-276: Suggestion no longer applies –Accountalready definessignAuthorization(optional).Likely an incorrect or invalid review comment.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "thirdweb": patch | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Version bump likely needs to be minor (public API surface).
If Account now exposes signAuthorization, this is a public API addition and should be a minor bump per guidelines. Use patch only if the method isn’t part of any exported/public type.
Apply this diff if it is public:
-"thirdweb": patch
+"thirdweb": minor📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "thirdweb": patch | |
| "thirdweb": minor |
🤖 Prompt for AI Agents
In .changeset/eighty-taxis-own.md around line 2, the changeset currently lists
"thirdweb": patch but the new Account API exposes signAuthorization which is a
public API addition; change the bump from patch to minor and update any
accompanying description/title in the changeset to reflect a minor release. If
signAuthorization is not exported/public, keep patch; otherwise edit the file to
use "thirdweb": minor and save.
PR-Codex overview
This PR introduces support for signing authorizations in the
1193provider of thethirdweblibrary by adding a new method.Detailed summary
signAuthorizationin thepackages/thirdweb/src/wallets/injected/index.tsfile.signAuthorizationmethod takes anAuthorizationRequestas a parameter.ox__Authorization.getSignPayloadand requests signing via theeth_signmethod.Summary by CodeRabbit
New Features
Chores