Update x402 default payment validity window #8317
Conversation
Allows for systems with retries to process at their own pace, and guards against slightly descynced block.timestamp
🦋 Changeset detectedLatest commit: 445e77e 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
0xFirekeeper
added
the
DO NOT MERGE
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. |
WalkthroughThe changes increase timeout and validity window durations in the x402 payment request flow. Default maxTimeoutSeconds is increased from 300 to 86400 seconds, and the validAfter calculation now subtracts 86400 seconds instead of 600 seconds, widening the validity window. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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
📜 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 (2)
packages/thirdweb/src/x402/common.ts(1 hunks)packages/thirdweb/src/x402/sign.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/x402/sign.tspackages/thirdweb/src/x402/common.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/x402/sign.tspackages/thirdweb/src/x402/common.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/x402/sign.tspackages/thirdweb/src/x402/common.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, vite)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
| mimeType: mimeType ?? "application/json", | ||
| payTo: getAddress(facilitator.address), // always pay to the facilitator address first | ||
| maxTimeoutSeconds: maxTimeoutSeconds ?? 300, | ||
| maxTimeoutSeconds: maxTimeoutSeconds ?? 86400, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that 24-hour timeout aligns with security and UX requirements.
The default maxTimeoutSeconds has been increased from 300 seconds (5 minutes) to 86400 seconds (24 hours). While this supports retry scenarios as mentioned in the PR description, consider the following:
- Security: A 24-hour validity window for payment authorizations significantly increases the window for potential replay attacks or misuse of stale authorizations.
- User Experience: Users expecting near-instant responses may experience confusion with such a long timeout.
- Breaking Change: Existing integrations may have assumptions about the 5-minute default that could affect timeout handling, monitoring, or user flows.
Verify that this timeout duration is appropriate for your payment flow. Consider whether a more granular approach (e.g., different defaults for different scenarios) would be more suitable. You may also want to search for best practices around EIP-2612 and EIP-3009 authorization validity windows:
🌐 Web query:
What are the recommended validity window durations for EIP-2612 Permit and EIP-3009 TransferWithAuthorization signatures?
💡 Result:
Short answer
-
EIP‑2612 (permit): the spec leaves deadline to the signer; common practice is to use short expiries (minutes→hours). Example usages and docs commonly show deadline = now + 3600s (1 hour) or shorter (e.g., 5–20 minutes) as best practice. [EIP‑2612][1][2][3]
-
EIP‑3009 (transferWithAuthorization): the spec provides validAfter/validBefore and recommends bounding authorizations; common examples use validBefore = now + 3600s (1 hour) and validAfter = now (or a short delay), i.e. a validity window (minutes→hours) chosen by the app. [EIP‑3009][4][5][6]
Notes
- Neither standard mandates a specific duration — choose based on UX/risk (short windows reduce misuse but increase fragility for relayers). See the EIPs and implementation examples. [1][4][5]
Sources
- EIP‑2612 specification (permit, deadline discussion). [EIP‑2612]. [2]
- OpenZeppelin / ERC20Permit implementation notes (deadline requirement). [3]
- Developer examples showing common deadline usage (now + 3600s). [7]
- EIP‑3009 specification (validAfter/validBefore fields and rationale). [4]
- EIP‑3009 examples and implementations using validBefore = now + 3600s. [5][6]
References
[1] https://eips.ethereum.org/EIPS/eip-2612
[2] https://eips-wg.github.io/EIPs/2612/
[3] https://docs.openzeppelin.com/contracts/ (ERC20Permit)
[4] https://eips.ethereum.org/EIPS/eip-3009
[5] https://eips-wg.github.io/EIPs/3009/
[6] example usages (libraries/docs showing validBefore/deadline = now + 3600s).
🏁 Script executed:
#!/bin/bash
# Inspect the file context to understand maxTimeoutSeconds purpose
cat packages/thirdweb/src/x402/common.tsLength of output: 10404
🏁 Script executed:
#!/bin/bash
# Search for references to maxTimeoutSeconds to understand its use case
rg -n "maxTimeoutSeconds" --type ts -C 3Length of output: 3838
🏁 Script executed:
#!/bin/bash
# Check for any comments or documentation about timeout durations
rg -n "timeout|Timeout" packages/thirdweb/src/x402/ --type ts -C 2Length of output: 1850
Reduce maxTimeoutSeconds default from 86400 to align with EIP-2612/EIP-3009 best practices.
The default timeout was increased from 300 seconds (5 minutes) to 86400 seconds (24 hours). This contradicts established standards:
- EIP-2612 and EIP-3009 best practice is short expiries of minutes→hours, commonly 1 hour or 5-20 minutes
- Your documentation examples in
verify-payment.tsandsettle-payment.tsusemaxTimeoutSeconds: 300(5 minutes) - The code sets
validBeforefor signature validity (line 38 insign.ts), directly affecting authorization window length
A 24-hour validity window creates a significant security gap: signatures remain valid for an entire day, increasing exposure to replay attacks or misuse of stale authorizations. Reduce the default to 3600 (1 hour) or provide documented security justification for the 24-hour window.
🤖 Prompt for AI Agents
In packages/thirdweb/src/x402/common.ts around line 110, the default
maxTimeoutSeconds is set to 86400 (24h) which is inconsistent with
EIP-2612/EIP-3009 best practices and existing examples; change the default to
3600 (1 hour) (or 300 if you prefer 5 minutes) to reduce the authorization
window, and ensure any related logic (e.g., the validBefore calculation in
sign.ts line 38) and documentation/examples (verify-payment.ts and
settle-payment.ts) are updated to reflect the new default so defaults remain
consistent across the codebase.
| const validAfter = BigInt( | ||
| Math.floor(Date.now() / 1000) - 600, // 10 minutes before | ||
| Math.floor(Date.now() / 1000) - 86400, // 24h before in case weird block timestamp behavior | ||
| ).toString(); |
There was a problem hiding this comment.
🧩 Analysis chain
Reconsider the 24-hour validAfter window for authorization signatures.
Setting validAfter to 24 hours in the past is excessive for handling blockchain timestamp desynchronization. Typical blockchain timestamp drift is measured in seconds or minutes, not hours. This creates significant security concerns:
- Stale Authorization Risk: A signed authorization remains valid for 24 hours after creation, even if conditions have changed (account balance, user intent, etc.).
- Replay Attack Surface: The extended validity window increases opportunities for replay attacks if authorizations are intercepted or leaked.
- Non-standard Practice: EIP-2612 and EIP-3009 implementations typically use much shorter validity windows (often 10-30 minutes maximum) to limit exposure.
The comment "weird block timestamp behavior" is vague. If this addresses a specific blockchain issue, it should be documented with details about which chains and what specific behavior is being mitigated.
Consider reducing this to a more conservative value (e.g., 600-1800 seconds) that still handles reasonable clock skew while limiting security exposure. Run the following to check for any documented requirements:
🏁 Script executed:
#!/bin/bash
# Search for documentation or comments about timestamp handling in x402 module
rg -n -C3 -g '*.{ts,md}' 'timestamp|validAfter|block.timestamp' packages/thirdweb/src/x402/Length of output: 3504
I'll verify the EIP-3009 specification and search for additional context in the codebase.
Now let me search for tests and related documentation in the x402 module to understand if there's a specific reason for this 24-hour window:
Let me search the codebase for tests and any specific rationale for the 24-hour window:Based on my thorough verification, I can now provide the rewritten review comment. The web search results confirm:
- Ethereum clients reject blocks with timestamps more than 15 seconds in the future
- Ethereum timestamp accuracy can vary up to a few tens of seconds
- Standard EIP-3009 implementations use validBefore with a 1-hour window and typically set validAfter to 0
The 24-hour (86400 seconds) backward window is significantly excessive for handling blockchain timestamp drift and introduces unnecessary security risks. The original review comment's concerns are valid and substantiated by industry standards.
Reduce the 24-hour validAfter window to a more reasonable value consistent with blockchain standards.
Ethereum clients only allow block timestamps up to 15 seconds in the future, and actual timestamp accuracy varies only up to a few tens of seconds. Setting validAfter to 24 hours in the past creates security risks:
- Extended Authorization Validity: Allows signed authorizations from 24 hours ago to remain valid, even if account conditions or user intent have changed.
- Increased Replay Surface: The wide validity window significantly increases exposure to replay attacks if authorizations are leaked or intercepted.
- Deviation from Standards: Standard EIP-3009 implementations use 1-hour validity windows and commonly set validAfter to 0, not 24 hours backward.
Consider reducing validAfter to a conservative value like 600-1800 seconds to handle reasonable clock skew while limiting security exposure. Update the comment to explain the specific blockchain issue being addressed, if applicable.
🤖 Prompt for AI Agents
packages/thirdweb/src/x402/sign.ts lines 34-36: the code sets validAfter to 24
hours in the past which is excessive and increases replay/authorization risk;
change the logic to use a much smaller backward skew (e.g., 600–1800 seconds) or
set validAfter to 0 per common EIP-3009 practice, update the numeric value and
accompanying comment to reflect the new conservative skew, and adjust any tests
or docs that assume the 24-hour window.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/good-ducks-eat.md (1)
1-5: Changeset structure and versioning look good.The changeset properly documents the patch bump for the thirdweb package, and the patch version level is appropriate for default parameter value changes that don't modify the public API surface.
Consider enhancing the description to be more specific about the business rationale—e.g., "increases x402 payment timeout from 5 to 1440 minutes to allow retries to complete and guard against block.timestamp desyncs" would give future maintainers clearer context than the current description.
📜 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 (1)
.changeset/good-ducks-eat.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.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/good-ducks-eat.md
⏰ 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). (3)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8317 +/- ##
=======================================
Coverage 54.89% 54.89%
=======================================
Files 919 919
Lines 60697 60697
Branches 4129 4129
=======================================
Hits 33318 33318
Misses 27277 27277
Partials 102 102
🚀 New features to boost your workflow:
|
Allows for systems with retries to process at their own pace, and guards against slightly descynced block.timestamp
PR-Codex overview
This PR updates the
maxTimeoutSecondsfor thex402transfer process to allow for a longer authorization timeout, changing it from 300 seconds to 86400 seconds (24 hours). This adjustment aims to accommodate potential issues with block timestamp behavior.Detailed summary
maxTimeoutSecondsfrom 300 to 86400 inpackages/thirdweb/src/x402/common.ts.validAfterinpackages/thirdweb/src/x402/sign.tsto allow for a 24-hour buffer instead of 10 minutes.Summary by CodeRabbit