fix(wallet): handle missing wallet, wrong network, trustline, balance, and user rejection - #150
Conversation
…, and user rejection - Add lib/stellar/stellarErrors.js: reusable error mapping utility - StellarProvider: wallet detection, network mismatch check, validateForPayment() - PaymentModal: no-wallet install prompts, network mismatch warning, trustline/balance checks - WalletConnectButton: install prompts when no extension, network mismatch badge - useStellarPayment: graceful user rejection handling
|
@BountySpaghetti is attempting to deploy a commit to the Deen Bridge Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughChangesStellar wallet payment flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WalletConnectButton
participant StellarProvider
participant PaymentModal
participant useStellarPayment
User->>WalletConnectButton: Open wallet controls
WalletConnectButton->>StellarProvider: Connect or inspect wallet state
StellarProvider-->>WalletConnectButton: Connection, extension, and network status
User->>PaymentModal: Open payment
PaymentModal->>StellarProvider: validateForPayment(price)
StellarProvider-->>PaymentModal: Wallet, network, trustline, and balance issues
User->>PaymentModal: Confirm valid payment
PaymentModal->>useStellarPayment: executePayment(paymentData)
useStellarPayment-->>PaymentModal: Success or mapped error
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 5
🧹 Nitpick comments (3)
components/stellar/PaymentModal.jsx (2)
286-306: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a stable key over array index for
preCheckIssues.map.Static analysis flags the array-index key here.
issue.type(orissue.title) is a more stable identity thanidxfor this list.♻️ Proposed fix
- {preCheckIssues.map((issue, idx) => ( - <div - key={idx} + {preCheckIssues.map((issue) => ( + <div + key={issue.type ?? issue.title}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/stellar/PaymentModal.jsx` around lines 286 - 306, Update the key in the preCheckIssues.map rendering within PaymentModal to use a stable issue identity such as issue.type or issue.title instead of the array index idx, while preserving the existing issue content and styling.Source: Linters/SAST tools
281-350: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrustline/balance warnings can render twice.
When
walletInfo.hasTrustlineis false,validateForPaymentpushes atrustline_missingissue intopreCheckIssues, which is rendered by the "Pre-check Issues" block (Lines 281-308). The "Wallet Connected - Preview" panel immediately below independently re-checks!walletInfo?.hasTrustline(Lines 339-343) andhasInsufficientBalance(Lines 344-348), so the same warning can show up twice on screen simultaneously. Consider sourcing these two inline checks frompreCheckIssues(or dropping the pre-check list in favor of the inline messages) to avoid redundant/conflicting copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/stellar/PaymentModal.jsx` around lines 281 - 350, Remove the duplicate trustline and balance warnings in the “Wallet Connected - Preview” panel by reusing the existing preCheckIssues results or eliminating the inline warning elements. Update the relevant rendering around walletInfo.hasTrustline and hasInsufficientBalance while preserving the pre-check issue display and avoiding redundant messages.components/stellar/WalletConnectButton.jsx (1)
67-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWallet install prompt duplicates markup already added in
PaymentModal.jsx.This "no wallet extension" dropdown re-implements the same three wallet links (Freighter, xBull, Albedo) and copy that
PaymentModal.jsx(Lines 189-234) also hardcodes. Both already shareWALLET_INSTALL_LINKSas the data source; extracting a small shared<WalletInstallPrompt />component (or a helper that maps overWALLET_INSTALL_LINKS) would keep the two surfaces in sync as wallets are added/removed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/stellar/WalletConnectButton.jsx` around lines 67 - 115, Extract the duplicated wallet-install prompt markup into a shared WalletInstallPrompt component or rendering helper, using WALLET_INSTALL_LINKS to map the Freighter, xBull, and Albedo entries and preserve the existing copy and actions. Replace the inline dropdown in WalletConnectButton and the corresponding hardcoded section in PaymentModal with the shared implementation, keeping the existing “Already installed? Connect” behavior where applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/stellar/StellarProvider.jsx`:
- Around line 291-299: Replace the floating-point comparisons in the
insufficient-balance check within the StellarProvider payment precheck with
exact decimal or base-unit arithmetic using the existing price and
walletInfo.usdcBalance values as strings. Preserve the current
insufficient_balance issue and message behavior, while ensuring equality and
fractional USDC amounts are compared accurately; do not introduce secret-key
handling or other floating-point amount arithmetic.
- Around line 121-126: Update the auth flow in StellarProvider around
StellarWalletsKit.authModal() to stop converting rejected authentication into an
empty success result. Let authModal() rejections propagate to the existing outer
catch, and only clear networkMismatch and setWalletNetwork after successful
authentication returns a valid address.
In `@hooks/useStellarPayment.js`:
- Around line 90-107: Update executePayment in useStellarPayment.js so it
returns a structured result distinguishing success, user cancellation, and
mapped or unexpected failures instead of returning false from every catch
branch. Preserve the existing toast behavior, include mapped error details and
the original error where needed, and expose cancellation through a cancelled
indicator so PaymentModal.handleConfirm can branch on result.cancelled or
result.error.
- Around line 41-49: Align the mapped-error toast field usage in the payment
handlers, including initializePayment and executePayment: use the same field
from mapStellarError() for the toast’s primary message in both places, while
preserving the existing nextStep description.
In `@lib/stellar/stellarErrors.js`:
- Around line 151-161: Make wallet-absence and cancellation detection mutually
exclusive in isNoWalletError and the rejection helper at
lib/stellar/stellarErrors.js lines 151-161 and 167-178. Remove the catch-all
error.code === -1 from isNoWalletError or require an explicit wallet-absence
signal, and update the rejection helper to recognize only a rejection-specific
code or message rather than the same ambiguous code.
---
Nitpick comments:
In `@components/stellar/PaymentModal.jsx`:
- Around line 286-306: Update the key in the preCheckIssues.map rendering within
PaymentModal to use a stable issue identity such as issue.type or issue.title
instead of the array index idx, while preserving the existing issue content and
styling.
- Around line 281-350: Remove the duplicate trustline and balance warnings in
the “Wallet Connected - Preview” panel by reusing the existing preCheckIssues
results or eliminating the inline warning elements. Update the relevant
rendering around walletInfo.hasTrustline and hasInsufficientBalance while
preserving the pre-check issue display and avoiding redundant messages.
In `@components/stellar/WalletConnectButton.jsx`:
- Around line 67-115: Extract the duplicated wallet-install prompt markup into a
shared WalletInstallPrompt component or rendering helper, using
WALLET_INSTALL_LINKS to map the Freighter, xBull, and Albedo entries and
preserve the existing copy and actions. Replace the inline dropdown in
WalletConnectButton and the corresponding hardcoded section in PaymentModal with
the shared implementation, keeping the existing “Already installed? Connect”
behavior where applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4bfca08b-1b55-4a0a-a439-f30dbc0f7859
📒 Files selected for processing (5)
components/stellar/PaymentModal.jsxcomponents/stellar/StellarProvider.jsxcomponents/stellar/WalletConnectButton.jsxhooks/useStellarPayment.jslib/stellar/stellarErrors.js
| const { address } = await StellarWalletsKit.authModal().catch(() => ({})); | ||
| // If we can reach this, wallet is available. | ||
| // The kit was initialized with the correct network, so if auth succeeds | ||
| // the wallet should be on the right network. | ||
| setNetworkMismatch(false); | ||
| setWalletNetwork(NETWORK); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='components/stellar/StellarProvider.jsx'
wc -l "$file"
sed -n '90,170p' "$file"Repository: Deen-Bridge/dnb-frontend
Length of output: 2555
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='components/stellar/StellarProvider.jsx'
rg -n "isNoWalletError|networkMismatch|setWalletNetwork|authModal|checkNetwork" "$file"
sed -n '1,120p' "$file"
sed -n '170,260p' "$file"Repository: Deen-Bridge/dnb-frontend
Length of output: 7157
Don’t swallow authModal() failures here. catch(() => ({})) turns a rejected auth attempt into the success path, so networkMismatch gets cleared and walletNetwork is set even when wallet auth fails. Let the rejection reach the outer catch, or only update state after a real address is returned.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 125-125: Avoid using the initial state variable in setState
Context: setWalletNetwork(NETWORK)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/stellar/StellarProvider.jsx` around lines 121 - 126, Update the
auth flow in StellarProvider around StellarWalletsKit.authModal() to stop
converting rejected authentication into an empty success result. Let authModal()
rejections propagate to the existing outer catch, and only clear networkMismatch
and setWalletNetwork after successful authentication returns a valid address.
| if ( | ||
| connectedWallet && | ||
| walletInfo && | ||
| parseFloat(walletInfo.usdcBalance || 0) < (price || 0) | ||
| ) { | ||
| issues.push({ | ||
| ...WALLET_ERRORS.insufficient_balance, | ||
| message: `You need $${price} USDC but only have $${parseFloat(walletInfo.usdcBalance || 0).toFixed(2)}.`, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use exact decimal/base-unit comparison for USDC.
parseFloat can miscompare payment amounts and incorrectly block or permit the precheck. Pass decimal strings or base units and compare with exact decimal/integer arithmetic.
As per path instructions, “Flag any deviation from this flow, any secret-key handling, and any amount arithmetic done with floating point.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/stellar/StellarProvider.jsx` around lines 291 - 299, Replace the
floating-point comparisons in the insufficient-balance check within the
StellarProvider payment precheck with exact decimal or base-unit arithmetic
using the existing price and walletInfo.usdcBalance values as strings. Preserve
the current insufficient_balance issue and message behavior, while ensuring
equality and fractional USDC amounts are compared accurately; do not introduce
secret-key handling or other floating-point amount arithmetic.
Source: Path instructions
| const mapped = mapStellarError(error); | ||
| if (mapped) { | ||
| toast.error(mapped.message, { | ||
| description: mapped.nextStep, | ||
| }); | ||
| } else { | ||
| const message = error.response?.data?.message || error.message; | ||
| toast.error(message); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "title|message|nextStep" lib/stellar/stellarErrors.jsRepository: Deen-Bridge/dnb-frontend
Length of output: 2744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== hooks/useStellarPayment.js ==="
nl -ba hooks/useStellarPayment.js | sed -n '1,180p'
echo
echo "=== search for errorDetail / Stellar error UI ==="
rg -n "errorDetail|mapped\.title|mapped\.message|nextStep|WALLET_ERRORS|STELLAR_ERRORS" -S .Repository: Deen-Bridge/dnb-frontend
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== hooks/useStellarPayment.js ==="
sed -n '1,180p' hooks/useStellarPayment.js | cat -n
echo
echo "=== matching error-related references ==="
rg -n "errorDetail|mapped\.title|mapped\.message|nextStep|WALLET_ERRORS|STELLAR_ERRORS" -S hooks lib components pages src .Repository: Deen-Bridge/dnb-frontend
Length of output: 13670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' lib/stellar/stellarErrors.js | cat -nRepository: Deen-Bridge/dnb-frontend
Length of output: 6787
Align Stellar error toast fields hooks/useStellarPayment.js uses different fields for the same mapped error shape: initializePayment shows message, while executePayment shows title. mapStellarError() returns title, message, and nextStep for every entry, so pick one convention and use it in both handlers to keep payment failures consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/useStellarPayment.js` around lines 41 - 49, Align the mapped-error
toast field usage in the payment handlers, including initializePayment and
executePayment: use the same field from mapStellarError() for the toast’s
primary message in both places, while preserving the existing nextStep
description.
| } catch (error) { | ||
| const message = error.response?.data?.message || error.message; | ||
|
|
||
| // Handle specific Stellar errors | ||
| if (message.includes("insufficient") || message.includes("underfunded")) { | ||
| toast.error("Insufficient USDC balance"); | ||
| } else if (message.includes("op_no_trust") || message.includes("trustline")) { | ||
| toast.error( | ||
| "You need to add USDC trustline to your wallet first" | ||
| ); | ||
| } else if (message.includes("rejected") || message.includes("cancelled")) { | ||
| toast.error("Transaction was cancelled"); | ||
| if (isUserRejection(error) || error.code === "USER_REJECTED") { | ||
| toast.info("Transaction cancelled", { | ||
| description: "You declined the signing request. No changes were made.", | ||
| }); | ||
| return false; | ||
| } | ||
|
|
||
| const mapped = mapStellarError(error); | ||
| if (mapped) { | ||
| toast.error(mapped.title, { | ||
| description: mapped.nextStep, | ||
| }); | ||
| } else { | ||
| const message = error.response?.data?.message || error.message; | ||
| toast.error(`Payment failed: ${message}`); | ||
| } | ||
| return false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
executePayment swallows all errors, silently breaking PaymentModal's structured error handling.
Every branch of this catch block returns false (or nothing rethrown) instead of propagating the error. PaymentModal.handleConfirm wraps await executePayment(paymentData) in its own try/catch expecting the promise to reject so it can run mapStellarError/isUserRejection and populate errorDetail (title + nextStep) or return silently to preview on cancellation. Since executePayment never throws, that catch block in PaymentModal.jsx (lines 114-127) is unreachable dead code: every failure — including user cancellation — falls into the generic else branch (setStep("error"); setError("Transaction failed. Please try again.")), overriding the toast's cancellation/mapped messaging with a raw fallback and defeating the “treat rejection as cancellation” and “show mapped nextStep” requirements from the linked issue.
Return a structured result instead of a bare boolean so the caller can distinguish success / cancellation / mapped failure:
🐛 Proposed fix
- } catch (error) {
- if (isUserRejection(error) || error.code === "USER_REJECTED") {
- toast.info("Transaction cancelled", {
- description: "You declined the signing request. No changes were made.",
- });
- return false;
- }
-
- const mapped = mapStellarError(error);
- if (mapped) {
- toast.error(mapped.title, {
- description: mapped.nextStep,
- });
- } else {
- const message = error.response?.data?.message || error.message;
- toast.error(`Payment failed: ${message}`);
- }
- return false;
+ } catch (error) {
+ if (isUserRejection(error) || error.code === "USER_REJECTED") {
+ toast.info("Transaction cancelled", {
+ description: "You declined the signing request. No changes were made.",
+ });
+ return { success: false, cancelled: true };
+ }
+
+ const mapped = mapStellarError(error);
+ if (mapped) {
+ toast.error(mapped.title, {
+ description: mapped.nextStep,
+ });
+ return { success: false, error: mapped };
+ }
+ const message = error.response?.data?.message || error.message;
+ toast.error(`Payment failed: ${message}`);
+ return { success: false, error: { message } };PaymentModal.handleConfirm would then branch on result.cancelled / result.error instead of relying on a thrown exception (see companion comment on that file).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| const message = error.response?.data?.message || error.message; | |
| // Handle specific Stellar errors | |
| if (message.includes("insufficient") || message.includes("underfunded")) { | |
| toast.error("Insufficient USDC balance"); | |
| } else if (message.includes("op_no_trust") || message.includes("trustline")) { | |
| toast.error( | |
| "You need to add USDC trustline to your wallet first" | |
| ); | |
| } else if (message.includes("rejected") || message.includes("cancelled")) { | |
| toast.error("Transaction was cancelled"); | |
| if (isUserRejection(error) || error.code === "USER_REJECTED") { | |
| toast.info("Transaction cancelled", { | |
| description: "You declined the signing request. No changes were made.", | |
| }); | |
| return false; | |
| } | |
| const mapped = mapStellarError(error); | |
| if (mapped) { | |
| toast.error(mapped.title, { | |
| description: mapped.nextStep, | |
| }); | |
| } else { | |
| const message = error.response?.data?.message || error.message; | |
| toast.error(`Payment failed: ${message}`); | |
| } | |
| return false; | |
| } catch (error) { | |
| if (isUserRejection(error) || error.code === "USER_REJECTED") { | |
| toast.info("Transaction cancelled", { | |
| description: "You declined the signing request. No changes were made.", | |
| }); | |
| return { success: false, cancelled: true }; | |
| } | |
| const mapped = mapStellarError(error); | |
| if (mapped) { | |
| toast.error(mapped.title, { | |
| description: mapped.nextStep, | |
| }); | |
| return { success: false, error: mapped }; | |
| } | |
| const message = error.response?.data?.message || error.message; | |
| toast.error(`Payment failed: ${message}`); | |
| return { success: false, error: { message } }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/useStellarPayment.js` around lines 90 - 107, Update executePayment in
useStellarPayment.js so it returns a structured result distinguishing success,
user cancellation, and mapped or unexpected failures instead of returning false
from every catch branch. Preserve the existing toast behavior, include mapped
error details and the original error where needed, and expose cancellation
through a cancelled indicator so PaymentModal.handleConfirm can branch on
result.cancelled or result.error.
| export function isNoWalletError(error) { | ||
| if (!error) return false; | ||
| const msg = typeof error === "string" ? error : error.message || ""; | ||
| return ( | ||
| msg.includes("no wallet") || | ||
| msg.includes("no extension") || | ||
| msg.includes("not installed") || | ||
| msg.includes("undefined") || | ||
| msg.includes("Cannot read") || | ||
| error.code === -1 | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make wallet-absence and cancellation classifications mutually exclusive.
error.code === -1 satisfies both helpers. The provider checks no-wallet first during connection but uses rejection handling during signing, so the same error can become either an install prompt or a cancellation.
lib/stellar/stellarErrors.js#L151-L161: remove the ambiguous-1no-wallet classification or require a specific no-wallet signal.lib/stellar/stellarErrors.js#L167-L178: use a rejection-specific code/message instead of the same catch-all code.
📍 Affects 1 file
lib/stellar/stellarErrors.js#L151-L161(this comment)lib/stellar/stellarErrors.js#L167-L178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/stellar/stellarErrors.js` around lines 151 - 161, Make wallet-absence and
cancellation detection mutually exclusive in isNoWalletError and the rejection
helper at lib/stellar/stellarErrors.js lines 151-161 and 167-178. Remove the
catch-all error.code === -1 from isNoWalletError or require an explicit
wallet-absence signal, and update the rejection helper to recognize only a
rejection-specific code or message rather than the same ambiguous code.
1 participant
Closes #72
What this PR does
Handles all the edge cases that can occur during wallet connection and Stellar USDC payments:
1. No wallet extension installed
2. Wrong network (mainnet vs testnet)
3. Missing USDC trustline
4. Insufficient USDC balance
5. User rejects signing request
Files changed
Summary by CodeRabbit
New Features
Bug Fixes