Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/thirdweb/src/x402/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export async function decodePaymentRequest(
description: description ?? "",
mimeType: mimeType ?? "application/json",
payTo: getAddress(facilitator.address), // always pay to the facilitator address first
maxTimeoutSeconds: maxTimeoutSeconds ?? 300,
maxTimeoutSeconds: maxTimeoutSeconds ?? 86400,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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:

  1. Security: A 24-hour validity window for payment authorizations significantly increases the window for potential replay attacks or misuse of stale authorizations.
  2. User Experience: Users expecting near-instant responses may experience confusion with such a long timeout.
  3. 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

  1. EIP‑2612 specification (permit, deadline discussion). [EIP‑2612]. [2]
  2. OpenZeppelin / ERC20Permit implementation notes (deadline requirement). [3]
  3. Developer examples showing common deadline usage (now + 3600s). [7]
  4. EIP‑3009 specification (validAfter/validBefore fields and rationale). [4]
  5. 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.ts

Length of output: 10404


🏁 Script executed:

#!/bin/bash
# Search for references to maxTimeoutSeconds to understand its use case
rg -n "maxTimeoutSeconds" --type ts -C 3

Length 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 2

Length 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.ts and settle-payment.ts use maxTimeoutSeconds: 300 (5 minutes)
  • The code sets validBefore for signature validity (line 38 in sign.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.

asset: getAddress(asset.address),
outputSchema: {
input: {
Expand Down
2 changes: 1 addition & 1 deletion packages/thirdweb/src/x402/sign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function preparePaymentHeader(
nonce: Hex,
): UnsignedPaymentPayload {
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();
Comment on lines 34 to 36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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:

  1. Stale Authorization Risk: A signed authorization remains valid for 24 hours after creation, even if conditions have changed (account balance, user intent, etc.).
  2. Replay Attack Surface: The extended validity window increases opportunities for replay attacks if authorizations are intercepted or leaked.
  3. 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:

  1. Ethereum clients reject blocks with timestamps more than 15 seconds in the future
  2. Ethereum timestamp accuracy can vary up to a few tens of seconds
  3. 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:

  1. Extended Authorization Validity: Allows signed authorizations from 24 hours ago to remain valid, even if account conditions or user intent have changed.
  2. Increased Replay Surface: The wide validity window significantly increases exposure to replay attacks if authorizations are leaked or intercepted.
  3. 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.

const validBefore = BigInt(
Math.floor(Date.now() / 1000 + paymentRequirements.maxTimeoutSeconds),
Expand Down
Loading