diff --git a/README.md b/README.md index a4505e2..a5ffb88 100644 --- a/README.md +++ b/README.md @@ -1641,6 +1641,12 @@ view would leave a wallet one click from deletion. the contract's own answer is read at signing time only to be compared with it, and a disagreement is a refusal rather than a preference for either value (`src/shared/transferAmount.js`) + - "Sign & Send" after the active network was switched in Settings while this + screen was open → nothing is signed; "The active network changed after + this transaction was reviewed, so it was not sent. Please go back and + review it again." in the errors box, no screen change. The transaction + carries the chain id of the network the screen was shown for, so it cannot + be signed for another - "Sign & Send" (wrong password) → "Wrong password." on the password error line, no screen change - "Back" → **Send** diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index 30d1c4d..4bb5f66 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -101,6 +101,10 @@ function show(txInfo) { feeStatus = FEE_PENDING; feeWei = null; maxSendFees = 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. + txInfo.chainId = currentNetwork().chainId; const isErc20 = txInfo.token !== "ETH"; // The raw symbol is the price-table key; the capped one is what the @@ -490,7 +494,12 @@ async function populateVerifyAndSend(connectedSigner, tx, fees = null) { ); request = await contract.transfer.populateTransaction(tx.to, amount); } - const populated = await connectedSigner.populateTransaction(request); + // The chain id of the network the screen was drawn for: ethers refuses to + // populate it for a provider on any other network. + const populated = await connectedSigner.populateTransaction({ + ...request, + chainId: tx.chainId, + }); assertWithinCeilings(populated); return connectedSigner.sendTransaction(populated); } @@ -547,6 +556,17 @@ function init(_ctx) { return; } + // Back from Settings only unhides this screen, still drawn for the + // network that was active when it was shown. + if (currentNetwork().chainId !== pendingTx.chainId) { + showSendError( + "The active network changed after this transaction was" + + " reviewed, so it was not sent. Please go back and review" + + " it again.", + ); + return; + } + const password = $("confirm-tx-password").value; if (!password) { showError( @@ -575,10 +595,6 @@ function init(_ctx) { $("btn-confirm-send").disabled = true; $("btn-confirm-send").classList.add("text-muted"); - // The network it is sent on. The wait, success and error screens - // label its amount by this, not by the network active when they draw. - pendingTx.chainId = currentNetwork().chainId; - let tx; try { const signer = getSignerForAddress( diff --git a/tests/confirmTxNetworkSwitch.test.js b/tests/confirmTxNetworkSwitch.test.js new file mode 100644 index 0000000..a612744 --- /dev/null +++ b/tests/confirmTxNetworkSwitch.test.js @@ -0,0 +1,259 @@ +// A transfer is sent only on the network its confirmation screen was drawn for +// (https://git.eeqj.de/sneak/AutistMask/issues/524). +// +// Settings is reachable from the confirmation screen, and Back from it only +// unhides the screen, still labelled for the network it was drawn for. Sign & +// Send used to take the network at the moment of the click, so 1 SepoliaETH +// reviewed on Sepolia went out as 1 ETH on mainnet once mainnet was chosen in +// Settings in between. +// +// 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. + +"use strict"; + +// The signed transactions the stub node was handed, with its network's chain id. +const mockNode = { broadcast: [] }; + +jest.mock("ethers", () => { + const actual = jest.requireActual("ethers"); + class StubProvider { + constructor(url, network) { + this.network = network; + } + async getNetwork() { + return this.network; + } + async getFeeData() { + return { + maxFeePerGas: 20000000000n, + maxPriorityFeePerGas: 1000000000n, + gasPrice: 10000000000n, + }; + } + async estimateGas() { + return 21000n; + } + async getCode() { + return "0x"; + } + async getTransactionCount() { + return 1; + } + async lookupAddress() { + return null; + } + async broadcastTransaction(signed) { + mockNode.broadcast.push({ chainId: this.network.chainId, signed }); + return { hash: "0x" + "ab".repeat(32) }; + } + } + return { ...actual, JsonRpcProvider: StubProvider }; +}); + +jest.mock("../src/shared/log", () => ({ + log: { + debugf: () => {}, + infof: () => {}, + warnf: () => {}, + errorf: () => {}, + }, + debugFetch: jest.fn(async () => ({ + ok: true, + status: 200, + json: async () => ({ items: [] }), + })), + urlOrigin: () => "", + setRuntimeDebug: () => {}, + isDebug: () => false, +})); + +// The wait screen polls for a receipt; these tests stop at the broadcast. +jest.mock("../src/popup/views/txStatus", () => ({ + showWait: jest.fn(), + showError: jest.fn(), +})); + +global.fetch = jest.fn(() => { + throw new Error("tests must not perform network requests"); +}); + +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) { + const handlers = new Map(); + return { + id, + textContent: "", + innerHTML: "", + value: "", + disabled: false, + style: {}, + dataset: {}, + classList: { + add() {}, + remove() {}, + toggle() {}, + contains: () => false, + }, + handlers, + addEventListener(name, fn) { + handlers.set(name, fn); + }, + querySelectorAll: () => [], + querySelector: () => null, + remove() {}, + focus() {}, + }; +} + +global.document = { + getElementById(id) { + if (!elements.has(id)) elements.set(id, makeEl(id)); + return elements.get(id); + }, + createElement: (tag) => makeEl(tag), + body: { prepend() {}, appendChild() {} }, + addEventListener() {}, +}; +global.navigator = { clipboard: { writeText() {} } }; + +const { BaseWallet, Transaction, Wallet } = require("ethers"); +const { encryptWithPassword } = require("../src/shared/vault"); +const { state, loadState } = require("../src/shared/state"); +const { onChainSwitch } = require("../src/shared/chainSwitch"); +const { NETWORKS } = require("../src/shared/networks"); +const txStatus = require("../src/popup/views/txStatus"); +const confirmTx = require("../src/popup/views/confirmTx"); + +const PRIVATE_KEY = "0x" + "11".repeat(32); +const HOLDER = new Wallet(PRIVATE_KEY).address; +const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe"; +const PASSWORD = "correct horse battery staple"; + +function el(id) { + return global.document.getElementById(id); +} + +let encryptedSecret; +let signTransaction; + +beforeAll(async () => { + await loadState(); + encryptedSecret = await encryptWithPassword(PRIVATE_KEY, PASSWORD); +}); + +beforeEach(() => { + elements.clear(); + mockNode.broadcast = []; + txStatus.showWait.mockClear(); + txStatus.showError.mockClear(); + signTransaction = jest.spyOn(BaseWallet.prototype, "signTransaction"); +}); + +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) { + await onChainSwitch(networkId); + state.wallets = [ + { + type: "key", + name: "Wallet 1", + encryptedSecret, + addresses: [{ address: HOLDER, balance: "5.0" }], + }, + ]; + state.selectedWallet = 0; + state.selectedAddress = 0; + confirmTx.init({}); + confirmTx.show({ + from: HOLDER, + to: RECIPIENT, + ensName: null, + amount: "1", + token: "ETH", + balance: "5.0", + tokenSymbol: null, + tokenBalance: null, + tokenDecimals: null, + max: false, + }); + for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); +} + +async function signAndSend() { + el("confirm-tx-password").value = PASSWORD; + await el("btn-confirm-send").handlers.get("click")(); +} + +test("a transfer reviewed on Sepolia and sent goes out on Sepolia", async () => { + await review("sepolia"); + expect(el("confirm-type").textContent).toBe("Native SepoliaETH transfer"); + expect(el("btn-confirm-send").disabled).toBe(false); + await signAndSend(); + expect(mockNode.broadcast).toHaveLength(1); + expect(mockNode.broadcast[0].chainId).toBe(11155111n); + expect(Transaction.from(mockNode.broadcast[0].signed).chainId).toBe( + 11155111n, + ); + expect(txStatus.showWait).toHaveBeenCalledTimes(1); +}); + +describe.each([ + ["sepolia", "mainnet", "SepoliaETH"], + ["mainnet", "sepolia", "ETH"], +])( + "a transfer reviewed on %s, with %s chosen in Settings before Sign & Send", + (reviewedOn, switchedTo, symbol) => { + test("is neither signed nor sent, and the screen says why", async () => { + await review(reviewedOn); + expect(el("confirm-type").textContent).toBe( + "Native " + symbol + " transfer", + ); + // Settings' network select, then Back, which only unhides the + // screen as it was drawn. + await onChainSwitch(switchedTo); + await signAndSend(); + + expect(signTransaction).not.toHaveBeenCalled(); + expect(mockNode.broadcast).toEqual([]); + expect(txStatus.showWait).not.toHaveBeenCalled(); + expect(txStatus.showError).not.toHaveBeenCalled(); + expect(el("confirm-errors").style.visibility).toBe("visible"); + expect(el("confirm-errors").innerHTML).toContain( + "The active network changed after this transaction was" + + " reviewed, so it was not sent. Please go back and review" + + " it again.", + ); + }); + }, +); + +// The check in the click handler tells the user; the send itself carries the +// chain id the screen was drawn for, so a signer on another network's node +// cannot sign it even if the network changes after that check. +test("the send is refused by a signer on another network's node", async () => { + const { getProvider } = require("../src/shared/balances"); + const signer = new Wallet(PRIVATE_KEY).connect( + getProvider("https://rpc.example.invalid", "mainnet"), + ); + await expect( + confirmTx.populateVerifyAndSend(signer, { + token: "ETH", + to: RECIPIENT, + amount: "1", + chainId: NETWORKS.sepolia.chainId, + }), + ).rejects.toThrow("transaction chainId mismatch"); + expect(signTransaction).not.toHaveBeenCalled(); + expect(mockNode.broadcast).toEqual([]); +});