fix: the balance lists and Home cut amounts to four decimals (closes #757)
The balance lists and Home's ETH line rounded with toFixed(4), so 0.99996 ETH read 1.0000 there and 0.9999 on Send. They now cut the stored decimal string with truncateAmount(), as the history rows do. balanceLine() takes the stored balance string instead of a parsed number, and escapes the quantity cell. The screens that state what is authorized keep the floor of truncateAmountNeverZero(). Model: opus-5-5
This commit is contained in:
1 parent
6d158bf1b1
commit
fb7c75101f
12 files changed
+146
-54
No files matched your search
@@ -1191,8 +1191,8 @@ These helpers write one kind of value on every screen that shows it:
|
||||
|
||||
Most screens show token and ETH amounts with four decimal places. History rows
|
||||
cut amounts to four decimals and never round up (`truncateAmount()` in
|
||||
`src/shared/amountDisplay.js`). The balance lists and Home's ETH balance round
|
||||
to four places. The screens that state what is being authorized cut as the
|
||||
`src/shared/amountDisplay.js`). The balance lists and Home's ETH balance cut
|
||||
them the same way. The screens that state what is being authorized cut as the
|
||||
history does, and extend an amount below 0.0001 to its first nonzero digit
|
||||
(`truncateAmountNeverZero()`). So a nonzero amount never reads as zero there.
|
||||
Those screens are:
|
||||
|
||||
@@ -159,10 +159,10 @@ function renderBalance() {
|
||||
const addr =
|
||||
state.wallets[state.selectedWallet].addresses[state.selectedAddress];
|
||||
const tokenId = state.selectedToken;
|
||||
let amount, price;
|
||||
let balance, price;
|
||||
if (tokenId === "ETH") {
|
||||
// null when no refresh has read the balance.
|
||||
amount = unknownableAmount(addr.balance);
|
||||
balance = addr.balance;
|
||||
price = getPrice("ETH");
|
||||
} else {
|
||||
const id = tokenId.toLowerCase();
|
||||
@@ -175,12 +175,13 @@ function renderBalance() {
|
||||
// null when the scale is unknown, or when no refresh has read the
|
||||
// token's balance: no quantity to show, and none to price.
|
||||
// balanceLine() states that rather than printing 0.0000.
|
||||
if (tb) amount = unknownableAmount(tb.balance);
|
||||
else amount = tracked && tracked.balanceNotFetched ? null : 0;
|
||||
if (tb) balance = tb.balance;
|
||||
else balance = tracked && tracked.balanceNotFetched ? null : "0";
|
||||
price = getPrice(tokenId);
|
||||
}
|
||||
|
||||
// USD total for this token only
|
||||
const amount = unknownableAmount(balance);
|
||||
const usdVal = price && amount !== null ? amount * price : null;
|
||||
const usdStr = formatUsd(usdVal);
|
||||
$("address-token-usd-total").innerHTML = escapeHtml(usdStr) || " ";
|
||||
@@ -188,7 +189,7 @@ function renderBalance() {
|
||||
// Single token balance line (no tokenId — not clickable here)
|
||||
$("address-token-balance").innerHTML = balanceLine(
|
||||
currentSymbol,
|
||||
amount,
|
||||
balance,
|
||||
price,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -79,8 +79,8 @@ function recoveryPathText(wallet) {
|
||||
// "Holds" means ETH or any ERC-20 the wallet knows about — an address with no
|
||||
// ETH and a five-figure stablecoin position must not get the blank line on
|
||||
// the one screen whose job is to warn. The sentence names no figure of its
|
||||
// own: the rendered lines round to four decimals, so a sentence built from a
|
||||
// rounded number would report "0.0000 ETH" for an address holding real money.
|
||||
// own: the rendered lines cut to four decimals, so a sentence built from their
|
||||
// figure would report "0.0000 ETH" for an address holding real money.
|
||||
// The lines below it carry the amounts, in the same format as Home and
|
||||
// AddressDetail, followed by the USD total when there is one to give — no
|
||||
// total line at all on testnet, before the first price fetch or while no
|
||||
|
||||
+14
-11
@@ -18,6 +18,7 @@
|
||||
const { formatEther } = require("ethers");
|
||||
const makeBlockie = require("ethereum-blockies-base64");
|
||||
const {
|
||||
truncateAmount,
|
||||
truncateAmountNeverZero,
|
||||
isBelowOneMillionth,
|
||||
} = require("../../shared/amountDisplay");
|
||||
@@ -373,13 +374,15 @@ function formatFee(wei, symbol, chainId) {
|
||||
// the row that issue #307 was reported against: every screen that lists a
|
||||
// holding renders through here.
|
||||
//
|
||||
// `amount` is null for a holding whose scale nothing knows
|
||||
// `balance` is the stored balance, a decimal string. It is null for a holding
|
||||
// whose scale nothing knows
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/349), and for a balance no
|
||||
// refresh has read. There is no quantity to print for it and no fiat value to
|
||||
// derive from one, and printing 0.0000 for a real holding is the failure this
|
||||
// whole rule exists to prevent, so the row says so instead.
|
||||
function balanceLine(symbol, amount, price, tokenId) {
|
||||
const qty = amount === null ? "quantity unknown" : amount.toFixed(4);
|
||||
function balanceLine(symbol, balance, price, tokenId) {
|
||||
const amount = unknownableAmount(balance);
|
||||
const qty = amount === null ? "quantity unknown" : truncateAmount(balance);
|
||||
const usd =
|
||||
price && amount !== null
|
||||
? escapeHtml(formatUsd(amount * price)) || " "
|
||||
@@ -394,7 +397,7 @@ function balanceLine(symbol, amount, price, tokenId) {
|
||||
`<div class="flex text-xs${clickClass}"${tokenAttr}>` +
|
||||
`<span class="flex justify-between w-[42ch] max-w-full">` +
|
||||
`<span>${escapeHtml(displaySymbol(symbol))}</span>` +
|
||||
`<span>${qty}</span>` +
|
||||
`<span>${escapeHtml(qty)}</span>` +
|
||||
`</span>` +
|
||||
`<span class="text-right text-muted flex-1">${usd}</span>` +
|
||||
`</div>`
|
||||
@@ -420,7 +423,7 @@ function markTrackedTokensNotFetched() {
|
||||
function balanceLinesForAddress(addr, trackedTokens, showZero) {
|
||||
let html = balanceLine(
|
||||
nativeCurrency(),
|
||||
unknownableAmount(addr.balance),
|
||||
addr.balance,
|
||||
getPrice("ETH"),
|
||||
"ETH",
|
||||
);
|
||||
@@ -438,7 +441,7 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
|
||||
if (bal === 0 && !showZero) continue;
|
||||
html += balanceLine(
|
||||
t.symbol,
|
||||
bal,
|
||||
t.balance,
|
||||
getPrice(t.address),
|
||||
t.address.toLowerCase(),
|
||||
);
|
||||
@@ -456,7 +459,7 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
|
||||
}
|
||||
html += balanceLine(
|
||||
t.symbol,
|
||||
t.balanceNotFetched ? null : 0,
|
||||
t.balanceNotFetched ? null : "0",
|
||||
getPrice(t.address),
|
||||
t.address.toLowerCase(),
|
||||
);
|
||||
@@ -467,10 +470,10 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
|
||||
|
||||
// Whether an address holds anything at all: ETH or any ERC-20 the wallet
|
||||
// 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.
|
||||
// under the remove-address warning leaves out too. Deliberately exact — the
|
||||
// rendered lines cut 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;
|
||||
// A null or absent ETH balance reads as unknown, so the address may hold
|
||||
|
||||
@@ -31,6 +31,7 @@ const {
|
||||
resetSendValidation,
|
||||
} = require("./send");
|
||||
const { deriveAddressFromXpub } = require("../../shared/wallet");
|
||||
const { truncateAmount } = require("../../shared/amountDisplay");
|
||||
const { canRemoveAddress } = require("../../shared/walletDelete");
|
||||
const {
|
||||
walletDefect,
|
||||
@@ -81,7 +82,7 @@ function renderTotalValue() {
|
||||
if (ethBal === null) {
|
||||
el.textContent = "unknown (" + nativeCurrency() + ")";
|
||||
} else {
|
||||
const ethStr = ethBal.toFixed(4) + " " + nativeCurrency();
|
||||
const ethStr = truncateAmount(addr.balance) + " " + nativeCurrency();
|
||||
const ethUsd = ethPrice
|
||||
? " (" + formatUsd(ethBal * ethPrice) + ")"
|
||||
: "";
|
||||
|
||||
@@ -25,10 +25,6 @@ const { ERC20_ABI } = require("../src/shared/constants");
|
||||
const { state } = require("../src/shared/state");
|
||||
const { decodeCalldata } = require("../src/shared/calldataDecoder");
|
||||
const uniswap = require("../src/shared/uniswap");
|
||||
const {
|
||||
truncateAmount,
|
||||
truncateAmountNeverZero,
|
||||
} = require("../src/shared/amountDisplay");
|
||||
|
||||
const iface = new Interface(ERC20_ABI);
|
||||
|
||||
@@ -183,15 +179,4 @@ describe("the four-decimal rule is otherwise unchanged", () => {
|
||||
test("a genuine zero still renders as zero", () => {
|
||||
expect(amount(transferData(0n), DAI).rawValue).toBe("0.0000");
|
||||
});
|
||||
|
||||
// The three truncators now share one module. The floor is a policy of the
|
||||
// approval and confirmation screens only: the history and balance lists
|
||||
// keep plain truncation, because the transaction detail view is the
|
||||
// authoritative record and already shows exact precision.
|
||||
test("the list rule stays unfloored", () => {
|
||||
expect(truncateAmount("0.000000000000000001")).toBe("0.0000");
|
||||
expect(truncateAmountNeverZero("0.000000000000000001")).toBe(
|
||||
"0.000000000000000001",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -41,13 +41,13 @@ describe("balanceLine", () => {
|
||||
// this payload before its id attribute, so an assertion about the
|
||||
// rest of the payload would pass on the cap and say nothing about
|
||||
// the escape.
|
||||
const html = balanceLine(HOSTILE_SYMBOL, 1, null, null);
|
||||
const html = balanceLine(HOSTILE_SYMBOL, "1", null, null);
|
||||
expect(html).not.toContain("<iframe");
|
||||
expect(html).toContain("<iframe");
|
||||
});
|
||||
|
||||
test("caps the symbol before rendering it", () => {
|
||||
const html = balanceLine("A".repeat(4096), 1, null, null);
|
||||
const html = balanceLine("A".repeat(4096), "1", null, null);
|
||||
expect(html).toContain("A".repeat(MAX_SYMBOL_LENGTH - 1) + "…");
|
||||
expect(html).not.toContain("A".repeat(MAX_SYMBOL_LENGTH + 1));
|
||||
});
|
||||
@@ -55,13 +55,13 @@ describe("balanceLine", () => {
|
||||
// The token id lands inside data-token="...", so a quote in it is a
|
||||
// way out of the attribute and into a new one.
|
||||
test("keeps a quote-bearing token id inside its attribute", () => {
|
||||
const html = balanceLine("TKN", 1, null, '" onclick="alert(1)');
|
||||
const html = balanceLine("TKN", "1", null, '" onclick="alert(1)');
|
||||
expect(html).not.toContain('onclick="');
|
||||
expect(html).toContain('data-token="" onclick="alert(1)"');
|
||||
});
|
||||
|
||||
test("renders an ordinary holding unchanged", () => {
|
||||
const html = balanceLine("USDC", 1.5, null, "0xabc");
|
||||
const html = balanceLine("USDC", "1.5", null, "0xabc");
|
||||
expect(html).toContain("<span>USDC</span>");
|
||||
expect(html).toContain("<span>1.5000</span>");
|
||||
expect(html).toContain('data-token="0xabc"');
|
||||
@@ -69,7 +69,7 @@ describe("balanceLine", () => {
|
||||
|
||||
// formatUsd() writes a value under a cent as "< $0.01".
|
||||
test("escapes the USD value along with the symbol", () => {
|
||||
const html = balanceLine("USDC", 0.001, 1, null);
|
||||
const html = balanceLine("USDC", "0.001", 1, null);
|
||||
expect(html).toContain("< $0.01");
|
||||
expect(html).not.toContain("< $0.01");
|
||||
});
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
// The balance lists and Home's ETH line show a balance as the history rows do
|
||||
// (README.md, Amounts): cut to four decimal places, never rounded up
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/757).
|
||||
//
|
||||
// Driven through the real popup over a stored profile on Ethereum Mainnet.
|
||||
|
||||
const {
|
||||
ADDRESS,
|
||||
bootPopup,
|
||||
cleanupPopup,
|
||||
unversionedValidProfile,
|
||||
} = require("./support/popupBoot");
|
||||
const { balanceLine } = require("../src/popup/views/helpers");
|
||||
|
||||
const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
|
||||
|
||||
afterEach(() => {
|
||||
cleanupPopup();
|
||||
});
|
||||
|
||||
// The quantity cell of a balance row.
|
||||
function quantity(row) {
|
||||
return row.match(/<span>([^<]*)<\/span><\/span>/)[1];
|
||||
}
|
||||
|
||||
// The popup on Home over one address holding `eth` ETH and `usdc` USDC.
|
||||
function bootHolding(eth, usdc) {
|
||||
const profile = unversionedValidProfile({
|
||||
trackedTokens: [{ address: USDC, symbol: "USDC", decimals: 6 }],
|
||||
});
|
||||
profile.wallets[0].addresses[0] = {
|
||||
address: ADDRESS,
|
||||
balance: eth,
|
||||
tokenBalances: [
|
||||
{ address: USDC, symbol: "USDC", decimals: 6, balance: usdc },
|
||||
],
|
||||
};
|
||||
return bootPopup(profile);
|
||||
}
|
||||
|
||||
// The balance row for `token` in Home's wallet list.
|
||||
function homeRow(page, token) {
|
||||
return page
|
||||
.node("wallet-list")
|
||||
.innerHTML.split("<div ")
|
||||
.find((row) => row.includes('data-token="' + token + '"'));
|
||||
}
|
||||
|
||||
describe("balanceLine", () => {
|
||||
test("cuts 0.99996 to 0.9999, never rounding it up to 1.0000", () => {
|
||||
expect(quantity(balanceLine("ETH", "0.99996", null, "ETH"))).toBe(
|
||||
"0.9999",
|
||||
);
|
||||
});
|
||||
|
||||
test("cuts 0.00004 to 0.0000", () => {
|
||||
expect(quantity(balanceLine("USDC", "0.00004", null, USDC))).toBe(
|
||||
"0.0000",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Home", () => {
|
||||
test("the ETH line and the balance list read 0.99996 ETH as 0.9999", async () => {
|
||||
const page = await bootHolding("0.99996", "0.00004");
|
||||
|
||||
expect(page.visibleViews()).toEqual(["main"]);
|
||||
expect(page.text("total-value")).toBe("0.9999 ETH");
|
||||
expect(quantity(homeRow(page, "ETH"))).toBe("0.9999");
|
||||
});
|
||||
|
||||
test("the balance list reads 0.00004 USDC as 0.0000", async () => {
|
||||
const page = await bootHolding("0.99996", "0.00004");
|
||||
|
||||
expect(quantity(homeRow(page, USDC.toLowerCase()))).toBe("0.0000");
|
||||
});
|
||||
});
|
||||
@@ -4,8 +4,8 @@
|
||||
// the copy is the substance and is tested as such. Two things it must not
|
||||
// get wrong: what it takes to get the address back — the app refuses both
|
||||
// obvious routes — and what counts as holding something, which is any
|
||||
// ERC-20 as well as ETH, at any size, including a balance that rounds to
|
||||
// zero at the four decimals the balance lines render. The DOM behaviour
|
||||
// ERC-20 as well as ETH, at any size, including a balance below the four
|
||||
// decimals the balance lines render. The DOM behaviour
|
||||
// around them is driven against the real popup by tests/e2e/run.js.
|
||||
|
||||
// helpers.js pulls in state.js, which reads chrome.storage.local at load.
|
||||
|
||||
+9
-9
@@ -829,12 +829,11 @@ test("a common-token quick-pick fills in the contract address (#150)", async (en
|
||||
await visible(env.page, "#view-address");
|
||||
});
|
||||
|
||||
// The native amount as the transaction list writes it (four decimals) and
|
||||
// as the detail screen writes it (full precision). Both are rendered here
|
||||
// from the fixture rather than read off the screen, so the assertions
|
||||
// compare against the wei the stub served.
|
||||
const NATIVE_ROW_TEXT =
|
||||
parseFloat(formatEther(STUB_NATIVE_VALUE_WEI)).toFixed(4) + " ETH";
|
||||
// The native amount as the transaction list writes it (cut to four decimals,
|
||||
// quantity() below) and as the detail screen writes it (full precision). Both
|
||||
// are rendered here from the fixture rather than read off the screen, so the
|
||||
// assertions compare against the wei the stub served.
|
||||
const NATIVE_ROW_TEXT = quantity(STUB_NATIVE_VALUE_WEI) + " ETH";
|
||||
const NATIVE_DETAIL_TEXT = formatEther(STUB_NATIVE_VALUE_WEI) + " ETH";
|
||||
|
||||
test("the native ETH transaction detail still renders (#151)", async (env) => {
|
||||
@@ -2724,10 +2723,11 @@ async function goToConfirm(page, send) {
|
||||
await assertAddressesFit(page, "the confirmation screen");
|
||||
}
|
||||
|
||||
// A balance as the main view renders it: balanceLinesForAddress() writes
|
||||
// every quantity with four decimal places.
|
||||
// An amount in wei as the main view's balance lines and the transaction list
|
||||
// write it: cut to four decimal places, never rounded, as feeEth() does.
|
||||
function quantity(wei) {
|
||||
return parseFloat(formatEther(wei)).toFixed(4);
|
||||
const [whole, frac = ""] = formatEther(wei).split(".");
|
||||
return whole + "." + (frac + "0000").slice(0, 4);
|
||||
}
|
||||
|
||||
// Wait on the main view until a changed balance fixture has been picked up.
|
||||
|
||||
@@ -1361,6 +1361,31 @@ describe("fetchRecentTransactions merge and dedup", () => {
|
||||
expect(txs[0].holders).toBe(3500000);
|
||||
});
|
||||
|
||||
// README.md, Amounts: a history row cuts to four decimals, with no floor.
|
||||
test("a token transfer below 0.0001 reads 0.0000", async () => {
|
||||
respondWith(
|
||||
[],
|
||||
[
|
||||
{
|
||||
transaction_hash: "0x" + "4".repeat(64),
|
||||
block_number: 21000040,
|
||||
timestamp: TS,
|
||||
from: { hash: ORDINARY_PEER },
|
||||
to: { hash: VICTIM },
|
||||
total: { value: "40", decimals: "6" },
|
||||
token: {
|
||||
symbol: "USDC",
|
||||
address_hash: USDC_CONTRACT,
|
||||
holders_count: "3500000",
|
||||
},
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
const txs = await fetchRecentTransactions(VICTIM, BLOCKSCOUT, "0x1");
|
||||
expect(txs[0].value).toBe("0.0000");
|
||||
});
|
||||
|
||||
test("two transfers of the same token in one transaction collapse to one entry", async () => {
|
||||
const hash = "0x" + "3".repeat(64);
|
||||
const leg = (value) => ({
|
||||
|
||||
@@ -96,7 +96,7 @@ afterEach(() => {
|
||||
describe("balanceLine", () => {
|
||||
test("an unknown quantity and a zero one render differently", () => {
|
||||
const unknown = balanceLine("NOVEL", null, null, NOVEL);
|
||||
const zero = balanceLine("NOVEL", 0, null, NOVEL);
|
||||
const zero = balanceLine("NOVEL", "0", null, NOVEL);
|
||||
expect(unknown).not.toBe(zero);
|
||||
expect(quantities(unknown)).toEqual(["NOVEL", "quantity unknown"]);
|
||||
expect(quantities(zero)).toEqual(["NOVEL", "0.0000"]);
|
||||
@@ -105,7 +105,7 @@ describe("balanceLine", () => {
|
||||
test("an unknown quantity produces no fiat figure, a zero one does", () => {
|
||||
prices[NOVEL] = 3;
|
||||
const unknown = balanceLine("NOVEL", null, 3, NOVEL);
|
||||
const zero = balanceLine("NOVEL", 0, 3, NOVEL);
|
||||
const zero = balanceLine("NOVEL", "0", 3, NOVEL);
|
||||
// A price times an unknown quantity is not $0.00: that is the same
|
||||
// claim of "nothing here" the quantity cell just refused to make.
|
||||
expect(unknown).toContain(
|
||||
|
||||
Reference in new issue
Block a user