From 467b849a13e85a22ac3090b7bf14691b3ae6e50a Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 11:07:37 +0200 Subject: [PATCH] fix: show balances and fees below 0.000001 as nonzero on the send screens (closes #343) The stored ETH and token balances and the send-confirm screen's fee were each cut to six decimal places, and a token holding cut to zero was dropped, so a value below 0.000001 read as zero. Balances are now stored exactly, whatever decimals a token declares, and every nonzero token holding is kept; the balance check reads a token balance to its first 18 places. The balance lists, the send-screen token selector, the address total and the remove-address warning leave out a holding below 0.000001 themselves, through isBelowOneMillionth(). The send and send-confirm screens' balances, reserve and insufficient-balance messages go through truncateAmountNeverZero(). The send-confirm and approval screens both render the fee through formatFee(), which prices the exact fee in USD. Model: opus-5-5 --- README.md | 30 +- TODO.md | 18 ++ src/popup/views/approval.js | 14 +- src/popup/views/confirmTx.js | 46 ++-- src/popup/views/deleteAddress.js | 3 +- src/popup/views/helpers.js | 32 ++- src/popup/views/send.js | 17 +- src/shared/amountDisplay.js | 24 +- src/shared/balances.js | 23 +- src/shared/prices.js | 4 + src/shared/txValidation.js | 10 +- tests/e2e/run.js | 33 ++- tests/sendDisplayFloor.test.js | 452 +++++++++++++++++++++++++++++++ tests/unknownScaleSend.test.js | 6 +- 14 files changed, 624 insertions(+), 88 deletions(-) create mode 100644 tests/sendDisplayFloor.test.js diff --git a/README.md b/README.md index bb0d196..1df5679 100644 --- a/README.md +++ b/README.md @@ -892,12 +892,21 @@ shows is a V4 exact-in `amountIn` of zero. The rule and its exception live in `src/shared/amountDisplay.js` as `truncateAmount()` and `truncateAmountNeverZero()`. Everything the approval and confirmation screens display goes through the floored one — the ERC-20 amount, -the ETH value and max fee (`src/popup/views/approval.js`), and the swap's -`Amount` and `Min. received` lines (`src/shared/uniswap.js`). The history and -balance lists (`src/shared/transactions.js`) use the unfloored one: the -transaction detail view is the authoritative record and already shows exact -precision. The 4-decimal rule is unchanged everywhere else, including for -amounts at or above the floor on the approval screens. +the ETH value and max fee (`src/popup/views/approval.js`), the swap's `Amount` +and `Min. received` lines (`src/shared/uniswap.js`), the Send screen's +`Current balance` (`src/popup/views/send.js`), and the balance and network fee +on the confirmation screen for the wallet's own send +(`src/popup/views/confirmTx.js`). Both screens render a network fee through +`formatFee()` in `src/popup/views/helpers.js`, which prices the exact fee in USD +rather than its truncated figure, so the same fee reads the same on both, USD +value included. Balances are stored exactly (`src/shared/balances.js`), whatever +decimals a token declares, so a balance below the floor reaches these screens as +it is. The history list (`src/shared/transactions.js`) uses the unfloored one: +the transaction detail view is the authoritative record and already shows exact +precision. The balance lists use neither: they round to four places with +`toFixed(4)` (`balanceLine()` in `src/popup/views/helpers.js`). The 4-decimal +rule is unchanged everywhere else, including for amounts at or above the floor +on the approval screens. The floor applies only where the token's scale is known. Where it is not, the approval screen states base units instead of a quantity — see Unknown token @@ -1055,8 +1064,13 @@ list from any other contract address is always dropped, and so is any token claiming a symbol that belongs to the native asset and therefore has no legitimate contract at all (`"ETH"`). That filter is unconditional — the "Hide tokens with fewer than 1,000 holders" setting governs the transaction history -and the send-screen token selector, not this list. Tracked tokens with a zero -balance are listed as well while "Show tracked tokens with zero balance" is on. +and the send-screen token selector, not this list. `fetchTokenBalances()` stores +every nonzero holding of a token it admits, however small, but a holding below +0.000001 is left out of the balance lists, the send-screen token selector, the +address total and the remove-address warning (`isBelowOneMillionth()` in +`src/shared/amountDisplay.js`). The Send and confirmation screens show it when +its token is the one being sent. Tracked tokens with a zero balance are listed +as well while "Show tracked tokens with zero balance" is on. #### Stored state and its version diff --git a/TODO.md b/TODO.md index 843c842..00b7845 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,24 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: The Send and confirmation screens no longer show an ETH balance, a + token balance or a network fee below 0.000001 as zero + ([#343](https://git.eeqj.de/sneak/AutistMask/issues/343)). The stored balances + (`src/shared/balances.js`) and the confirmation screen's fee were each cut to + six decimal places by a rule of their own, and a token holding cut to zero was + dropped. Balances are now stored exactly, whatever decimals a token declares, + and every nonzero token holding is kept; the balance check reads a token + balance to its first 18 places, the most an amount can have. The balance + lists, the send-screen token selector, the address total and the + remove-address warning leave out a holding below 0.000001 themselves, as + before. The Send screen's `Current balance`, and the confirmation screen's + balance, fee, reserve and insufficient-balance messages, go through + `truncateAmountNeverZero()` in `src/shared/amountDisplay.js`, the helper the + approval screen already used. The confirmation and approval screens both + render the fee through `formatFee()` in `src/popup/views/helpers.js`, which + prices the exact fee in USD, so the same fee reads the same on both, USD value + included. + - 2026-10-04: The flash line keeps to the one line it reserves at any message length ([#252](https://git.eeqj.de/sneak/AutistMask/issues/252)). A message that wrapped pushed the whole screen below it down. `#flash-msg` no longer diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 2428dc5..b3738c5 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -8,6 +8,7 @@ const { renderAddressHtml, attachCopyHandlers, onViewLeave, + formatFee, } = require("./helpers"); const { state, saveState } = require("../../shared/state"); const { networkByChainId } = require("../../shared/networks"); @@ -207,7 +208,7 @@ function showPhishingWarning(elementId, isPhishing) { // and the nonce. The background compares every one of them against the signed // artifact, so every one of them has to be on the screen — a number that is // verified but never displayed is verified against nothing the user agreed to. -function showTxFee(approvedTx, ethPrice) { +function showTxFee(approvedTx) { const network = networkByChainId(approvedTx.chainId); $("approve-tx-network").textContent = network ? network.name @@ -215,12 +216,9 @@ function showTxFee(approvedTx, ethPrice) { const gasLimit = BigInt(approvedTx.gasLimit); const feePerGas = BigInt(approvedTx.maxFeePerGas || approvedTx.gasPrice); - const maxFeeEth = formatTxValue(formatEther(gasLimit * feePerGas)); - const usdStr = formatUsd( - ethPrice ? parseFloat(maxFeeEth) * ethPrice : null, - ); - $("approve-tx-fee").textContent = - maxFeeEth + " ETH" + (usdStr ? " (" + usdStr + ")" : ""); + // Through formatFee(), as the confirmation screen's fee is, so the same + // fee reads the same on both. + $("approve-tx-fee").textContent = formatFee(gasLimit * feePerGas); let detail = gasLimit.toString() + @@ -332,7 +330,7 @@ function showTxApproval(details) { $("approve-tx-value").textContent = ethValueFormatted + " ETH" + (usdStr ? " (" + usdStr + ")" : ""); - showTxFee(approvedTx, ethPrice); + showTxFee(approvedTx); // Decode calldata (reuse decoded from above) const decodedEl = $("approve-tx-decoded"); diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index 338cbde..9560383 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -15,6 +15,7 @@ const { attachCopyHandlers, goBack, onViewLeave, + formatFee, } = require("./helpers"); const { state } = require("../../shared/state"); const { getSignerForAddress } = require("../../shared/wallet"); @@ -31,6 +32,9 @@ const { transferAmountUnits, } = require("../../shared/transferAmount"); const { assertWithinCeilings } = require("../../shared/approvalVerify"); +// The balance lines, the fee reserve and the insufficient-balance messages go +// through it, as the approval screen's amounts do. +const { truncateAmountNeverZero } = require("../../shared/amountDisplay"); const { CODES, FEE_PENDING, @@ -150,11 +154,17 @@ function show(txInfo) { $("confirm-balance").textContent = bal == null ? "unknown (" + symbol + ")" - : valueWithUsd(bal + " " + symbol, balUsd); + : valueWithUsd( + truncateAmountNeverZero(bal) + " " + symbol, + balUsd, + ); } else { const bal = txInfo.balance || "0"; const balUsd = ethPrice ? parseFloat(bal) * ethPrice : null; - $("confirm-balance").textContent = valueWithUsd(bal + " ETH", balUsd); + $("confirm-balance").textContent = valueWithUsd( + truncateAmountNeverZero(bal) + " ETH", + balUsd, + ); } // Check for warnings (synchronous local checks) @@ -249,7 +259,7 @@ function renderValidation(txInfo) { : "Insufficient " + symbol + " balance. You have " + - txInfo.tokenBalance + + truncateAmountNeverZero(txInfo.tokenBalance) + " " + symbol + " but are trying to send " + @@ -262,7 +272,7 @@ function renderValidation(txInfo) { if (codes.includes(CODES.INSUFFICIENT_ETH)) { messages.push( "Insufficient balance. You have " + - txInfo.balance + + truncateAmountNeverZero(txInfo.balance || "0") + " ETH but are trying to send " + txInfo.amount + " ETH.", @@ -305,14 +315,6 @@ function setVisible(id, visible) { $(id).style.visibility = visible ? "visible" : "hidden"; } -// A fee in wei as an ETH string, truncated to 6 decimal places. -function formatFeeEth(wei) { - const parts = formatEther(wei).split("."); - const dec = - parts.length > 1 ? parts[1].slice(0, 6).replace(/0+$/, "") || "0" : "0"; - return parts[0] + "." + dec + " ETH"; -} - async function estimateGas(txInfo) { try { const provider = getProvider(state.rpcUrl, state.networkId); @@ -359,26 +361,20 @@ async function estimateGas(txInfo) { // flight; a stale fee must not reach the screen or the balance check. if (pendingTx !== txInfo) return; - const ethPrice = getPrice("ETH"); - const usd = (wei) => - ethPrice ? parseFloat(formatEther(wei)) * ethPrice : null; - + // The fee line goes through formatFee(), as the approval screen's + // does, so the same fee reads the same on both. if (estimateWei !== null && estimateWei < gasCostWei) { - $("confirm-fee-amount").textContent = valueWithUsd( - "~" + formatFeeEth(estimateWei), - usd(estimateWei), - ); + $("confirm-fee-amount").textContent = "~" + formatFee(estimateWei); $("confirm-fee-reserve").textContent = - "up to " + formatFeeEth(gasCostWei) + " reserved"; + "up to " + + truncateAmountNeverZero(formatEther(gasCostWei)) + + " ETH reserved"; setVisible("confirm-fee-reserve", true); } else { // No spread to report: either there is no estimate, or the node // quotes a gas price at or above maxFeePerGas, so the expected // cost is not below the reserve. Show the reserve alone. - $("confirm-fee-amount").textContent = valueWithUsd( - formatFeeEth(gasCostWei), - usd(gasCostWei), - ); + $("confirm-fee-amount").textContent = formatFee(gasCostWei); setVisible("confirm-fee-reserve", false); } feeStatus = FEE_KNOWN; diff --git a/src/popup/views/deleteAddress.js b/src/popup/views/deleteAddress.js index ce40e84..c8d2a9e 100644 --- a/src/popup/views/deleteAddress.js +++ b/src/popup/views/deleteAddress.js @@ -87,7 +87,8 @@ function recoveryPathText(wallet) { // AddressDetail, followed by the USD total when there is one to give — no // total line at all on testnet or before the first price fetch, and no figure // when every holding here is one with no price, since "$0.00" directly under -// "This address holds a balance." is a contradiction. +// "This address holds a balance." is a contradiction. A token holding below +// 0.000001 does not count, as the lines below leave it out. function balanceWarningHtml(addr) { if (!addressHoldsFunds(addr)) return " "; const line = formatAddressTotal(getAddressValue(addr)); diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index 374513f..3e96760 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -12,6 +12,11 @@ // escapeHtml lives in src/shared/html.js, where the escape and the // reasoning behind it are; it is re-exported below so views keep importing // it from here. +const { formatEther } = require("ethers"); +const { + truncateAmountNeverZero, + isBelowOneMillionth, +} = require("../../shared/amountDisplay"); const { DEBUG } = require("../../shared/constants"); const { escapeHtml } = require("../../shared/html"); const { isDebug } = require("../../shared/log"); @@ -247,6 +252,19 @@ function unknownableAmount(balance) { return Number.isFinite(n) ? n : null; } +// A network fee in wei as the confirmation and approval screens both show it: +// the ETH figure through truncateAmountNeverZero(), then its USD value when the +// ETH price is known. The USD value is of the exact fee, not of the truncated +// figure. +function formatFee(wei) { + const eth = formatEther(wei); + const ethPrice = getPrice("ETH"); + const usd = ethPrice ? formatUsd(parseFloat(eth) * ethPrice) : ""; + return ( + truncateAmountNeverZero(eth) + " ETH" + (usd ? " (" + usd + ")" : "") + ); +} + // One row of the balance list: symbol, quantity, fiat value. // // `symbol` is the ERC-20's own symbol() as the block explorer reported it, @@ -292,6 +310,9 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) { ); const seen = new Set(); for (const t of addr.tokenBalances || []) { + // A holding below 0.000001 is not listed, tracked or not. A tracked + // token then gets the zero row below while showZero is on. + if (isBelowOneMillionth(t.balance)) continue; // A null balance is a holding of an unstatable amount, not a holding // of zero, so the show-zero setting has no say over it: hiding it // would be asserting the zero nobody established. Anything that does @@ -322,14 +343,16 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) { } // Whether an address holds anything at all: ETH or any ERC-20 the wallet -// knows about. Deliberately unrounded — the rendered lines round to four -// decimals, so a dust balance displays as 0.0000 while still being real -// money at a real address. Callers that warn about holdings must ask this, -// not the rendered figure. +// knows about, except a token holding below 0.000001, which the balance list +// under the remove-address warning leaves out too. Deliberately unrounded — +// the rendered lines round to four decimals, so a dust balance displays as +// 0.0000 while still being real money at a real address. Callers that warn +// about holdings must ask this, not the rendered figure. function addressHoldsFunds(addr) { if (!addr) return false; if (parseFloat(addr.balance || "0") > 0) return true; for (const t of addr.tokenBalances || []) { + if (isBelowOneMillionth(t.balance)) continue; // A null balance is a holding whose amount could not be stated — // balances.js drops a row of zero base units before the scale is // consulted, so a row that survived with no quantity is holding @@ -614,6 +637,7 @@ module.exports = { balanceLinesForAddress, addressHoldsFunds, unknownableAmount, + formatFee, addressColor, addressDotHtml, escapeHtml, diff --git a/src/popup/views/send.js b/src/popup/views/send.js index 1600d14..87c1cd8 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -16,6 +16,10 @@ const { resolveTokenDecimals } = require("../../shared/approvalAmount"); const { resolveSymbol } = require("../../shared/tokenList"); const { isLowHolderCount } = require("../../shared/holders"); const { isSpoofedSymbol } = require("../../shared/symbolSpoof"); +const { + truncateAmountNeverZero, + isBelowOneMillionth, +} = require("../../shared/amountDisplay"); const { getAddress } = require("ethers"); const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000"; @@ -125,6 +129,10 @@ function renderSendTokenSelect(addr) { (state.fraudContracts || []).map((a) => a.toLowerCase()), ); for (const t of addr.tokenBalances || []) { + // A holding below 0.000001 is left out, as the balance lists leave it + // out. Its token's own screen can still send it: there + // state.selectedToken picks the token, not this list. + if (isBelowOneMillionth(t.balance)) continue; if (isSpoofedSymbol(t.symbol, t.address)) continue; if (fraudSet.has(t.address.toLowerCase())) continue; // An unknown holder count does not withhold a token the user holds: @@ -150,7 +158,9 @@ function updateSendBalance() { const token = state.selectedToken || $("send-token").value; if (token === "ETH") { $("send-balance").textContent = - "Current balance: " + (addr.balance || "0") + " ETH"; + "Current balance: " + + truncateAmountNeverZero(addr.balance || "0") + + " ETH"; } else { const tb = (addr.tokenBalances || []).find( (t) => t.address.toLowerCase() === token.toLowerCase(), @@ -167,7 +177,10 @@ function updateSendBalance() { $("send-balance").textContent = bal == null ? "Current balance: unknown (" + symbol + ")" - : "Current balance: " + bal + " " + symbol; + : "Current balance: " + + truncateAmountNeverZero(bal) + + " " + + symbol; } } diff --git a/src/shared/amountDisplay.js b/src/shared/amountDisplay.js index 0b5d3ae..ee2c441 100644 --- a/src/shared/amountDisplay.js +++ b/src/shared/amountDisplay.js @@ -6,10 +6,10 @@ // (`src/shared/uniswap.js`) — and a fix applied to one of them left the other // two showing a different number for the same value. // -// The two functions below are the two policies, not two implementations of -// one: summary lists truncate, and the screens that state what is being -// authorized truncate with a floor. Keeping them adjacent is the point, so a -// change to the rule cannot reach one screen and miss another. +// The two truncation functions below are the two policies, not two +// implementations of one: summary lists truncate, and the screens that state +// what is being authorized truncate with a floor. Keeping them adjacent is the +// point, so a change to the rule cannot reach one screen and miss another. // Truncate to exactly four decimal places. Truncation, never rounding: an // amount must never be displayed as larger than it is, so 0.99999 stays @@ -43,4 +43,18 @@ function truncateAmountNeverZero(val) { return parts[0] + "." + parts[1].slice(0, sig + 1); } -module.exports = { truncateAmount, truncateAmountNeverZero }; +// Whether a stored token balance is a holding below 0.000001. The balance +// lists, the send-screen token selector, the address total and the +// remove-address warning leave such a holding out; the Send and confirmation +// screens show it when its token is the one being sent. Exact, because +// src/shared/balances.js stores plain decimal digits: below 0.000001 the +// balance reads "0.000000" and then more digits. +function isBelowOneMillionth(balance) { + return typeof balance === "string" && balance.startsWith("0.000000"); +} + +module.exports = { + truncateAmount, + truncateAmountNeverZero, + isBelowOneMillionth, +}; diff --git a/src/shared/balances.js b/src/shared/balances.js index 0e9ad5c..fcff065 100644 --- a/src/shared/balances.js +++ b/src/shared/balances.js @@ -52,19 +52,16 @@ function requireNetworkId(networkId) { return net; } -function formatBalance(wei) { - const eth = formatEther(wei); - const parts = eth.split("."); - if (parts.length === 1) return eth + ".0"; - const dec = parts[1].slice(0, 6).replace(/0+$/, "") || "0"; - return parts[0] + "." + dec; -} - +// A token balance as an exact decimal string, never cut: a cut stores a small +// nonzero holding as zero. fetchTokenBalances() stores every nonzero holding of +// a token it admits, however small; the screens that leave out one below +// 0.000001 decide that themselves, through isBelowOneMillionth() in +// src/shared/amountDisplay.js. function formatTokenBalance(raw, decimals) { const val = formatUnits(raw, decimals); const parts = val.split("."); if (parts.length === 1) return val + ".0"; - const dec = parts[1].slice(0, 6).replace(/0+$/, "") || "0"; + const dec = parts[1].replace(/0+$/, "") || "0"; return parts[0] + "." + dec; } @@ -149,11 +146,7 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) { const scale = known !== null ? known : decimals; // null is a holding of an amount that cannot be stated, which is // not the same as a holding of zero, and must never render as one. - // With a scale, the display filter proper applies: a balance that - // rounds to zero at six places is dust and is not listed. Without - // one there is no such judgement to make, and the row is kept. const bal = scale === null ? null : formatTokenBalance(raw, scale); - if (bal === "0.0") continue; // null means the explorer reported no count, which is not the // same as a count of zero. This gate is not the low-holder // display filter: it has no user-facing off switch and governs @@ -221,7 +214,9 @@ async function refreshBalances( provider .getBalance(addr.address) .then((bal) => { - addr.balance = formatBalance(bal); + // Exact, never cut: a cut here stores a small nonzero + // balance as zero. + addr.balance = formatEther(bal); log.debugf("ETH balance", addr.address, addr.balance); }) .catch((e) => { diff --git a/src/shared/prices.js b/src/shared/prices.js index 02a1618..d6166ad 100644 --- a/src/shared/prices.js +++ b/src/shared/prices.js @@ -1,6 +1,7 @@ // Price fetching with 5-minute cache, USD formatting, value aggregation. const { getTopTokenPrices } = require("./tokenList"); +const { isBelowOneMillionth } = require("./amountDisplay"); const PRICE_CACHE_TTL = 300000; // 5 minutes @@ -78,6 +79,9 @@ function getAddressValue(addr) { let usd = parseFloat(addr.balance || "0") * prices.ETH; let partial = false; for (const token of addr.tokenBalances || []) { + // A holding below 0.000001 is left out, as the balance lists leave it + // out, so the total never counts a holding the list does not show. + if (isBelowOneMillionth(token.balance)) continue; // A null balance is a holding whose scale nothing knows, so it has no // quantity to price — but it is still a holding, and a total that // silently omits it would read as complete. That is exactly what diff --git a/src/shared/txValidation.js b/src/shared/txValidation.js index c55c66b..49e6314 100644 --- a/src/shared/txValidation.js +++ b/src/shared/txValidation.js @@ -139,7 +139,15 @@ function validateTransfer({ const feeFp = known ? feeWei : null; if (isErc20) { - const tokenFp = toFixedPoint(tokenBalance) ?? 0n; + // A token can declare more than 18 decimals, and its balance is + // stored with all of them. Only the first 18 places (SCALE_DECIMALS) + // are read: an amount with more was refused above, so the places + // after them cannot decide whether the amount fits. + const tokenText = + typeof tokenBalance === "string" + ? tokenBalance.replace(/(\.\d{18})\d+$/, "$1") + : tokenBalance; + const tokenFp = toFixedPoint(tokenText) ?? 0n; if (amountFp > tokenFp) codes.push(CODES.INSUFFICIENT_TOKEN); if (feeFp !== null && feeFp > ethFp) { codes.push(CODES.INSUFFICIENT_ETH_FOR_FEE); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 21ed29b..c3b61de 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -1463,9 +1463,10 @@ test("an over-long flash message keeps to one line (#252)", async (env) => { // on opposite sides of the reserve while sitting on the same side of the // estimate. -// The balance the funded fixture serves, and the amounts sent against it. +// The balance the funded fixture serves, as the Send and confirmation screens +// show it, and the amounts sent against it. const FUNDED_ETH_WEI = 10n ** 18n; -const FUNDED_ETH_TEXT = "1.0"; +const FUNDED_ETH_TEXT = "1.0000"; const COMFORTABLE_AMOUNT = "0.1"; const OVER_BALANCE_AMOUNT = "2.0"; @@ -1479,7 +1480,7 @@ const GAP_AMOUNT = formatEther(FUNDED_ETH_WEI - FEE_ESTIMATE_WEI); // fee test: it covers the expected cost to the wei and falls short of the // reserve, so the same swap flips this assertion too — through a different // balance and a different message than the ETH path uses. -const TOKEN_BALANCE_TEXT = "1.5"; +const TOKEN_BALANCE_TEXT = "1.5000"; const TOKEN_AMOUNT = "0.25"; const OVER_TOKEN_AMOUNT = "9.0"; const FEE_ONLY_ETH_WEI = FEE_ESTIMATE_WEI; @@ -1488,16 +1489,15 @@ function toHexWei(wei) { return "0x" + wei.toString(16); } -// A fee in wei as the confirmation screen writes it. Deliberately a second -// implementation of formatFeeEth() from src/popup/views/confirmTx.js rather -// than an import of it: that module pulls in the whole popup and cannot be -// required outside a browser, and asserting against an independent rendering -// is stronger than asserting a function equals itself. +// A fee in wei as the confirmation screen writes it: truncated to four decimal +// places (README.md, Display Consistency). Deliberately a second +// implementation rather than an import of src/shared/amountDisplay.js: +// asserting against an independent rendering is stronger than asserting a +// function equals itself. The fixture's fees are above 0.0001 ETH, so the +// nonzero floor never applies here. function feeEth(wei) { - const parts = formatEther(wei).split("."); - const dec = - parts.length > 1 ? parts[1].slice(0, 6).replace(/0+$/, "") || "0" : "0"; - return parts[0] + "." + dec + " ETH"; + const [whole, frac = ""] = formatEther(wei).split("."); + return whole + "." + (frac + "0000").slice(0, 4) + " ETH"; } // What the confirmation screen is showing right now, read out of the DOM in @@ -1554,11 +1554,10 @@ async function backToAddress(page) { // Drive the popup to the confirmation screen for one send. // -// It waits for the send screen to be showing `balance` before filling -// anything in. That figure is the exact number the spend gate compares -// against, so waiting for it — rather than for a refresh to have probably -// landed — is what keeps every assertion below deterministic after a -// fixture change. +// It waits for the send screen to be showing `balance`, the fixture's balance +// as that screen displays it, before filling anything in. Waiting for it — +// rather than for a refresh to have probably landed — is what keeps every +// assertion below deterministic after a fixture change. async function goToConfirm(page, { token, balance, amount }) { await backToAddress(page); await page.click("#btn-send"); diff --git a/tests/sendDisplayFloor.test.js b/tests/sendDisplayFloor.test.js new file mode 100644 index 0000000..a05821b --- /dev/null +++ b/tests/sendDisplayFloor.test.js @@ -0,0 +1,452 @@ +// The balance and fee lines of the Send and confirmation screens, and the fee +// line they must share with the approval screen. +// +// An ETH balance, a token balance or a fee below 0.000001 rendered as zero on +// these screens (https://git.eeqj.de/sneak/AutistMask/issues/343): the stored +// balances and the fee were each cut to six decimal places, a rule of their +// own, and a token holding cut to zero was dropped, while the approval screen +// showed the same fee through src/shared/amountDisplay.js with the nonzero +// floor. The balances are now stored exactly, every nonzero token holding +// kept, and the screens show them and the fee through that helper. +// +// Driven through the real refreshBalances(), Send screen, confirmation screen +// and approval screen, with only the node, the explorer and the DOM stubbed: a +// balance written onto state by hand would skip the place the cut happened. + +"use strict"; + +// What the stub node answers. Each test sets what it needs. +const mockNode = { + balanceWei: 0n, + feeData: { maxFeePerGas: 1n, gasPrice: 1n }, +}; + +// The token rows the stub explorer reports for the address. +const mockExplorer = { items: [] }; + +jest.mock("ethers", () => { + const actual = jest.requireActual("ethers"); + class StubProvider { + async getBalance() { + return mockNode.balanceWei; + } + async lookupAddress() { + return null; + } + async getFeeData() { + return mockNode.feeData; + } + async estimateGas() { + return 21000n; + } + async getCode() { + return "0x"; + } + async getTransactionCount() { + return 1; + } + } + return { + ...actual, + JsonRpcProvider: StubProvider, + Network: { from: () => ({}) }, + }; +}); + +jest.mock("../src/shared/log", () => ({ + log: { + debugf: () => {}, + infof: () => {}, + warnf: () => {}, + errorf: () => {}, + }, + // The explorer's token list, which refreshBalances() also fetches. + debugFetch: jest.fn(async () => ({ + ok: true, + status: 200, + json: async () => mockExplorer.items, + })), + setRuntimeDebug: () => {}, + isDebug: () => false, +})); + +// The confirmation screen's Etherscan label lookup is the only fetch() these +// screens make; it fails, as it does offline. +global.fetch = jest.fn(() => { + throw new Error("tests must not perform network requests"); +}); + +// The approval the background hands the approval screen. Set per test. +let approvalDetails = null; + +const { makeStorageStub } = require("./support/storageStub"); +global.chrome = { + storage: makeStorageStub(), + runtime: { + connect: () => ({ + postMessage() {}, + disconnect() {}, + onDisconnect: { addListener() {} }, + }), + sendMessage(message, callback) { + callback( + message.type === "AUTISTMASK_GET_APPROVAL" + ? approvalDetails + : undefined, + ); + }, + }, +}; + +// A stub DOM: every id resolves to a recording element. +const elements = new Map(); + +function makeEl(id) { + const handlers = new Map(); + return { + id, + textContent: "", + innerHTML: "", + value: "", + disabled: false, + style: {}, + dataset: {}, + classList: { + add() {}, + remove() {}, + toggle() {}, + contains: () => false, + }, + handlers, + children: [], + addEventListener(name, fn) { + handlers.set(name, fn); + }, + appendChild(child) { + this.children.push(child); + return child; + }, + querySelectorAll: () => [], + querySelector: () => null, + remove() {}, + focus() {}, + }; +} + +global.document = { + getElementById(id) { + if (!elements.has(id)) elements.set(id, makeEl(id)); + return elements.get(id); + }, + createElement: (tag) => makeEl(tag), + body: { prepend() {}, appendChild() {} }, + addEventListener() {}, +}; +global.navigator = { clipboard: { writeText() {} } }; + +const { refreshBalances } = require("../src/shared/balances"); +const { state } = require("../src/shared/state"); +const { + prices, + clearPrices, + formatAddressTotal, + getAddressValue, +} = require("../src/shared/prices"); +const send = require("../src/popup/views/send"); +const confirmTx = require("../src/popup/views/confirmTx"); +const approval = require("../src/popup/views/approval"); +const { + addressHoldsFunds, + balanceLinesForAddress, +} = require("../src/popup/views/helpers"); + +const HOLDER = "0x" + "a".repeat(40); +const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe"; + +// 0.0000005 ETH, or 0.0000005 of an 18-decimal token. +const HALF_MICRO_ETH = 500000000000n; + +// A token the bundled list does not know. +const TOKEN = "0x" + "d".repeat(40); + +// The explorer's row for TOKEN, holding `value` base units. With only five +// holders, it is listed only when the user tracks the token. +function tokenRow(value, token = {}) { + return { + value: String(value), + token: { + type: "ERC-20", + address_hash: TOKEN, + symbol: "TOK", + name: "Token", + decimals: "18", + holders_count: "5", + ...token, + }, + }; +} + +function text(id) { + return global.document.getElementById(id).textContent; +} + +function errors() { + return global.document.getElementById("confirm-errors").innerHTML; +} + +// The ETH balance the node reports and the token rows the explorer reports, +// fetched and stored exactly where the popup stores them. +async function refreshWith(balanceWei, tokenItems = []) { + mockNode.balanceWei = balanceWei; + mockExplorer.items = tokenItems; + state.wallets = [{ name: "Wallet 1", addresses: [{ address: HOLDER }] }]; + state.selectedWallet = 0; + state.selectedAddress = 0; + await refreshBalances( + state.wallets, + "https://rpc.example.invalid", + "https://blockscout.example/api/v2", + state.trackedTokens, + "mainnet", + ); +} + +// Press Review on the Send screen for a send of `token` ("ETH" or a token +// address), and show the confirmation screen it leads to with its fee estimate +// settled. +async function confirmSend(amount, token = "ETH") { + let txInfo = null; + send.init({ showConfirmTx: (info) => (txInfo = info) }); + state.selectedToken = token; + global.document.getElementById("send-to").value = RECIPIENT; + global.document.getElementById("send-amount").value = amount; + await global.document + .getElementById("btn-send-review") + .handlers.get("click")(); + confirmTx.show(txInfo); + for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); +} + +// The approval screen for a dApp transaction of 21000 gas, the gas the stub +// node estimates for the send above. +async function approveTxWithFeePerGas(maxFeePerGas) { + approvalDetails = { + type: "tx", + hostname: "dapp.example", + approvedFrom: HOLDER, + approvedTx: { + to: RECIPIENT, + value: "0", + data: "0x", + chainId: "0x1", + gasLimit: "21000", + maxFeePerGas: String(maxFeePerGas), + nonce: 0, + }, + }; + await approval.show("1"); +} + +beforeEach(() => { + elements.clear(); + state.selectedToken = null; + state.trackedTokens = []; + state.fraudContracts = []; + state.currentView = null; + mockNode.feeData = { maxFeePerGas: 1n, gasPrice: 1n }; +}); + +describe("an ETH balance below 0.000001 never renders as zero", () => { + test("on the Send screen", async () => { + await refreshWith(HALF_MICRO_ETH); + state.selectedToken = "ETH"; + send.updateSendBalance(); + expect(text("send-balance")).toBe("Current balance: 0.0000005 ETH"); + }); + + test("on the confirmation screen", async () => { + await refreshWith(HALF_MICRO_ETH); + await confirmSend("0.0000001"); + expect(text("confirm-balance")).toBe("0.0000005 ETH"); + }); + + test("while a balance above the floor keeps four decimals", async () => { + await refreshWith(1234567890000000000n); + await confirmSend("0.1"); + expect(text("confirm-balance")).toBe("1.2345 ETH"); + }); +}); + +// A token the user tracks stays on the balance list when the explorer's row is +// dropped, so a holding of it below 0.000001 reached these screens as zero, and +// the send was checked against zero. +describe("a tracked token holding below 0.000001 never renders as zero", () => { + beforeEach(() => { + state.trackedTokens = [ + { address: TOKEN, symbol: "TOK", name: "Token", decimals: 18 }, + ]; + }); + + test("on the Send screen", async () => { + await refreshWith(10n ** 18n, [tokenRow(HALF_MICRO_ETH)]); + state.selectedToken = TOKEN; + send.updateSendBalance(); + expect(text("send-balance")).toBe("Current balance: 0.0000005 TOK"); + }); + + test("on the confirmation screen, which checks the send against it", async () => { + await refreshWith(10n ** 18n, [tokenRow(HALF_MICRO_ETH)]); + await confirmSend("0.0000005", TOKEN); + expect(text("confirm-balance")).toBe("0.0000005 TOK"); + expect(errors()).toBe(""); + await confirmSend("0.0000006", TOKEN); + expect(errors()).toContain( + "You have 0.0000005 TOK but are trying to send 0.0000006 TOK.", + ); + }); + + // The stored balance keeps all 24 places, and the balance check reads the + // first 18 of them rather than refusing it as no balance at all. + test("with more than 18 decimals, the send is checked against 18 of them", async () => { + state.trackedTokens[0].decimals = 24; + // 1.5 plus one base unit. + const value = 15n * 10n ** 23n + 1n; + await refreshWith(10n ** 18n, [tokenRow(value, { decimals: "24" })]); + await confirmSend("1.5", TOKEN); + expect(text("confirm-balance")).toBe("1.5000 TOK"); + expect(errors()).toBe(""); + }); + + test("with more than 18 decimals and a holding below 10^-18", async () => { + state.trackedTokens[0].decimals = 24; + // One base unit, 0.000000000000000000000001 TOK. + await refreshWith(10n ** 18n, [tokenRow(1n, { decimals: "24" })]); + state.selectedToken = TOKEN; + send.updateSendBalance(); + expect(text("send-balance")).toBe( + "Current balance: 0.000000000000000000000001 TOK", + ); + await confirmSend("0.000000000000000001", TOKEN); + expect(text("confirm-balance")).toBe("0.000000000000000000000001 TOK"); + expect(errors()).toContain( + "You have 0.000000000000000000000001 TOK but are trying to send" + + " 0.000000000000000001 TOK.", + ); + }); +}); + +// A token the user does not track, with enough holders to be admitted. The +// balance fetch dropped a holding of it below 0.000001, but the token stays +// selected while its own screen is open: after sending 2 of a 2.0000003 +// holding, the user is back on that screen, and Send read the missing row as +// zero. +describe("an untracked token holding below 0.000001 never renders as zero", () => { + const row = () => tokenRow(HALF_MICRO_ETH, { holders_count: "50000" }); + + test("on the Send screen", async () => { + await refreshWith(10n ** 18n, [row()]); + state.selectedToken = TOKEN; + send.updateSendBalance(); + expect(text("send-balance")).toBe("Current balance: 0.0000005 TOK"); + }); + + test("on the confirmation screen, which checks the send against it", async () => { + await refreshWith(10n ** 18n, [row()]); + await confirmSend("0.0000005", TOKEN); + expect(text("confirm-balance")).toBe("0.0000005 TOK"); + expect(errors()).toBe(""); + await confirmSend("0.0000006", TOKEN); + expect(errors()).toContain( + "You have 0.0000005 TOK but are trying to send 0.0000006 TOK.", + ); + }); +}); + +// The fetch keeps every holding, so the screens that showed only what it kept +// leave out a holding below 0.000001 themselves, and look as they did. +describe("a token holding below 0.000001 is still not listed", () => { + afterEach(() => { + clearPrices(); + }); + + test("for a token the user does not track", async () => { + prices.ETH = 3000; + await refreshWith(0n, [ + tokenRow(HALF_MICRO_ETH, { holders_count: "50000" }), + ]); + const addr = state.wallets[0].addresses[0]; + expect(balanceLinesForAddress(addr, [], true)).not.toContain(TOKEN); + expect(balanceLinesForAddress(addr, [], false)).not.toContain(TOKEN); + send.renderSendTokenSelect(addr); + const options = global.document.getElementById("send-token").children; + expect(options.map((o) => o.value)).toEqual([]); + // Not an unpriced token in the total, and not funds on the + // remove-address warning. + expect(formatAddressTotal(getAddressValue(addr))).toBe("Total: $0.00"); + expect(addressHoldsFunds(addr)).toBe(false); + }); + + // As a tracked token holding nothing: listed only while zero balances are + // shown. + test("for a tracked token, unless zero balances are shown", async () => { + state.trackedTokens = [ + { address: TOKEN, symbol: "TOK", name: "Token", decimals: 18 }, + ]; + await refreshWith(0n, [tokenRow(HALF_MICRO_ETH)]); + const addr = state.wallets[0].addresses[0]; + expect( + balanceLinesForAddress(addr, state.trackedTokens, false), + ).not.toContain(TOKEN); + expect( + balanceLinesForAddress(addr, state.trackedTokens, true), + ).toContain(`data-token="${TOKEN}"`); + }); +}); + +describe("a fee below 0.000001 ETH never renders as zero", () => { + test("when the estimate and the reserve are the same", async () => { + await refreshWith(10n ** 18n); + await confirmSend("0.1"); + // 21000 gas at 1 wei is 0.000000000000021 ETH, shown to its first + // significant digit. + expect(text("confirm-fee-amount")).toBe("0.00000000000002 ETH"); + }); + + test("when they differ, on both lines", async () => { + mockNode.feeData = { maxFeePerGas: 2n, gasPrice: 1n }; + await refreshWith(10n ** 18n); + await confirmSend("0.1"); + expect(text("confirm-fee-amount")).toBe("~0.00000000000002 ETH"); + expect(text("confirm-fee-reserve")).toBe( + "up to 0.00000000000004 ETH reserved", + ); + }); +}); + +// The confirmation screen shows the reserve alone when the node quotes no +// cheaper estimate, and that reserve is the same gas limit times maximum fee +// per gas that the approval screen calls the max fee. An ETH price is set, as +// it is on mainnet, so the USD value has to match too. +describe("the same fee reads the same on the confirmation and approval screens", () => { + beforeEach(() => { + prices.ETH = 3000; + }); + afterEach(() => { + clearPrices(); + }); + + test.each([ + // 21000 gas at 1 wei. + ["below the floor", 1n, "0.00000000000002 ETH (< $0.01)"], + // 0.001235294117631 ETH, which is $3.71. Pricing the truncated + // 0.0012 instead would read $3.60. + ["with more than four decimals", 58823529411n, "0.0012 ETH ($3.71)"], + ])("%s", async (_label, feePerGas, expected) => { + mockNode.feeData = { maxFeePerGas: feePerGas, gasPrice: feePerGas }; + await refreshWith(10n ** 18n); + await confirmSend("0.1"); + expect(text("confirm-fee-amount")).toBe(expected); + await approveTxWithFeePerGas(feePerGas); + expect(text("approve-tx-fee")).toBe(expected); + }); +}); diff --git a/tests/unknownScaleSend.test.js b/tests/unknownScaleSend.test.js index 9be1839..2c326d7 100644 --- a/tests/unknownScaleSend.test.js +++ b/tests/unknownScaleSend.test.js @@ -389,7 +389,7 @@ describe("a scale the explorer's own rows disagree about", () => { expect(txInfo.tokenBalance).toBe("5.0"); confirmTx.show(txInfo); await settle(); - expect(text("confirm-balance")).toBe("5.0 NOVEL"); + expect(text("confirm-balance")).toBe("5.0000 NOVEL"); expect(errors()).toBe(""); expect(sendDisabled()).toBe(false); }); @@ -431,7 +431,7 @@ describe("the confirmation screen tells an unknown balance from a zero one", () const zero = await render("0.0"); expect(unknown.balance).not.toBe(zero.balance); expect(unknown.balance).toBe("unknown (NOVEL)"); - expect(zero.balance).toBe("0.0 NOVEL"); + expect(zero.balance).toBe("0.0000 NOVEL"); }); // Both hit INSUFFICIENT_TOKEN — an unknown balance is treated as nothing to @@ -443,7 +443,7 @@ describe("the confirmation screen tells an unknown balance from a zero one", () expect(unknown.errors).not.toBe(zero.errors); expect(unknown.errors).toContain("This token's balance is unknown"); expect(unknown.errors).not.toContain("You have"); - expect(zero.errors).toContain("You have 0.0 NOVEL"); + expect(zero.errors).toContain("You have 0.0000 NOVEL"); expect(zero.errors).not.toContain("balance is unknown"); }); });