feat(wallet): implement live wallet balance, asset management, and escrow summary - #179
Conversation
|
@Josue19-08 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
@Josue19-08 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughReplaces mock wallet UI with live Stellar smart-wallet integration: adds env vars, Stellar lib modules (assets, horizon), React Query wallet hooks (assets, transactions, escrow), updates wallet page and components to fetch and render real balances, activity, and escrow data with loading/error states. Changes
Sequence DiagramsequenceDiagram
actor User
participant WalletPage as Wallet Page
participant Hooks as useWalletData Hooks
participant ReactQuery as React Query Cache
participant StellarRPC as Stellar RPC / Smart Wallet Contract
participant Horizon as Stellar Horizon
participant CoinGecko as CoinGecko API
User->>WalletPage: Open wallet page
rect rgba(100,150,200,0.5)
Note over WalletPage,Hooks: Asset balance flow
WalletPage->>Hooks: useWalletAssets(address)
Hooks->>ReactQuery: check cache
ReactQuery-->>Hooks: cache miss / stale
Hooks->>StellarRPC: getSACBalance / fetchAssetBalance per asset
StellarRPC-->>Hooks: balances (stroops -> units)
Hooks->>CoinGecko: fetchAssetPricesUsd()
CoinGecko-->>Hooks: USD prices
Hooks->>ReactQuery: store WalletAsset[] with usdValue
ReactQuery-->>WalletPage: render assets
end
rect rgba(150,200,100,0.5)
Note over WalletPage,Horizon: Transaction history flow
WalletPage->>Hooks: useWalletTransactions(address)
Hooks->>ReactQuery: check cache
ReactQuery-->>Hooks: cache miss
Hooks->>Horizon: payments().forAccount(address)
Horizon-->>Hooks: payment operations
Hooks->>Hooks: map -> WalletActivity[] (extract counterparty)
Hooks->>ReactQuery: store transactions
ReactQuery-->>WalletPage: render activity
end
rect rgba(200,150,100,0.5)
Note over WalletPage,StellarRPC: Escrow summary flow
WalletPage->>Hooks: useEscrowSummary(address)
Hooks->>ReactQuery: check cache
ReactQuery-->>Hooks: cache miss
Hooks->>StellarRPC: read EscrowLocked key from contract
StellarRPC-->>Hooks: XDR value -> stroops
Hooks->>CoinGecko: get XLM price (fallback)
CoinGecko-->>Hooks: price
Hooks->>ReactQuery: store EscrowSummaryData (totalLocked USD)
ReactQuery-->>WalletPage: render escrow card
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 7
🧹 Nitpick comments (8)
hooks/use-wallet-data.ts (2)
28-35: Transactions don't auto-refresh.
useWalletAssetshasrefetchInterval: 30_000, butuseWalletTransactionshas onlystaleTime: 2 * 60 * 1000with norefetchInterval, so new incoming/outgoing payments only appear on window focus or manual invalidation. PR objectives mention "auto-refreshing balance fetches" — consider adding a refetch interval here (e.g. 30–60 s) for parity, or documenting that history is fetched on-demand.♻️ Proposed refactor
export function useWalletTransactions(address: string | null) { return useQuery({ queryKey: walletKeys.transactions(address ?? ""), queryFn: () => fetchAccountTransactions(address!), enabled: !!address, - staleTime: 2 * 60 * 1000, + staleTime: 2 * 60 * 1000, + refetchInterval: 60 * 1000, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` around lines 28 - 35, useWalletTransactions currently only sets staleTime so transactions won't auto-refresh; update the query in useWalletTransactions (the call to useQuery using walletKeys.transactions and fetchAccountTransactions) to add a refetchInterval (e.g. refetchInterval: 30_000 or 60_000) alongside the existing staleTime and enabled flags so transaction history is polled automatically; ensure you keep enabled: !!address and queryFn: () => fetchAccountTransactions(address!) unchanged and pick the same interval used in useWalletAssets for parity or document the behavior if you prefer on-demand fetching.
56-68: Duplicaterpc.Serverinstantiation.Same observation as for
lib/stellar/assets.ts#fetchAssetBalance: a freshrpc.Server(SMART_WALLET_CONFIG.rpcUrl)is created per call. Extracting a sharedgetSorobanServer()helper (e.g. inlib/smart-wallet/configor a newlib/stellar/soroban.ts) and reusing it from both files keeps the connection setup centralized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` around lines 56 - 68, The code repeatedly instantiates rpc.Server(SMART_WALLET_CONFIG.rpcUrl) (seen in the rpc.Server(...) call and used to call getContractData), so extract and reuse a single Soroban server factory: add a getSorobanServer() helper (e.g. in lib/smart-wallet/config or lib/stellar/soroban.ts) that returns a singleton rpc.Server configured with SMART_WALLET_CONFIG.rpcUrl, replace direct rpc.Server(...) calls in use-wallet-data.ts (the block using walletAddr, key, and server.getContractData) and lib/stellar/assets.ts#fetchAssetBalance to call getSorobanServer() instead, and update imports accordingly so the connection setup is centralized and not re-created per call.app/wallet/page.tsx (1)
48-50:onConnectreturns a Promise whose rejection is unhandled.
connectfromuseSmartWalletis() => Promise<void>, but theButton'sonClickdiscards the returned promise, so any rejection (passkey cancellation, network failure, etc.) surfaces as an unhandled promise rejection with no user feedback. Consider wrapping to catch and optionally surface via a toast.♻️ Proposed fix
- <Button size="lg" onClick={onConnect}> + <Button + size="lg" + onClick={() => { + void onConnect(); + }} + > Connect with Passkey </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/wallet/page.tsx` around lines 48 - 50, The Button's onClick currently calls onConnect which returns a Promise (from useSmartWallet.connect) that may reject and is not handled; wrap the call in a try/catch (or attach .catch) inside the onConnect handler to await connect and handle errors, then surface failures to the user (e.g., show a toast or set an error state) so passkey cancellations or network errors do not cause unhandled promise rejections; update the onConnect implementation used by the Button to perform this error handling and logging..env.example (1)
47-52: Document which smart-wallet vars are required vs optional.All five smart wallet variables are added with empty defaults and no inline documentation.
NEXT_PUBLIC_SMART_ACCOUNT_WASM_HASHandNEXT_PUBLIC_WEBAUTHN_VERIFIER_ADDRESSare presumably required for the wallet to function at all, while indexer/relayer URLs may be optional; a one-line comment per var (like you have for Stellar contracts above) will save new contributors debugging time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 47 - 52, Annotate the smart wallet env vars to indicate which are required vs optional and add a one-line comment for each of NEXT_PUBLIC_SMART_ACCOUNT_WASM_HASH, NEXT_PUBLIC_WEBAUTHN_VERIFIER_ADDRESS, NEXT_PUBLIC_NATIVE_TOKEN_CONTRACT, NEXT_PUBLIC_SMART_WALLET_INDEXER_URL, and NEXT_PUBLIC_SMART_WALLET_RELAYER_URL describing their purpose and whether they are required (e.g., mark NEXT_PUBLIC_SMART_ACCOUNT_WASM_HASH and NEXT_PUBLIC_WEBAUTHN_VERIFIER_ADDRESS as required for wallet operation, and note that indexer/relayer URLs are optional but recommended), following the style used for the Stellar contract comments so new contributors know what to provide.components/wallet/balance-card.tsx (1)
110-126: Empty asset list renders a collapsed grid.When
walletInfo.assetsis empty (no balances yet or all filtered out), the bottom section renders as an emptygridafter the divider. Consider an empty-state message (e.g., "No assets yet — fund your wallet to get started") for a cleaner first-run UX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/balance-card.tsx` around lines 110 - 126, The bottom grid renders nothing when walletInfo.assets is empty; update the BalanceCard component to conditionally render a friendly empty-state message when walletInfo.assets.length === 0 instead of the empty grid. Inside the block that currently maps walletInfo.assets (referencing walletInfo.assets and formatCurrency), replace or wrap the map with a conditional that shows a centered text like "No assets yet — fund your wallet to get started" (styled consistent with existing classes) when length is 0, otherwise render the existing map output; ensure the divider and surrounding layout remain unchanged.lib/stellar/assets.ts (2)
55-73: Cache therpc.Serverinstance.A new
rpc.Serveris constructed on everyfetchAssetBalancecall. With 3 supported assets and a 30 s refetch interval inuseWalletAssets, that's 6 allocations per minute per tab. Mirror the singleton pattern you already use inlib/stellar/horizon.ts:♻️ Proposed refactor
const STROOPS_PER_UNIT = 10_000_000; +let sorobanServerInstance: rpc.Server | null = null; +function getSorobanServer(): rpc.Server { + if (!sorobanServerInstance) { + sorobanServerInstance = new rpc.Server(SMART_WALLET_CONFIG.rpcUrl); + } + return sorobanServerInstance; +} + export async function fetchAssetBalance( walletContractId: string, asset: Asset, ): Promise<number> { try { - const server = new rpc.Server(SMART_WALLET_CONFIG.rpcUrl); + const server = getSorobanServer(); const result = await server.getSACBalance(
fetchEscrowSummaryinhooks/use-wallet-data.tscould consume the same helper (consider exporting it) rather than constructing its own.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/assets.ts` around lines 55 - 73, fetchAssetBalance constructs a new rpc.Server on every call; change it to reuse a cached singleton rpc.Server instance (similar to the pattern used in lib/stellar/horizon.ts) instead of new rpc.Server(... SMART_WALLET_CONFIG.rpcUrl) each time; either import/export the existing helper from horizon.ts or move the singleton helper to a shared module and replace rpc.Server creation in fetchAssetBalance (and in fetchEscrowSummary if present) with the shared getServer/getRpcServer helper so SMART_WALLET_CONFIG.rpcUrl and network usage remains the same while avoiding repeated allocations.
75-91: Remove redundant fetch cache option and address misleading silent fallbacks.The hardcoded price fallbacks silently return stale data if CoinGecko is unreachable, which can mislead users about USD totals with no indication the data is unavailable. Since React Query already controls freshness via
staleTimeandrefetchInterval, either: (1) returnnull/undefinedon failure and let the UI display a "prices unavailable" state, or (2) log failures to observability so prolonged fallback use is detectable.Also remove
cache: "no-store"from the fetch options—it's redundant since React Query already manages when this function is called.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/assets.ts` around lines 75 - 91, In fetchAssetPricesUsd: remove the fetch option { cache: "no-store" } and stop returning silent hardcoded prices; instead change the signature to Promise<Record<string, number> | null>, parse and return the fetched prices when response.ok, and on non-OK or any catch call your observability/logging (e.g. console.error or telemetry) with the error/response details and return null so the UI can show a "prices unavailable" state rather than stale defaults.components/wallet/escrow-summary.tsx (1)
50-56: Usenext/linkfor internal navigation.Plain
<a>triggers a full-page reload and loses Next.js prefetching/client-side transitions. Swap toLinkfor in-app bounty routes.♻️ Proposed fix
"use client"; import { Lock, ExternalLink } from "lucide-react"; +import Link from "next/link"; import { Skeleton } from "@/components/ui/skeleton";- <a + <Link href={`/bounty/${entry.bountyId}`} className="flex items-center gap-1 text-primary hover:underline text-xs" > Bounty #{entry.bountyId.slice(0, 8)} <ExternalLink className="h-3 w-3" /> - </a> + </Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/escrow-summary.tsx` around lines 50 - 56, Replace the plain anchor used for the bounty link with Next.js client-side navigation: import Link from 'next/link' and change the <a href={`/bounty/${entry.bountyId}`} ...> to <Link href={`/bounty/${entry.bountyId}`} className="flex items-center gap-1 text-primary hover:underline text-xs"> so the text "Bounty #{entry.bountyId.slice(0,8)}" and the ExternalLink icon remain inside the Link; keep entry.bountyId and ExternalLink references intact and ensure the new Link import is added at the top of the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/wallet/balance-card.tsx`:
- Around line 62-68: The displayed total uses formatCurrency which always
formats as en-US USD while the caption reads walletInfo.balanceCurrency and the
code adds walletInfo.balance + pendingEarnings (mixing USD with
asset-denominated escrow from fetchEscrowSummary), causing mismatched units; fix
by making formatCurrency accept and use walletInfo.balanceCurrency (or
explicitly render "USD" if you choose to keep USD formatting), and ensure the
sum walletInfo.balance + pendingEarnings is computed in the same currency
(convert escrow/asset values to USD before adding or display them separately) —
update the BalanceCard component usages of formatCurrency, walletInfo.balance,
pendingEarnings, and address the related unit conversion in
fetchEscrowSummary/escrow-summary.tsx so labels and formatted output match the
actual currency.
In `@components/wallet/escrow-summary.tsx`:
- Around line 35-40: The displayed totalLocked is being treated as USD but
fetchEscrowSummary returns the native asset amount (stroops→units) — update the
rendering in escrow-summary.tsx to either convert that unit amount to USD using
the same price feed used elsewhere (call the price lookup from
fetchAssetPricesUsd / the hook that provides asset USD prices, multiply
data.totalLocked by the asset USD price, then pass the result to
formatCurrency), or change the UI to render the native asset unit and label
(avoid formatCurrency and instead format as asset units with the asset symbol);
apply the same fix pattern for pendingEarnings in BalanceCard so amounts and
labels match the underlying denomination.
In `@hooks/use-wallet-data.ts`:
- Around line 49-85: fetchEscrowSummary currently always returns entries: [] —
update it to enumerate per-bounty escrow records for the given wallet and
populate entries with { bountyId, amount, asset, status }. Inside
fetchEscrowSummary, use the contract storage access patterns (e.g., read the
EscrowPool or per-wallet map/vector via server.getContractData or by adding a
helper on the contract client that exposes DepositRecord listings) to fetch all
DepositRecord/EscrowPool items for walletContractId, convert each record's
amount using the correct stroop conversion (use scValToNative but validate types
beyond bigint|number and log/throw on unexpected types), map fields into
entries, and return { totalLocked, entries } (keeping CORE_ESCROW_CONTRACT_ID
checks and the existing error handling). Ensure you reference and use the
EscrowPool and DepositRecord structures and keep scValToNative conversion
consistent with the contract encoding.
In `@lib/stellar/assets.ts`:
- Around line 93-116: The current fetchAllAssetBalances function filters out
zero balances (results.filter((a) => a.amount > 0)), hiding supported tokens;
remove that filter so fetchAllAssetBalances returns the full supported assets
list (including zero-amount entries) and instead apply amount > 0 filtering
where a BalanceCard or similar UI breakdown needs only non-zero balances; update
references to fetchAllAssetBalances and callers (e.g., BalanceCard rendering) to
perform downstream filtering as needed.
In `@lib/stellar/horizon.ts`:
- Around line 49-50: The current assignment for the local variable asset uses
op.asset_type === "native" ? "XLM" : (op.asset_code ?? "XLM"), which incorrectly
labels non-native assets with missing asset_code as "XLM"; change the fallback
to a neutral value (e.g., use "UNKNOWN") or skip emitting the record when
op.asset_code is absent. Update the expression that sets asset (the variable
named asset in this file) to use "UNKNOWN" instead of "XLM" for the non-native
fallback, or add logic to ignore the operation when op.asset_code is undefined.
- Around line 52-60: The mapping in lib/stellar/horizon.ts currently sets
incoming payments to type "earning" which should be "deposit" per ActivityType
(types/wallet.ts). Update the return object where type is assigned (the block
using isIncoming) so that when isIncoming is true it returns "deposit" (as
const) instead of "earning"; leave the false branch as "withdrawal". Ensure any
references to transaction mapping or the variable names op, isIncoming, amount,
asset, transactionHash remain unchanged.
- Around line 45-63: The mapping currently assumes every HorizonOpRecord has an
amount and treats incoming ops as "earning", which silently drops create_account
and account_merge ops and mislabels incoming activity; update the map/filter to
first switch on op.type and only handle supported types (e.g., "payment",
"path_payment_strict_receive", "path_payment_strict_send", "create_account",
"account_merge"), extracting amounts appropriately (use op.amount for
payment/path_payment types, op.starting_balance for create_account, and handle
account_merge semantics or set a sensible amount/description), convert incoming
type label from "earning" to "deposit" to match WalletActivity, and ensure
unsupported op.type values are ignored early (before amount parsing) to avoid
silent drops and incorrect results when mapping response.records
(HorizonOpRecord -> WalletActivity).
---
Nitpick comments:
In @.env.example:
- Around line 47-52: Annotate the smart wallet env vars to indicate which are
required vs optional and add a one-line comment for each of
NEXT_PUBLIC_SMART_ACCOUNT_WASM_HASH, NEXT_PUBLIC_WEBAUTHN_VERIFIER_ADDRESS,
NEXT_PUBLIC_NATIVE_TOKEN_CONTRACT, NEXT_PUBLIC_SMART_WALLET_INDEXER_URL, and
NEXT_PUBLIC_SMART_WALLET_RELAYER_URL describing their purpose and whether they
are required (e.g., mark NEXT_PUBLIC_SMART_ACCOUNT_WASM_HASH and
NEXT_PUBLIC_WEBAUTHN_VERIFIER_ADDRESS as required for wallet operation, and note
that indexer/relayer URLs are optional but recommended), following the style
used for the Stellar contract comments so new contributors know what to provide.
In `@app/wallet/page.tsx`:
- Around line 48-50: The Button's onClick currently calls onConnect which
returns a Promise (from useSmartWallet.connect) that may reject and is not
handled; wrap the call in a try/catch (or attach .catch) inside the onConnect
handler to await connect and handle errors, then surface failures to the user
(e.g., show a toast or set an error state) so passkey cancellations or network
errors do not cause unhandled promise rejections; update the onConnect
implementation used by the Button to perform this error handling and logging.
In `@components/wallet/balance-card.tsx`:
- Around line 110-126: The bottom grid renders nothing when walletInfo.assets is
empty; update the BalanceCard component to conditionally render a friendly
empty-state message when walletInfo.assets.length === 0 instead of the empty
grid. Inside the block that currently maps walletInfo.assets (referencing
walletInfo.assets and formatCurrency), replace or wrap the map with a
conditional that shows a centered text like "No assets yet — fund your wallet to
get started" (styled consistent with existing classes) when length is 0,
otherwise render the existing map output; ensure the divider and surrounding
layout remain unchanged.
In `@components/wallet/escrow-summary.tsx`:
- Around line 50-56: Replace the plain anchor used for the bounty link with
Next.js client-side navigation: import Link from 'next/link' and change the <a
href={`/bounty/${entry.bountyId}`} ...> to <Link
href={`/bounty/${entry.bountyId}`} className="flex items-center gap-1
text-primary hover:underline text-xs"> so the text "Bounty
#{entry.bountyId.slice(0,8)}" and the ExternalLink icon remain inside the Link;
keep entry.bountyId and ExternalLink references intact and ensure the new Link
import is added at the top of the file.
In `@hooks/use-wallet-data.ts`:
- Around line 28-35: useWalletTransactions currently only sets staleTime so
transactions won't auto-refresh; update the query in useWalletTransactions (the
call to useQuery using walletKeys.transactions and fetchAccountTransactions) to
add a refetchInterval (e.g. refetchInterval: 30_000 or 60_000) alongside the
existing staleTime and enabled flags so transaction history is polled
automatically; ensure you keep enabled: !!address and queryFn: () =>
fetchAccountTransactions(address!) unchanged and pick the same interval used in
useWalletAssets for parity or document the behavior if you prefer on-demand
fetching.
- Around line 56-68: The code repeatedly instantiates
rpc.Server(SMART_WALLET_CONFIG.rpcUrl) (seen in the rpc.Server(...) call and
used to call getContractData), so extract and reuse a single Soroban server
factory: add a getSorobanServer() helper (e.g. in lib/smart-wallet/config or
lib/stellar/soroban.ts) that returns a singleton rpc.Server configured with
SMART_WALLET_CONFIG.rpcUrl, replace direct rpc.Server(...) calls in
use-wallet-data.ts (the block using walletAddr, key, and server.getContractData)
and lib/stellar/assets.ts#fetchAssetBalance to call getSorobanServer() instead,
and update imports accordingly so the connection setup is centralized and not
re-created per call.
In `@lib/stellar/assets.ts`:
- Around line 55-73: fetchAssetBalance constructs a new rpc.Server on every
call; change it to reuse a cached singleton rpc.Server instance (similar to the
pattern used in lib/stellar/horizon.ts) instead of new rpc.Server(...
SMART_WALLET_CONFIG.rpcUrl) each time; either import/export the existing helper
from horizon.ts or move the singleton helper to a shared module and replace
rpc.Server creation in fetchAssetBalance (and in fetchEscrowSummary if present)
with the shared getServer/getRpcServer helper so SMART_WALLET_CONFIG.rpcUrl and
network usage remains the same while avoiding repeated allocations.
- Around line 75-91: In fetchAssetPricesUsd: remove the fetch option { cache:
"no-store" } and stop returning silent hardcoded prices; instead change the
signature to Promise<Record<string, number> | null>, parse and return the
fetched prices when response.ok, and on non-OK or any catch call your
observability/logging (e.g. console.error or telemetry) with the error/response
details and return null so the UI can show a "prices unavailable" state rather
than stale defaults.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46f0e01b-ea7e-425f-8148-115e8112a1f6
📒 Files selected for processing (7)
.env.exampleapp/wallet/page.tsxcomponents/wallet/balance-card.tsxcomponents/wallet/escrow-summary.tsxhooks/use-wallet-data.tslib/stellar/assets.tslib/stellar/horizon.ts
- balance-card: show static "USD" caption instead of walletInfo.balanceCurrency - transaction-history: treat "deposit" same as "earning" for green color/sign - use-wallet-data: convert escrow XLM stroops to USD using live price feed - assets: return all assets including zero-balance ones in fetchAllAssetBalances - horizon: use "UNKNOWN" fallback for missing asset_code; map incoming payments to "deposit" type
There was a problem hiding this comment.
Hey @Josue19-08, nice work on the live data wiring. A few things I'd like you to address beyond what CodeRabbit already flagged.
The biggest concern is the ?preview=1 query param in app/wallet/page.tsx. It completely bypasses the wallet auth check (isPreview short-circuits both walletLoading and the connect-wallet prompt), and renders mockWalletWithAssets. That means anyone who hits /wallet?preview=1 sees a fake wallet with no sign-in required, and the mock data still ships in the production bundle. This wasn't in the issue scope and isn't mentioned in the PR description. Please either remove it or gate it behind process.env.NODE_ENV === "development" so it can't be triggered in prod.
In components/wallet/escrow-summary.tsx the "per-bounty escrow" breakdown is sourced from useActiveBountiesQuery, which returns every active bounty on the platform. You then sum rewardAmount across all of them as this user's "Escrow Locked" when data.totalLocked is zero. A random user viewing the page will see the platform's total active rewards as their own locked funds, which is very misleading. This needs to be filtered to bounties where the current wallet is the creator or the escrow funder.
The trackedAssets state + onAssetsChange plumbing in app/wallet/page.tsx is effectively dead code. The ternary !isPreview && assets ? assets : trackedAssets.length > 0 ? trackedAssets : baseInfo.assets only falls to trackedAssets when assets is null, but assetsLoading is already handling that branch with a skeleton. Users adding a trustline via the Manage panel see a toast but the list doesn't update until useWalletAssets refetches (and the asset only appears then if it has a non-zero balance, which defeats the trustline management intent). Either wire the "Add" action into a React Query mutation that invalidates walletKeys.assets(address), or rework the state so the tracked list actually drives the render.
fetchAssetBalance and fetchEscrowSummary both swallow every error with catch {} and return 0 / empty. When the Soroban RPC fails transiently or CoinGecko is down, the user silently sees "0 balance" with no indication something went wrong, and the developer has no console trace to debug. At minimum log the error; ideally surface a "couldn't load" state so refresh is actionable.
useSearchParams in the default export isn't wrapped in a <Suspense> boundary. Same Next.js 15 issue flagged on PR #177. Either extract the body into an inner component wrapped in Suspense or read the param another way.
CI/CD fails because pnpm-lock.yaml is out of sync with package.json after the main merge. Run pnpm install and commit the updated lockfile.
Nice work overall. Ping me when these are in.
Also Sync with main branch |
|
Working on it! |
Replace mock data on the wallet page with real Stellar network data: - Add lib/stellar/assets.ts: lazy-initialized supported assets (XLM, USDC, EURC) with fetchAssetBalance (getSACBalance via Soroban RPC) and fetchAssetPricesUsd (CoinGecko with fallback). - Add lib/stellar/horizon.ts: Horizon API client to fetch payment operations and map them to WalletActivity[]. - Add hooks/use-wallet-data.ts: React Query hooks useWalletAssets, useWalletTransactions, and useEscrowSummary (queries CORE_ESCROW contract via Soroban RPC, falls back gracefully). - Add components/wallet/escrow-summary.tsx: sidebar card showing total escrow-locked funds with per-bounty breakdown. - Update app/wallet/page.tsx: wire real data from useSmartWallet + new hooks; add unauthenticated connect-wallet prompt and loading skeletons. - Update components/wallet/balance-card.tsx: remove hardcoded mock values (pendingEarnings, change24h); accept pendingEarnings and isLoading props; add skeleton loading state. - Update .env.example: document USDC/EURC issuer vars and smart wallet configuration vars. Closes boundlessfi#151
…e management, escrow breakdown - types/wallet.ts: add counterparty field to WalletActivity - lib/stellar/horizon.ts: include counterparty (from/to address) in fetched payment operations - components/wallet/transaction-history.tsx: add Counterparty column showing the other party address via AccountLink (hidden on mobile) - components/wallet/assets-list.tsx: full trustline management — list current trustlines with balances, Manage panel shows all supported assets from config with Active badge or Add button that fetches the SAC balance on-chain and registers the asset - components/wallet/escrow-summary.tsx: per-bounty escrow breakdown using useActiveBountiesQuery (GraphQL); shows each active bounty title, reward amount, currency, and link to its detail page - app/wallet/page.tsx: move useState before early returns (hooks rules), pass walletAddress and onAssetsChange to AssetsList
- balance-card: show static "USD" caption instead of walletInfo.balanceCurrency - transaction-history: treat "deposit" same as "earning" for green color/sign - use-wallet-data: convert escrow XLM stroops to USD using live price feed - assets: return all assets including zero-balance ones in fetchAllAssetBalances - horizon: use "UNKNOWN" fallback for missing asset_code; map incoming payments to "deposit" type
- page.tsx: gate ?preview=1 behind NODE_ENV=development; wrap useSearchParams in Suspense boundary; remove dead trackedAssets state and onAssetsChange plumbing - escrow-summary.tsx: remove misleading platform-wide bounties fallback; show only wallet-specific on-chain escrow data from Soroban contract - assets-list.tsx: replace onAssetsChange callback with React Query cache invalidation (walletKeys.assets) so trustline additions immediately refresh the live asset list - assets.ts, horizon.ts, use-wallet-data.ts: log errors in catch blocks instead of silently swallowing them - pnpm-lock.yaml: sync with package.json after rebase onto main
d5c54fb to
a95d719
Compare
Hi! All 6 points have been addressed:
Ready for re-review! |
|
This is solid @Josue19-08 , preview gating, on-chain escrow query, dead
The per-bounty escrow breakdown in
Errors are now logged but the UI still silently shows UI matches the rest of |
- page.tsx: null-guard providerInfo in preview mode using mockWalletWithAssets fallback so /wallet?preview=1 no longer crashes without a connected wallet - page.tsx: show DataErrorBanner when asset or activity queries fail - escrow-summary.tsx: remove dead per-bounty entries UI (contract always returns entries: []); accept isError prop and show error state - assets-list.tsx: rename "Manage/Add" panel to "Supported Assets / Refresh" with honest copy — the button re-fetches the on-chain balance and invalidates the cache, it does not submit a changeTrust transaction
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
components/wallet/transaction-history.tsx (3)
54-74: CSV export is out of sync with the table — counterparty is missing.The new Counterparty column was added to the UI but
headers/rowshere weren't updated. Users exporting transactions will silently lose the field. Also worth aligning column order with the table for predictability.♻️ Proposed change
const headers = [ "ID", "Type", "Description", "Amount", "Currency", "Date", + "Counterparty", "Transaction", "Status", ]; const rows = filteredActivity.map((item) => [ item.id, item.type, item.description || "", item.amount.toString(), item.currency, formatSafeDate(item.date, "yyyy-MM-dd HH:mm:ss"), + item.counterparty || "", item.transactionHash || "", item.status, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/transaction-history.tsx` around lines 54 - 74, The CSV export in handleExportCsv is missing the new Counterparty column and the headers/rows are out of sync with the UI table; update the headers array and each row mapping in rows (which currently maps filteredActivity items) to include the counterparty field in the correct position and order used by the table (e.g., insert item.counterparty or item.counterparty?.toString() between Description and Amount or wherever the UI shows it), keeping the rest of the fields (id, type, description, amount.toString(), currency, formatSafeDate(item.date, "yyyy-MM-dd HH:mm:ss"), item.transactionHash || "", item.status) intact so CSV columns align with the table.
24-29: Search filter doesn't include the new Counterparty column.The filter matches
description,type, andcurrency, but notcounterparty. Users seeing a counterparty address in the table will reasonably expect to be able to search by it.♻️ Proposed change
const filteredActivity = activity.filter( (item) => item.description?.toLowerCase().includes(search.toLowerCase()) || item.type.toLowerCase().includes(search.toLowerCase()) || - item.currency.toLowerCase().includes(search.toLowerCase()), + item.currency.toLowerCase().includes(search.toLowerCase()) || + item.counterparty?.toLowerCase().includes(search.toLowerCase()), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/transaction-history.tsx` around lines 24 - 29, The search filter in filteredActivity only checks description, type, and currency but omits the counterparty column; update the activity.filter callback (the filteredActivity computation) to also check item.counterparty (e.g., item.counterparty?.toLowerCase().includes(search.toLowerCase())) alongside the existing checks so a search term matches counterparty values while preserving the existing null/undefined-safe checks.
164-172:colSpan={7}doesn't account for the responsively hidden Counterparty column.The Counterparty header/cell uses
hidden lg:table-cell, so on viewports< lgonly 6 columns are rendered while the empty-state row still declarescolSpan={7}. Browsers tolerate this, but it's inconsistent. Consider matching the responsive behavior:♻️ Proposed change
- <tr> - <td - colSpan={7} - className="py-12 text-center text-muted-foreground" - > - No activity found. - </td> - </tr> + <tr> + <td + colSpan={6} + className="py-12 text-center text-muted-foreground lg:hidden" + > + No activity found. + </td> + <td + colSpan={7} + className="py-12 text-center text-muted-foreground hidden lg:table-cell" + > + No activity found. + </td> + </tr>Or, simpler: drop the
hidden lg:table-cellon the Counterparty column so it's always present andcolSpan={7}is always correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/transaction-history.tsx` around lines 164 - 172, The empty-state row uses colSpan={7} but the Counterparty column header/cell is rendered with the responsive class "hidden lg:table-cell", so on small viewports only 6 columns exist; update the empty-state to match that responsive behavior by either making the Counterparty column always present (remove the "hidden lg:table-cell" class on the Counterparty header/cell) so colSpan={7} is always correct, or compute the colSpan dynamically (e.g., use 6 when the Counterparty column is hidden and 7 when visible) to keep the empty-state row consistent with the table columns rendered by filteredActivity and the Counterparty column..env.example (1)
47-52: Add brief comments documenting the Smart Wallet variables.The Stellar/Soroban and Asset Issuers sections include explanatory comments for each variable, but the new Smart Wallet block only has a header. The PR objectives state these vars are "documented"; consider adding one-liners explaining what each value should be (WASM hash source, verifier contract, native token contract address, indexer/relayer service URLs) so users filling in
.envknow what to provide.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 47 - 52, Add brief one-line comments above each Smart Wallet env var describing what value to provide: document NEXT_PUBLIC_SMART_ACCOUNT_WASM_HASH as the deployed smart account WASM hash or artifact source, NEXT_PUBLIC_WEBAUTHN_VERIFIER_ADDRESS as the verifier contract address used for WebAuthn, NEXT_PUBLIC_NATIVE_TOKEN_CONTRACT as the native token/soroban contract address used by the wallet, NEXT_PUBLIC_SMART_WALLET_INDEXER_URL as the indexer service URL for wallet transaction/history lookups, and NEXT_PUBLIC_SMART_WALLET_RELAYER_URL as the relayer service URL for submitting transactions; place each comment directly above its corresponding variable so users know the expected value/source.components/wallet/assets-list.tsx (1)
38-39: Optional: memoizesupportedAssetsandactiveSymbols.
getSupportedAssets()builds a new array (and constructsAssetinstances) on every render, andactiveSymbolsrebuilds theSet. Wrapping both inuseMemo(withassetsas the dep for the latter) avoids unnecessary work and stabilizes references for any downstream memoization.♻️ Proposed refactor
- const supportedAssets = getSupportedAssets(); - const activeSymbols = new Set(assets.map((a) => a.tokenSymbol)); + const supportedAssets = useMemo(() => getSupportedAssets(), []); + const activeSymbols = useMemo( + () => new Set(assets.map((a) => a.tokenSymbol)), + [assets], + );Add
useMemoto the React import at the top.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/assets-list.tsx` around lines 38 - 39, Wrap the call to getSupportedAssets() in React's useMemo so supportedAssets is only rebuilt when needed, and memoize activeSymbols with useMemo using assets as a dependency so the Set only recalculates when assets change; update the import to include useMemo and replace const supportedAssets = getSupportedAssets() with a useMemo that returns getSupportedAssets(), and replace const activeSymbols = new Set(assets.map((a) => a.tokenSymbol)) with a useMemo that returns that Set and depends on assets.hooks/use-wallet-data.ts (3)
40-50:EscrowEntryandEscrowSummaryData.entriesare effectively dead.
fetchEscrowSummaryalways returnsentries: []andcomponents/wallet/escrow-summary.tsxno longer renders a per-bounty breakdown, so theentriesfield onEscrowSummaryDataand the entireEscrowEntrytype are unused. Either:
- Remove the type and field until per-bounty data is actually queryable, or
- Keep them but add a TODO/comment so a future contributor knows they're scaffolding for unimplemented work and where the data would come from (e.g., enumerating
EscrowPool/DepositRecordentries on the core escrow contract).Currently it reads as if the feature exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` around lines 40 - 50, The EscrowEntry type and the entries field on EscrowSummaryData are dead (fetchEscrowSummary always returns entries: [] and components/wallet/escrow-summary.tsx no longer uses per-bounty data); either remove the EscrowEntry interface and the entries property from EscrowSummaryData and adjust any references, including fetchEscrowSummary, or if you prefer to keep scaffolding, add a clear TODO comment on EscrowEntry and EscrowSummaryData.entries explaining they are placeholders for future per-bounty data (e.g., enumerating EscrowPool/DepositRecord on the core escrow contract) and reference fetchEscrowSummary and components/wallet/escrow-summary.tsx so future contributors know where to implement it.
73-84: Hardcoded XLM fallback price duplicated; minor brittleness.
0.12is repeated here and inlib/stellar/assets.ts(fetchAssetPricesUsdfallback). If you ever bump the fallback, both must be updated. Consider exporting the fallback price map (e.g.,FALLBACK_PRICES_USD) fromlib/stellar/assets.tsand reusing it here. Also worth noting:prices["XLM"]returned fromfetchAssetPricesUsdalready defaults to0.12in the failure path, so the?? 0.12here only protects against the symbol literally being absent from the record — which today it never is. You can drop the fallback or move it to a shared constant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` around lines 73 - 84, Replace the duplicated hardcoded XLM fallback by exporting a shared fallback map from lib/stellar/assets.ts (e.g., export const FALLBACK_PRICES_USD = { XLM: 0.12, ... }) and import it into this hook; then compute totalLocked using prices["XLM"] ?? FALLBACK_PRICES_USD["XLM"] (or simply prices["XLM"] if you prefer to rely on fetchAssetPricesUsd), replacing the literal 0.12 used when calculating totalLocked (the scValToNative conversion and xlmUnits calculation remain unchanged).
31-38: Inconsistent auto-refresh between assets and transactions.
useWalletAssetsusesrefetchInterval: 30 * 1000for live polling, butuseWalletTransactionsdoes not. New payment activity arriving on-chain will not surface until the user navigates away and back (or the 2-minutestaleTimelapses and a refocus/remount happens). Consider applying the same polling cadence (or a slightly slower one, e.g., 60s) so the activity tab tracks reality. Same applies touseEscrowSummaryif escrow values can change without the user taking action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-data.ts` around lines 31 - 38, useWalletTransactions currently lacks a refetchInterval so it won't poll for new on‑chain payments like useWalletAssets does; update useWalletTransactions (and consider useEscrowSummary if relevant) to include a refetchInterval (e.g., 30_000 or 60_000) in the useQuery options so transactions are polled on the same cadence as assets, keeping queryKey walletKeys.transactions(address ?? "") and queryFn fetchAccountTransactions(address!) unchanged and gated by enabled: !!address.components/wallet/balance-card.tsx (1)
31-31: Minor: redundant alias.
availableForWithdrawalis just a rename ofwalletInfo.balanceand is used in a single place at line 84. You can inline it to reduce indirection, or compute it as something more meaningful (e.g.,walletInfo.balance - pendingEarnings) if "available" is meant to exclude locked funds. As-is the label "Available now" and the "Escrow Locked" amount are not deducted from "Total Balance"; they are presented as orthogonal numbers, which is confusing ifwalletInfo.balanceis meant to be liquid only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/balance-card.tsx` at line 31, The variable availableForWithdrawal is a redundant alias of walletInfo.balance and adds indirection; either inline walletInfo.balance where availableForWithdrawal is used (reference: availableForWithdrawal and walletInfo.balance) or change the calculation to reflect true liquid funds (e.g., compute walletInfo.balance minus any locked/escrow/pending amounts such as pendingEarnings or escrowLocked) and update the displayed labels ("Available now", "Escrow Locked", "Total Balance") accordingly so numbers add up and are not orthogonal; modify the usage in the component (where availableForWithdrawal is referenced) to use the inlined value or the revised computation and ensure the UI text matches the new semantics.components/wallet/escrow-summary.tsx (1)
13-16: Optional: extractformatCurrencyto a shared util.The same
Intl.NumberFormat("en-US", { style: "currency", currency: "USD" })helper now exists incomponents/wallet/escrow-summary.tsx,components/wallet/balance-card.tsx, andcomponents/wallet/assets-list.tsx. Lifting it into something likelib/utils/format.tskeeps all wallet currency formatting consistent (and makes a future locale-switch a one-line change).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/escrow-summary.tsx` around lines 13 - 16, Extract the duplicated Intl.NumberFormat call into a single shared utility by moving the current formatCurrency implementation into a new exported function (e.g. formatCurrency(amount: number, locale?: string, currency?: string)) and replace the in-file helpers in escrow-summary.tsx, balance-card.tsx, and assets-list.tsx with imports of that utility; keep the default behavior as "en-US" and "USD" but accept optional locale and currency parameters for future switching, and ensure you export the function so all three components import and use the same implementation.lib/stellar/assets.ts (1)
94-117: Optional: hoist therpc.Serverinstance.
fetchAllAssetBalancestriggers onefetchAssetBalancecall per supported asset, and each call constructs a freshnew rpc.Server(...). For the typical 3-asset list this is fine, but it's wasteful and easy to fix by accepting a sharedServerinstance (or memoizing one at module scope). Useful if/when the supported-asset list grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/assets.ts` around lines 94 - 117, fetchAllAssetBalances currently calls fetchAssetBalance for each supported asset and each fetchAssetBalance creates a new rpc.Server(...) which is wasteful; modify the code to reuse a single rpc.Server instance by either (A) adding an optional rpc.Server parameter to fetchAllAssetBalances and threading that shared server into fetchAssetBalance calls, or (B) memoizing a module-scoped rpc.Server singleton used by fetchAssetBalance; update function signatures (fetchAllAssetBalances and fetchAssetBalance) and call sites accordingly, and ensure rpc.Server is constructed once (referenced by the rpc.Server symbol) and passed into or used by fetchAssetBalance to avoid reconstructing it per asset.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/wallet/assets-list.tsx`:
- Around line 139-148: The "Refresh" Button (using RefreshCw) currently toggles
the manage panel via setShowManage(show => !show) instead of refreshing asset
data; update the handler on that Button to either rename it to "Manage" (and
keep setShowManage behavior) or — if keeping the "Refresh" label/icon — call
queryClient.invalidateQueries({ queryKey: walletKeys.assets(walletAddress ?? "")
}) (or the equivalent refetch helper) in addition to toggling showManage so
clicking the Button will re-pull balances; locate the Button with props
onClick={() => setShowManage(!showManage)} and modify the handler to perform the
invalidateQueries call referencing queryClient, walletKeys.assets, and
walletAddress.
- Around line 48-69: The function handleAddTrustline is misnamed because it only
refreshes cached balances; rename handleAddTrustline to handleRefreshAsset (and
update all callers) and change the supported-assets panel header text to clearly
state it only refreshes the cached balance for already-supported assets (update
the message that currently references "Use Refresh to update the balance..." to
mention "refresh cached balance" and that no on-chain trustline is created);
also update toast/log text (e.g., `${supported.symbol} balance refreshed`) and
any references to getSACBalance/fetchAssetBalance to reflect the refresh intent.
If instead you choose to implement the original behavior, replace the refresh
logic in handleAddTrustline by building and submitting an
Operation.changeTrust({ asset: supported.asset }) signed by the smart wallet,
then call queryClient.invalidateQueries(walletKeys.assets(...)) and refresh the
balance—pick one of these two paths and make all related identifiers
(handleAddTrustline → handleRefreshAsset or the new changeTrust flow) consistent
across the component.
- Around line 250-296: The Supported Assets list never shows the "Refresh/Add"
button because activeSymbols is built from fetchAllAssetBalances which returns
every supported asset (even amount==0), so activeSymbols.has(supported.symbol)
is always true; update the logic that builds activeSymbols to only include
symbols with amount > 0 (e.g. use assets.filter(a => a.amount > 0).map(...)) so
items with zero balance render the non-active branch, or if you prefer to
disable the UI entirely remove the Supported Assets panel until trustline
management is implemented; locate the code that calls
fetchAllAssetBalances/getSupportedAssets and the variable activeSymbols, and
adjust that construction accordingly (or remove the Supported Assets render
block that uses supportedAssets and handleAddTrustline).
In `@lib/stellar/assets.ts`:
- Around line 70-73: fetchAssetBalance and fetchAllAssetBalances currently
swallow errors (logging and returning 0 or never throwing), causing the UI to
treat failures as zero balances; change the logic so that fetchAssetBalance
re-throws errors instead of returning 0, and update fetchAllAssetBalances to
detect if every per-asset fetch failed (or if the prices fetch failed) and throw
a consolidated error so React Query can enter isError; alternatively (if you
prefer finer-grained UX), have fetchAllAssetBalances return per-asset result
objects like { amount, error?: string } and update BalanceCard/AssetsList to
display load errors, but do not keep the current behavior of returning zeros on
failure.
- Around line 89-91: The catch block that returns hardcoded fallback prices
should capture and log the error before returning, matching the pattern used in
fetchAssetBalance; update the anonymous catch to "catch (err)" and call
console.error with a clear message (e.g., "Failed to fetch CoinGecko prices in
<function name>" or similar) along with the error object before returning the {
XLM: 0.12, USDC: 1.0, EURC: 1.08 } fallback so failures are surfaced for
debugging.
---
Nitpick comments:
In @.env.example:
- Around line 47-52: Add brief one-line comments above each Smart Wallet env var
describing what value to provide: document NEXT_PUBLIC_SMART_ACCOUNT_WASM_HASH
as the deployed smart account WASM hash or artifact source,
NEXT_PUBLIC_WEBAUTHN_VERIFIER_ADDRESS as the verifier contract address used for
WebAuthn, NEXT_PUBLIC_NATIVE_TOKEN_CONTRACT as the native token/soroban contract
address used by the wallet, NEXT_PUBLIC_SMART_WALLET_INDEXER_URL as the indexer
service URL for wallet transaction/history lookups, and
NEXT_PUBLIC_SMART_WALLET_RELAYER_URL as the relayer service URL for submitting
transactions; place each comment directly above its corresponding variable so
users know the expected value/source.
In `@components/wallet/assets-list.tsx`:
- Around line 38-39: Wrap the call to getSupportedAssets() in React's useMemo so
supportedAssets is only rebuilt when needed, and memoize activeSymbols with
useMemo using assets as a dependency so the Set only recalculates when assets
change; update the import to include useMemo and replace const supportedAssets =
getSupportedAssets() with a useMemo that returns getSupportedAssets(), and
replace const activeSymbols = new Set(assets.map((a) => a.tokenSymbol)) with a
useMemo that returns that Set and depends on assets.
In `@components/wallet/balance-card.tsx`:
- Line 31: The variable availableForWithdrawal is a redundant alias of
walletInfo.balance and adds indirection; either inline walletInfo.balance where
availableForWithdrawal is used (reference: availableForWithdrawal and
walletInfo.balance) or change the calculation to reflect true liquid funds
(e.g., compute walletInfo.balance minus any locked/escrow/pending amounts such
as pendingEarnings or escrowLocked) and update the displayed labels ("Available
now", "Escrow Locked", "Total Balance") accordingly so numbers add up and are
not orthogonal; modify the usage in the component (where availableForWithdrawal
is referenced) to use the inlined value or the revised computation and ensure
the UI text matches the new semantics.
In `@components/wallet/escrow-summary.tsx`:
- Around line 13-16: Extract the duplicated Intl.NumberFormat call into a single
shared utility by moving the current formatCurrency implementation into a new
exported function (e.g. formatCurrency(amount: number, locale?: string,
currency?: string)) and replace the in-file helpers in escrow-summary.tsx,
balance-card.tsx, and assets-list.tsx with imports of that utility; keep the
default behavior as "en-US" and "USD" but accept optional locale and currency
parameters for future switching, and ensure you export the function so all three
components import and use the same implementation.
In `@components/wallet/transaction-history.tsx`:
- Around line 54-74: The CSV export in handleExportCsv is missing the new
Counterparty column and the headers/rows are out of sync with the UI table;
update the headers array and each row mapping in rows (which currently maps
filteredActivity items) to include the counterparty field in the correct
position and order used by the table (e.g., insert item.counterparty or
item.counterparty?.toString() between Description and Amount or wherever the UI
shows it), keeping the rest of the fields (id, type, description,
amount.toString(), currency, formatSafeDate(item.date, "yyyy-MM-dd HH:mm:ss"),
item.transactionHash || "", item.status) intact so CSV columns align with the
table.
- Around line 24-29: The search filter in filteredActivity only checks
description, type, and currency but omits the counterparty column; update the
activity.filter callback (the filteredActivity computation) to also check
item.counterparty (e.g.,
item.counterparty?.toLowerCase().includes(search.toLowerCase())) alongside the
existing checks so a search term matches counterparty values while preserving
the existing null/undefined-safe checks.
- Around line 164-172: The empty-state row uses colSpan={7} but the Counterparty
column header/cell is rendered with the responsive class "hidden lg:table-cell",
so on small viewports only 6 columns exist; update the empty-state to match that
responsive behavior by either making the Counterparty column always present
(remove the "hidden lg:table-cell" class on the Counterparty header/cell) so
colSpan={7} is always correct, or compute the colSpan dynamically (e.g., use 6
when the Counterparty column is hidden and 7 when visible) to keep the
empty-state row consistent with the table columns rendered by filteredActivity
and the Counterparty column.
In `@hooks/use-wallet-data.ts`:
- Around line 40-50: The EscrowEntry type and the entries field on
EscrowSummaryData are dead (fetchEscrowSummary always returns entries: [] and
components/wallet/escrow-summary.tsx no longer uses per-bounty data); either
remove the EscrowEntry interface and the entries property from EscrowSummaryData
and adjust any references, including fetchEscrowSummary, or if you prefer to
keep scaffolding, add a clear TODO comment on EscrowEntry and
EscrowSummaryData.entries explaining they are placeholders for future per-bounty
data (e.g., enumerating EscrowPool/DepositRecord on the core escrow contract)
and reference fetchEscrowSummary and components/wallet/escrow-summary.tsx so
future contributors know where to implement it.
- Around line 73-84: Replace the duplicated hardcoded XLM fallback by exporting
a shared fallback map from lib/stellar/assets.ts (e.g., export const
FALLBACK_PRICES_USD = { XLM: 0.12, ... }) and import it into this hook; then
compute totalLocked using prices["XLM"] ?? FALLBACK_PRICES_USD["XLM"] (or simply
prices["XLM"] if you prefer to rely on fetchAssetPricesUsd), replacing the
literal 0.12 used when calculating totalLocked (the scValToNative conversion and
xlmUnits calculation remain unchanged).
- Around line 31-38: useWalletTransactions currently lacks a refetchInterval so
it won't poll for new on‑chain payments like useWalletAssets does; update
useWalletTransactions (and consider useEscrowSummary if relevant) to include a
refetchInterval (e.g., 30_000 or 60_000) in the useQuery options so transactions
are polled on the same cadence as assets, keeping queryKey
walletKeys.transactions(address ?? "") and queryFn
fetchAccountTransactions(address!) unchanged and gated by enabled: !!address.
In `@lib/stellar/assets.ts`:
- Around line 94-117: fetchAllAssetBalances currently calls fetchAssetBalance
for each supported asset and each fetchAssetBalance creates a new
rpc.Server(...) which is wasteful; modify the code to reuse a single rpc.Server
instance by either (A) adding an optional rpc.Server parameter to
fetchAllAssetBalances and threading that shared server into fetchAssetBalance
calls, or (B) memoizing a module-scoped rpc.Server singleton used by
fetchAssetBalance; update function signatures (fetchAllAssetBalances and
fetchAssetBalance) and call sites accordingly, and ensure rpc.Server is
constructed once (referenced by the rpc.Server symbol) and passed into or used
by fetchAssetBalance to avoid reconstructing it per asset.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e272a58-b9c5-4836-a338-b3500c575c99
📒 Files selected for processing (10)
.env.exampleapp/wallet/page.tsxcomponents/wallet/assets-list.tsxcomponents/wallet/balance-card.tsxcomponents/wallet/escrow-summary.tsxcomponents/wallet/transaction-history.tsxhooks/use-wallet-data.tslib/stellar/assets.tslib/stellar/horizon.tstypes/wallet.ts
✅ Files skipped from review due to trivial changes (1)
- types/wallet.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/stellar/horizon.ts
- app/wallet/page.tsx
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="flex-1 sm:flex-none" | ||
| onClick={() => setShowManage(!showManage)} | ||
| > | ||
| <RefreshCw className="mr-2 h-4 w-4" /> | ||
| Refresh | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
"Refresh" button toggles a panel rather than refreshing data.
The button labeled Refresh with the RefreshCw icon actually toggles showManage — it doesn't invalidate or refetch the assets query. Users will reasonably expect clicking it to re-pull balances. Either:
- Rename the button to
Manage(orAssets), or - Wire it to also call
queryClient.invalidateQueries({ queryKey: walletKeys.assets(walletAddress ?? "") })in addition to opening the panel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/wallet/assets-list.tsx` around lines 139 - 148, The "Refresh"
Button (using RefreshCw) currently toggles the manage panel via
setShowManage(show => !show) instead of refreshing asset data; update the
handler on that Button to either rename it to "Manage" (and keep setShowManage
behavior) or — if keeping the "Refresh" label/icon — call
queryClient.invalidateQueries({ queryKey: walletKeys.assets(walletAddress ?? "")
}) (or the equivalent refetch helper) in addition to toggling showManage so
clicking the Button will re-pull balances; locate the Button with props
onClick={() => setShowManage(!showManage)} and modify the handler to perform the
invalidateQueries call referencing queryClient, walletKeys.assets, and
walletAddress.
| {supportedAssets.map((supported) => { | ||
| const isActive = activeSymbols.has(supported.symbol); | ||
| const isAdding = addingAsset === supported.id; | ||
| return ( | ||
| <div | ||
| key={supported.id} | ||
| className="flex items-center justify-between py-2" | ||
| > | ||
| <div className="flex items-center gap-3"> | ||
| <div className="flex h-9 w-9 items-center justify-center rounded-full bg-primary/10 text-xs font-bold text-primary-foreground dark:text-primary"> | ||
| {supported.symbol} | ||
| </div> | ||
| <div> | ||
| <div className="text-sm font-medium"> | ||
| {supported.symbol} | ||
| </div> | ||
| <div className="text-xs text-muted-foreground"> | ||
| {supported.name} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div className="flex items-center gap-2"> | ||
| {isActive ? ( | ||
| <Badge | ||
| variant="outline" | ||
| className="text-green-500 border-green-500/20 bg-green-500/5 text-[10px] h-5" | ||
| > | ||
| <CheckCircle2 className="h-3 w-3 mr-1" /> | ||
| Active | ||
| </Badge> | ||
| ) : ( | ||
| <Button | ||
| size="sm" | ||
| className="flex-1 sm:flex-none" | ||
| onClick={() => handleSort('tokenSymbol')} | ||
| title="Sort by Symbol" | ||
| > | ||
| <ArrowUpDown className="mr-2 h-4 w-4" /> | ||
| Sort Symbol | ||
| </Button> | ||
| <Button | ||
| variant={hideSmallBalances ? "default" : "outline"} | ||
| size="sm" | ||
| className="flex-1 sm:flex-none" | ||
| onClick={() => setHideSmallBalances(!hideSmallBalances)} | ||
| title={hideSmallBalances ? "Showing only >$1" : "Filter small balances"} | ||
| > | ||
| <Filter className="mr-2 h-4 w-4" /> | ||
| {hideSmallBalances ? "Filtering" : "Filter"} | ||
| </Button> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="rounded-xl border border-border overflow-hidden"> | ||
| <div className="overflow-x-auto"> | ||
| <table className="w-full text-sm"> | ||
| <thead className="bg-muted/50 border-b border-border"> | ||
| <tr> | ||
| <th className="text-left py-3 px-4 font-medium text-muted-foreground uppercase tracking-wider text-[10px]">Asset</th> | ||
| <th className="text-right py-3 px-4 font-medium text-muted-foreground uppercase tracking-wider text-[10px]">Balance</th> | ||
| <th className="text-right py-3 px-4 font-medium text-muted-foreground uppercase tracking-wider text-[10px]">Price</th> | ||
| <th className="text-right py-3 px-4 font-medium text-muted-foreground uppercase tracking-wider text-[10px]">Value</th> | ||
| <th className="text-right py-3 px-4 font-medium text-muted-foreground uppercase tracking-wider text-[10px] hidden md:table-cell">Portfolio %</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody className="divide-y divide-border"> | ||
| {filteredAndSortedAssets.length === 0 ? ( | ||
| <tr> | ||
| <td colSpan={5} className="py-12 text-center text-muted-foreground"> | ||
| No assets found matching your search. | ||
| </td> | ||
| </tr> | ||
| ) : ( | ||
| filteredAndSortedAssets.map((asset) => ( | ||
| <tr key={asset.id} className="hover:bg-muted/30 transition-colors cursor-pointer group"> | ||
| <td className="py-4 px-4"> | ||
| <div className="flex items-center gap-3"> | ||
| <div className="flex h-10 w-10 items-center justify-center rounded-full bg-primary/10 text-xs font-bold text-primary-foreground dark:text-primary"> | ||
| {asset.tokenSymbol} | ||
| </div> | ||
| <div> | ||
| <div className="font-semibold">{asset.tokenSymbol}</div> | ||
| <div className="text-xs text-muted-foreground">{asset.tokenName}</div> | ||
| </div> | ||
| </div> | ||
| </td> | ||
| <td className="py-4 px-4 text-right"> | ||
| <div className="font-medium">{asset.amount.toLocaleString()}</div> | ||
| <div className="text-xs text-muted-foreground">{asset.tokenSymbol}</div> | ||
| </td> | ||
| <td className="py-4 px-4 text-right"> | ||
| {formatCurrency(asset.amount ? asset.usdValue / asset.amount : 0)} | ||
| </td> | ||
| <td className="py-4 px-4 text-right"> | ||
| <div className="font-medium">{formatCurrency(asset.usdValue)}</div> | ||
| </td> | ||
| <td className="py-4 px-4 text-right hidden md:table-cell"> | ||
| <div className="text-xs font-medium"> | ||
| {(totalUsd ? (asset.usdValue / totalUsd) * 100 : 0).toFixed(1)}% | ||
| </div> | ||
| </td> | ||
| </tr> | ||
| )) | ||
| )} | ||
| </tbody> | ||
| </table> | ||
| variant="outline" | ||
| className="h-7 text-xs" | ||
| disabled={isAdding || !walletAddress} | ||
| onClick={() => handleAddTrustline(supported.id)} | ||
| > | ||
| {isAdding ? ( | ||
| <Loader2 className="h-3 w-3 animate-spin mr-1" /> | ||
| ) : ( | ||
| <RefreshCw className="h-3 w-3 mr-1" /> | ||
| )} | ||
| Refresh | ||
| </Button> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
The !isActive branch is dead in production.
fetchAllAssetBalances in lib/stellar/assets.ts returns one entry per element of getSupportedAssets() (it no longer filters by amount, per the earlier review). Therefore assets.map(a => a.tokenSymbol) will always contain every supported symbol, activeSymbols.has(supported.symbol) is always true, and the Refresh button branch (lines 280–295) never renders in practice. Combined with the previous comment on handleAddTrustline, this whole "Supported Assets" panel collapses to a list of Active badges with no actionable controls.
Either:
- Differentiate "supported but balance == 0" from "active (balance > 0)" in
activeSymbols, e.g.new Set(assets.filter(a => a.amount > 0).map(...)), so users with no USDC see the Refresh/Add control, or - Remove the panel entirely until the trustline-management story is implemented.
🛠️ Suggested narrowing
- const activeSymbols = new Set(assets.map((a) => a.tokenSymbol));
+ const activeSymbols = new Set(
+ assets.filter((a) => a.amount > 0).map((a) => a.tokenSymbol),
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/wallet/assets-list.tsx` around lines 250 - 296, The Supported
Assets list never shows the "Refresh/Add" button because activeSymbols is built
from fetchAllAssetBalances which returns every supported asset (even amount==0),
so activeSymbols.has(supported.symbol) is always true; update the logic that
builds activeSymbols to only include symbols with amount > 0 (e.g. use
assets.filter(a => a.amount > 0).map(...)) so items with zero balance render the
non-active branch, or if you prefer to disable the UI entirely remove the
Supported Assets panel until trustline management is implemented; locate the
code that calls fetchAllAssetBalances/getSupportedAssets and the variable
activeSymbols, and adjust that construction accordingly (or remove the Supported
Assets render block that uses supportedAssets and handleAddTrustline).
| } catch { | ||
| return { XLM: 0.12, USDC: 1.0, EURC: 1.08 }; | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling: missing log.
fetchAssetBalance (line 71) logs failures via console.error, but the catch here silently falls through to hardcoded fallback prices. If CoinGecko is unreachable or rate-limited, debugging is harder and the user sees stale fallback valuations with no signal. Match the logging pattern used elsewhere in this file.
🛠️ Proposed fix
- } catch {
+ } catch (err) {
+ console.error("[stellar] fetchAssetPricesUsd failed:", err);
return { XLM: 0.12, USDC: 1.0, EURC: 1.08 };
}📝 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 { | |
| return { XLM: 0.12, USDC: 1.0, EURC: 1.08 }; | |
| } | |
| } catch (err) { | |
| console.error("[stellar] fetchAssetPricesUsd failed:", err); | |
| return { XLM: 0.12, USDC: 1.0, EURC: 1.08 }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/stellar/assets.ts` around lines 89 - 91, The catch block that returns
hardcoded fallback prices should capture and log the error before returning,
matching the pattern used in fetchAssetBalance; update the anonymous catch to
"catch (err)" and call console.error with a clear message (e.g., "Failed to
fetch CoinGecko prices in <function name>" or similar) along with the error
object before returning the { XLM: 0.12, USDC: 1.0, EURC: 1.08 } fallback so
failures are surfaced for debugging.
Re-throw in fetchAccountTransactions and guard fetchAllAssetBalances so React Query isError flag triggers and the UI error banner renders.
…banner renders Returning 0 silently hid Soroban RPC errors from React Query.
Implemented @Benjtalkshow |
The function no longer creates a trustline — it just refreshes the on-chain balance — so the original name was misleading after the panel was renamed to "Supported Assets / Refresh". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (4)
lib/stellar/assets.ts (1)
89-91:⚠️ Potential issue | 🟡 MinorSilent CoinGecko fallback — log the error to match the file's pattern.
Per the prior review,
fetchAssetBalance(line 71) logs failures viaconsole.error, but thiscatchsilently swallows network/parse errors and returns hardcoded prices. If CoinGecko is unreachable or rate-limited, valuations look correct-ish (~$0.12XLM,$1.00USDC,$1.08EURC) but are stale, with no debugging signal.♻️ Suggested fix
- } catch { + } catch (err) { + console.error("[stellar] fetchAssetPricesUsd failed:", err); return { XLM: 0.12, USDC: 1.0, EURC: 1.08 }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/assets.ts` around lines 89 - 91, The catch block that returns hardcoded prices should log the error like the rest of the file instead of silently swallowing it: update the anonymous catch in the price-fetching code to catch (err) and call console.error with a clear message (e.g., "fetchAssetPrices failed" or similar) and the caught error before returning the fallback { XLM: 0.12, USDC: 1.0, EURC: 1.08 }; match the logging style used by fetchAssetBalance to ensure failures are visible during debugging.components/wallet/assets-list.tsx (2)
139-148:⚠️ Potential issue | 🟡 MinorThe "Refresh" button doesn't refresh — it toggles the manage panel.
Per the prior review, this
Refresh-labeled button only flipsshowManageand never re-pulls live balances. Users will reasonably click it expecting their balances to refetch. Either rename the button toManage(orAssets) to match what it actually does, or invalidate the query alongside the toggle:♻️ Suggested change
<Button variant="outline" size="sm" className="flex-1 sm:flex-none" - onClick={() => setShowManage(!showManage)} + onClick={() => { + if (walletAddress) { + queryClient.invalidateQueries({ + queryKey: walletKeys.assets(walletAddress), + }); + } + setShowManage((prev) => !prev); + }} > <RefreshCw className="mr-2 h-4 w-4" /> Refresh </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/assets-list.tsx` around lines 139 - 148, The Refresh button currently calls onClick={() => setShowManage(!showManage)} so it only toggles the manage panel (showManage) instead of refetching balances; update the handler on the Button (or rename the Button label) so it matches intent: either change the label/icon from "Refresh" to "Manage"/"Assets" to reflect setShowManage usage, or keep the "Refresh" label and additionally invalidate/refetch the balances query (call your query client/refetch function alongside setShowManage, e.g., invoke the relevant invalidate/refetch method for the balances query) so clicking the button both toggles showManage and re-pulls live balances.
39-39:⚠️ Potential issue | 🟠 Major
activeSymbolsmakes the!isActivebranch unreachable.Per the previous review,
fetchAllAssetBalancesreturns one entry per supported asset (regardless ofamount), soassets.map(a => a.tokenSymbol)always contains every supported symbol andactiveSymbols.has(supported.symbol)is alwaystrue. Combined with the panel header at lines 244–246, the "Supported Assets" section reduces to a list ofActivebadges with no actionable controls — theRefreshbutton branch (lines 281–294) never renders.Fix by narrowing the active set to non-zero balances:
♻️ Suggested change
- const activeSymbols = new Set(assets.map((a) => a.tokenSymbol)); + const activeSymbols = new Set( + assets.filter((a) => a.amount > 0).map((a) => a.tokenSymbol), + );Or remove the entire "Supported Assets" panel until trustline management is actually implemented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/assets-list.tsx` at line 39, The activeSymbols set currently uses all symbols from assets (assets.map(a => a.tokenSymbol)), making isActive always true; update the activeSymbols declaration to only include symbols for assets with non-zero balances (e.g., filter assets by a non-zero amount/value before mapping) so the isActive branch and the Refresh button branch (which relies on non-active entries) can render correctly; reference fetchAllAssetBalances, activeSymbols, assets, and isActive when locating and updating the code.lib/stellar/horizon.ts (1)
45-65:⚠️ Potential issue | 🟡 Minor
create_accountandaccount_mergepayments are still silently dropped.The previous reviewer comment on filtering by
op.typehas not been addressed. The Horizonpayments()endpoint returns multiple operation types (payment,create_account,path_payment_strict_*,account_merge, and on newer SDKsinvoke_host_function):
create_accountcarriesstarting_balance(notamount), soparseFloat(op.amount ?? "0")yields0and the record is dropped by theamount > 0filter — the funding tx that creates the wallet is never shown.account_mergehas no amount field at all, also silently dropped.- Path-payment strict-receive/send may carry the relevant amount in
source_amount/amountdifferently; worth verifying.Filter to handled types up front and pull the right field per type:
♻️ Suggested change
return (response.records as unknown as HorizonOpRecord[]) + .filter( + (op) => + op.type === "payment" || + op.type === "path_payment_strict_receive" || + op.type === "path_payment_strict_send" || + op.type === "create_account", + ) .map((op, index) => { - const amount = parseFloat(op.amount ?? "0"); - const isIncoming = op.to === address; + const rawAmount = + op.type === "create_account" + ? (op as HorizonOpRecord & { starting_balance?: string }) + .starting_balance + : op.amount; + const amount = parseFloat(rawAmount ?? "0"); + const counterpartyAddr = + op.type === "create_account" + ? (op as HorizonOpRecord & { account?: string; funder?: string }) + .funder + : op.from; + const recipient = + op.type === "create_account" + ? (op as HorizonOpRecord & { account?: string }).account + : op.to; + const isIncoming = recipient === address; const asset = op.asset_type === "native" ? "XLM" : (op.asset_code ?? "UNKNOWN"); - const counterparty = isIncoming ? op.from : op.to; + const counterparty = isIncoming ? counterpartyAddr : recipient;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/horizon.ts` around lines 45 - 65, The mapping silently drops non-`payment` operations; update the transform that maps over response.records (the map in lib/stellar/horizon.ts) to first filter/handle operation types explicitly (whitelist: "payment", "create_account", "path_payment_strict_receive", "path_payment_strict_send", "account_merge", "invoke_host_function"), and compute amount using the correct field per type (e.g., use op.starting_balance for "create_account", op.source_amount or op.amount for path payments, and fallback to op.amount || op.starting_balance || op.source_amount for others), keep counterparty logic based on op.type (e.g., "create_account" uses op.account or op.funder), and only then apply the amount > 0 filter so create_account and account_merge are represented correctly; update references to op.amount, op.starting_balance, op.source_amount, and op.type in the mapping and ensure transaction fields (id, transaction_hash, created_at) remain filled.
🧹 Nitpick comments (6)
lib/stellar/assets.ts (3)
60-65: Constructing a newrpc.Serverper call is wasteful.
fetchAssetBalanceis invoked once per supported asset on every refresh (every 30s peruseWalletAssets), and a freshrpc.Serveris allocated each time. Lift the instance to module scope (mirroring the lazygetHorizonServerpattern inlib/stellar/horizon.ts):♻️ Suggested change
+let rpcServer: rpc.Server | null = null; +function getRpcServer(): rpc.Server { + if (!rpcServer) rpcServer = new rpc.Server(SMART_WALLET_CONFIG.rpcUrl); + return rpcServer; +} + export async function fetchAssetBalance( walletContractId: string, asset: Asset, ): Promise<number> { try { - const server = new rpc.Server(SMART_WALLET_CONFIG.rpcUrl); + const server = getRpcServer(); const result = await server.getSACBalance(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/assets.ts` around lines 60 - 65, fetchAssetBalance currently creates a new rpc.Server on every call (const server = new rpc.Server(...)) which is wasteful; instead, lift the rpc.Server instance to module scope with a lazy initializer (mirror getHorizonServer in lib/stellar/horizon.ts) and have fetchAssetBalance use that shared server when calling server.getSACBalance(walletContractId, asset, SMART_WALLET_CONFIG.networkPassphrase); ensure the module-scoped initializer respects SMART_WALLET_CONFIG.rpcUrl and only constructs the rpc.Server once (or reinitializes if config changes).
106-118: One asset's RPC failure now blacks out every other balance.After the recent change to make
fetchAssetBalancethrow,Promise.all(supportedAssets.map(...))rejects if any single asset'sgetSACBalancefails — e.g., a misconfiguredNEXT_PUBLIC_USDC_ISSUERon testnet, or the Soroban contract for one token being unavailable, will surface asisErrorfor the entire wallet, hiding the user's XLM balance too.Consider
Promise.allSettledand only throwing if every asset failed (or per-asset error objects), so partial data still renders:♻️ Suggested change
- const results = await Promise.all( + const settled = await Promise.allSettled( supportedAssets.map(async (supported) => { const amount = await fetchAssetBalance(walletContractId, supported.asset); const priceUsd = prices[supported.symbol] ?? 0; return { id: supported.id, tokenSymbol: supported.symbol, tokenName: supported.name, amount, usdValue: amount * priceUsd, }; }), ); - - return results; + const results = settled.flatMap((r) => + r.status === "fulfilled" ? [r.value] : [], + ); + if (results.length === 0) { + throw new Error("[stellar] all asset balance fetches failed"); + } + return results; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/assets.ts` around lines 106 - 118, The current Promise.all(supportedAssets.map(...)) causes a single fetchAssetBalance failure to reject the whole batch; change this to use Promise.allSettled over the supportedAssets.map so each asset's fetchAssetBalance is handled independently, then transform the settled results into the same per-asset objects (id, tokenSymbol, tokenName, amount, usdValue) for fulfilled entries and attach an error marker/object for rejected entries (or amount = 0 and include the error) so partial balances render; only throw or mark global error if every settled result is rejected. Ensure usdValue is computed using priceUsd = prices[supported.symbol] ?? 0 for fulfilled amounts.
78-82: Add a timeout to the CoinGecko fetch.CoinGecko's free tier is rate-limited and occasionally hangs. With no
AbortSignal, a slow response will block the entireuseWalletAssetsquery (which gates skeleton dismissal on the page) until the network stack times out. UseAbortSignal.timeoutto cap the wait:♻️ Suggested change
const response = await fetch( "https://api.coingecko.com/api/v3/simple/price?ids=stellar%2Cusd-coin%2Ceuro-coin&vs_currencies=usd", - { cache: "no-store" }, + { cache: "no-store", signal: AbortSignal.timeout(5_000) }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/assets.ts` around lines 78 - 82, The fetch call in useWalletAssets (the CoinGecko fetch in lib/stellar/assets.ts) needs an AbortSignal.timeout to avoid hanging; create a signal via AbortSignal.timeout(desiredMs) and pass it in the fetch options (e.g., { cache: "no-store", signal }) and ensure any AbortError is handled/converted to a meaningful error before throwing so the caller can proceed; update the fetch invocation and error handling around the response.ok check accordingly.lib/stellar/horizon.ts (2)
53-54: Fallbackidcan collide across pages or with missing-id records.
id: op.id || \tx-${index}`indexes into the current 50-record page. Horizon payment records always include anid, so this fallback should be unreachable in practice — but if it does fire (e.g., partial deserialization or an SDK type mismatch), two records lackingidin different fetches would both betx-0, breaking React list keys and React Query equality checks. Prefer a stable identifier such asop.transaction_hashcombined withindex`:♻️ Suggested change
- id: op.id || `tx-${index}`, + id: op.id || `${op.transaction_hash}-${index}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/horizon.ts` around lines 53 - 54, The fallback id value uses a page-local index and can collide; replace the fallback in the returned object (the id property currently set as op.id || `tx-${index}`) with a stable identifier that combines op.transaction_hash and the index (e.g., use op.id if present, otherwise `${op.transaction_hash}-${index}`), and ensure you handle the unlikely case where transaction_hash is missing by falling back to a unique value (e.g., include index or generate a short unique token) so list keys and React Query identity remain stable.
5-10: Futurenet would be misrouted to the testnet Horizon endpoint.The
getHorizonUrlheuristic looks for the substring"Test SDF", which matches bothNetworks.TESTNET("Test SDF Network ; September 2015") andNetworks.FUTURENET("Test SDF Future Network ; October 2022"). If anyone configuresNEXT_PUBLIC_STELLAR_NETWORK_PASSPHRASEto Futurenet, payments will be fetched fromhorizon-testnet.stellar.organd silently return no records for that account.Consider matching by exact passphrase against
Networksconstants for clarity:♻️ Suggested change
+import { Networks } from "@stellar/stellar-sdk"; + function getHorizonUrl(): string { - const isTestnet = SMART_WALLET_CONFIG.networkPassphrase.includes("Test SDF"); - return isTestnet - ? "https://horizon-testnet.stellar.org" - : "https://horizon.stellar.org"; + switch (SMART_WALLET_CONFIG.networkPassphrase) { + case Networks.PUBLIC: + return "https://horizon.stellar.org"; + case Networks.FUTURENET: + return "https://horizon-futurenet.stellar.org"; + case Networks.TESTNET: + default: + return "https://horizon-testnet.stellar.org"; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/horizon.ts` around lines 5 - 10, The current getHorizonUrl uses a substring check on SMART_WALLET_CONFIG.networkPassphrase which erroneously treats Futurenet as Testnet; update getHorizonUrl to compare SMART_WALLET_CONFIG.networkPassphrase against exact Stellar Networks constants (e.g. Networks.TESTNET, Networks.PUBLIC, Networks.FUTURENET) and return the appropriate Horizon URL for each exact match (horizon-testnet.stellar.org, horizon.stellar.org, and the Futurenet endpoint if applicable), falling back to a safe default if no known constant matches; edit the getHorizonUrl function and references to SMART_WALLET_CONFIG.networkPassphrase to use exact equality checks with those Network constants.components/wallet/assets-list.tsx (1)
41-46: Sort UI only ever sorts bytokenSymbol.The single
Sortbutton hardcodeshandleSort("tokenSymbol")while the table headers (Balance,Price,Value,Portfolio %) advertise sortable-looking columns but aren't clickable. The internalsortConfigalready supports"tokenSymbol" | "amount" | "usdValue"; consider making the column headers clickable (or expanding the menu) so the sort state is actually reachable from the UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/assets-list.tsx` around lines 41 - 46, The UI only triggers sorting for tokenSymbol; update the table header cells (the headers labeled Balance, Price, Value, Portfolio %) to be clickable and call the existing handleSort with the appropriate keys ("amount" for Balance, "usdValue" for Price/Value as appropriate, and "tokenSymbol" for the symbol column), or convert the single Sort button into a dropdown that calls handleSort with those keys; ensure you use the existing setSortConfig/sortConfig state and update the header markup to include onClick handlers, a visible sort indicator based on sortConfig.direction, and accessible affordances (role/button or tabindex/aria-sort) so users can reach all three sort modes from the UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/wallet/assets-list.tsx`:
- Around line 139-148: The Refresh button currently calls onClick={() =>
setShowManage(!showManage)} so it only toggles the manage panel (showManage)
instead of refetching balances; update the handler on the Button (or rename the
Button label) so it matches intent: either change the label/icon from "Refresh"
to "Manage"/"Assets" to reflect setShowManage usage, or keep the "Refresh" label
and additionally invalidate/refetch the balances query (call your query
client/refetch function alongside setShowManage, e.g., invoke the relevant
invalidate/refetch method for the balances query) so clicking the button both
toggles showManage and re-pulls live balances.
- Line 39: The activeSymbols set currently uses all symbols from assets
(assets.map(a => a.tokenSymbol)), making isActive always true; update the
activeSymbols declaration to only include symbols for assets with non-zero
balances (e.g., filter assets by a non-zero amount/value before mapping) so the
isActive branch and the Refresh button branch (which relies on non-active
entries) can render correctly; reference fetchAllAssetBalances, activeSymbols,
assets, and isActive when locating and updating the code.
In `@lib/stellar/assets.ts`:
- Around line 89-91: The catch block that returns hardcoded prices should log
the error like the rest of the file instead of silently swallowing it: update
the anonymous catch in the price-fetching code to catch (err) and call
console.error with a clear message (e.g., "fetchAssetPrices failed" or similar)
and the caught error before returning the fallback { XLM: 0.12, USDC: 1.0, EURC:
1.08 }; match the logging style used by fetchAssetBalance to ensure failures are
visible during debugging.
In `@lib/stellar/horizon.ts`:
- Around line 45-65: The mapping silently drops non-`payment` operations; update
the transform that maps over response.records (the map in
lib/stellar/horizon.ts) to first filter/handle operation types explicitly
(whitelist: "payment", "create_account", "path_payment_strict_receive",
"path_payment_strict_send", "account_merge", "invoke_host_function"), and
compute amount using the correct field per type (e.g., use op.starting_balance
for "create_account", op.source_amount or op.amount for path payments, and
fallback to op.amount || op.starting_balance || op.source_amount for others),
keep counterparty logic based on op.type (e.g., "create_account" uses op.account
or op.funder), and only then apply the amount > 0 filter so create_account and
account_merge are represented correctly; update references to op.amount,
op.starting_balance, op.source_amount, and op.type in the mapping and ensure
transaction fields (id, transaction_hash, created_at) remain filled.
---
Nitpick comments:
In `@components/wallet/assets-list.tsx`:
- Around line 41-46: The UI only triggers sorting for tokenSymbol; update the
table header cells (the headers labeled Balance, Price, Value, Portfolio %) to
be clickable and call the existing handleSort with the appropriate keys
("amount" for Balance, "usdValue" for Price/Value as appropriate, and
"tokenSymbol" for the symbol column), or convert the single Sort button into a
dropdown that calls handleSort with those keys; ensure you use the existing
setSortConfig/sortConfig state and update the header markup to include onClick
handlers, a visible sort indicator based on sortConfig.direction, and accessible
affordances (role/button or tabindex/aria-sort) so users can reach all three
sort modes from the UI.
In `@lib/stellar/assets.ts`:
- Around line 60-65: fetchAssetBalance currently creates a new rpc.Server on
every call (const server = new rpc.Server(...)) which is wasteful; instead, lift
the rpc.Server instance to module scope with a lazy initializer (mirror
getHorizonServer in lib/stellar/horizon.ts) and have fetchAssetBalance use that
shared server when calling server.getSACBalance(walletContractId, asset,
SMART_WALLET_CONFIG.networkPassphrase); ensure the module-scoped initializer
respects SMART_WALLET_CONFIG.rpcUrl and only constructs the rpc.Server once (or
reinitializes if config changes).
- Around line 106-118: The current Promise.all(supportedAssets.map(...)) causes
a single fetchAssetBalance failure to reject the whole batch; change this to use
Promise.allSettled over the supportedAssets.map so each asset's
fetchAssetBalance is handled independently, then transform the settled results
into the same per-asset objects (id, tokenSymbol, tokenName, amount, usdValue)
for fulfilled entries and attach an error marker/object for rejected entries (or
amount = 0 and include the error) so partial balances render; only throw or mark
global error if every settled result is rejected. Ensure usdValue is computed
using priceUsd = prices[supported.symbol] ?? 0 for fulfilled amounts.
- Around line 78-82: The fetch call in useWalletAssets (the CoinGecko fetch in
lib/stellar/assets.ts) needs an AbortSignal.timeout to avoid hanging; create a
signal via AbortSignal.timeout(desiredMs) and pass it in the fetch options
(e.g., { cache: "no-store", signal }) and ensure any AbortError is
handled/converted to a meaningful error before throwing so the caller can
proceed; update the fetch invocation and error handling around the response.ok
check accordingly.
In `@lib/stellar/horizon.ts`:
- Around line 53-54: The fallback id value uses a page-local index and can
collide; replace the fallback in the returned object (the id property currently
set as op.id || `tx-${index}`) with a stable identifier that combines
op.transaction_hash and the index (e.g., use op.id if present, otherwise
`${op.transaction_hash}-${index}`), and ensure you handle the unlikely case
where transaction_hash is missing by falling back to a unique value (e.g.,
include index or generate a short unique token) so list keys and React Query
identity remain stable.
- Around line 5-10: The current getHorizonUrl uses a substring check on
SMART_WALLET_CONFIG.networkPassphrase which erroneously treats Futurenet as
Testnet; update getHorizonUrl to compare SMART_WALLET_CONFIG.networkPassphrase
against exact Stellar Networks constants (e.g. Networks.TESTNET,
Networks.PUBLIC, Networks.FUTURENET) and return the appropriate Horizon URL for
each exact match (horizon-testnet.stellar.org, horizon.stellar.org, and the
Futurenet endpoint if applicable), falling back to a safe default if no known
constant matches; edit the getHorizonUrl function and references to
SMART_WALLET_CONFIG.networkPassphrase to use exact equality checks with those
Network constants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19570258-041e-4e1e-af88-752e66895aee
📒 Files selected for processing (3)
components/wallet/assets-list.tsxlib/stellar/assets.tslib/stellar/horizon.ts
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
LGTM!
Pushed a small commit (3ec9afa) on your branch to rename handleAddTrustline to handleRefreshAsset since the panel was renamed and the old name was misleading. Everything else looks ready — merging this in. Thanks for the solid follow-up work.
Thank you for the well-defined corrections; it's good to work on them. |
2 participants
Description
Replaces the mock data on the wallet page (
app/wallet/) with real Stellar network integration, fulfilling the remaining acceptance criteria for issue #151.Changes
New files
lib/stellar/assets.ts— Lazy-initialized supported asset definitions (XLM, USDC, EURC). ExposesgetSupportedAssets(),fetchAssetBalance()(Soroban RPCgetSACBalance),fetchAssetPricesUsd()(CoinGecko with hardcoded fallbacks), andfetchAllAssetBalances()to aggregate all balances + USD values in one call.lib/stellar/horizon.ts— Horizon API client (getHorizonServer) andfetchAccountTransactions()that retrieves payment operations for an account and maps them toWalletActivity[].hooks/use-wallet-data.ts— Three React Query hooks:useWalletAssets(2 min stale / 30 s auto-refresh),useWalletTransactions, anduseEscrowSummary(queriesCORE_ESCROW_CONTRACT_IDvia SorobangetContractData, falls back gracefully to{ totalLocked: 0, entries: [] }when the key is absent).components/wallet/escrow-summary.tsx— Sidebar card showing escrow-locked total + per-bounty breakdown with links to bounty detail pages.Modified files
app/wallet/page.tsx— WiresuseSmartWallet+ the three new hooks; replaces allmockWalletWithAssetsreferences with real data. Adds a skeleton loading state and a connect-wallet prompt for unauthenticated users.components/wallet/balance-card.tsx— Removes hardcodedpendingEarnings = 150andchange24h = 12.5; acceptspendingEarnings?: number(fed from escrow summary) andisLoading?: boolean(renders a skeleton)..env.example— DocumentsNEXT_PUBLIC_USDC_ISSUER,NEXT_PUBLIC_EURC_ISSUER, and the smart-wallet vars that were previously undocumented.Acceptance criteria
getSACBalance; USDC/EURC when issuer env vars are set)payments().forAccount())CORE_ESCROW_CONTRACT_ID, falls back to 0)Closes
Closes #151
Notes
NEXT_PUBLIC_USDC_ISSUER/NEXT_PUBLIC_EURC_ISSUER) are explicitly set, preventing invalid-issuer errors during SSR/build.xdr.ScVal.scvVec(["EscrowLocked", walletAddress])as the storage key. If the contract uses a different key format, the hook returns zero without throwing — the UI degrades gracefully.Summary by CodeRabbit
New Features
Chores