Skip to content

Conversation

@joaquim-verges
Copy link
Member

@joaquim-verges joaquim-verges commented Nov 20, 2025


PR-Codex overview

This PR introduces a new optional property, fundWalletLink, to the x402 responses in the thirdweb package, enhancing the functionality of payment settlement and verification processes.

Detailed summary

  • Added fundWalletLink to the response in settle-payment.ts.
  • Added fundWalletLink to the response in verify-payment.ts.
  • Updated types.ts to include fundWalletLink as an optional string.
  • Updated schemas.ts to add fundWalletLink in FacilitatorSettleResponseSchema and FacilitatorVerifyResponseSchema.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features
    • Payment verification and settlement error responses now include an optional wallet funding link, enabling users to quickly resolve payment issues.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Nov 20, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs-v2 Ready Ready Preview Comment Nov 20, 2025 2:06am
nebula Ready Ready Preview Comment Nov 20, 2025 2:06am
thirdweb_playground Ready Ready Preview Comment Nov 20, 2025 2:06am
thirdweb-www Ready Ready Preview Comment Nov 20, 2025 2:06am
wallet-ui Ready Ready Preview Comment Nov 20, 2025 2:06am

@changeset-bot
Copy link

changeset-bot bot commented Nov 20, 2025

🦋 Changeset detected

Latest commit: 69a9eff

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
thirdweb Patch
@thirdweb-dev/nebula Patch
@thirdweb-dev/wagmi-adapter Patch
wagmi-inapp Patch

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 20, 2025

Walkthrough

This PR extends the X402 payment module by adding an optional fundWalletLink property to response schemas, types, and implementations. The field is populated in error responses for both settlement and verification failures, sourced from corresponding settlement/verification objects.

Changes

Cohort / File(s) Summary
X402 Payment Schema & Type Definitions
packages/thirdweb/src/x402/schemas.ts, packages/thirdweb/src/x402/types.ts
Added optional fundWalletLink: string field to FacilitatorSettleResponseSchema, FacilitatorVerifyResponseSchema, and PaymentRequiredResult.responseBody type.
X402 Payment Implementation
packages/thirdweb/src/x402/settle-payment.ts, packages/thirdweb/src/x402/verify-payment.ts
Updated error response handling to include fundWalletLink populated from settlement/verification objects in failed response payloads.
Changelog Entry
.changeset/real-bugs-boil.md
Added changeset documenting patch-level update for thirdweb package.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify fundWalletLink field is consistently defined across all schema and type declarations.
  • Confirm the field is properly sourced from settlement.fundWalletLink and verification.fundWalletLink in error response paths.
  • Ensure the optional nature of the field is preserved throughout (no breaking changes to existing callers).

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete. While the PR-Codex section provides technical details, the required template sections (title format confirmation, issue tag, notes for reviewer, and how to test) are missing or unfilled. Fill in the required template sections: confirm the title format matches [SDK/Dashboard/Portal], provide the Linear issue tag (TEAM-0000), add notes for the reviewer, and document testing instructions.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding a fundWalletLink property to x402 responses in the SDK.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch _SDK_Add_fundWalletLink_prop_to_x402_responses

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • TEAM-0000: Entity not found: Issue - Could not find referenced Issue.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Nov 20, 2025
@joaquim-verges joaquim-verges marked this pull request as ready for review November 20, 2025 01:53
@joaquim-verges joaquim-verges requested review from a team as code owners November 20, 2025 01:53
Copy link
Member Author

joaquim-verges commented Nov 20, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
packages/thirdweb/src/x402/types.ts (1)

61-62: Consider clarifying the comment wording.

The comment "link to a wallet to fund the wallet of the payer" is slightly redundant. Consider simplifying to "link to fund the payer's wallet" for improved clarity.

-    /** Optional link to a wallet to fund the wallet of the payer */
+    /** Optional link to fund the payer's wallet */
     fundWalletLink?: string;
packages/thirdweb/src/x402/verify-payment.ts (1)

115-115: LGTM! Consider adding fundWalletLink to the catch block for consistency.

The fundWalletLink is correctly added to the error response when verification fails. However, note that the catch block (lines 120-135) doesn't include fundWalletLink in its error response, which creates an inconsistency. If the facilitator can provide a funding link in exception scenarios, consider adding it there as well.

