Show subdust coins #189
Show subdust coins #189
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the VTXO (Virtual Transaction Output) display functionality to show subdust coins that were previously filtered out. The change retrieves all VTXOs including recoverable ones, rather than only displaying spendable VTXOs above the dust threshold.
Key changes:
- Fetches all VTXOs (including recoverable ones) instead of only spendable VTXOs
- Updates the display logic to show unspent VTXOs regardless of dust threshold
- Adds type casting for wallet parameter consistency
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/screens/Settings/Vtxos.tsx | Adds fetching of all VTXOs including recoverable ones, updates display logic to show all unspent coins instead of only spendable ones |
| src/lib/vtxo.ts | Adds type cast to ServiceWorkerWallet for wallet parameter in getVtxos call |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/screens/Settings/Vtxos.tsx
Outdated
| Your virtual coins with amount and expiration | ||
| </Text> | ||
| {vtxos.spendable?.map((v: Vtxo) => ( | ||
| {listableVtxos.map((v: ExtendedVirtualCoin) => ( |
There was a problem hiding this comment.
The VtxoLine component expects a Vtxo type parameter but is now receiving ExtendedVirtualCoin. This type mismatch could cause runtime errors if the properties differ between these types. Update VtxoLine's type signature to accept ExtendedVirtualCoin or ensure the types are compatible.
| {listableVtxos.map((v: ExtendedVirtualCoin) => ( | |
| {listableVtxos.map((v) => ( |
| const getOrphanVtxos = async (wallet: IWallet): Promise<ExtendedVirtualCoin[]> => { | ||
| const now = Date.now() | ||
| const { spendable } = await getVtxos(wallet) | ||
| const { spendable } = await getVtxos(wallet as ServiceWorkerWallet) |
There was a problem hiding this comment.
Using a type cast suggests that the function signature should be updated instead. Consider changing the getOrphanVtxos function parameter type from IWallet to ServiceWorkerWallet, or update getVtxos to accept IWallet if it should support the broader interface.
There was a problem hiding this comment.
it seems to me getVtxos may take the interface ?
Deploying wallet-signet with
|
| Latest commit: |
3fb377c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://436922cb.wallet-23u.pages.dev |
| Branch Preview URL: | https://show-subdust-coins.wallet-23u.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
3fb377c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://89b96a80.arkade-wallet.pages.dev |
| Branch Preview URL: | https://show-subdust-coins.arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
3fb377c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1c6fef02.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://show-subdust-coins.wallet-bitcoin.pages.dev |
| import { maxPercentage } from './constants' | ||
| import { getVtxos } from './asp' | ||
|
|
||
| // this should never happen, but just in case |
There was a problem hiding this comment.
nit: why should never happen ?
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR extends transaction and VTXO UI to support expiry fields, adds preconfirmed boarding status visualization, narrows wallet typing to ServiceWorkerWallet in VTXO retrieval, and enhances the Vtxos screen to display both virtual coins and boarding UTXOs with expiry information. Changes
Sequence DiagramsequenceDiagram
participant User
participant TransactionScreen as Transaction<br/>Screen
participant State as App State
User->>TransactionScreen: View transaction
TransactionScreen->>State: Check tx.boardingTx & preconfirmed
alt Pending Boarding
State-->>TransactionScreen: Not settled + boarding exists
TransactionScreen->>TransactionScreen: Show "Pending boarding" status
TransactionScreen->>TransactionScreen: Check expiry: elapsed > boardingExitDelay
Note over TransactionScreen: If expired → mark as expired boarding
TransactionScreen->>User: Render "Complete boarding" button
else Settled or Preconfirmed
State-->>TransactionScreen: Return settled/preconfirmed state
TransactionScreen->>User: Render "Settle transaction" button
end
TransactionScreen->>User: Display transaction with expiry field & Info block
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes The diff involves heterogeneous changes across five files with varying complexity. Transaction.tsx contains logic-dense boarding state determination and expiry timing logic. Vtxos.tsx introduces significant structural refactoring with new component composition and data collection flows. VTXO library changes include type narrowing that affects retrieval logic. UI component changes are more straightforward but require coordination understanding. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/Wallet/Transaction.tsx (1)
170-172: Remove|| truefrom the disabled prop—resend functionality must be implemented per the TODO at line 81.The button is permanently disabled due to
disabled={resending || true}. Line 81 contains// TODO implement resend, confirming this is placeholder code. When implementing the resend logic, remove|| trueso the button respects theresendingstate variable:disabled={resending}.
♻️ Duplicate comments (1)
src/lib/vtxo.ts (1)
5-5: Clarify or remove “this should never happen” comment.Explain under which conditions “orphan VTXOs” might occur, or remove the comment to avoid confusion.
🧹 Nitpick comments (5)
src/lib/vtxo.ts (1)
1-1: Avoid unsafe cast; widen getVtxos to IWallet and drop the import.Casting
wallet as ServiceWorkerWalletis unnecessary ifgetVtxosonly callswallet.getVtxos(). Prefer updatinggetVtxosto acceptIWallet, then remove the cast and the now‑unusedServiceWorkerWalletimport here. This improves type safety and aligns with our prior guidance to keep wallet APIs IWallet‑based. Based on learnings.Apply locally:
-import { ExtendedVirtualCoin, IWallet, ServiceWorkerWallet, VtxoManager } from '@arkade-os/sdk' +import { ExtendedVirtualCoin, IWallet, VtxoManager } from '@arkade-os/sdk'- const { spendable } = await getVtxos(wallet as ServiceWorkerWallet) + const { spendable } = await getVtxos(wallet)And in
src/lib/asp.ts(external to this diff), adjust signature:// before export const getVtxos = async (wallet: ServiceWorkerWallet) => { // after export const getVtxos = async (wallet: IWallet) => {Also applies to: 8-8
src/components/TransactionsList.tsx (1)
28-35: Minor readability: factor a single boolean for preconfirmed boarding.You repeat
tx.preconfirmed && tx.boardingTxid. Consider:+ const isPreconfirmedBoarding = Boolean(tx.preconfirmed && tx.boardingTxid) const Fiat = () => { - const color = + const color = config.currencyDisplay === CurrencyDisplay.Both ? 'dark50' : tx.type === 'received' ? 'green' - : tx.boardingTxid && tx.preconfirmed + : isPreconfirmedBoarding ? 'orange' : ''- const Icon = () => tx.type === 'sent' ? <SentIcon /> : tx.preconfirmed && tx.boardingTxid ? <PreconfirmedIcon /> : <ReceivedIcon /> + const Icon = () => tx.type === 'sent' ? <SentIcon /> : isPreconfirmedBoarding ? <PreconfirmedIcon /> : <ReceivedIcon />- <Text color={tx.type === 'received' ? (tx.preconfirmed && tx.boardingTxid ? 'orange' : 'green') : ''} thin> + <Text color={tx.type === 'received' ? (isPreconfirmedBoarding ? 'orange' : 'green') : ''} thin>Also applies to: 50-50
src/screens/Wallet/Transaction.tsx (2)
36-41: Harden expiry-time calculation and button label updates.
- Use integer seconds to avoid fractional comparisons.
- Ensure the button label reacts if the boarding state changes.
-const defaultButtonLabel = boardingTx ? 'Complete boarding' : 'Settle transaction' -const boardingExitDelay = Number(aspInfo?.boardingExitDelay || 0) +const defaultButtonLabel = boardingTx ? 'Complete boarding' : 'Settle transaction' +const boardingExitDelay = Number(aspInfo?.boardingExitDelay || 0) -const expiredBoardingTx = - !tx?.settled && boardingTx && tx?.createdAt && Date.now() / 1000 - tx?.createdAt > boardingExitDelay +const nowSec = Math.floor(Date.now() / 1000) +const expiredBoardingTx = + !tx?.settled && boardingTx && tx?.createdAt && nowSec - tx.createdAt > boardingExitDelayAnd a matching effect tweak (near Lines 53‑55):
-useEffect(() => { - setButtonLabel(settling ? 'Settling...' : defaultButtonLabel) -}, [settling]) +useEffect(() => { + setButtonLabel(settling ? 'Settling...' : defaultButtonLabel) +}, [settling, defaultButtonLabel])
139-142: Copy nit: “Onboarding” instead of “Onboard”.Minor grammar fix in the info box title/body.
-<Info color='orange' icon={<VtxosIcon />} title='Pending boarding'> - <Text wrap>Onboard transaction confirmed on-chain.</Text> +<Info color='orange' icon={<VtxosIcon />} title='Pending onboarding'> + <Text wrap>Onboarding transaction confirmed on-chain.</Text>src/screens/Settings/Vtxos.tsx (1)
190-201: Consistency: show “Unknown” expiry when block time is missing.
VtxoLineshows'Unknown'when expiry is not available; mirror that inUtxoLine.- const expiry = utxo.status.block_time ? prettyAgo(utxo.status.block_time + expiration) : '' + const expiry = utxo.status.block_time ? prettyAgo(utxo.status.block_time + expiration) : 'Unknown'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Details.tsx(3 hunks)src/components/TransactionsList.tsx(3 hunks)src/lib/vtxo.ts(1 hunks)src/screens/Settings/Vtxos.tsx(6 hunks)src/screens/Wallet/Transaction.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-30T18:33:29.839Z
Learnt from: bordalix
PR: arkade-os/wallet#114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.
Applied to files:
src/lib/vtxo.ts
🧬 Code graph analysis (5)
src/components/Details.tsx (1)
src/icons/Date.tsx (1)
DateIcon(1-13)
src/lib/vtxo.ts (1)
src/lib/asp.ts (1)
getVtxos(124-135)
src/screens/Settings/Vtxos.tsx (6)
src/lib/logs.ts (1)
consoleError(40-44)src/components/Text.tsx (1)
Text(25-79)src/components/FlexRow.tsx (1)
FlexRow(16-45)src/lib/types.ts (1)
Vtxo(95-95)src/lib/format.ts (3)
prettyNumber(85-88)prettyHide(69-74)prettyAgo(12-25)src/components/Empty.tsx (1)
EmptyCoinsList(26-34)
src/screens/Wallet/Transaction.tsx (3)
src/components/Info.tsx (1)
Info(14-30)src/icons/Vtxos.tsx (1)
VtxosIcon(1-13)src/components/Text.tsx (1)
Text(25-79)
src/components/TransactionsList.tsx (4)
src/icons/Sent.tsx (1)
SentIcon(1-13)src/icons/Preconfirmed.tsx (1)
PreconfirmedIcon(1-12)src/icons/Received.tsx (1)
ReceivedIcon(1-13)src/components/Text.tsx (2)
Text(25-79)TextSecondary(91-97)
🪛 Biome (2.1.2)
src/components/TransactionsList.tsx
[error] 48-48: Do not shadow the global "Date" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (3)
src/components/Details.tsx (1)
23-23: Expiry support looks good; ensure consistent formatting.The new
expiryfield and row render are fine. Please confirm callers always provide a preformatted string (e.g.,prettyDate/prettyAgo) to avoid mixed formats.Also applies to: 40-55, 74-74
src/components/TransactionsList.tsx (1)
45-46: Confirm intent: Preconfirmed icon only for received transactions.Current logic renders
SentIconfor all sent txs, even ifpreconfirmed && boardingTxid. Should sent preconfirmed/boarding txs show a special icon or color as well?src/screens/Settings/Vtxos.tsx (1)
127-148: Tags and lines look solid.Good separation via
CoinLineand consistent tags (subdust/settled/swept/unconfirmed). Expiry math for VTXOs uses ms and is handled byprettyAgocorrectly; UTXOs use seconds + delay—looks right.Please confirm
aspInfo.dustandboardingExitDelayare in sats/seconds respectively across providers.Also applies to: 177-188
| const Icon = () => | ||
| tx.type === 'sent' ? <SentIcon /> : tx.preconfirmed && tx.boardingTxid ? <PreconfirmedIcon /> : <ReceivedIcon /> | ||
| const Kind = () => <Text thin>{tx.type === 'sent' ? 'Sent' : 'Received'}</Text> | ||
| const Date = () => <TextSecondary>{date}</TextSecondary> |
There was a problem hiding this comment.
Rename local Date component to avoid shadowing the global.
This triggers Biome’s noShadowRestrictedNames. Rename to TxDate (and its usage).
- const Date = () => <TextSecondary>{date}</TextSecondary>
+ const TxDate = () => <TextSecondary>{date}</TextSecondary>- <Date />
+ <TxDate />Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 48-48: Do not shadow the global "Date" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In src/components/TransactionsList.tsx around line 48, the local component is
named "Date" which shadows the global Date and triggers Biome's
noShadowRestrictedNames; rename the component to "TxDate" and update all usages
in this file accordingly (replace the component declaration const Date = () =>
... with const TxDate = () => ... and change any <Date/> renders to <TxDate/>),
keeping props/children the same and ensuring imports/exports (if any) reflect
the new name.
| <IonGrid> | ||
| <IonRow class='ion-align-items-end'> | ||
| <IonCol size='4'> | ||
| <Text>{amount}</Text> | ||
| </IonCol> | ||
| <IonCol size='4'>{tags}</IonCol> | ||
| <IonCol size='4'> | ||
| <Text right>{expiry}</Text> | ||
| </IonCol> | ||
| </IonRow> | ||
| </IonGrid> |
There was a problem hiding this comment.
Use className instead of class on React components.
React/TSX expects className. Using class can cause typing/lint issues.
- <IonRow class='ion-align-items-end'>
+ <IonRow className='ion-align-items-end'>📝 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.
| <IonGrid> | |
| <IonRow class='ion-align-items-end'> | |
| <IonCol size='4'> | |
| <Text>{amount}</Text> | |
| </IonCol> | |
| <IonCol size='4'>{tags}</IonCol> | |
| <IonCol size='4'> | |
| <Text right>{expiry}</Text> | |
| </IonCol> | |
| </IonRow> | |
| </IonGrid> | |
| <IonGrid> | |
| <IonRow className='ion-align-items-end'> | |
| <IonCol size='4'> | |
| <Text>{amount}</Text> | |
| </IonCol> | |
| <IonCol size='4'>{tags}</IonCol> | |
| <IonCol size='4'> | |
| <Text right>{expiry}</Text> | |
| </IonCol> | |
| </IonRow> | |
| </IonGrid> |
🤖 Prompt for AI Agents
In src/screens/Settings/Vtxos.tsx around lines 161 to 171, the IonRow uses the
HTML attribute class='ion-align-items-end' which should be the React prop
className; change class='ion-align-items-end' to className='ion-align-items-end'
(and verify there are no other JSX elements in this file using class instead of
className).
@tiero @Kukks please review
Summary by CodeRabbit