diff --git a/README.md b/README.md index a4f6f98..74bde42 100644 --- a/README.md +++ b/README.md @@ -1712,32 +1712,39 @@ view would leave a wallet one click from deletion. the reserve the transaction does not use. If the balance no longer covers the fee, the amount is left as it was and the amount-plus-fee error below blocks the send - - Your balance: value + symbol (USD in parentheses), or `unknown (SYMBOL)` - for a token whose scale is unknown + - Your balance: the From address's balance on the network the screen is + drawn for, read from that network's RPC endpoint each time the screen is + shown, so a screen the popup reopens onto after the network was switched + states and checks the balance on the new network. "Loading..." until the + read returns, then value + symbol (USD in parentheses), or + `unknown (SYMBOL)` for a token whose scale is unknown or when the read + failed. The amount check uses the same balances: the ETH balance, and for + a token transfer the token balance as well - Network fee: "Estimating..." then two lines, or "Unable to estimate", - fetched async. The first line is what the transfer is expected to cost, - `gasLimit * gasPrice` (USD in parentheses); the second is the - `gasLimit * maxFeePerGas` reserve the node requires, which is what the - balance check gates on. The second line is omitted on a network with no - type-2 pricing, where the two are the same number, but its space is - reserved either way + fetched async once the balance is read. The first line is what the + transfer is expected to cost, `gasLimit * gasPrice` (USD in parentheses); + the second is the `gasLimit * maxFeePerGas` reserve the node requires, + which is what the balance check gates on. The second line is omitted on a + network with no type-2 pricing, where the two are the same number, but its + space is reserved either way - Warnings: inline warnings from the local checks (scam address, self-send) plus four reserved warning boxes made visible by the async checks — recipient with no transaction history, recipient is a contract, burn address, and an Etherscan phishing/scam label - - Errors (insufficient balance), plus three reserved error boxes — the - amount plus the fee exceeds the balance (ETH transfers), not enough ETH to - pay the fee for the transfer (ERC-20 transfers), and the fee could not be - estimated. The first two are mutually exclusive per transfer type, so only - the applicable one holds space. The last names its cause: for a token - whose scale is unknown the fee can never be estimated, and it says the - wallet does not know how many decimal places the token uses and that the - transaction cannot be sent; for any other failure it asks the user to go - back and try again + - Errors (insufficient balance, or a balance that could not be read, which + asks the user to go back and try again), plus three reserved error boxes — + the amount plus the fee exceeds the balance (ETH transfers), not enough + ETH to pay the fee for the transfer (ERC-20 transfers), and the fee could + not be estimated. The first two are mutually exclusive per transfer type, + so only the applicable one holds space. The last names its cause: for a + token whose scale is unknown the fee can never be estimated, and it says + the wallet does not know how many decimal places the token uses and that + the transaction cannot be sent; for any other failure it asks the user to + go back and try again - Password: an inline field on this screen, not a modal, with its own error line - - "Sign & Send" button (disabled if errors, and while the network fee - estimate is pending or unavailable) + - "Sign & Send" button (disabled if errors, while the balance is being read, + and while the network fee estimate is pending or unavailable) - **Transitions**: - "Sign & Send" (correct password) → broadcast tx → **WaitTx** - "Sign & Send" (correct password) → broadcast fails → **ErrorTx** diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index 90c74ae..d2f4e06 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -2,7 +2,13 @@ // Shows transaction details, warnings, errors. On Sign & Send, // reads inline password, decrypts secret, signs and broadcasts. -const { parseEther, parseUnits, formatEther, Contract } = require("ethers"); +const { + parseEther, + parseUnits, + formatEther, + formatUnits, + Contract, +} = require("ethers"); const { $, showError, @@ -32,6 +38,7 @@ const { const { ERC20_ABI, isBurnAddress } = require("../../shared/constants"); const { displayedDecimals, + toDecimals, transferAmountUnits, } = require("../../shared/transferAmount"); const { assertWithinCeilings } = require("../../shared/approvalVerify"); @@ -60,6 +67,14 @@ let feeWei = null; // amount was derived from. Null for any other send, which ethers prices from // the node at signing time. let maxSendFees = null; +// The From address's balances on the network this screen is drawn for, read +// from that network's node by readBalances() each time the screen is shown. +// The popup can reopen onto this screen after the network was switched, so +// any balance Send knew may be another network's. Null until the read +// returns; then `eth` and `token` are decimal strings, `token` null for an +// ETH send or a token whose scale is unknown, and both null if the read +// failed. +let balances = null; function restore() { const d = state.viewData; @@ -96,11 +111,38 @@ function renderAmount(txInfo) { ); } +// The balance line, with its USD value: "Loading..." until readBalances() +// returns, then the balance it read. +function renderBalance(txInfo) { + const isErc20 = txInfo.token !== "ETH"; + const rawSymbol = isErc20 ? txInfo.tokenSymbol || "?" : nativeCurrency(); + const symbol = displaySymbol(rawSymbol); + if (balances === null) { + $("confirm-balance").textContent = "Loading..."; + return; + } + // A null balance is one nobody established, for a token whose scale is + // unknown or after a read that failed, and is stated as unknown, never as + // zero (https://git.eeqj.de/sneak/AutistMask/issues/349). + const bal = isErc20 ? balances.token : balances.eth; + if (bal === null) { + $("confirm-balance").textContent = "unknown (" + symbol + ")"; + return; + } + const price = isErc20 ? getPrice(rawSymbol) : getPrice("ETH"); + const balUsd = price ? parseFloat(bal) * price : null; + $("confirm-balance").textContent = valueWithUsd( + truncateAmountNeverZero(bal) + " " + symbol, + balUsd, + ); +} + function show(txInfo) { pendingTx = txInfo; feeStatus = FEE_PENDING; feeWei = null; maxSendFees = null; + balances = null; // The network this screen is drawn for, and the only one it sends on: // Settings can switch the active network while the screen is open. The // wait, success and error screens label the amount by it too. @@ -152,33 +194,7 @@ function show(txInfo) { $("confirm-to-ens").classList.add("hidden"); renderAmount(txInfo); - - // Balance (with inline USD) - const ethPrice = getPrice("ETH"); - const tokenPrice = getPrice(rawSymbol); - if (isErc20) { - // null is a balance whose scale nothing knows, not a balance of zero - // (https://git.eeqj.de/sneak/AutistMask/issues/349). The send is - // refused at encode time for the same missing scale; what this line - // must not do is state a quantity nobody established. - const bal = txInfo.tokenBalance; - const balUsd = - tokenPrice && bal != null ? parseFloat(bal) * tokenPrice : null; - $("confirm-balance").textContent = - bal == null - ? "unknown (" + symbol + ")" - : valueWithUsd( - truncateAmountNeverZero(bal) + " " + symbol, - balUsd, - ); - } else { - const bal = txInfo.balance || "0"; - const balUsd = ethPrice ? parseFloat(bal) * ethPrice : null; - $("confirm-balance").textContent = valueWithUsd( - truncateAmountNeverZero(bal) + " " + symbol, - balUsd, - ); - } + renderBalance(txInfo); // Check for warnings (synchronous local checks) const localWarnings = getLocalWarnings(txInfo.to, { @@ -262,25 +278,68 @@ function show(txInfo) { $("confirm-burn-warning").style.visibility = "visible"; } - estimateGas(txInfo); + // The fee is estimated once the balances are read, because a max ETH + // send's amount is the ETH balance minus the estimate's reserve. + readBalances(txInfo).then(() => estimateGas(txInfo)); checkRecipientHistory(txInfo); } +// Read the From address's balances from the node of the network this screen +// is drawn for, the node the fee estimate asks: ETH, and for a token send the +// token, at the scale the screen renders the amount with. A token whose scale +// is unknown has no balance to read. Never rejects. +async function readBalances(txInfo) { + let read; + try { + const provider = getProvider(state.rpcUrl, state.networkId); + const decimals = toDecimals(txInfo.tokenDecimals); + const readToken = txInfo.token !== "ETH" && decimals !== null; + const [ethWei, tokenUnits] = await Promise.all([ + provider.getBalance(txInfo.from), + readToken + ? new Contract(txInfo.token, ERC20_ABI, provider).balanceOf( + txInfo.from, + ) + : null, + ]); + read = { + eth: formatEther(ethWei), + token: readToken ? formatUnits(tokenUnits, decimals) : null, + }; + } catch (e) { + log.errorf("balance read failed:", e.shortMessage || e.message); + read = { eth: null, token: null }; + } + // The user may have left this transaction while the read was in flight. + if (pendingTx !== txInfo) return; + balances = read; + renderBalance(txInfo); + renderValidation(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. +// show(), again when the balances are read, 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 ? displaySymbol(txInfo.tokenSymbol || "?") : nativeCurrency(); + // Null while the balances are being read and after a read that failed. + // validateTransfer() takes a missing balance as nothing to spend from, so + // sending stays blocked either way, and no message below quotes one. + const ethBalance = balances ? balances.eth : null; + const tokenBalance = balances ? balances.token : null; + const balanceRead = ethBalance !== null; + const { canSend, codes } = validateTransfer({ isErc20, amount: txInfo.amount, tokenDecimals: txInfo.tokenDecimals, - ethBalance: txInfo.balance, - tokenBalance: txInfo.tokenBalance, + ethBalance, + tokenBalance, feeStatus, feeWei, }); @@ -292,9 +351,16 @@ function renderValidation(txInfo) { if (codes.includes(CODES.AMOUNT_INVALID)) { messages.push("Please enter a valid amount to send."); } - if (codes.includes(CODES.INSUFFICIENT_TOKEN)) { + if (balances !== null && !balanceRead) { messages.push( - txInfo.tokenBalance == null + "Your balance could not be read from the network, so this" + + " transaction cannot be checked against it. Please go back" + + " and try again.", + ); + } + if (balanceRead && codes.includes(CODES.INSUFFICIENT_TOKEN)) { + messages.push( + tokenBalance == null ? "This token's balance is unknown, because nothing this" + " wallet can consult reports how many decimal places it" + " uses, so the amount you are trying to send cannot be" + @@ -302,7 +368,7 @@ function renderValidation(txInfo) { : "Insufficient " + symbol + " balance. You have " + - truncateAmountNeverZero(txInfo.tokenBalance) + + truncateAmountNeverZero(tokenBalance) + " " + symbol + " but are trying to send " + @@ -312,10 +378,10 @@ function renderValidation(txInfo) { ".", ); } - if (codes.includes(CODES.INSUFFICIENT_ETH)) { + if (balanceRead && codes.includes(CODES.INSUFFICIENT_ETH)) { messages.push( "Insufficient balance. You have " + - truncateAmountNeverZero(txInfo.balance || "0") + + truncateAmountNeverZero(ethBalance) + " " + symbol + " but are trying to send " + @@ -339,11 +405,11 @@ function renderValidation(txInfo) { setVisible( "confirm-amount-fee-error", - codes.includes(CODES.INSUFFICIENT_ETH_WITH_FEE), + balanceRead && codes.includes(CODES.INSUFFICIENT_ETH_WITH_FEE), ); setVisible( "confirm-gas-error", - codes.includes(CODES.INSUFFICIENT_ETH_FOR_FEE), + balanceRead && codes.includes(CODES.INSUFFICIENT_ETH_FOR_FEE), ); setVisible( "confirm-fee-unknown-error", @@ -432,14 +498,14 @@ async function estimateGas(txInfo) { } feeStatus = FEE_KNOWN; feeWei = gasCostWei; - // A max ETH send is the balance minus this estimate's reserve, not the - // Send screen's, and is signed with this estimate's fee fields: fees - // fetched again at signing could exceed the reserve it leaves, and the - // node would refuse it for want of funds. Where the balance no longer - // covers the fee, the amount is left as it is and the balance check - // below says so. + // A max ETH send is the balance readBalances() read minus this + // estimate's reserve, not the Send screen's, and is signed with this + // estimate's fee fields: fees fetched again at signing could exceed + // the reserve it leaves, and the node would refuse it for want of + // funds. Where the balance is unknown or no longer covers the fee, the + // amount is left as it is and the balance check below says so. if (txInfo.max && txInfo.token === "ETH") { - const amount = maxEthAmount(txInfo.balance, gasCostWei); + const amount = maxEthAmount(balances?.eth, gasCostWei); if (amount !== null) { txInfo.amount = amount; renderAmount(txInfo); diff --git a/src/popup/views/send.js b/src/popup/views/send.js index 29350d6..6556c9f 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -164,8 +164,9 @@ function renderSendTokenSelect(addr) { } } -// The token balance and scale the Send screen states and hands the -// confirmation screen, so the two screens describe the holding the same way. +// The token balance the Send screen states, and the scale it hands the +// confirmation screen, which reads the balance itself from the network it is +// drawn for. // // The scale is resolved the same way balances.js resolved the scale it // DISPLAYED this token's balance at: bundled list, then the user's tracked @@ -188,13 +189,11 @@ function renderSendTokenSelect(addr) { // // The two resolutions can therefore differ, and where they do, the stored // `balance` is a quantity computed at a scale this screen has just declined to -// stand behind. Stating it would leave validateTransfer() checking the amount -// against a number the wallet does not vouch for, so it is withdrawn: unknown -// scale means unknown balance. It is null rather than "0": both screens state -// an unknown balance as unknown, and validateTransfer() treats it as no balance -// to spend from, which is the fail-closed side of an amount nobody can check. -// Only a stored quantity is withdrawn: the "0" for a token that has no row at -// all is an absence of holdings, which is true at every scale. +// stand behind, so it is withdrawn: unknown scale means unknown balance. It is +// null rather than "0": this screen states it as unknown, as the confirmation +// screen does, and Max fills nothing in for it. Only a stored quantity is +// withdrawn: the "0" for a token that has no row at all is an absence of +// holdings, which is true at every scale. function tokenBalanceAndDecimals(addr, token) { const tb = (addr.tokenBalances || []).find( (t) => t.address.toLowerCase() === token.toLowerCase(), @@ -378,7 +377,6 @@ function init(_ctx) { const addr = currentAddress(); let tokenSymbol = null; - let tokenBalance = null; let tokenDecimals = null; if (token !== "ETH") { tokenSymbol = resolveSymbol( @@ -386,10 +384,7 @@ function init(_ctx) { addr.tokenBalances, state.trackedTokens, ); - ({ tokenBalance, tokenDecimals } = tokenBalanceAndDecimals( - addr, - token, - )); + ({ tokenDecimals } = tokenBalanceAndDecimals(addr, token)); } // The confirmation screen's own amount check, so it is never shown @@ -424,9 +419,7 @@ function init(_ctx) { ensName: ensName, amount: amount, token: token, - balance: addr.balance, tokenSymbol: tokenSymbol, - tokenBalance: tokenBalance, tokenDecimals: tokenDecimals, max: amountIsMax, }); diff --git a/src/shared/txValidation.js b/src/shared/txValidation.js index 4c9d5df..ed256cc 100644 --- a/src/shared/txValidation.js +++ b/src/shared/txValidation.js @@ -111,8 +111,8 @@ function isValidAmount(amount, tokenDecimals = null) { // The most ETH a send can carry: the exact balance minus the fee reserve from // feeReserveWei(), as a decimal string, so validateTransfer() passes it with -// exactly that reserve left behind. `ethBalance` is the exact decimal string -// balances.js stores, never a rounded one. Null when the balance does not +// exactly that reserve left behind. `ethBalance` is an exact decimal string as +// formatEther() writes it, never a rounded one. Null when the balance does not // leave anything to send once the fee is paid, or when either input is // unusable. function maxEthAmount(ethBalance, feeWei) { diff --git a/tests/addressKey.test.js b/tests/addressKey.test.js index 631cfe5..89e779f 100644 --- a/tests/addressKey.test.js +++ b/tests/addressKey.test.js @@ -291,7 +291,6 @@ async function confirmSend(from, selected) { to: RECIPIENT, amount: "0.5", token: "ETH", - balance: "1.0", }); await settle(); node("confirm-tx-password").value = PASSWORD; diff --git a/tests/confirmTxNetworkSwitch.test.js b/tests/confirmTxNetworkSwitch.test.js index a612744..3a670b1 100644 --- a/tests/confirmTxNetworkSwitch.test.js +++ b/tests/confirmTxNetworkSwitch.test.js @@ -7,17 +7,28 @@ // reviewed on Sepolia went out as 1 ETH on mainnet once mainnet was chosen in // Settings in between. // +// Its balance line and balance check are that network's too +// (https://git.eeqj.de/sneak/AutistMask/issues/558). The popup can reopen onto +// a confirmation screen saved before the network was switched in another +// popup, and the screen, drawn again for the new network, showed and checked +// the balance Send had read on the old one. +// // Driven through the real confirmation screen, signer and chain switch, with // only the node and the DOM stubbed. The stub node is on the network its -// provider was built for, so a broadcast records the network it went to. +// provider was built for, so a broadcast records the network it went to, and +// a balance read answers with that network's balance. "use strict"; -// The signed transactions the stub node was handed, with its network's chain id. -const mockNode = { broadcast: [] }; +// The balances the stub node reports on each network, by network name: ETH +// in wei and the token in its base units. And the signed transactions it was +// handed, with its network's chain id. +const mockNode = { balanceWei: {}, tokenUnits: {}, broadcast: [] }; jest.mock("ethers", () => { const actual = jest.requireActual("ethers"); + // The selector of balanceOf(address). + const BALANCE_OF = "0x70a08231"; class StubProvider { constructor(url, network) { this.network = network; @@ -25,6 +36,20 @@ jest.mock("ethers", () => { async getNetwork() { return this.network; } + async getBalance() { + return mockNode.balanceWei[this.network.name]; + } + // balanceOf() is the one contract call the screen makes before Sign + // & Send. + async call(tx) { + if (!tx.data.startsWith(BALANCE_OF)) { + throw new Error("the stub node answers only balanceOf()"); + } + return actual.AbiCoder.defaultAbiCoder().encode( + ["uint256"], + [mockNode.tokenUnits[this.network.name]], + ); + } async getFeeData() { return { maxFeePerGas: 20000000000n, @@ -123,7 +148,13 @@ global.document = { }; global.navigator = { clipboard: { writeText() {} } }; -const { BaseWallet, Transaction, Wallet } = require("ethers"); +const { + BaseWallet, + JsonRpcProvider, + Transaction, + Wallet, + parseEther, +} = require("ethers"); const { encryptWithPassword } = require("../src/shared/vault"); const { state, loadState } = require("../src/shared/state"); const { onChainSwitch } = require("../src/shared/chainSwitch"); @@ -135,6 +166,8 @@ const PRIVATE_KEY = "0x" + "11".repeat(32); const HOLDER = new Wallet(PRIVATE_KEY).address; const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe"; const PASSWORD = "correct horse battery staple"; +// A token with 6 decimal places, which the bundled list does not know. +const TOKEN = "0x" + "d".repeat(40); function el(id) { return global.document.getElementById(id); @@ -150,6 +183,11 @@ beforeAll(async () => { beforeEach(() => { elements.clear(); + mockNode.balanceWei = { + mainnet: parseEther("5"), + sepolia: parseEther("5"), + }; + mockNode.tokenUnits = { mainnet: 0n, sepolia: 0n }; mockNode.broadcast = []; txStatus.showWait.mockClear(); txStatus.showError.mockClear(); @@ -160,16 +198,23 @@ afterEach(() => { signTransaction.mockRestore(); }); -// Show the confirmation screen for 1 of the native token on `networkId`, as -// Review on the Send screen does, with its fee estimate settled. -async function review(networkId) { +// show() reads the balances and estimates the fee without awaiting either; +// this lets both settle. +async function settle() { + for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); +} + +// Show the confirmation screen for 1 of the native token on `networkId`, or +// for what `send` describes instead, as Review on the Send screen does, with +// its balance read and its fee estimate settled. +async function review(networkId, send = {}) { await onChainSwitch(networkId); state.wallets = [ { type: "key", name: "Wallet 1", encryptedSecret, - addresses: [{ address: HOLDER, balance: "5.0" }], + addresses: [{ address: HOLDER }], }, ]; state.selectedWallet = 0; @@ -181,13 +226,32 @@ async function review(networkId) { ensName: null, amount: "1", token: "ETH", - balance: "5.0", tokenSymbol: null, - tokenBalance: null, tokenDecimals: null, max: false, + ...send, }); - for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); + await settle(); +} + +// Switch the network to `networkId` as Settings in another popup does, then +// open the popup again onto the confirmation screen it saved. +async function reopenOn(networkId) { + const saved = JSON.parse(JSON.stringify(state.viewData)); + await onChainSwitch(networkId); + elements.clear(); + state.viewData = saved; + confirmTx.init({}); + confirmTx.restore(); + await settle(); +} + +function errors() { + return el("confirm-errors").innerHTML; +} + +function sendDisabled() { + return el("btn-confirm-send").disabled; } async function signAndSend() { @@ -257,3 +321,117 @@ test("the send is refused by a signer on another network's node", async () => { expect(signTransaction).not.toHaveBeenCalled(); expect(mockNode.broadcast).toEqual([]); }); + +describe("a confirmation screen saved on mainnet and reopened after a switch to Sepolia", () => { + test("states and checks the Sepolia balance, which covers the send", async () => { + mockNode.balanceWei = { + mainnet: parseEther("0.5"), + sepolia: parseEther("5"), + }; + await review("mainnet"); + expect(el("confirm-balance").textContent).toBe("0.5000 ETH"); + expect(sendDisabled()).toBe(true); + + await reopenOn("sepolia"); + expect(el("confirm-type").textContent).toBe( + "Native SepoliaETH transfer", + ); + expect(el("confirm-balance").textContent).toBe("5.0000 SepoliaETH"); + expect(errors()).toBe(""); + expect(el("confirm-amount-fee-error").style.visibility).toBe("hidden"); + expect(sendDisabled()).toBe(false); + }); + + test("states and checks the Sepolia balance, which does not cover the send", async () => { + mockNode.balanceWei = { + mainnet: parseEther("5"), + sepolia: parseEther("0.5"), + }; + await review("mainnet"); + expect(el("confirm-balance").textContent).toBe("5.0000 ETH"); + expect(sendDisabled()).toBe(false); + + await reopenOn("sepolia"); + expect(el("confirm-balance").textContent).toBe("0.5000 SepoliaETH"); + expect(errors()).toContain( + "Insufficient balance. You have 0.5000 SepoliaETH but are trying" + + " to send 1 SepoliaETH.", + ); + expect(sendDisabled()).toBe(true); + }); + + const tokenSend = { + token: TOKEN, + tokenSymbol: "TOK", + tokenDecimals: 6, + }; + + test("states and checks the Sepolia token balance, which covers the send", async () => { + mockNode.tokenUnits = { mainnet: 0n, sepolia: 10000000n }; + await review("mainnet", tokenSend); + expect(el("confirm-balance").textContent).toBe("0.0000 TOK"); + expect(sendDisabled()).toBe(true); + + await reopenOn("sepolia"); + expect(el("confirm-balance").textContent).toBe("10.0000 TOK"); + expect(errors()).toBe(""); + expect(el("confirm-gas-error").style.visibility).toBe("hidden"); + expect(sendDisabled()).toBe(false); + }); + + test("states and checks the Sepolia token balance, which does not cover the send", async () => { + mockNode.tokenUnits = { mainnet: 10000000n, sepolia: 500000n }; + await review("mainnet", tokenSend); + expect(el("confirm-balance").textContent).toBe("10.0000 TOK"); + expect(sendDisabled()).toBe(false); + + await reopenOn("sepolia"); + expect(el("confirm-balance").textContent).toBe("0.5000 TOK"); + expect(errors()).toContain( + "Insufficient TOK balance. You have 0.5000 TOK but are trying to" + + " send 1 TOK.", + ); + expect(sendDisabled()).toBe(true); + }); +}); + +describe("the balance read", () => { + test("leaves the balance line Loading... and Sign & Send disabled until it returns", async () => { + let answer; + const getBalance = jest + .spyOn(JsonRpcProvider.prototype, "getBalance") + .mockImplementationOnce(() => new Promise((r) => (answer = r))); + try { + await review("sepolia"); + } finally { + getBalance.mockRestore(); + } + expect(el("confirm-balance").textContent).toBe("Loading..."); + expect(el("confirm-fee-amount").textContent).toBe("Estimating..."); + expect(el("confirm-errors").style.visibility).toBe("hidden"); + expect(sendDisabled()).toBe(true); + + answer(parseEther("5")); + await settle(); + expect(el("confirm-balance").textContent).toBe("5.0000 SepoliaETH"); + expect(sendDisabled()).toBe(false); + }); + + test("that fails states the balance as unknown, not zero, and blocks sending", async () => { + const getBalance = jest + .spyOn(JsonRpcProvider.prototype, "getBalance") + .mockRejectedValueOnce(new Error("the node did not answer")); + try { + await review("sepolia"); + } finally { + getBalance.mockRestore(); + } + expect(el("confirm-balance").textContent).toBe("unknown (SepoliaETH)"); + expect(errors()).toBe( + '
Your balance could not be read from the' + + " network, so this transaction cannot be checked against it." + + " Please go back and try again.
", + ); + expect(sendDisabled()).toBe(true); + }); +}); diff --git a/tests/e2e/network.js b/tests/e2e/network.js index 123f6cd..e26a38a 100644 --- a/tests/e2e/network.js +++ b/tests/e2e/network.js @@ -244,16 +244,21 @@ function latestBlock() { }; } -// keccak("decimals()")[0:4], and the same for symbol() and name(). +// keccak("decimals()")[0:4], and the same for symbol(), name() and +// balanceOf(address). const SELECTOR_DECIMALS = "0x313ce567"; const SELECTOR_SYMBOL = "0x95d89b41"; const SELECTOR_NAME = "0x06fdde03"; +const SELECTOR_BALANCE_OF = "0x70a08231"; -// Every eth_call still answers with a zero word except decimals(), symbol() -// and name() on the stub token. The wallet reads decimals() back at signing -// time to compare with the scale the confirmation screen rendered (issue -// #305). Adding the token by its contract address reads all three (issue -// #295); symbol() and name() answer what the explorer reports for it. +// Every eth_call still answers with a zero word except decimals(), symbol(), +// name() and balanceOf() on the stub token. The wallet reads decimals() back +// at signing time to compare with the scale the confirmation screen rendered +// (issue #305). Adding the token by its contract address reads the first three +// (issue #295); symbol() and name() answer what the explorer reports for it. +// The confirmation screen reads balanceOf() +// (https://git.eeqj.de/sneak/AutistMask/issues/558), which answers the holding +// the explorer reports, or zero when no holding is seeded. // // opts.tokenDecimalsOverride is the lying contract: set it and decimals() // answers something other than the value this same fixture reports through @@ -278,6 +283,9 @@ function ethCallResult(req, opts) { if (data.startsWith(SELECTOR_NAME)) { return abi.encode(["string"], [tokenObject(opts).name]); } + if (data.startsWith(SELECTOR_BALANCE_OF) && opts.seedTokenBalance) { + return word(tokenBalanceItems(opts)[0].value); + } return ZERO_WORD; } diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 52a8883..07cceec 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -2303,6 +2303,16 @@ async function goToConfirm(page, { token, balance, amount }) { await page.fill("#send-amount", amount); await page.click("#btn-send-review"); await visible(page, "#view-confirm-tx"); + // The screen reads the balance itself before it estimates the fee, so + // waiting for it here means every state below is read with the balance + // in place, the pending fee included. + await page.waitForFunction( + () => + document.getElementById("confirm-balance").textContent.trim() !== + "Loading...", + null, + { timeout: 60000 }, + ); await assertAddressesFit(page, "the confirmation screen"); } diff --git a/tests/nativeCurrency.test.js b/tests/nativeCurrency.test.js index 8bd8bb7..2be8265 100644 --- a/tests/nativeCurrency.test.js +++ b/tests/nativeCurrency.test.js @@ -17,6 +17,10 @@ jest.mock("ethers", () => { async lookupAddress() { return null; } + // The 1.5 of the native token the address holds on either network. + async getBalance() { + return 1500000000000000000n; + } // 10 gwei expected, 20 gwei reserved per gas. async getFeeData() { return { maxFeePerGas: 20000000000n, gasPrice: 10000000000n }; diff --git a/tests/persistedFieldContract.test.js b/tests/persistedFieldContract.test.js index 637d2ab..6d45119 100644 --- a/tests/persistedFieldContract.test.js +++ b/tests/persistedFieldContract.test.js @@ -767,7 +767,6 @@ const WELL_FORMED_DATA = { from: ADDRESS, to: ADDRESS, amount: "1", - balance: "2", }, pendingWait: { hash: "0x1", diff --git a/tests/sendAmountRule.test.js b/tests/sendAmountRule.test.js index 407b13c..224ff4a 100644 --- a/tests/sendAmountRule.test.js +++ b/tests/sendAmountRule.test.js @@ -23,7 +23,10 @@ jest.mock("../src/shared/log", () => ({ isDebug: () => false, })); -// What the confirmation screen asks the node for. +const { AbiCoder } = require("ethers"); + +// What the confirmation screen asks the node for. The address holds the 1 ETH +// and 250 TOK the wallet below records. const mockProvider = { getFeeData: async () => ({ maxFeePerGas: 2000000000n, @@ -32,7 +35,11 @@ const mockProvider = { estimateGas: async () => 21000n, getCode: async () => "0x", getTransactionCount: async () => 1, - getBalance: async () => 0n, + getBalance: async () => 1000000000000000000n, + // The one contract call the screen makes, balanceOf(), answered at the + // token's 6 decimal places. + call: async () => + AbiCoder.defaultAbiCoder().encode(["uint256"], [250000000n]), }; jest.mock("../src/shared/balances", () => { diff --git a/tests/sendDisplayFloor.test.js b/tests/sendDisplayFloor.test.js index 1e0b885..7dc921d 100644 --- a/tests/sendDisplayFloor.test.js +++ b/tests/sendDisplayFloor.test.js @@ -18,6 +18,7 @@ // What the stub node answers. Each test sets what it needs. const mockNode = { balanceWei: 0n, + tokenUnits: 0n, feeData: { maxFeePerGas: 1n, gasPrice: 1n }, }; @@ -30,6 +31,13 @@ jest.mock("ethers", () => { async getBalance() { return mockNode.balanceWei; } + // balanceOf(), the one contract call these screens make. + async call() { + return actual.AbiCoder.defaultAbiCoder().encode( + ["uint256"], + [mockNode.tokenUnits], + ); + } async lookupAddress() { return null; } @@ -196,9 +204,12 @@ function errors() { } // The ETH balance the node reports and the token rows the explorer reports, -// fetched and stored exactly where the popup stores them. +// fetched and stored exactly where the popup stores them. The node reports the +// token holding the explorer does. async function refreshWith(balanceWei, tokenItems = []) { mockNode.balanceWei = balanceWei; + mockNode.tokenUnits = + tokenItems.length > 0 ? BigInt(tokenItems[0].value) : 0n; mockExplorer.items = tokenItems; state.wallets = [{ name: "Wallet 1", addresses: [{ address: HOLDER }] }]; state.selectedWallet = 0; diff --git a/tests/sendMax.test.js b/tests/sendMax.test.js index 1c3f0b9..997e43e 100644 --- a/tests/sendMax.test.js +++ b/tests/sendMax.test.js @@ -17,6 +17,7 @@ // What the stub node answers, and the signed transactions it was handed. const mockNode = { balanceWei: 0n, + tokenUnits: 0n, feeData: null, broadcast: [], }; @@ -30,6 +31,14 @@ jest.mock("ethers", () => { async getBalance() { return mockNode.balanceWei; } + // balanceOf(), the one contract call these screens make before Sign + // & Send. + async call() { + return actual.AbiCoder.defaultAbiCoder().encode( + ["uint256"], + [mockNode.tokenUnits], + ); + } async lookupAddress() { return null; } @@ -187,9 +196,12 @@ function text(id) { } // The ETH balance the node reports and the token rows the explorer reports, -// fetched and stored exactly where the popup stores them. +// fetched and stored exactly where the popup stores them. The node reports the +// token holding the explorer does. async function refreshWith(balanceWei, tokenItems = []) { mockNode.balanceWei = balanceWei; + mockNode.tokenUnits = + tokenItems.length > 0 ? BigInt(tokenItems[0].value) : 0n; mockExplorer.items = tokenItems; state.wallets = [ { diff --git a/tests/unknownScaleSend.test.js b/tests/unknownScaleSend.test.js index e264584..3c35a23 100644 --- a/tests/unknownScaleSend.test.js +++ b/tests/unknownScaleSend.test.js @@ -39,7 +39,8 @@ jest.mock("../src/shared/log", () => ({ // Everything the confirmation screen would reach the network for. The gas // estimate is the point: with a usable scale it must succeed, so that a failure -// in these tests is a failure of the scale and not of the stub. +// in these tests is a failure of the scale and not of the stub. The address +// holds the 1 ETH the wallets below record, and `tokenUnits` of the token. const mockProvider = { getFeeData: async () => ({ maxFeePerGas: 2000000000n, @@ -48,7 +49,14 @@ const mockProvider = { estimateGas: async () => 21000n, getCode: async () => "0x", getTransactionCount: async () => 1, - getBalance: async () => 0n, + getBalance: async () => 1000000000000000000n, + tokenUnits: 0n, + // The one contract call the screen makes, balanceOf(). + call: async () => + AbiCoder.defaultAbiCoder().encode( + ["uint256"], + [mockProvider.tokenUnits], + ), }; jest.mock("../src/shared/balances", () => { @@ -114,7 +122,7 @@ global.document = { }; global.navigator = { clipboard: { writeText() {} } }; -const { parseUnits } = require("ethers"); +const { AbiCoder, parseUnits } = require("ethers"); const { fetchTokenBalances } = require("../src/shared/balances"); const { debugFetch } = require("../src/shared/log"); const { state } = require("../src/shared/state"); @@ -155,8 +163,9 @@ function row(token = {}, value = FIVE_WETH) { } // Fetch the explorer's rows through the real fetcher and put them exactly where -// refreshBalances() puts them. +// refreshBalances() puts them. The node reports the holding the explorer does. async function fetchOnto(items) { + mockProvider.tokenUnits = BigInt(items[0].value); debugFetch.mockImplementation(async () => ({ ok: true, status: 200, @@ -181,6 +190,7 @@ async function fetchOnto(items) { // is from the first. Two addresses is what it takes to reach // explorerDecimals()'s disagreement check, which is only reachable across rows. async function fetchOntoBoth(itemsA, itemsB) { + mockProvider.tokenUnits = BigInt(itemsA[0].value); debugFetch.mockImplementation(async () => ({ ok: true, status: 200, @@ -270,7 +280,6 @@ describe("the Send screen resolves the scale rather than reading the stored one" const txInfo = await reviewSend(WETH, "1.5"); expect(txInfo.tokenDecimals).toBe(18); expect(txInfo.tokenDecimals).not.toBe(balances[0].decimals); - expect(txInfo.tokenBalance).toBe("5.0"); }); test("that scale estimates a fee and leaves Send enabled", async () => { @@ -307,7 +316,6 @@ describe("the Send screen resolves the scale rather than reading the stored one" // No fallback was introduced: resolution answers null here, and the // null is what goes forward. expect(txInfo.tokenDecimals).toBeNull(); - expect(txInfo.tokenBalance).toBeNull(); confirmTx.show(txInfo); await settle(); expect(text("confirm-balance")).toBe("unknown (NOVEL)"); @@ -355,7 +363,8 @@ describe("a scale the explorer's own rows disagree about", () => { const txInfo = await reviewSend(NOVEL, "1.5"); expect(txInfo.tokenDecimals).toBeNull(); // The regression this closes: null scale alongside a non-null balance. - expect(txInfo.tokenBalance).toBeNull(); + send.updateSendBalance(); + expect(text("send-balance")).toBe("Current balance: unknown (NOVEL)"); }); test("so the user is told the balance is unknown, not only that the fee failed", async () => { @@ -434,7 +443,6 @@ describe("a scale the explorer's own rows disagree about", () => { expect(text("send-balance")).toBe("Current balance: 5.0000 NOVEL"); const txInfo = await reviewSend(NOVEL, "1.5"); expect(txInfo.tokenDecimals).toBe(6); - expect(txInfo.tokenBalance).toBe("5.0"); confirmTx.show(txInfo); await settle(); expect(text("confirm-balance")).toBe("5.0000 NOVEL"); @@ -444,21 +452,10 @@ describe("a scale the explorer's own rows disagree about", () => { }); describe("the confirmation screen tells an unknown balance from a zero one", () => { - function txInfo(tokenBalance) { - return { - from: HOLDER, - to: RECIPIENT, - ensName: null, - amount: "1.5", - token: NOVEL, - balance: "1.0", - tokenSymbol: "NOVEL", - tokenBalance, - tokenDecimals: tokenBalance === null ? null : 18, - }; - } - - async function render(tokenBalance) { + // A send of 1.5 NOVEL from an address the node reports none of, with the + // token's scale `tokenDecimals`, or null for a scale nobody knows. + async function render(tokenDecimals) { + mockProvider.tokenUnits = 0n; state.wallets = [ { name: "Wallet 1", @@ -469,14 +466,22 @@ describe("the confirmation screen tells an unknown balance from a zero one", () ]; state.selectedWallet = 0; state.selectedAddress = 0; - confirmTx.show(txInfo(tokenBalance)); + confirmTx.show({ + from: HOLDER, + to: RECIPIENT, + ensName: null, + amount: "1.5", + token: NOVEL, + tokenSymbol: "NOVEL", + tokenDecimals, + }); await settle(); return { balance: text("confirm-balance"), errors: errors() }; } test("the balance line states unknown rather than a quantity of zero", async () => { const unknown = await render(null); - const zero = await render("0.0"); + const zero = await render(18); expect(unknown.balance).not.toBe(zero.balance); expect(unknown.balance).toBe("unknown (NOVEL)"); expect(zero.balance).toBe("0.0000 NOVEL"); @@ -487,7 +492,7 @@ describe("the confirmation screen tells an unknown balance from a zero one", () // about the holding, and this one has no established quantity to claim. test("the insufficient-balance message names the reason, not a figure", async () => { const unknown = await render(null); - const zero = await render("0.0"); + const zero = await render(18); expect(unknown.errors).not.toBe(zero.errors); expect(unknown.errors).toContain("This token's balance is unknown"); expect(unknown.errors).not.toContain("You have");