Skip to content

Conversation

@joaquim-verges
Copy link
Member

@joaquim-verges joaquim-verges commented Dec 4, 2025


PR-Codex overview

This PR focuses on enhancing the handling of payment methods by introducing an optional quote property and a hasEnoughBalance boolean. It updates types and logic to improve balance checks and ensures better integration of token pricing.

Detailed summary

  • Changed quote to optional in Bridge/types.ts.
  • Introduced hasEnoughBalance boolean in Bridge/types.ts.
  • Updated SwapWidget.tsx to use hasEnoughBalance.
  • Replaced balance calculation logic with hasEnoughBalance in TokenSelection.tsx.
  • Updated PaymentSelection.tsx to use TokenWithPrices instead of Token.
  • Modified usePaymentMethods to handle optional quote.
  • Enhanced the logic for filtering and sorting payment methods based on balance and token prices.

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

@changeset-bot
Copy link

changeset-bot bot commented Dec 4, 2025

🦋 Changeset detected

Latest commit: 130de87

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

@vercel
Copy link

vercel bot commented Dec 4, 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 Dec 4, 2025 0:54am
nebula Ready Ready Preview Comment Dec 4, 2025 0:54am
thirdweb_playground Ready Ready Preview Comment Dec 4, 2025 0:54am
thirdweb-www Ready Ready Preview Comment Dec 4, 2025 0:54am
wallet-ui Ready Ready Preview Comment Dec 4, 2025 0:54am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch _SDK_Optimize_payment_methods_query_by_removing_quotes_and_improving_balance_checks

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 Dec 4, 2025
Copy link
Member Author

joaquim-verges commented Dec 4, 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.

@joaquim-verges joaquim-verges marked this pull request as ready for review December 4, 2025 00:31
@joaquim-verges joaquim-verges requested review from a team as code owners December 4, 2025 00:31
@graphite-app graphite-app bot changed the base branch from _SDK_Add_payment_selection_tracking to graphite-base/8493 December 4, 2025 00:32
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

size-limit report 📦

Path Size
@thirdweb-dev/nexus (esm) 105.66 KB (0%)
@thirdweb-dev/nexus (cjs) 319.47 KB (0%)

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 0% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.62%. Comparing base (551ec68) to head (130de87).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...thirdweb/src/react/core/hooks/usePaymentMethods.ts 0.00% 19 Missing ⚠️
...web/ui/Bridge/payment-selection/TokenSelection.tsx 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8493      +/-   ##
==========================================
- Coverage   54.62%   54.62%   -0.01%     
==========================================
  Files         920      920              
  Lines       61135    61142       +7     
  Branches     4145     4143       -2     
==========================================
  Hits        33397    33397              
- Misses      27636    27643       +7     
  Partials      102      102              
Flag Coverage Δ
packages 54.62% <0.00%> (-0.01%) ⬇️
Files with missing lines Coverage Δ
...b/ui/Bridge/payment-selection/PaymentSelection.tsx 6.96% <ø> (ø)
...web/ui/Bridge/payment-selection/TokenSelection.tsx 5.49% <0.00%> (+0.08%) ⬆️
...thirdweb/src/react/core/hooks/usePaymentMethods.ts 5.83% <0.00%> (-0.54%) ⬇️
🚀 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.

Comment on lines +129 to +136
const requiredUsdValue =
(destinationToken.prices?.["USD"] ?? 0) * Number(destinationAmount);

return finalQuotes.map((x) => {
const tokenUsdValue =
(x.originToken.prices?.["USD"] ?? 0) *
Number(toTokens(x.balance, x.originToken.decimals));
const hasEnoughBalance = tokenUsdValue >= requiredUsdValue;
Copy link

Choose a reason for hiding this comment

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

Suggested change
const requiredUsdValue =
(destinationToken.prices?.["USD"] ?? 0) * Number(destinationAmount);
return finalQuotes.map((x) => {
const tokenUsdValue =
(x.originToken.prices?.["USD"] ?? 0) *
Number(toTokens(x.balance, x.originToken.decimals));
const hasEnoughBalance = tokenUsdValue >= requiredUsdValue;
return finalQuotes.map((x) => {
let hasEnoughBalance = false;
// Priority 1: Use quote's originAmount if available (most accurate)
if (x.quote) {
hasEnoughBalance = x.balance >= x.quote.originAmount;
} else {
// Priority 2: Fall back to USD comparison only if destination price is available
const destinationUsdPrice = destinationToken.prices?.["USD"];
if (destinationUsdPrice && destinationUsdPrice > 0) {
const requiredUsdValue =
destinationUsdPrice * Number(destinationAmount);
const tokenUsdPrice = x.originToken.prices?.["USD"] || 0;
const tokenUsdValue =
tokenUsdPrice * Number(toTokens(x.balance, x.originToken.decimals));
hasEnoughBalance = tokenUsdValue >= requiredUsdValue;
}
// Priority 3: If destination price is missing, default to false (fail-safe)
}

The balance check calculation is fundamentally flawed: it compares USD values between tokens without accounting for fees, slippage, and price impact. Additionally, if the destination token's USD price is missing or zero, the check incorrectly marks all tokens as having sufficient balance.

View Details

Analysis

Balance check calculation doesn't account for fees and fails with missing destination token price

What fails: The usePaymentMethods hook's hasEnoughBalance calculation (lines 129-136) incorrectly marks tokens as having sufficient balance when destination token price is missing, and ignores fees/slippage when calculating required amount.

How to reproduce:

  1. Payment method with destination token that has no USD price (prices["USD"] = undefined)
  2. Call usePaymentMethods with any token with non-zero balance and price
  3. Result: hasEnoughBalance = true even though destination amount is unknown

What happens:

  • When destinationToken.prices?.["USD"] is undefined, requiredUsdValue = 0
  • Any token with positive balance gets marked as sufficient (hasEnoughBalance = true)
  • Button is enabled allowing user to attempt swap that will fail

Expected behavior: When destination token price is missing, assume insufficient balance (fail-safe). Additionally, when quote data is available, use quote.originAmount (which includes fees from the API's calculation) instead of naive USD comparison.

Why this matters: Users select payment methods that appear to have enough balance but fail at transaction time, resulting in poor UX. Per Bridge Quote documentation, originAmount is "the necessary originAmount to receive the desired destinationAmount" and includes all fees.

Solution implemented:

  1. Use quote.originAmount when available (most accurate, accounts for all fees)
  2. Fall back to USD comparison only if destination price exists and is > 0
  3. Default to hasEnoughBalance = false if destination price is missing (fail-safe)

@joaquim-verges joaquim-verges force-pushed the _SDK_Optimize_payment_methods_query_by_removing_quotes_and_improving_balance_checks branch from fe7c3b3 to 130de87 Compare December 4, 2025 00:41
@joaquim-verges joaquim-verges changed the base branch from graphite-base/8493 to _SDK_Add_payment_selection_tracking December 4, 2025 00:42
@socket-security
Copy link

@joaquim-verges joaquim-verges deleted the _SDK_Optimize_payment_methods_query_by_removing_quotes_and_improving_balance_checks branch December 4, 2025 01:00
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