[SDK] Cache and reuse x402 permit signatures for upto schemes #8538
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: a9ab172 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 |
WalkthroughAdds AsyncStorage-backed caching for x402 "upto" permit signatures and threads an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/App
participant Storage as AsyncStorage
participant Signing as Payment Signing
participant OnChain as On‑chain (ERC20)
participant Server as x402 Server
Client->>Server: Initial request (no permit)
Server-->>Client: 402 Payment Required + requirements
alt "upto" permit flow
Signing->>Storage: getPermitSignatureFromCache(chainId, asset, owner, spender)
Storage-->>Signing: cached permit? (payload, deadline, maxAmount) / null
alt cached & not expired
Signing->>OnChain: query allowance(owner, spender, asset)
OnChain-->>Signing: allowance amount
alt allowance >= required
Signing-->>Client: reuse cached permit header
else
Signing->>Signing: create new permit signature
Signing->>Storage: savePermitSignatureToCache(...)
Signing-->>Client: new permit header
end
else no cached permit
Signing->>Signing: create new permit signature
Signing->>Storage: savePermitSignatureToCache(...)
Signing-->>Client: new permit header
end
end
Client->>Server: Retry with signed header
Server-->>Client: Success OR 402 (second 402)
alt second 402 response
Client->>Storage: clearPermitSignatureFromCache(chainId, asset, owner, spender)
Storage-->>Client: cleared
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts (1)
92-100: Avoid spreadingoptionswhen it may beundefined
optionsis optional, but this call:return useFetchWithPaymentCore(client, { ...options, storage: options?.storage ?? nativeLocalStorage, });will throw if
optionsisundefinedbecause object spread cannot spreadundefined.Wrap the spread with a safe fallback:
-export function useFetchWithPayment( - client: ThirdwebClient, - options?: UseFetchWithPaymentOptions, -) { - // Native version doesn't show modal, errors bubble up naturally - return useFetchWithPaymentCore(client, { - ...options, - storage: options?.storage ?? nativeLocalStorage, - }); -} +export function useFetchWithPayment( + client: ThirdwebClient, + options?: UseFetchWithPaymentOptions, +) { + // Native version doesn't show modal, errors bubble up naturally + return useFetchWithPaymentCore(client, { + ...(options ?? {}), + storage: options?.storage ?? nativeLocalStorage, + }); +}This preserves existing behavior while safely defaulting storage for permit caching.
🧹 Nitpick comments (2)
packages/thirdweb/src/x402/sign.ts (1)
101-152: Permit caching logic is solid; consider trimming debug log and unused fieldsThe new
"Permit"branch for the"upto"scheme looks correct:
- Only caches when
scheme === "upto"andstorageis provided.- Reuse path validates:
- Deadline (
BigInt(cached.deadline) > now), and- On-chain allowance via
allowance(...), comparing againstminAmountRequiredwhen present, otherwisemaxAmountRequired.This should prevent reusing signatures when they are expired or underfunded.
Two minor follow-ups:
- Debug logging in SDK code
if (currentAllowance >= threshold) { console.log("re-using cached signature"); return cached.payload; }A bare
console.login library code can be noisy for consumers. Consider removing it or routing through whatever structured/debug logging facility the SDK uses (or guarding it behind a debug flag).
- Possibly unused
maxAmountinCachedPermitSignatureWhen caching:
await savePermitSignatureToCache( storage, cacheParams, signedPayload, unsignedPaymentHeader.payload.authorization.validBefore, paymentRequirements.maxAmountRequired, );the
maxAmountfield is persisted, but in this file you only readpayloadanddeadlinefrom the cache. If no other call sites usemaxAmount, you could drop it from the cached data to keep the payload lean; if you plan to use it later (e.g., for debugging or additional validation), keeping it is fine.Also applies to: 173-193
packages/thirdweb/src/x402/permitSignatureStorage.ts (1)
1-99: AsyncStorage-backed permit cache helpers look robust and low-riskThe cache key derivation and helpers are well-structured:
- Key normalization with
toLowerCase()on addresses avoids case-sensitivity bugs.getPermitSignatureFromCachesafely returnsnullon missing or malformed entries.savePermitSignatureToCacheandclearPermitSignatureFromCacheswallow errors, which is reasonable since caching is optional.The
maxAmountfield inCachedPermitSignatureis currently only written, not read in the shown code; if it’s just informational, that’s fine, but you could drop it later to keep the cache footprint minimal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/warm-clouds-judge.md(1 hunks)packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts(3 hunks)packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts(3 hunks)packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx(2 hunks)packages/thirdweb/src/x402/fetchWithPayment.ts(4 hunks)packages/thirdweb/src/x402/permitSignatureStorage.ts(1 hunks)packages/thirdweb/src/x402/sign.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript 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 in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.tspackages/thirdweb/src/x402/sign.tspackages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsxpackages/thirdweb/src/x402/permitSignatureStorage.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.tspackages/thirdweb/src/x402/sign.tspackages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsxpackages/thirdweb/src/x402/permitSignatureStorage.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.tspackages/thirdweb/src/x402/sign.tspackages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsxpackages/thirdweb/src/x402/permitSignatureStorage.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.tspackages/thirdweb/src/x402/sign.tspackages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsxpackages/thirdweb/src/x402/permitSignatureStorage.ts
🧬 Code graph analysis (4)
packages/thirdweb/src/x402/fetchWithPayment.ts (1)
packages/thirdweb/src/x402/permitSignatureStorage.ts (1)
clearPermitSignatureFromCache(89-99)
packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts (1)
packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts (1)
useFetchWithPaymentCore(41-159)
packages/thirdweb/src/x402/sign.ts (3)
packages/thirdweb/src/x402/permitSignatureStorage.ts (3)
PermitCacheKeyParams(16-21)getPermitSignatureFromCache(40-54)savePermitSignatureToCache(64-82)packages/thirdweb/src/chains/utils.ts (1)
getCachedChain(79-89)packages/thirdweb/src/exports/x402.ts (1)
ERC20TokenAmount(17-17)
packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx (1)
packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts (1)
useFetchWithPaymentCore(41-159)
⏰ 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). (7)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
.changeset/warm-clouds-judge.md (1)
1-5: Changeset description matches behavior and scopeThe summary clearly reflects the new x402 permit signature caching behavior and is scoped as a patch for
thirdweb, which is appropriate.packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts (2)
5-23: Storage option threading in core hook looks correctExtending
UseFetchWithPaymentOptionswithstorage?: AsyncStorageand forwarding it towrapFetchWithPaymentkeeps the core hook platform-agnostic while enabling permit signature caching. The option remains fully optional, so existing callers are preserved.
86-95: ForwardingstoragetowrapFetchWithPaymentis consistent with new options typePassing
storage: options?.storagealongsidemaxValueandpaymentRequirementsSelectorcleanly exposes caching behavior to all consumers of the core hook without changing its public signature.packages/thirdweb/src/x402/sign.ts (1)
232-245: ThreadingstorageintocreatePaymentHeaderis consistent withsignPaymentHeaderAccepting an optional
storage?: AsyncStorageincreatePaymentHeaderand simply forwarding it tosignPaymentHeaderkeeps the external API minimal while enabling permit caching where desired.
| // Default to webLocalStorage for permit signature caching | ||
| const resolvedOptions = useMemo( | ||
| () => ({ | ||
| ...options, | ||
| storage: options?.storage ?? webLocalStorage, | ||
| }), | ||
| [options], | ||
| ); | ||
|
|
||
| return useFetchWithPaymentCore( | ||
| client, | ||
| options, | ||
| resolvedOptions, | ||
| showErrorModal, |
There was a problem hiding this comment.
Fix spreading of possibly-undefined options (runtime TypeError risk)
options is optional, but this block:
const resolvedOptions = useMemo(
() => ({
...options,
storage: options?.storage ?? webLocalStorage,
}),
[options],
);will throw at runtime when options is undefined because object spread cannot spread undefined.
Use a safe fallback object when options is not provided:
- const resolvedOptions = useMemo(
- () => ({
- ...options,
- storage: options?.storage ?? webLocalStorage,
- }),
- [options],
- );
+ const resolvedOptions = useMemo(
+ () => ({
+ ...(options ?? {}),
+ storage: options?.storage ?? webLocalStorage,
+ }),
+ [options],
+ );Additionally, consider updating the hook JSDoc above to document the new options.storage parameter and its default (webLocalStorage), for parity with the native hook docs.
🤖 Prompt for AI Agents
In packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx around
lines 259 to 271, the code spreads the possibly-undefined options object which
will throw if options is undefined; change the memo to spread a safe fallback
(e.g., options ?? {}) and set storage with options?.storage ?? webLocalStorage
so the spread never receives undefined, then pass that resolvedOptions into
useFetchWithPaymentCore; also update the hook JSDoc above to document the
options.storage parameter and its default (webLocalStorage).
size-limit report 📦
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8538 +/- ##
=======================================
Coverage 54.66% 54.66%
=======================================
Files 921 921
Lines 61163 61163
Branches 4151 4149 -2
=======================================
Hits 33434 33434
Misses 27627 27627
Partials 102 102
🚀 New features to boost your workflow:
|
joaquim-verges
force-pushed
the
Cache_and_reuse_x402_permit_signatures_for_upto_schemes
branch
from
3aa6e70 to
cb376ab
Compare
github-actions
bot
added
the
Portal
joaquim-verges
force-pushed
the
Cache_and_reuse_x402_permit_signatures_for_upto_schemes
branch
from
cb376ab to
390dc19
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/x402/fetchWithPayment.ts (1)
143-172: Cache invalidation inconsistency with default storage.The code defaults to
webLocalStoragewhen caching signatures (line 143), but only clears the cache whenoptions?.storageis explicitly provided (line 165). This means when callers don't passstorage, signatures are cached inwebLocalStoragebut never invalidated on a subsequent 402.Compute the effective storage once and use it consistently:
export function wrapFetchWithPayment( fetch: typeof globalThis.fetch, client: ThirdwebClient, wallet: Wallet, options?: { maxValue?: bigint; paymentRequirementsSelector?: ( paymentRequirements: RequestedPaymentRequirements[], ) => RequestedPaymentRequirements | undefined; storage?: AsyncStorage; }, ) { return async (input: RequestInfo, init?: RequestInit) => { + const effectiveStorage = options?.storage ?? webLocalStorage; // ... existing code ... const paymentHeader = await createPaymentHeader( client, account, selectedPaymentRequirements, x402Version, - options?.storage ?? webLocalStorage, + effectiveStorage, ); // ... existing code ... // If payment was rejected (still 402), clear cached signature - if (secondResponse.status === 402 && options?.storage) { - await clearPermitSignatureFromCache(options.storage, { + if (secondResponse.status === 402) { + await clearPermitSignatureFromCache(effectiveStorage, { chainId: paymentChainId, asset: selectedPaymentRequirements.asset, owner: getAddress(account.address), spender: getAddress(selectedPaymentRequirements.payTo), }); }
🧹 Nitpick comments (1)
packages/thirdweb/src/x402/sign.ts (1)
103-112: Redundant storage check.Since
shouldCacheis defined aspaymentRequirements.scheme === "upto" && storage !== undefined, the subsequentstoragecheck on line 115 and 182 is redundant. The code is correct but could be simplified.- if (shouldCache && storage) { + if (shouldCache) { const cached = await getPermitSignatureFromCache(storage, cacheParams);Also apply to line 182.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.changeset/warm-clouds-judge.md(1 hunks)apps/portal/src/app/x402/server/page.mdx(1 hunks)packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts(3 hunks)packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts(3 hunks)packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx(2 hunks)packages/thirdweb/src/x402/fetchWithPayment.ts(4 hunks)packages/thirdweb/src/x402/permitSignatureStorage.ts(1 hunks)packages/thirdweb/src/x402/sign.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts
- packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts
- packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx
- packages/thirdweb/src/x402/permitSignatureStorage.ts
- .changeset/warm-clouds-judge.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript 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 in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/sign.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/sign.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/sign.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/sign.ts
🧬 Code graph analysis (1)
packages/thirdweb/src/x402/sign.ts (4)
packages/thirdweb/src/x402/types.ts (2)
x402Version(14-14)ERC20TokenAmount(125-138)packages/thirdweb/src/x402/permitSignatureStorage.ts (3)
PermitCacheKeyParams(16-21)getPermitSignatureFromCache(40-54)savePermitSignatureToCache(64-82)packages/thirdweb/src/chains/utils.ts (1)
getCachedChain(79-89)packages/thirdweb/src/exports/x402.ts (1)
ERC20TokenAmount(17-17)
⏰ 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). (2)
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/portal/src/app/x402/server/page.mdx (1)
135-135: Documentation accurately reflects the new caching behavior.The statement correctly informs users that the SDK now handles caching and reuse of payment data automatically, removing the need for manual backend storage management.
packages/thirdweb/src/x402/sign.ts (3)
133-147: Threshold logic is sound.The fallback from
minAmountRequiredtomaxAmountRequiredcorrectly handles both cases. The optional chaining safely handles whenextraorminAmountRequiredis undefined.
181-192: Cache storage implementation is correct.The signature is properly cached with the deadline from
validBeforeand themaxAmountRequired, which aligns with the retrieval threshold logic.
232-246: Clean parameter threading.The
storageparameter is properly threaded through tosignPaymentHeaderwith accurate JSDoc documentation.
joaquim-verges
force-pushed
the
Cache_and_reuse_x402_permit_signatures_for_upto_schemes
branch
from
390dc19 to
e681bbd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/thirdweb/src/x402/fetchWithPayment.ts (1)
64-69: Critical: Storage default inconsistency remains unfixed.The past review flagged this exact issue as addressed, but the bug is still present:
- Line 143 defaults to
webLocalStoragewhenoptions?.storageis undefined- Line 165 only clears the cache when
options?.storageis explicitly provided (truthy)Impact: When callers don't pass
options.storage, permit signatures are cached inwebLocalStoragebut never invalidated on a second 402 response. This breaks the intended cache-invalidation behavior for the default case.Apply this diff to compute effective storage once and use it consistently:
export function wrapFetchWithPayment( fetch: typeof globalThis.fetch, client: ThirdwebClient, wallet: Wallet, options?: { maxValue?: bigint; paymentRequirementsSelector?: ( paymentRequirements: RequestedPaymentRequirements[], ) => RequestedPaymentRequirements | undefined; /** * Storage for caching permit signatures (for "upto" scheme). * When provided, permit signatures will be cached and reused if the on-chain allowance is sufficient. + * Defaults to `webLocalStorage` in browser environments. */ storage?: AsyncStorage; }, ) { return async (input: RequestInfo, init?: RequestInit) => { + const effectiveStorage = options?.storage ?? webLocalStorage; + const response = await fetch(input, init); // ... (existing code) const paymentHeader = await createPaymentHeader( client, account, selectedPaymentRequirements, x402Version, - options?.storage ?? webLocalStorage, + effectiveStorage, ); // ... (existing code) const secondResponse = await fetch(input, newInit); // If payment was rejected (still 402), clear cached signature - if (secondResponse.status === 402 && options?.storage) { - await clearPermitSignatureFromCache(options.storage, { + if (secondResponse.status === 402) { + await clearPermitSignatureFromCache(effectiveStorage, { chainId: paymentChainId, asset: selectedPaymentRequirements.asset, owner: getAddress(account.address), spender: getAddress(selectedPaymentRequirements.payTo), }); } return secondResponse; }; }Also applies to: 143-172
🧹 Nitpick comments (1)
packages/thirdweb/src/x402/sign.ts (1)
115-149: Consider clarifying the threshold logic and improving type safety.The caching reuse logic is sound but has two areas for improvement:
Type safety issue (lines 134-138): The type assertion assumes
extracan containminAmountRequired, but the actual type (ERC20TokenAmount["asset"]["eip712"]) doesn't include this property. This is fragile and could break if the type evolves.Missing explanation (lines 139-141): The logic prefers
minAmountRequiredovermaxAmountRequiredfor the allowance threshold, but this choice isn't documented. Adding a comment would help future maintainers understand why.Apply this diff to improve type safety and clarity:
// Determine threshold - use minAmountRequired if present, else maxAmountRequired const extra = paymentRequirements.extra as | (ERC20TokenAmount["asset"]["eip712"] & { minAmountRequired?: string; }) | undefined; + // For "upto" schemes, prefer minAmountRequired (actual payment) over maxAmountRequired (permit ceiling) + // to avoid requesting a new signature when the cached permit still covers the minimum payment. const threshold = extra?.minAmountRequired ? BigInt(extra.minAmountRequired) : BigInt(paymentRequirements.maxAmountRequired);Additionally, consider defining a proper type for the extended
extrafield inschemas.tsortypes.ts:// In types.ts or schemas.ts export type UptoSchemeExtra = ERC20TokenAmount["asset"]["eip712"] & { minAmountRequired?: string; };Then use it:
- const extra = paymentRequirements.extra as - | (ERC20TokenAmount["asset"]["eip712"] & { - minAmountRequired?: string; - }) - | undefined; + const extra = paymentRequirements.extra as UptoSchemeExtra | undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.changeset/warm-clouds-judge.md(1 hunks)apps/portal/src/app/x402/server/page.mdx(1 hunks)packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts(3 hunks)packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts(3 hunks)packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx(2 hunks)packages/thirdweb/src/x402/fetchWithPayment.ts(4 hunks)packages/thirdweb/src/x402/permitSignatureStorage.ts(1 hunks)packages/thirdweb/src/x402/sign.ts(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/warm-clouds-judge.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts
- packages/thirdweb/src/x402/permitSignatureStorage.ts
- packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx
- apps/portal/src/app/x402/server/page.mdx
- packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript 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 in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.ts
🧬 Code graph analysis (2)
packages/thirdweb/src/x402/sign.ts (3)
packages/thirdweb/src/x402/types.ts (2)
x402Version(14-14)ERC20TokenAmount(125-138)packages/thirdweb/src/x402/permitSignatureStorage.ts (3)
PermitCacheKeyParams(16-21)getPermitSignatureFromCache(40-54)savePermitSignatureToCache(64-82)packages/thirdweb/src/chains/utils.ts (1)
getCachedChain(79-89)
packages/thirdweb/src/x402/fetchWithPayment.ts (1)
packages/thirdweb/src/x402/permitSignatureStorage.ts (1)
clearPermitSignatureFromCache(89-99)
⏰ 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). (7)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Build Packages
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/thirdweb/src/x402/fetchWithPayment.ts (1)
3-7: LGTM: Imports are appropriate.The new imports support the caching and address normalization requirements for the x402 permit signature flow.
packages/thirdweb/src/x402/sign.ts (5)
103-112: LGTM: Cache setup is correct.The cache key parameters properly identify a unique permit signature by chainId, asset, owner, and spender. The
shouldCacheflag correctly restricts caching to the "upto" scheme with explicit storage.
118-131: LGTM: Deadline and allowance validation is correct.The cache reuse logic properly validates both temporal (deadline) and on-chain state (allowance) before reusing a cached signature. The RPC call to check allowance is necessary for security, even though it adds latency.
172-191: LGTM: Signature caching logic is correct.The code properly persists the newly generated signature along with its deadline and max amount. The try-catch in
savePermitSignatureToCache(from the storage module) ensures caching failures don't break the signing flow.
6-6: LGTM: Imports support the new caching functionality.The imports for
allowance(on-chain check),AsyncStorage(caching interface), and permit signature storage utilities are all necessary for the caching implementation.Also applies to: 10-10, 14-18
81-81: LGTM: Storage parameter threading is correct.The
storage?: AsyncStorageparameter is properly threaded throughsignPaymentHeaderandcreatePaymentHeaderto enable optional signature caching.Also applies to: 236-236
joaquim-verges
force-pushed
the
Cache_and_reuse_x402_permit_signatures_for_upto_schemes
branch
from
e681bbd to
a9ab172
Compare
joaquim-verges
deleted the
Cache_and_reuse_x402_permit_signatures_for_upto_schemes
branch
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/thirdweb/src/x402/fetchWithPayment.ts (1)
64-69: Critical: Cache invalidation doesn't use the same storage as caching (default case bug)This issue was previously flagged but not fully resolved. When
options.storageis not provided:
- Line 143: Signatures are cached to
webLocalStorage(via the??fallback)- Line 165: Cache clearing only occurs when
options?.storageis explicitly truthy- Result: Cached signatures in
webLocalStorageare never invalidated on subsequent 402 responsesThis breaks the intended "clear on second 402" behavior for the default storage case.
Fix: Compute the effective storage once and use it consistently:
export function wrapFetchWithPayment( fetch: typeof globalThis.fetch, client: ThirdwebClient, wallet: Wallet, options?: { maxValue?: bigint; paymentRequirementsSelector?: ( paymentRequirements: RequestedPaymentRequirements[], ) => RequestedPaymentRequirements | undefined; storage?: AsyncStorage; }, ) { return async (input: RequestInfo, init?: RequestInit) => { + const storage = options?.storage ?? webLocalStorage; + const response = await fetch(input, init); // ... const paymentHeader = await createPaymentHeader( client, account, selectedPaymentRequirements, x402Version, - options?.storage ?? webLocalStorage, + storage, ); // ... - if (secondResponse.status === 402 && options?.storage) { - await clearPermitSignatureFromCache(options.storage, { + if (secondResponse.status === 402) { + await clearPermitSignatureFromCache(storage, { chainId: paymentChainId, asset: selectedPaymentRequirements.asset, owner: getAddress(account.address), spender: getAddress(selectedPaymentRequirements.payTo), }); }Also applies to: 143-143, 165-172
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.changeset/warm-clouds-judge.md(1 hunks)apps/portal/src/app/x402/server/page.mdx(1 hunks)packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts(3 hunks)packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts(3 hunks)packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx(2 hunks)packages/thirdweb/src/x402/fetchWithPayment.ts(4 hunks)packages/thirdweb/src/x402/permitSignatureStorage.ts(1 hunks)packages/thirdweb/src/x402/sign.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts
- packages/thirdweb/src/x402/permitSignatureStorage.ts
- packages/thirdweb/src/react/web/hooks/x402/useFetchWithPayment.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript 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 in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/x402/sign.tspackages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts
🧬 Code graph analysis (2)
packages/thirdweb/src/x402/sign.ts (5)
packages/thirdweb/src/x402/schemas.ts (2)
RequestedPaymentRequirements(40-42)RequestedPaymentPayload(24-26)packages/thirdweb/src/x402/types.ts (2)
x402Version(14-14)ERC20TokenAmount(125-138)packages/thirdweb/src/x402/permitSignatureStorage.ts (3)
PermitCacheKeyParams(16-21)getPermitSignatureFromCache(40-54)savePermitSignatureToCache(64-82)packages/thirdweb/src/exports/extensions/erc20.ts (1)
allowance(35-35)packages/thirdweb/src/exports/x402.ts (1)
ERC20TokenAmount(17-17)
packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts (1)
packages/thirdweb/src/react/core/hooks/x402/useFetchWithPaymentCore.ts (1)
useFetchWithPaymentCore(41-159)
⏰ 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, webpack)
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/portal/src/app/x402/server/page.mdx (1)
135-135: LGTM!The documentation accurately reflects the new automatic caching behavior for permit signatures, correctly noting that backend storage is no longer needed for this purpose.
packages/thirdweb/src/react/native/hooks/x402/useFetchWithPayment.ts (1)
97-100: LGTM!The storage defaulting logic is correct and consistent with the web implementation. When
options?.storageis not provided, it properly falls back tonativeLocalStoragefor the React Native environment.packages/thirdweb/src/x402/sign.ts (1)
228-246: LGTM!The storage parameter is correctly threaded through
createPaymentHeadertosignPaymentHeader, enabling the caching behavior for the "upto" scheme.
PR-Codex overview
This PR introduces caching for permit signatures in the
thirdweblibrary, specifically for the "upto" scheme. It enhances theuseFetchWithPaymentfunctionality to automatically reuse cached signatures, improving efficiency in payment handling.Detailed summary
storageoption for caching permit signatures inuseFetchWithPaymentandwrapFetchWithPayment.signPaymentHeaderto reuse signatures based on allowance checks.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.