docs: RULES.md states the branch-per-issue model (closes #193)

Replaces the "merge to main when done" checklist line with the wording
sneak approved on the issue.

Model: opus-5-5

README.md

@@ -1187,17 +1187,20 @@ These helpers write one kind of value on every screen that shows it:
   times;
 - `renderAddressHtml()` in the same file: most addresses.
 
-Every screen shows token and ETH amounts the same way, through
+Most screens show token and ETH amounts with four decimal places. History rows
-`truncateAmountNeverZero()` in `src/shared/amountDisplay.js`:
+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
+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:
 
-- an amount is cut to four decimal places, never rounded, so 0.99996 reads
+- TxApproval;
-  `0.9999`;
+- SignApproval's Permit warning;
-- an amount below 0.0001 is shown to its first nonzero digit, so 0.00004 reads
+- ConfirmTx, and Send's current balance;
-  `0.00004`.
+- WaitTx, SuccessTx and ErrorTx.
 
-So an amount never reads as more than it is, and a nonzero amount never reads as
+TransactionDetail shows the exact amount and its base units.
-zero. TransactionDetail is the one exception: it shows the exact amount and its
-base units.
 
 An amount to send is valid when it is a number above zero, written as digits
 with at most one decimal point. Nothing but zeros may follow the token's last

RULES.md

@@ -108,7 +108,10 @@ contradicts either, the originals govern.
 - [ ] All tool invocations via `make` targets, never directly
 - [ ] `make check` = `make test` + `make lint` + `make fmt-check`
 - [ ] `main` always passes `make check`
-- [ ] Feature branches for all changes, merge to main when done
+- [ ] Branch per issue, cut from `next`. One PR per issue, based on `next`,
+      squash-merged into `next` on a passed independent review. `main` is
+      reached only by the per-milestone PR from `next`; releases are tagged from
+      `main`. `next` and `main` must stay green; feature branches may be red.
 - [ ] No force-push to `main`
 - [ ] Pre-commit hook runs `make check`
 

src/popup/views/addressToken.js

@@ -159,10 +159,10 @@ function renderBalance() {
     const addr =
         state.wallets[state.selectedWallet].addresses[state.selectedAddress];
     const tokenId = state.selectedToken;
-    let balance, price;
+    let amount, price;
     if (tokenId === "ETH") {
         // null when no refresh has read the balance.
-        balance = addr.balance;
+        amount = unknownableAmount(addr.balance);
         price = getPrice("ETH");
     } else {
         const id = tokenId.toLowerCase();
@@ -175,13 +175,12 @@ 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) balance = tb.balance;
+        if (tb) amount = unknownableAmount(tb.balance);
-        else balance = tracked && tracked.balanceNotFetched ? null : "0";
+        else amount = 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) || " ";
@@ -189,7 +188,7 @@ function renderBalance() {
     // Single token balance line (no tokenId — not clickable here)
     $("address-token-balance").innerHTML = balanceLine(
         currentSymbol,
-        balance,
+        amount,
         price,
     );
 }

src/popup/views/helpers.js

@@ -394,16 +394,13 @@ function formatFee(wei, symbol, chainId) {
 // the row that issue #307 was reported against: every screen that lists a
 // holding renders through here.
 //
-// `balance` is the stored balance, a decimal string. It is null for a holding
+// `amount` is null for a holding whose scale nothing knows
-// 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, balance, price, tokenId) {
+function balanceLine(symbol, amount, price, tokenId) {
-    const amount = unknownableAmount(balance);
+    const qty = amount === null ? "quantity unknown" : amount.toFixed(4);
-    const qty =
-        amount === null ? "quantity unknown" : truncateAmountNeverZero(balance);
     const usd =
         price && amount !== null
             ? escapeHtml(formatUsd(amount * price)) || " "
@@ -418,7 +415,7 @@ function balanceLine(symbol, balance, 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>${escapeHtml(qty)}</span>` +
+        `<span>${qty}</span>` +
         `</span>` +
         `<span class="text-right text-muted flex-1">${usd}</span>` +
         `</div>`
@@ -444,7 +441,7 @@ function markTrackedTokensNotFetched() {
 function balanceLinesForAddress(addr, trackedTokens, showZero) {
     let html = balanceLine(
         nativeCurrency(),
-        addr.balance,
+        unknownableAmount(addr.balance),
         getPrice("ETH"),
         "ETH",
     );
@@ -462,7 +459,7 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
         if (bal === 0 && !showZero) continue;
         html += balanceLine(
             t.symbol,
-            t.balance,
+            bal,
             getPrice(t.address),
             t.address.toLowerCase(),
         );
@@ -480,7 +477,7 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
             }
             html += balanceLine(
                 t.symbol,
-                t.balanceNotFetched ? null : "0",
+                t.balanceNotFetched ? null : 0,
                 getPrice(t.address),
                 t.address.toLowerCase(),
             );
@@ -491,8 +488,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. Callers that warn about
+// under the remove-address warning leaves out too. Deliberately unrounded —
-// holdings must ask this, not the rendered lines.
+// 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;
     // A null or absent ETH balance reads as unknown, so the address may hold

src/popup/views/home.js

@@ -31,7 +31,6 @@ const {
     resetSendValidation,
 } = require("./send");
 const { deriveAddressFromXpub } = require("../../shared/wallet");
-const { truncateAmountNeverZero } = require("../../shared/amountDisplay");
 const { canRemoveAddress } = require("../../shared/walletDelete");
 const {
     walletDefect,
@@ -82,8 +81,7 @@ function renderTotalValue() {
     if (ethBal === null) {
         el.textContent = "unknown (" + nativeCurrency() + ")";
     } else {
-        const ethStr =
+        const ethStr = ethBal.toFixed(4) + " " + nativeCurrency();
-            truncateAmountNeverZero(addr.balance) + " " + nativeCurrency();
         const ethUsd = ethPrice
             ? " (" + formatUsd(ethBal * ethPrice) + ")"
             : "";

src/shared/amountDisplay.js

@@ -1,6 +1,15 @@
-// The amount rule from README.md's Amounts section. Every screen that shows a
+// The 4-decimal amount rule from README.md's Amounts section, and
-// token or ETH amount, except TransactionDetail, shows it through
+// the one exception to it, in one place. Three call sites had grown their own
-// truncateAmountNeverZero(), so one value reads the same on every screen.
+// copy of the truncation — the history and balance lists
+// (`src/shared/transactions.js`), the approval screen's ERC-20 amount line
+// (`src/popup/views/approval.js`) and its Uniswap swap detail lines
+// (`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 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
@@ -11,13 +20,12 @@ function truncateAmount(val) {
     return parts[0] + "." + (parts[1] + "0000").slice(0, 4);
 }
 
-// Four decimal places, truncated, and a nonzero amount never renders as zero.
+// The same rule, plus the invariant the approval and confirmation screens
-// Truncating alone would show an amount below 0.0001 as 0.0000, and a real
+// hold: a nonzero amount never renders as zero. Truncating to four decimals
-// holding, transfer or allowance would then read as nothing.
+// does exactly that to an amount below 0.0001 — one base unit of an 18-decimal
-//
+// token, 500 of an 8-decimal one — and a real transfer or allowance then reads
-// `val` is the exact decimal string: a stored balance, or what formatEther()
+// as "nothing is being moved" on the screen whose whole job is to say what is
-// or formatUnits() returns. Never a JavaScript number, whose own rounding
+// being authorized.
-// would happen before the truncation.
 //
 // When the truncated string carries no significant digit and the value does,
 // the amount is extended to its first significant digit instead. It stays in
@@ -46,6 +54,7 @@ function isBelowOneMillionth(balance) {
 }
 
 module.exports = {
+    truncateAmount,
     truncateAmountNeverZero,
     isBelowOneMillionth,
 };

src/shared/transactions.js

@@ -16,7 +16,10 @@ const { nativeCurrencyByChainId } = require("./networks");
 // a scale is either reported or it is unknown, and "unknown" must mean the
 // same thing here as it does on the screens that refuse to format one.
 const { toDecimals } = require("./transferAmount");
-const { truncateAmountNeverZero } = require("./amountDisplay");
+// The plain 4-decimal rule. The history and balance lists deliberately keep
+// truncation without the approval screens' nonzero floor: the transaction
+// detail view is the authoritative record and already shows exact precision.
+const { truncateAmount: formatTxValue } = require("./amountDisplay");
 const { routerRelease } = require("./uniswap");
 const { storedSettingIsOn } = require("./storedSettings");
 
@@ -37,7 +40,7 @@ function parseTx(tx, addrLower, chainId) {
 
     // For contract calls, produce a meaningful label instead of "0.0000 ETH"
     let symbol = nativeCurrencyByChainId(chainId);
-    let value = truncateAmountNeverZero(formatEther(rawWei));
+    let value = formatTxValue(formatEther(rawWei));
     let exactValue = formatEther(rawWei);
     let rawAmount = rawWei;
     let rawUnit = "wei";
@@ -119,9 +122,7 @@ function parseTokenTransfer(tt, addrLower, chainId) {
     // The exact figure is not lost — it is the base-unit line below, which is
     // the one number that needs no scale to be true.
     const formatted =
-        decimals === null
+        decimals === null ? "" : formatTxValue(formatUnits(rawVal, decimals));
-            ? ""
-            : truncateAmountNeverZero(formatUnits(rawVal, decimals));
     const exact = decimals === null ? "" : formatUnits(rawVal, decimals);
     return {
         hash: tt.transaction_hash,

tests/approvalDisplayFloor.test.js

@@ -25,7 +25,10 @@ const { ERC20_ABI } = require("../src/shared/constants");
 const { state } = require("../src/shared/state");
 const { decodeCalldata } = require("../src/popup/views/approval");
 const uniswap = require("../src/shared/uniswap");
-const { truncateAmountNeverZero } = require("../src/shared/amountDisplay");
+const {
+    truncateAmount,
+    truncateAmountNeverZero,
+} = require("../src/shared/amountDisplay");
 
 const iface = new Interface(ERC20_ABI);
 
@@ -181,7 +184,12 @@ describe("the four-decimal rule is otherwise unchanged", () => {
         expect(amount(transferData(0n), DAI).rawValue).toBe("0.0000");
     });
 
-    test("one base unit of an 18-decimal token reads as itself", () => {
+    // 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",
         );

tests/balanceLineEscaping.test.js

@@ -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("&lt;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="&quot; onclick=&quot;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("&lt; $0.01");
         expect(html).not.toContain("< $0.01");
     });

tests/balanceTruncation.test.js

@@ -1,78 +0,0 @@
-// The balance lists and Home's ETH line show a balance as Send does (README.md,
-// Amounts): cut to four decimal places, never rounded up, and a nonzero amount
-// never shown as zero
-// (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("shows 0.00004 as 0.00004, not 0.0000", () => {
-        expect(quantity(balanceLine("USDC", "0.00004", null, USDC))).toBe(
-            "0.00004",
-        );
-    });
-});
-
-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.00004", async () => {
-        const page = await bootHolding("0.99996", "0.00004");
-
-        expect(quantity(homeRow(page, USDC.toLowerCase()))).toBe("0.00004");
-    });
-});

