fix: Sign & Send refuses a transfer reviewed on another network (closes #524 )

Settings is reachable from the confirmation screen, and Back from it only unhides the screen as it was drawn. Sign & Send took the network at the click, so a transfer reviewed on Sepolia went out on mainnet after a switch in between. show() now records the chain id of the network it draws for. Sign & Send refuses with a message on the screen when the active network differs, and the send carries that chain id, so ethers will not populate it for a provider on any other network. Model: opus-5-5
2026-10-14 03:16:01 +00:00
5 changed files with 289 additions and 187 deletions
@@ -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**
@@ -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(
@@ -183,7 +183,7 @@ function render() {
        if (el) el.classList.add("hidden");
    }

-    loadFullTxDetails(tx.hash, tx.chainId);
+    loadFullTxDetails(tx.hash, tx.to, tx.chainId);

    const isoStr = isoDate(tx.timestamp);
    $("tx-detail-time").innerHTML =
@@ -291,7 +291,7 @@ function populateOnChainDetails(txData, chainId) {
    }
 }

-async function loadFullTxDetails(txHash, chainId) {
+async function loadFullTxDetails(txHash, toAddress, chainId) {
    const section = $("tx-detail-calldata-section");
    const actionEl = $("tx-detail-calldata-action");
    const detailsEl = $("tx-detail-calldata-details");
@@ -313,10 +313,7 @@ async function loadFullTxDetails(txHash, chainId) {
        const inputData = txData.raw_input || txData.input || null;
        if (!inputData || inputData === "0x") return;

-        // Decoded against the address the transaction was sent to, as the
-        // explorer reports it, not the history entry's `to`: for a token
-        // transfer that is the transfer's recipient, not the token contract.
-        const decoded = decodeCalldata(inputData, txData.to?.hash || "");
+        const decoded = decodeCalldata(inputData, toAddress || "");
        if (decoded) {
            // Render decoded calldata matching approval view style
            actionEl.textContent = decoded.name;
@@ -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([]);
+});
@@ -1,176 +0,0 @@
-// The decoded details on the transaction detail screen of a token transfer
-// (https://git.eeqj.de/sneak/AutistMask/issues/515).
-//
-// A history row for a token transfer holds the transfer's recipient as its
-// `to`, not the contract the transaction was sent to. The screen used to decode
-// the transaction's data against that `to`, so its Token line named the
-// recipient, and the amount read "decimals unknown" for a token the wallet
-// tracks. It decodes against the address the explorer reports the transaction
-// was sent to.
-//
-// The row is the one the real history parser makes from the explorer's answer,
-// opened through transactionDetail.show(), which is what a click on the row
-// calls. Driven against a minimal DOM stub in the shape
-// tests/contractCreation.test.js uses.
-
-const { Interface } = require("ethers");
-
-jest.mock("../src/shared/log", () => ({
-    log: {
-        debugf: () => {},
-        infof: () => {},
-        warnf: () => {},
-        errorf: () => {},
-    },
-    debugFetch: async (url) => ({
-        ok: true,
-        json: async () => mockExplorerAnswer(url),
-    }),
-    setRuntimeDebug: () => {},
-    isDebug: () => false,
-}));
-
-globalThis.chrome = {
-    storage: { local: { get: async () => ({}), set: async () => {} } },
-};
-
-const { state } = require("../src/shared/state");
-const { fetchRecentTransactions } = require("../src/shared/transactions");
-const transactionDetail = require("../src/popup/views/transactionDetail");
-
-const EXPLORER = "https://explorer.example/api/v2";
-const HOLDER = "0x0000000000000000000000000000000000000a11";
-const RECIPIENT = "0x711e691962E5c054b78071C789A0C8CE657377aD";
-// A token the wallet tracks and the bundled token list does not name.
-const TOKEN = "0x74de5d4FCbf63E00296fd95d33236B9794016631";
-const TX_HASH =
-    "0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84";
-
-// HOLDER sends 1234.5 of the 6-decimal token to RECIPIENT.
-const TRANSFER_DATA = new Interface([
-    "function transfer(address to, uint256 amount)",
-]).encodeFunctionData("transfer", [RECIPIENT, 1234500000n]);
-
-// The explorer's answers: the transfer in HOLDER's token transfers, and the
-// transaction itself, sent to the token contract.
-function mockExplorerAnswer(url) {
-    if (
-        url ===
-        EXPLORER + "/addresses/" + HOLDER + "/token-transfers?type=ERC-20"
-    ) {
-        return {
-            items: [
-                {
-                    transaction_hash: TX_HASH,
-                    block_number: 100,
-                    timestamp: "2026-10-01T00:00:00Z",
-                    from: { hash: HOLDER },
-                    to: { hash: RECIPIENT },
-                    total: { value: "1234500000", decimals: "6" },
-                    token: {
-                        address_hash: TOKEN,
-                        symbol: "TDOL",
-                        holders_count: "5000",
-                    },
-                },
-            ],
-        };
-    }
-    if (url === EXPLORER + "/transactions/" + TX_HASH) {
-        return { to: { hash: TOKEN }, raw_input: TRANSFER_DATA };
-    }
-    return { items: [] };
-}
-
-function makeElement(id) {
-    const classes = new Set();
-    const el = {
-        id,
-        textContent: "",
-        value: "",
-        innerHTML: "",
-        style: {},
-        dataset: {},
-        classList: {
-            add: (...names) => names.forEach((n) => classes.add(n)),
-            remove: (...names) => names.forEach((n) => classes.delete(n)),
-            contains: (n) => classes.has(n),
-            toggle: () => false,
-        },
-        addEventListener: () => {},
-        querySelectorAll: () => [],
-        appendChild: () => {},
-    };
-    // Views reach for .parentElement to hide whole sections.
-    Object.defineProperty(el, "parentElement", {
-        get: () => node(id + "-parent"),
-    });
-    return el;
-}
-
-function makeDocument() {
-    const els = new Map();
-    return {
-        getElementById(id) {
-            // The debug banner is created on demand by helpers.js; absent
-            // is the state a non-debug, non-testnet popup is in.
-            if (id === "debug-banner") return null;
-            if (!els.has(id)) els.set(id, makeElement(id));
-            return els.get(id);
-        },
-        createElement: () => makeElement("created"),
-        body: { prepend: () => {} },
-    };
-}
-
-function node(id) {
-    return globalThis.document.getElementById(id);
-}
-
-// One line of the decoded details: its label and what is drawn under it.
-function decodedLine(label) {
-    return node("tx-detail-calldata-details")
-        .innerHTML.split('<div class="mb-2">')
-        .find((line) =>
-            line.startsWith(`<div class="text-muted">${label}</div>`),
-        );
-}
-
-beforeEach(() => {
-    globalThis.document = makeDocument();
-    globalThis.window = { location: { search: "" } };
-    state.blockscoutUrl = EXPLORER;
-    state.wallets = [
-        {
-            name: "Main",
-            type: "key",
-            addresses: [{ address: HOLDER, balance: "0.0000" }],
-        },
-    ];
-    state.trackedTokens = [{ address: TOKEN, symbol: "TDOL", decimals: 6 }];
-    state.viewData = {};
-    state.viewStack = [];
-    state.currentView = null;
-});
-
-test("a token transfer's decoded details name the token contract and the amount at its scale", async () => {
-    const [row] = await fetchRecentTransactions(HOLDER, EXPLORER, 1);
-    expect(row.to).toBe(RECIPIENT);
-    expect(row.contractAddress).toBe(TOKEN.toLowerCase());
-
-    transactionDetail.show(row);
-    // The decoded details are drawn once the explorer has answered.
-    for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
-
-    expect(node("tx-detail-calldata-action").textContent).toBe(
-        "Token Transfer",
-    );
-    const token = decodedLine("Token");
-    expect(token).toContain('<div class="font-bold">TDOL</div>');
-    expect(token).toContain(TOKEN);
-    expect(token).not.toContain(RECIPIENT);
-    expect(decodedLine("Recipient")).toContain(RECIPIENT);
-    expect(decodedLine("Amount")).toContain(
-        '<div class="font-bold">1234.5000 TDOL</div>',
-    );
-});