diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index a7b50fe..6569743 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -609,6 +609,9 @@ async function checkRecipientHistory(txInfo) { fromAddress: txInfo.from, chainId: txInfo.chainId, }); + // The user may have left this transaction while the checks were in + // flight; its warnings must not reach another recipient's screen. + if (pendingTx !== txInfo) return; for (const w of asyncWarnings) { if (w.type === "contract") { $("confirm-contract-warning").style.visibility = "visible"; diff --git a/tests/confirmTxLateResult.test.js b/tests/confirmTxLateResult.test.js new file mode 100644 index 0000000..780912b --- /dev/null +++ b/tests/confirmTxLateResult.test.js @@ -0,0 +1,221 @@ +// A late answer for a confirmation screen the user has left changes nothing on +// the screen shown since (https://git.eeqj.de/sneak/AutistMask/issues/575). +// +// The confirmation screen asks the node and the explorer about its transaction +// without waiting for the answers: the recipient checks, which can show three +// warning boxes, the balance read and the fee estimate. Back, a new recipient +// and Review again show the screen for another transaction while those +// questions may still be open. The recipient checks showed their boxes +// whenever they answered, so a box about the first recipient could appear on +// the second recipient's screen. +// +// Driven through the real confirmation screen, with only the node, the +// explorer and the DOM stubbed. Each test holds back the first screen's answer +// to one question, and gives it once the screen it tests is shown. + +"use strict"; + +// The From address of every transaction here. The jest.mock() factory below +// may only name outside variables whose names start with "mock". +const mockFrom = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; + +jest.mock("ethers", () => { + const actual = jest.requireActual("ethers"); + // Every answer is a mock, so a test can hold one back. Every address has + // sent one transaction and has no code, and only the From address holds + // anything: 5 ETH. + class StubProvider {} + Object.assign(StubProvider.prototype, { + getBalance: jest.fn(async (address) => + address === mockFrom ? actual.parseEther("5") : 0n, + ), + getCode: jest.fn(async () => "0x"), + getTransactionCount: jest.fn(async () => 1), + // 10 gwei expected, 20 gwei reserved per gas. + getFeeData: jest.fn(async () => ({ + maxFeePerGas: 20000000000n, + maxPriorityFeePerGas: 1000000000n, + gasPrice: 10000000000n, + })), + estimateGas: jest.fn(async () => 21000n), + }); + return { ...actual, JsonRpcProvider: StubProvider }; +}); + +// An explorer address page whose title is `title`. +function explorerPage(title) { + return { ok: true, text: async () => "" + title + "" }; +} + +// The explorer labels no address. +global.fetch = jest.fn(async () => explorerPage("Etherscan")); + +const { makeStorageStub } = require("./support/storageStub"); +global.chrome = { storage: makeStorageStub(), runtime: { sendMessage() {} } }; + +// A stub DOM: every id resolves to a recording element. +const elements = new Map(); + +function makeEl(id) { + return { + id, + textContent: "", + innerHTML: "", + value: "", + disabled: false, + style: {}, + dataset: {}, + classList: { + add() {}, + remove() {}, + toggle() {}, + contains: () => false, + }, + addEventListener() {}, + querySelectorAll: () => [], + remove() {}, + }; +} + +global.document = { + getElementById(id) { + if (!elements.has(id)) elements.set(id, makeEl(id)); + return elements.get(id); + }, +}; + +const { JsonRpcProvider, parseEther } = require("ethers"); +const { loadState } = require("../src/shared/state"); +const confirmTx = require("../src/popup/views/confirmTx"); + +const node = JsonRpcProvider.prototype; +const FIRST = "0xC0FfEE0000000000000000000000000000c0fFEe"; +const SECOND = "0xE2E0000000000000000000000000000000000E2e"; + +function el(id) { + return global.document.getElementById(id); +} + +function shown(id) { + return el(id).style.visibility === "visible"; +} + +// Show the confirmation screen for 1 ETH to `to` on mainnet, as Review on the +// Send screen does. +function review(to) { + confirmTx.show({ + from: mockFrom, + to, + ensName: null, + amount: "1", + token: "ETH", + tokenSymbol: null, + tokenDecimals: null, + max: false, + }); +} + +// show() does not wait for its questions; this lets every answer not held +// back arrive. +async function settle() { + for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); +} + +// Hold back the next answer `question` gives. Returns the function that gives +// it. +function holdNextAnswer(question) { + let give; + question.mockImplementationOnce(() => new Promise((r) => (give = r))); + return (answer) => give(answer); +} + +beforeAll(async () => { + await loadState(); +}); + +beforeEach(() => { + elements.clear(); +}); + +// The three boxes the recipient checks show, each with the question whose +// answer shows it for FIRST, and that answer. FIRST holds no ETH, so the +// answer that it has sent no transaction is what shows the box saying so. +describe.each([ + { + box: "confirm-contract-warning", + question: node.getCode, + answer: "0x6080604052", + }, + { + box: "confirm-recipient-warning", + question: node.getTransactionCount, + answer: 0, + }, + { + box: "confirm-etherscan-warning", + question: global.fetch, + answer: explorerPage( + "Fake_Phishing1 | Address: " + FIRST + " | Etherscan", + ), + }, +])("the $box box", ({ box, question, answer }) => { + test("is shown when the answer about the recipient arrives while its screen is shown", async () => { + const give = holdNextAnswer(question); + review(FIRST); + await settle(); + expect(shown(box)).toBe(false); + + give(answer); + await settle(); + expect(shown(box)).toBe(true); + }); + + test("is not shown on the next recipient's screen when the answer about the first arrives after that screen is shown", async () => { + const give = holdNextAnswer(question); + review(FIRST); + await settle(); + review(SECOND); + await settle(); + expect(shown(box)).toBe(false); + + give(answer); + await settle(); + expect(shown(box)).toBe(false); + }); +}); + +test("a balance read answered after the next screen is shown leaves that screen's balance and balance check as they are", async () => { + const give = holdNextAnswer(node.getBalance); + review(FIRST); + await settle(); + review(SECOND); + await settle(); + expect(el("confirm-balance").textContent).toBe("5.0000 ETH"); + + give(parseEther("0.5")); + await settle(); + expect(el("confirm-balance").textContent).toBe("5.0000 ETH"); + expect(shown("confirm-balance-error")).toBe(false); + expect(el("btn-confirm-send").disabled).toBe(false); +}); + +test("a fee estimate answered after the next screen is shown leaves that screen's fee as it is", async () => { + const give = holdNextAnswer(node.getFeeData); + review(FIRST); + await settle(); + review(SECOND); + await settle(); + expect(el("confirm-fee-amount").textContent).toBe("~0.0002 ETH"); + + // 100 times the fee the second screen was quoted. + give({ + maxFeePerGas: 2000000000000n, + maxPriorityFeePerGas: 100000000000n, + gasPrice: 1000000000000n, + }); + await settle(); + expect(el("confirm-fee-amount").textContent).toBe("~0.0002 ETH"); + expect(el("confirm-fee-reserve").textContent).toBe( + "up to 0.0004 ETH reserved", + ); +});