diff --git a/TODO.md b/TODO.md index 1529c97..5bd5d1e 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,10 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-11: Network fee counted in the confirmation-screen balance check for + both ETH and ERC-20 sends, with the arithmetic in a pure, unit-tested + `src/shared/txValidation.js` + ([#154](https://git.eeqj.de/sneak/AutistMask/issues/154)). - 2026-08-11: `TODO.md` Workflow rewritten to the branch-and-PR-per-issue model on `next`, with Status and Next Step refreshed ([#191](https://git.eeqj.de/sneak/AutistMask/issues/191)). diff --git a/src/popup/index.html b/src/popup/index.html index a38e901..da22d5d 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -647,6 +647,31 @@ class="mb-2 border border-border border-dashed p-2" style="visibility: hidden; min-height: 1.25rem" > + + +
tokenBal) { - errors.push( - "Insufficient " + - symbol + - " balance. You have " + - txInfo.tokenBalance + - " " + - symbol + - " but are trying to send " + - txInfo.amount + - " " + - symbol + - ".", - ); - } - } else if (parseFloat(txInfo.amount) > parseFloat(txInfo.balance)) { - errors.push( - "Insufficient balance. You have " + - txInfo.balance + - " ETH but are trying to send " + - txInfo.amount + - " ETH.", - ); - } + // The two fee messages are mutually exclusive per transaction type, and + // the type is known here, before the first paint. Drop the one that can + // never apply and reserve the space of the one that can, so the async + // estimate landing later never moves anything. + $("confirm-amount-fee-error").classList.toggle("hidden", isErc20); + $("confirm-gas-error").classList.toggle("hidden", !isErc20); - const errorsEl = $("confirm-errors"); - const sendBtn = $("btn-confirm-send"); - if (errors.length > 0) { - errorsEl.innerHTML = errors - .map((e) => `
${e}
`) - .join(""); - errorsEl.style.visibility = "visible"; - sendBtn.disabled = true; - sendBtn.classList.add("text-muted"); - } else { - errorsEl.innerHTML = ""; - errorsEl.style.visibility = "hidden"; - sendBtn.disabled = false; - sendBtn.classList.remove("text-muted"); - } + renderValidation(txInfo); // Reset password field and error $("confirm-tx-password").value = ""; @@ -224,6 +201,89 @@ function show(txInfo) { checkRecipientHistory(txInfo); } +// Render the balance check for the transaction on screen. Called once during +// show() and again when the fee estimate resolves or fails. Every element it +// touches already occupies its space, so re-running it never moves anything. +function renderValidation(txInfo) { + const isErc20 = txInfo.token !== "ETH"; + const symbol = isErc20 ? txInfo.tokenSymbol || "?" : "ETH"; + + const { canSend, codes } = validateTransfer({ + isErc20, + amount: txInfo.amount, + ethBalance: txInfo.balance, + tokenBalance: txInfo.tokenBalance, + feeStatus, + feeWei, + }); + + // Messages carrying the user's own numbers are built here; the fixed + // sentences live in the reserved elements in index.html. + const messages = []; + if (codes.includes(CODES.AMOUNT_INVALID)) { + messages.push("Please enter a valid amount to send."); + } + if (codes.includes(CODES.INSUFFICIENT_TOKEN)) { + messages.push( + "Insufficient " + + symbol + + " balance. You have " + + txInfo.tokenBalance + + " " + + symbol + + " but are trying to send " + + txInfo.amount + + " " + + symbol + + ".", + ); + } + if (codes.includes(CODES.INSUFFICIENT_ETH)) { + messages.push( + "Insufficient balance. You have " + + txInfo.balance + + " ETH but are trying to send " + + txInfo.amount + + " ETH.", + ); + } + + const errorsEl = $("confirm-errors"); + if (messages.length > 0) { + errorsEl.innerHTML = messages + .map((m) => `
${escapeHtml(m)}
`) + .join(""); + errorsEl.style.visibility = "visible"; + } else { + errorsEl.innerHTML = ""; + errorsEl.style.visibility = "hidden"; + } + + setVisible( + "confirm-amount-fee-error", + codes.includes(CODES.INSUFFICIENT_ETH_WITH_FEE), + ); + setVisible( + "confirm-gas-error", + codes.includes(CODES.INSUFFICIENT_ETH_FOR_FEE), + ); + setVisible( + "confirm-fee-unknown-error", + codes.includes(CODES.FEE_UNAVAILABLE), + ); + + // While the estimate is in flight there is no error to show — the fee + // line already reads "Estimating..." — but sending stays blocked so a + // transaction the fee would break cannot be signed in the meantime. + const sendBtn = $("btn-confirm-send"); + sendBtn.disabled = !canSend; + sendBtn.classList.toggle("text-muted", !canSend); +} + +function setVisible(id, visible) { + $(id).style.visibility = visible ? "visible" : "hidden"; +} + async function estimateGas(txInfo) { try { const provider = getProvider(state.rpcUrl); @@ -247,6 +307,10 @@ async function estimateGas(txInfo) { } const gasCostWei = gasLimit * gasPrice; + // The user may have left this transaction while the estimate was in + // flight; a stale fee must not reach the screen or the balance check. + if (pendingTx !== txInfo) return; + const gasCostEth = formatEther(gasCostWei); // Format to 6 significant decimal places const parts = gasCostEth.split("."); @@ -258,9 +322,16 @@ async function estimateGas(txInfo) { const ethPrice = getPrice("ETH"); const feeUsd = ethPrice ? parseFloat(gasCostEth) * ethPrice : null; $("confirm-fee-amount").textContent = valueWithUsd(feeStr, feeUsd); + feeStatus = FEE_KNOWN; + feeWei = gasCostWei; + renderValidation(txInfo); } catch (e) { log.errorf("gas estimation failed:", e.message); + if (pendingTx !== txInfo) return; $("confirm-fee-amount").textContent = "Unable to estimate"; + feeStatus = FEE_UNAVAILABLE; + feeWei = null; + renderValidation(txInfo); } } diff --git a/src/shared/txValidation.js b/src/shared/txValidation.js new file mode 100644 index 0000000..f081725 --- /dev/null +++ b/src/shared/txValidation.js @@ -0,0 +1,114 @@ +// Balance arithmetic for the transaction confirmation screen. +// +// Pure: no DOM, no network, no state. Everything is exact integer math on +// 18-decimal fixed point (wei for ETH), so it can be unit tested directly +// instead of through the confirmation view. The caller maps the returned +// codes to the reserved message elements on the screen. +// +// Human decimal strings ("1.25") are scaled to 18 decimals for comparison. +// That scale is independent of a token's own decimals: both the amount and +// the token balance arrive as human decimal strings, so comparing them at a +// common scale is exact. + +const { parseUnits } = require("ethers"); + +const SCALE_DECIMALS = 18; + +// Whether the asynchronous fee estimate has arrived yet. +const FEE_PENDING = "pending"; +const FEE_KNOWN = "known"; +const FEE_UNAVAILABLE = "unavailable"; + +const CODES = { + // The amount is not a number we can do exact arithmetic on. + AMOUNT_INVALID: "amount-invalid", + // ERC-20: the token amount exceeds the token balance. + INSUFFICIENT_TOKEN: "insufficient-token", + // ETH: the amount alone already exceeds the ETH balance. + INSUFFICIENT_ETH: "insufficient-eth", + // ETH: the amount fits, the amount plus the network fee does not. + INSUFFICIENT_ETH_WITH_FEE: "insufficient-eth-with-fee", + // ERC-20: the token balance covers the transfer, the ETH balance does + // not cover the network fee it costs. + INSUFFICIENT_ETH_FOR_FEE: "insufficient-eth-for-fee", + // The fee estimate has not arrived yet. + FEE_PENDING: "fee-pending", + // The fee estimate failed. Unknown is never treated as zero. + FEE_UNAVAILABLE: "fee-unavailable", +}; + +// Scale a human decimal string to 18-decimal fixed point. Returns null when +// the value is not a decimal number or carries more precision than the scale +// can hold, which the caller must treat as unusable rather than as zero. +function toFixedPoint(value) { + if (typeof value !== "string" && typeof value !== "number") return null; + const text = String(value).trim(); + if (text === "") return null; + try { + return parseUnits(text, SCALE_DECIMALS); + } catch (e) { + return null; + } +} + +// Validate a pending transfer against the balances that must cover it. +// +// isErc20 — token transfer rather than a native ETH transfer +// amount — human decimal string being sent +// ethBalance — human decimal string, the sender's ETH balance +// tokenBalance — human decimal string, the sender's token balance +// feeStatus — FEE_PENDING, FEE_KNOWN or FEE_UNAVAILABLE +// feeWei — bigint estimated network fee in wei, when FEE_KNOWN +// +// Returns { canSend, codes }. Every code blocks sending: canSend is true +// only when nothing was found. +function validateTransfer({ + isErc20 = false, + amount, + ethBalance, + tokenBalance, + feeStatus = FEE_PENDING, + feeWei = null, +} = {}) { + const codes = []; + + const amountFp = toFixedPoint(amount); + const ethFp = toFixedPoint(ethBalance) ?? 0n; + + if (amountFp === null) { + codes.push(CODES.AMOUNT_INVALID); + return { canSend: false, codes }; + } + + const feeFp = + feeStatus === FEE_KNOWN && typeof feeWei === "bigint" ? feeWei : null; + + if (isErc20) { + const tokenFp = toFixedPoint(tokenBalance) ?? 0n; + if (amountFp > tokenFp) codes.push(CODES.INSUFFICIENT_TOKEN); + if (feeFp !== null && feeFp > ethFp) { + codes.push(CODES.INSUFFICIENT_ETH_FOR_FEE); + } + } else if (amountFp > ethFp) { + codes.push(CODES.INSUFFICIENT_ETH); + } else if (feeFp !== null && amountFp + feeFp > ethFp) { + codes.push(CODES.INSUFFICIENT_ETH_WITH_FEE); + } + + // An unknown fee is never assumed to be zero: sending stays blocked + // until the estimate arrives, and stays blocked if it never does. + if (feeStatus === FEE_PENDING) codes.push(CODES.FEE_PENDING); + if (feeStatus === FEE_UNAVAILABLE) codes.push(CODES.FEE_UNAVAILABLE); + + return { canSend: codes.length === 0, codes }; +} + +module.exports = { + CODES, + FEE_PENDING, + FEE_KNOWN, + FEE_UNAVAILABLE, + SCALE_DECIMALS, + toFixedPoint, + validateTransfer, +}; diff --git a/tests/txValidation.test.js b/tests/txValidation.test.js new file mode 100644 index 0000000..6e23d3d --- /dev/null +++ b/tests/txValidation.test.js @@ -0,0 +1,185 @@ +const { parseEther } = require("ethers"); +const { + CODES, + FEE_PENDING, + FEE_KNOWN, + FEE_UNAVAILABLE, + toFixedPoint, + validateTransfer, +} = require("../src/shared/txValidation"); + +// A plausible mainnet fee: 21000 gas at 20 gwei. +const FEE = 21000n * 20000000000n; // 0.00042 ETH + +describe("toFixedPoint", () => { + test("scales human decimals to 18 places", () => { + expect(toFixedPoint("1.5")).toBe(parseEther("1.5")); + expect(toFixedPoint("0")).toBe(0n); + }); + + test("rejects values it cannot represent exactly", () => { + expect(toFixedPoint("not a number")).toBe(null); + expect(toFixedPoint("")).toBe(null); + expect(toFixedPoint(null)).toBe(null); + // More precision than 18 decimals can hold. + expect(toFixedPoint("0.0000000000000000001")).toBe(null); + }); +}); + +describe("validateTransfer, native ETH", () => { + const eth = (over) => ({ + isErc20: false, + amount: "0.5", + ethBalance: "1.0", + feeStatus: FEE_KNOWN, + feeWei: FEE, + ...over, + }); + + test("allows a send comfortably within balance", () => { + const r = validateTransfer(eth()); + expect(r).toEqual({ canSend: true, codes: [] }); + }); + + test("blocks a send whose amount plus fee exceeds the balance", () => { + // The whole balance: passes an amount-only check, fails once the fee + // is counted. This is the bug this module exists to prevent. + const r = validateTransfer(eth({ amount: "1.0", ethBalance: "1.0" })); + expect(r.canSend).toBe(false); + expect(r.codes).toEqual([CODES.INSUFFICIENT_ETH_WITH_FEE]); + }); + + test("blocks a send left short by less than one fee", () => { + const balance = "1.0"; + // One wei less headroom than the fee needs. + const amount = "0.99958000000000001"; // 1.0 - 0.00042 + 1e-17 + const r = validateTransfer(eth({ amount, ethBalance: balance })); + expect(r.codes).toEqual([CODES.INSUFFICIENT_ETH_WITH_FEE]); + }); + + test("allows a send that leaves exactly the fee behind", () => { + const r = validateTransfer( + eth({ amount: "0.99958", ethBalance: "1.0" }), + ); + expect(r).toEqual({ canSend: true, codes: [] }); + }); + + test("reports plain insufficient balance when the amount alone is too big", () => { + const r = validateTransfer(eth({ amount: "2.0", ethBalance: "1.0" })); + expect(r.codes).toEqual([CODES.INSUFFICIENT_ETH]); + }); + + test("blocks while the fee estimate is still pending", () => { + const r = validateTransfer( + eth({ feeStatus: FEE_PENDING, feeWei: null }), + ); + expect(r.canSend).toBe(false); + expect(r.codes).toEqual([CODES.FEE_PENDING]); + }); + + test("blocks when the fee estimate failed, without assuming zero", () => { + const r = validateTransfer( + eth({ + amount: "1.0", + ethBalance: "1.0", + feeStatus: FEE_UNAVAILABLE, + feeWei: null, + }), + ); + expect(r.canSend).toBe(false); + expect(r.codes).toEqual([CODES.FEE_UNAVAILABLE]); + // A zero fee would have let this exact transfer through. + expect( + validateTransfer( + eth({ amount: "1.0", ethBalance: "1.0", feeWei: 0n }), + ).canSend, + ).toBe(true); + }); + + test("still reports an over-balance amount before the estimate lands", () => { + const r = validateTransfer( + eth({ + amount: "2.0", + ethBalance: "1.0", + feeStatus: FEE_PENDING, + feeWei: null, + }), + ); + expect(r.codes).toEqual([CODES.INSUFFICIENT_ETH, CODES.FEE_PENDING]); + }); + + test("rejects an amount it cannot do exact arithmetic on", () => { + const r = validateTransfer(eth({ amount: "abc" })); + expect(r.canSend).toBe(false); + expect(r.codes).toEqual([CODES.AMOUNT_INVALID]); + }); + + test("treats a missing balance as zero, not as unlimited", () => { + const r = validateTransfer(eth({ ethBalance: undefined })); + expect(r.codes).toEqual([CODES.INSUFFICIENT_ETH]); + }); +}); + +describe("validateTransfer, ERC-20", () => { + const erc20 = (over) => ({ + isErc20: true, + amount: "100.0", + tokenBalance: "250.0", + ethBalance: "1.0", + feeStatus: FEE_KNOWN, + feeWei: FEE, + ...over, + }); + + test("allows a transfer with tokens to spend and ETH for the fee", () => { + expect(validateTransfer(erc20())).toEqual({ canSend: true, codes: [] }); + }); + + test("checks the token amount against the token balance", () => { + const r = validateTransfer(erc20({ amount: "250.000001" })); + expect(r.codes).toEqual([CODES.INSUFFICIENT_TOKEN]); + }); + + test("does not charge the fee against the token balance", () => { + // The full token balance is sendable: the fee is paid in ETH. + expect(validateTransfer(erc20({ amount: "250.0" })).canSend).toBe(true); + }); + + test("blocks when the ETH balance does not cover the fee", () => { + const r = validateTransfer(erc20({ ethBalance: "0.0001" })); + expect(r.canSend).toBe(false); + expect(r.codes).toEqual([CODES.INSUFFICIENT_ETH_FOR_FEE]); + }); + + test("allows a fee exactly equal to the ETH balance", () => { + const r = validateTransfer(erc20({ ethBalance: "0.00042" })); + expect(r).toEqual({ canSend: true, codes: [] }); + }); + + test("reports both shortfalls when tokens and ETH are both short", () => { + const r = validateTransfer( + erc20({ amount: "300.0", ethBalance: "0.0" }), + ); + expect(r.codes).toEqual([ + CODES.INSUFFICIENT_TOKEN, + CODES.INSUFFICIENT_ETH_FOR_FEE, + ]); + }); + + test("blocks while the fee estimate is pending or failed", () => { + expect( + validateTransfer(erc20({ feeStatus: FEE_PENDING, feeWei: null })) + .codes, + ).toEqual([CODES.FEE_PENDING]); + expect( + validateTransfer( + erc20({ feeStatus: FEE_UNAVAILABLE, feeWei: null }), + ).codes, + ).toEqual([CODES.FEE_UNAVAILABLE]); + }); + + test("treats a missing token balance as zero", () => { + const r = validateTransfer(erc20({ tokenBalance: undefined })); + expect(r.codes).toEqual([CODES.INSUFFICIENT_TOKEN]); + }); +});