fix(wallet): don't render token amounts as sats - #652
Open
dmnyc wants to merge 2 commits into
Open
Conversation
A wallet restored from the same seed in an app that offers token
conversion sends token payments back through listPayments, and we read
every Payment.amount as satoshis. A 15.77 USDB transfer displayed as
"+15,766,673 sats" — off by eight orders of magnitude, in the direction
that looks like a windfall.
This is not stablecoin support. There is no way to hold, convert, send
or receive a token in this app before this change or after it, and
GetInfoResponse.tokenBalances stays unread. What it accommodates is a
shared seed: the other app's payments come back down the same
listPayments feed whether we want them or not, so the only choice is
how to render rows we did not create. Today we render them as a false
statement about the user's money.
The SDK is explicit — Payment.amount is "satoshis OR token base units" —
and PaymentDetails.Token carries the metadata to tell them apart.
PaymentMethod.TOKEN is the fallback discriminator, since the SDK notes
the details can be empty. Read via property access rather than
destructuring, because the case's arity differs across SDK versions.
Token rows carry assetTicker / assetAmount / assetFee scaled by the
token's own decimals, and their sats fields are forced to zero: there is
no honest sats value for a token transfer, so nothing sats-denominated
can derive a number from one. The token branch sits BEFORE the fiat
branch in the row — the fiat rate converts sats, so running it would
only produce a second wrong number.
Rows show two decimal places ("15.77 USDB"): USDB is dollars, and six
places is both unreadable at a glance and wrong for what the number
means. The expanded detail keeps full precision, and dust that would
round to "0.00" keeps its precision rather than reading as nothing
having arrived.
Also fixes a latent overflow: payment.amount is a BigInteger and
.toLong() wraps silently past Long.MAX_VALUE, so a large token amount
would have rendered as a 0-sat payment. Scaling works on the decimal
string instead; a test pins u128 max.
Ported from wisp-ios#451. 14 tests.
A sats-to-USDB conversion is one payment on each side of the swap, and both rendered as a bare "Received" with a green down-arrow — money arriving from someone. Nothing arrived: the funds changed shape inside the wallet. Conversion rows now read "Converted from bitcoin" / "Converted from USDB", matching how Glow presents the same history. The green and the arrow were making the same false claim as the label, so both go too. The icon becomes a swap glyph in the accent color, and the amount takes the neutral label color — green means money arrived and red means it left, and a conversion is neither. The +/- sign stays, since it still says which leg of the swap the row is. The source asset comes from conversionDetails.conversions.firstOrNull()?.from?.asset?.ticker. The step list is ordered [cross-chain, AMM] for receives and [AMM, cross-chain] for sends, so the FIRST step is the true origin in both directions rather than an intermediate hop. "BTC" renders lowercase as "bitcoin" — in that sentence it is the asset, not a ticker symbol. Tokens keep their uppercase ticker. The expanded detail carries the full direction: "Conversion · BTC → USDB". Note a sats row can be a conversion leg without being a token transfer (USDB back into sats), so the two flags are independent. Ported from wisp-ios#451. 18 tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports barrydeen/wisp-ios#451 — see that PR for screenshots of the before / after.
A wallet restored from the same seed in an app that offers token conversion sends token payments back through
listPayments, and we read everyPayment.amountas satoshis. A 15.77 USDB transfer displayed as "+15,766,673 sats" — off by eight orders of magnitude, in the direction that looks like a windfall.This is not stablecoin support
To be unambiguous about scope, because "handle USDB" reads like the opposite of what this does:
No stablecoin balances are being added, and this doesn't move toward them. There is no way to hold, convert, send or receive USDB — or any token — in this app before this change or after it. No conversion UI, no token balance on the dashboard, nothing in the send or receive paths.
GetInfoResponse.tokenBalancesstays unread.What it accommodates is a shared seed. The other app's token payments come back down the same
listPaymentsfeed on the next sync whether we want them or not — the SDK returns the wallet's history, and the wallet is shared. The only choice available is how to render rows we did not create. Today we render them as a false statement about the user's money.Cause
The SDK is explicit —
Payment.amountis "Amount in satoshis or token base units" — andPaymentDetails.Tokencarries the metadata to tell them apart.PaymentMethod.TOKENis the fallback discriminator, since the SDK notes the details can be empty.Read via property access rather than positional destructuring: the case's arity differs between SDK versions, and property access compiles against both.
Fix — display only
assetTicker/assetAmount/assetFee, scaled by the token's owndecimals.15.77 USDB) — USDB is dollars, and six places is unreadable at a glance and wrong for what the number means. The expanded detail keeps full precision, and dust that would round to0.00keeps its precision rather than reading as nothing having arrived.Latent overflow, also fixed
payment.amountis aBigIntegerand.toLong()wraps silently pastLong.MAX_VALUE. Token base units are u128, so a large amount would have rendered as a 0-sat payment. Scaling works on the decimal string instead; a test pins u128 max.Conversion rows
A sats-to-USDB conversion is one payment on each side of the swap, and both rendered as a bare "Received" with a green down-arrow — money arriving from someone. Nothing arrived: the funds changed shape inside the wallet.
Rows now read "Converted from bitcoin" / "Converted from USDB", matching how Glow presents the same history. The green and the arrow were making the same false claim as the label, so both go too:
The
+/-sign stays — it still says which leg of the swap the row is — but nothing about it implies income.Source asset comes from
conversionDetails.conversions.first?.from.asset.ticker. The step list is ordered[cross-chain, AMM]for receives and[AMM, cross-chain]for sends, so the first step is the true origin in both directions rather than an intermediate hop. "BTC" renders lowercase as "bitcoin" — in that sentence it's the asset, not a ticker symbol. The expanded detail carries the full direction:Conversion · BTC → USDB.Worth noting the two states are independent: a sats row can be a conversion leg without being a token transfer (USDB converted back into sats), and there's a test pinning that.
Testing
18 tests on the pure formatting: decimal scaling (u128 max, sub-unit amounts keeping their leading zero, trailing-zero trimming), the two-place compact form, dust preservation, and the model's token/sats discrimination.