tests/deleteAddress.test.js

@@ -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 below the four
+// ERC-20 as well as ETH, at any size, including a balance that rounds to
-// decimals the balance lines render. The DOM behaviour
+// zero at 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.
@@ -102,7 +102,7 @@ describe("whether an address holds anything", () => {
         expect(addressHoldsFunds(TOKEN_ONLY)).toBe(true);
     });
 
-    // 0.00001 ETH is below four decimals. It is still money.
+    // 0.00001 ETH renders as "0.0000" at four decimals. It is still money.
     test("an ETH balance below the displayed precision counts", () => {
         expect(addressHoldsFunds(DUST)).toBe(true);
     });
@@ -141,12 +141,13 @@ describe("the balance warning on the removal confirmation", () => {
         expect(html).toContain("2500.0000");
     });
 
-    // The rendered line shows the balance to its first nonzero digit, as Home
+    // The rendered line says 0.0000 for this address — that is the display
-    // and AddressDetail do (README.md, Amounts).
+    // format, shared with Home and AddressDetail — and the warning is shown
-    test("an ETH balance below 0.0001 is warned about, and reads as itself", () => {
+    // all the same, because the balance is not zero.
+    test("an ETH balance that renders as 0.0000 is warned about", () => {
         const html = balanceWarningHtml(DUST);
         expect(html).toContain("This address holds a balance.");
-        expect(html).toContain("<span>0.00001</span>");
+        expect(html).toContain("<span>0.0000</span>");
     });
 
     test("an ETH balance no refresh has read is warned about, and reads as unknown", () => {

tests/transactions.test.js

@@ -1361,31 +1361,6 @@ describe("fetchRecentTransactions merge and dedup", () => {
         expect(txs[0].holders).toBe(3500000);
     });
 
-    // README.md, Amounts: a nonzero amount never reads as zero.
-    test("a token transfer below 0.0001 shows its first nonzero digit", 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.00004");
-    });
-
     test("two transfers of the same token in one transaction collapse to one entry", async () => {
         const hash = "0x" + "3".repeat(64);
         const leg = (value) => ({

tests/unknownScaleDisplay.test.js

@@ -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(