packages/thirdweb/src/x402/settle-payment.ts (1)

170-170: LGTM! Consider adding fundWalletLink to the catch block for consistency.

The fundWalletLink is correctly added to the error response when settlement fails. However, note that the catch block (lines 175-190) doesn't include fundWalletLink in its error response, which creates an inconsistency. If the facilitator can provide a funding link in exception scenarios, consider adding it there as well.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 82ba2b2 and 69a9eff.

📒 Files selected for processing (5)
  • .changeset/real-bugs-boil.md (1 hunks)
  • packages/thirdweb/src/x402/schemas.ts (1 hunks)
  • packages/thirdweb/src/x402/settle-payment.ts (1 hunks)
  • packages/thirdweb/src/x402/types.ts (1 hunks)
  • packages/thirdweb/src/x402/verify-payment.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-26T19:46:04.024Z
Learnt from: gregfromstl
Repo: thirdweb-dev/js PR: 7450
File: packages/thirdweb/src/bridge/Webhook.ts:57-81
Timestamp: 2025-06-26T19:46:04.024Z
Learning: In the onramp webhook schema (`packages/thirdweb/src/bridge/Webhook.ts`), the `currencyAmount` field is intentionally typed as `z.number()` while other amount fields use `z.string()` because `currencyAmount` represents fiat currency amounts in decimals (like $10.50), whereas other amount fields represent token amounts in wei (very large integers that benefit from bigint representation). The different naming convention (`currencyAmount` vs `amount`) reflects this intentional distinction.

Applied to files:

  • packages/thirdweb/src/x402/schemas.ts
📚 Learning: 2025-08-28T20:50:33.170Z
Learnt from: joaquim-verges
Repo: thirdweb-dev/js PR: 7922
File: apps/playground-web/src/app/ai/ai-sdk/components/chat-container.tsx:167-181
Timestamp: 2025-08-28T20:50:33.170Z
Learning: The thirdweb-dev/ai-sdk-provider schemas use snake_case field naming convention (e.g., chain_id, transaction_hash) rather than camelCase, as defined in the zod schemas in packages/ai-sdk-provider/src/tools.ts.

Applied to files:

  • packages/thirdweb/src/x402/schemas.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: Size
  • GitHub Check: Unit Tests
  • GitHub Check: E2E Tests (pnpm, webpack)
  • GitHub Check: E2E Tests (pnpm, esbuild)
  • GitHub Check: E2E Tests (pnpm, vite)
  • GitHub Check: Lint Packages
  • GitHub Check: Build Packages
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
.changeset/real-bugs-boil.md (1)

1-5: LGTM!

The changeset correctly documents the addition of the fundWalletLink property as a patch-level update.

packages/thirdweb/src/x402/schemas.ts (2)

44-44: LGTM!

The fundWalletLink field is correctly added to the settlement response schema as an optional string, consistent with the type definitions.


52-52: LGTM!

The fundWalletLink field is correctly added to the verification response schema as an optional string, consistent with the type definitions.

@joaquim-verges joaquim-verges merged commit f5f6848 into main Nov 20, 2025
20 of 23 checks passed
@joaquim-verges joaquim-verges deleted the _SDK_Add_fundWalletLink_prop_to_x402_responses branch November 20, 2025 01:56
@joaquim-verges joaquim-verges mentioned this pull request Nov 20, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 20, 2025

size-limit report 📦

Path Size
@thirdweb-dev/nexus (esm) 104.88 KB (0%)
@thirdweb-dev/nexus (cjs) 316.6 KB (0%)

@codecov
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.83%. Comparing base (82ba2b2) to head (69a9eff).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8443   +/-   ##
=======================================
  Coverage   54.83%   54.83%           
=======================================
  Files         919      919           
  Lines       60868    60868           
  Branches     4141     4142    +1     
=======================================
+ Hits        33375    33379    +4     
+ Misses      27391    27387    -4     
  Partials      102      102           
Flag Coverage Δ
packages 54.83% <ø> (+<0.01%) ⬆️
see 3 files with indirect coverage changes
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

packages SDK Involves changes to the thirdweb SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants