diff --git a/README.md b/README.md index ed60c3f..10b7fdf 100644 --- a/README.md +++ b/README.md @@ -708,6 +708,32 @@ Both are click-copyable. Truncating to 4 decimals in summary views is acceptable for scannability, but the detail view must never discard precision — it is the one place the user can always use to verify exact details. +**Specific Exception — nonzero floor on the approval screens:** A nonzero amount +must never render as zero. Truncating to 4 decimals does exactly that to an +amount below 0.0001 — 1 base unit of an 18-decimal token, 500 base units of an +8-decimal one — and on the dApp approval screen and the wait/success/error +screens that carry its amount forward, a real transfer or allowance then reads +as "nothing is being moved". A swap's `Min. received` is the sharper case: a +slippage floor shown as `0.0000` states that the swap may return nothing. + +On those screens, when the truncated string would contain no digit from 1 to 9 +and the value does, the amount is extended to its first significant digit +instead: `0.000000000000000001 DAI`, not `0.0000 DAI`. The test is on the whole +truncated string, integer part included, so `1.00005` still shows as `1.0000` — +the exception only fires where the entire displayed figure would read as zero. A +genuine zero still renders `0.0000`, and truncation stays truncation: `0.99999` +shows as `0.9999`, never rounded up. + +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. + #### Partial USD totals Prices are fetched for the top 25 tokens only, so an address can hold assets the diff --git a/TODO.md b/TODO.md index f82e2cf..d7f171d 100644 --- a/TODO.md +++ b/TODO.md @@ -68,6 +68,22 @@ but the review is broader than any of them. unchanged — the README said "nothing under `dist/` that the build did not write", which was broader than that. Documentation only; no executable line changed. +- 2026-08-23: An amount below the 4-decimal display floor no longer reads as + zero on the approval screens + ([#322](https://git.eeqj.de/sneak/AutistMask/issues/322)). With the token's + true scale resolved, the 4-decimal truncation still printed a small amount as + `0.0000` — 1 base unit of an 18-decimal token, 500 of an 8-decimal one — so a + real transfer, allowance or swap was stated as nothing on the one screen whose + job is to say what is being authorized, and a swap's `Min. received` claimed + the user might receive nothing. Three copies of that truncation existed; they + now share `src/shared/amountDisplay.js`. Everything the approval and + confirmation screens render (`src/popup/views/approval.js`, + `src/shared/uniswap.js`) extends to the first significant digit when the + truncated figure would otherwise read as zero, keeping the amount in token + units rather than switching to base units mid-line. The history and balance + lists (`src/shared/transactions.js`) keep the unfloored rule, which is out of + scope by the issue's definition of done. `README.md`'s Display Consistency + section records the exception. - 2026-08-20: A second extension page can no longer silently delete a wallet ([#304](https://git.eeqj.de/sneak/AutistMask/issues/304)). `saveState()` wrote the entire state blob, and every extension page — the toolbar popup, a dApp diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index f353bdd..1c10a1d 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -25,6 +25,12 @@ const { resolveTokenDecimals, unknownDecimalsAmount, } = require("../../shared/approvalAmount"); +// Four decimals, with the nonzero floor these screens hold: every amount this +// view renders — the ERC-20 line, the ETH value, the max fee — and every one +// it carries forward to the wait/success/error screens goes through it. +const { + truncateAmountNeverZero: formatTxValue, +} = require("../../shared/amountDisplay"); const { decryptWithPassword } = require("../../shared/vault"); const { getSignerForAddress } = require("../../shared/wallet"); const { walletDefect } = require("../../shared/walletDefects"); @@ -40,13 +46,6 @@ function approvalAddressHtml(address) { return renderAddressHtml(address, { title }); } -function formatTxValue(val) { - const parts = val.split("."); - if (parts.length === 1) return val + ".0000"; - const dec = (parts[1] + "0000").slice(0, 4); - return parts[0] + "." + dec; -} - // The amount line for a decoded ERC-20 call. With a known scale it is the // token quantity; with `decimals` null it is the base-unit integer with the // unknown scale stated, because formatting it with an assumed scale is what diff --git a/src/shared/amountDisplay.js b/src/shared/amountDisplay.js new file mode 100644 index 0000000..0b5d3ae --- /dev/null +++ b/src/shared/amountDisplay.js @@ -0,0 +1,46 @@ +// The 4-decimal amount rule from README.md's Display Consistency section, and +// the one exception to it, in one place. Three call sites had grown their own +// 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 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 +// 0.9999. +function truncateAmount(val) { + const parts = val.split("."); + if (parts.length === 1) return val + ".0000"; + return parts[0] + "." + (parts[1] + "0000").slice(0, 4); +} + +// The same rule, plus the invariant the approval and confirmation screens +// hold: a nonzero amount never renders as zero. Truncating to four decimals +// 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 +// as "nothing is being moved" on the screen whose whole job is to say what is +// being authorized. +// +// 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 +// token units, the same unit as the symbol printed beside it. A genuine zero +// still renders 0.0000, and anything at or above the floor is untouched. +function truncateAmountNeverZero(val) { + const truncated = truncateAmount(val); + // Tests the whole truncated string, integer part included: 1.00005 has a + // significant digit already and stays 1.0000. + if (/[1-9]/.test(truncated)) return truncated; + const parts = val.split("."); + if (parts.length === 1) return truncated; + const sig = parts[1].search(/[1-9]/); + if (sig === -1) return truncated; + return parts[0] + "." + parts[1].slice(0, sig + 1); +} + +module.exports = { truncateAmount, truncateAmountNeverZero }; diff --git a/src/shared/transactions.js b/src/shared/transactions.js index 0a87103..033543f 100644 --- a/src/shared/transactions.js +++ b/src/shared/transactions.js @@ -11,6 +11,10 @@ const { log, debugFetch } = require("./log"); const { TOKEN_BY_ADDRESS } = require("./tokenList"); const { parseHoldersCount, isLowHolderCount } = require("./holders"); const { isSpoofedSymbol } = require("./symbolSpoof"); +// 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"); // Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum // over the address, not part of its identity. Every address comparison in @@ -20,13 +24,6 @@ function normalizeAddress(addr) { return (addr || "").toLowerCase(); } -function formatTxValue(val) { - const parts = val.split("."); - if (parts.length === 1) return val + ".0000"; - const dec = (parts[1] + "0000").slice(0, 4); - return parts[0] + "." + dec; -} - function parseTx(tx, addrLower) { const from = tx.from?.hash || ""; const to = tx.to?.hash || ""; diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index 09f8c6a..6c0b7ac 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -3,6 +3,7 @@ const { Interface, AbiCoder, getBytes, formatUnits } = require("ethers"); const { TOKEN_BY_ADDRESS } = require("./tokenList"); +const { truncateAmountNeverZero } = require("./amountDisplay"); const coder = AbiCoder.defaultAbiCoder(); @@ -34,11 +35,13 @@ const COMMAND_NAMES = { 0x21: "Execute Sub-Plan", }; +// The swap's Amount and Min. received lines land on the same approval screen, +// and Amount is carried to the wait/success/error screens as the ERC-20 line +// is, so they take the same nonzero floor: a swap of an amount below 0.0001 is +// not "0.0000", and a slippage floor of one base unit does not read as "you may +// receive nothing". function formatAmount(raw, decimals) { - const parts = formatUnits(raw, decimals).split("."); - if (parts.length === 1) return parts[0] + ".0000"; - const dec = (parts[1] + "0000").slice(0, 4); - return parts[0] + "." + dec; + return truncateAmountNeverZero(formatUnits(raw, decimals)); } function tokenInfo(address) { diff --git a/tests/approvalDisplayFloor.test.js b/tests/approvalDisplayFloor.test.js new file mode 100644 index 0000000..03f617b --- /dev/null +++ b/tests/approvalDisplayFloor.test.js @@ -0,0 +1,190 @@ +// The floor of the approval screen's amount line. +// +// Amounts are truncated to four decimal places for scannability (README.md, +// Display Consistency). With the token's true scale resolved, that truncation +// can still take a real amount below the floor and print it as `0.0000`: one +// base unit of an 18-decimal token, or a few hundred of an 8-decimal one. On +// the one screen whose job is to state what is being authorized, a nonzero +// transfer or allowance then reads as nothing. +// +// The invariant asserted here is narrow: a nonzero amount never renders as +// zero. The four-decimal rule itself is unchanged, and the string the +// confirmation screens carry as `txInfo.amount` is the same one, so it is +// asserted on `rawValue` alongside the displayed line. +// +// Both amount paths of that screen are covered: the ERC-20 line decoded by +// `src/popup/views/approval.js`, and the swap's `Amount` and `Min. received` +// lines decoded by `src/shared/uniswap.js`. + +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { AbiCoder, Interface } = require("ethers"); +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 { + truncateAmount, + truncateAmountNeverZero, +} = require("../src/shared/amountDisplay"); + +const iface = new Interface(ERC20_ABI); + +// Bundled tokens, so the scale and the symbol both come from the list. +const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; // 6 decimals +const WBT = "0x925206b8a707096Ed26ae47C84747fE0bb734F59"; // 8 decimals +const DAI = "0x6B175474E89094C44Da98b954EedeAC495271d0F"; // 18 decimals +// Outside the list, so the scale comes from what the user tracks and the line +// carries no symbol. +const NOVEL = "0xE2E0000000000000000000000000000000000E2e"; + +const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe"; +const SPENDER = "0x1111111111111111111111111111111111111111"; + +// The Uniswap swap lines land on this same approval screen. +const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af"; +const USDT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; // 6 decimals +const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"; // 18 decimals + +const coder = AbiCoder.defaultAbiCoder(); +const routerIface = new Interface([ + "function execute(bytes commands, bytes[] inputs, uint256 deadline)", +]); + +// A V2_SWAP_EXACT_IN (command 0x08) execute() call: `amountIn` of USDT for at +// least `amountOutMin` of WETH. +function swapData(amountIn, amountOutMin) { + const input = coder.encode( + ["address", "uint256", "uint256", "address[]", "bool"], + [RECIPIENT, amountIn, amountOutMin, [USDT, WETH], true], + ); + return routerIface.encodeFunctionData("execute", [ + "0x08", + [input], + 9999999999n, + ]); +} + +function swapDetail(amountIn, amountOutMin, label) { + const decoded = uniswap.decode(swapData(amountIn, amountOutMin), ROUTER); + return decoded.details.find((d) => d.label === label); +} + +function transferData(amount) { + return iface.encodeFunctionData("transfer", [RECIPIENT, amount]); +} + +function approveData(amount) { + return iface.encodeFunctionData("approve", [SPENDER, amount]); +} + +// The Amount detail as the approval screen renders it: `value` is the line on +// the screen, `rawValue` is what is carried to the wait/success/error screens. +function amount(data, token) { + const decoded = decodeCalldata(data, token); + return decoded.details.find((d) => d.label === "Amount"); +} + +beforeEach(() => { + state.trackedTokens = []; + state.wallets = []; +}); + +describe("a nonzero amount never renders as zero", () => { + test("500 base units of a 6-decimal token", () => { + const detail = amount(transferData(500n), USDC); + expect(detail.value).toBe("0.0005 USDC"); + expect(detail.rawValue).toBe("0.0005"); + }); + + test("1 base unit of an 18-decimal token", () => { + const detail = amount(transferData(1n), DAI); + expect(detail.value).toBe("0.000000000000000001 DAI"); + expect(detail.rawValue).toBe("0.000000000000000001"); + }); + + test("500 base units of an 8-decimal token", () => { + expect(amount(transferData(500n), WBT).rawValue).toBe("0.000005"); + }); + + test("an allowance below the floor is not rendered as zero either", () => { + expect(amount(approveData(1n), DAI).value).toBe( + "0.000000000000000001 DAI", + ); + }); + + // The floor holds at any scale, not only the three above: for every + // decimals a token can declare, one base unit has to show a digit. + test("one base unit shows a significant digit at every scale", () => { + for (let decimals = 0; decimals <= 30; decimals++) { + state.trackedTokens = [{ address: NOVEL, decimals }]; + expect(amount(transferData(1n), NOVEL).rawValue).toMatch(/[1-9]/); + } + }); +}); + +// The swap decoder formats its own amounts, so the same floor has to hold on +// the swap lines of the same screen. `Min. received` is the sharper of the +// two: the slippage floor rendered as `0.0000` states that the swap may return +// nothing. +describe("a swap's amounts never render as zero either", () => { + test("a swap input below the floor keeps a significant digit", () => { + // 50 base units of a 6-decimal token is 0.00005. + const detail = swapDetail(50n, 10n ** 15n, "Amount"); + expect(detail.value).toBe("0.00005 USDT"); + expect(detail.rawValue).toBe("0.00005"); + }); + + test("a min-received below the floor keeps a significant digit", () => { + // 1 wei of an 18-decimal token. + expect(swapDetail(10n ** 6n, 1n, "Min. received").value).toBe( + "0.000000000000000001 WETH", + ); + }); + + test("swap amounts at or above the floor are still truncated", () => { + expect(swapDetail(1000000n, 10n ** 15n, "Amount").rawValue).toBe( + "1.0000", + ); + expect( + swapDetail(1000000n, 999999999999999999n, "Min. received").value, + ).toBe("0.9999 WETH"); + }); +}); + +describe("the four-decimal rule is otherwise unchanged", () => { + test("a whole amount keeps exactly four decimals", () => { + expect(amount(transferData(5000000000n), USDC).rawValue).toBe( + "5000.0000", + ); + }); + + test("precision beyond four decimals is still truncated", () => { + expect(amount(transferData(1234567890123456789n), DAI).rawValue).toBe( + "1.2345", + ); + }); + + test("an amount at the floor is not extended", () => { + expect(amount(transferData(100000000000000n), DAI).rawValue).toBe( + "0.0001", + ); + }); + + 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", + ); + }); +});