fix: a confirmation screen drawn again keeps the network it was reviewed for (closes #586)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run

ConfirmTx set the transaction's chain id to the active network's every
time it drew the screen, so a popup reopened onto a saved confirmation
after a network switch in Settings described the reviewed transfer as
the new network's and sent it there.

show() now sets the chain id only on a transaction Send hands over, and
the screen describes the transaction entirely on that network: native
currency, USD values, explorer links, and the node and explorer its
checks ask. explorerUrl(), renderAddressHtml(), getPrice(), formatFee()
and checkEtherscanLabel() take an optional chain id for this. A saved
transaction whose chain id names no network is not restored.

Model: opus-5-5
This commit was merged in pull request #587.
This commit is contained in:
2026-10-19 07:38:15 +02:00
parent b5d2cedb57
commit 1f5cdeb7b4
9 changed files with 435 additions and 72 deletions
+288 -9
View File
@@ -7,25 +7,52 @@
// reviewed on Sepolia went out as 1 ETH on mainnet once mainnet was chosen in
// Settings in between.
//
// The same holds when the popup was closed after the switch and reopened, and
// Back drew the screen again from the transaction it saved
// (https://git.eeqj.de/sneak/AutistMask/issues/586): the screen was drawn for
// the new network, and Sign & Send sent the reviewed transfer there. Drawn
// again, the screen describes the transaction entirely on the network it was
// reviewed for, because choosing that network again in Settings and pressing
// Back only unhides it, and Sign & Send then sends from it.
//
// 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.
// only the node, fetch 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: [] };
// The questions the stub node was asked for the fee and the recipient checks,
// by method, network name and RPC URL, in order. The signed transactions it
// was handed, with its network's chain id and RPC URL. And whether it quotes
// fees as a network with no type-2 pricing does.
const mockNode = { asked: [], broadcast: [], legacyFees: false };
jest.mock("ethers", () => {
const actual = jest.requireActual("ethers");
class StubProvider {
constructor(url, network) {
this.url = url;
this.network = network;
}
record(method) {
mockNode.asked.push({
method,
network: this.network.name,
url: this.url,
});
}
async getNetwork() {
return this.network;
}
async getFeeData() {
this.record("getFeeData");
if (mockNode.legacyFees) {
return {
maxFeePerGas: null,
maxPriorityFeePerGas: null,
gasPrice: 10000000000n,
};
}
return {
maxFeePerGas: 20000000000n,
maxPriorityFeePerGas: 1000000000n,
@@ -36,16 +63,22 @@ jest.mock("ethers", () => {
return 21000n;
}
async getCode() {
this.record("getCode");
return "0x";
}
async getTransactionCount() {
this.record("getTransactionCount");
return 1;
}
async lookupAddress() {
return null;
}
async broadcastTransaction(signed) {
mockNode.broadcast.push({ chainId: this.network.chainId, signed });
mockNode.broadcast.push({
chainId: this.network.chainId,
url: this.url,
signed,
});
return { hash: "0x" + "ab".repeat(32) };
}
}
@@ -75,6 +108,8 @@ jest.mock("../src/popup/views/txStatus", () => ({
showError: jest.fn(),
}));
// The Etherscan label check is its only caller here: the explorer pages it
// asks for are recorded, and nothing is fetched.
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
@@ -125,15 +160,27 @@ global.navigator = { clipboard: { writeText() {} } };
const { BaseWallet, Transaction, Wallet } = require("ethers");
const { encryptWithPassword } = require("../src/shared/vault");
const { state, loadState } = require("../src/shared/state");
const { state, loadState, saveState } = require("../src/shared/state");
const { onChainSwitch } = require("../src/shared/chainSwitch");
const { NETWORKS } = require("../src/shared/networks");
const { prices } = require("../src/shared/prices");
const {
goBack,
pushCurrentView,
setBackRenderer,
showView,
} = require("../src/popup/views/helpers");
const {
makeBackRenderer,
resetRenderedViews,
} = require("../src/popup/viewRouter");
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 TOKEN = "0x7070707070707070707070707070707070707070";
const PASSWORD = "correct horse battery staple";
function el(id) {
@@ -150,7 +197,10 @@ beforeAll(async () => {
beforeEach(() => {
elements.clear();
mockNode.asked = [];
mockNode.broadcast = [];
mockNode.legacyFees = false;
global.fetch.mockClear();
txStatus.showWait.mockClear();
txStatus.showError.mockClear();
signTransaction = jest.spyOn(BaseWallet.prototype, "signTransaction");
@@ -160,10 +210,16 @@ 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 the confirmation screen for 1 of the native token on `networkId`, or
// for the transfer `changes` makes of it, as Review on the Send screen does,
// with its fee estimate settled.
async function review(networkId, changes = {}) {
await onChainSwitch(networkId);
await reviewOnActiveNetwork(changes);
}
// review() on the network already active, without switching to it again.
async function reviewOnActiveNetwork(changes = {}) {
state.wallets = [
{
type: "key",
@@ -186,7 +242,90 @@ async function review(networkId) {
tokenBalance: null,
tokenDecimals: null,
max: false,
...changes,
});
await settle();
}
// The user's own node for a network, set in Settings.
function ownNode(networkId) {
return "https://" + networkId + ".node.example.invalid";
}
// After review() on `reviewedOn`: the user's own node set for it, the Settings
// gear, `switchedTo` chosen there, the popup closed and reopened onto
// Settings, and Back, which draws the confirmation screen again from the
// transaction it saved. The reopened page holds an ETH price of $2,000 and a
// USDC price of $1 whichever network is active, so each USD value shows which
// network it was priced for.
async function switchReopenAndGoBack(reviewedOn, switchedTo) {
state.rpcUrl = ownNode(reviewedOn);
pushCurrentView();
showView("settings");
await onChainSwitch(switchedTo);
await loadState();
elements.clear();
resetRenderedViews();
setBackRenderer(makeBackRenderer(state, { confirmTx }));
confirmTx.init({});
prices.ETH = 2000;
prices.USDC = 1;
mockNode.asked = [];
global.fetch.mockClear();
goBack();
await settle();
}
// The USD value `text` as the screen appends it for `networkId`: nothing on a
// testnet, which has no USD values.
function usdOn(networkId, text) {
return NETWORKS[networkId].isTestnet ? "" : " (" + text + ")";
}
// The screen for the transfer review() shows by default, described on
// `networkId`: in its native currency, priced by it, linking to its explorer.
function expectNativeTransferOn(networkId) {
const { nativeCurrency: symbol, explorerUrl } = NETWORKS[networkId];
expect(el("confirm-type").textContent).toBe(
"Native " + symbol + " transfer",
);
expect(el("confirm-amount").textContent).toBe(
"1 " + symbol + usdOn(networkId, "$2,000.00"),
);
expect(el("confirm-balance").textContent).toBe(
"5.0000 " + symbol + usdOn(networkId, "$10,000.00"),
);
expect(el("confirm-fee-amount").textContent).toBe(
"~0.0002 " + symbol + usdOn(networkId, "$0.42"),
);
expect(el("confirm-fee-reserve").textContent).toBe(
"up to 0.0004 " + symbol + " reserved",
);
expect(el("confirm-gas-error").textContent).toBe(
"You do not have enough " +
symbol +
" to pay the network fee for this transfer. Please add " +
symbol +
" to this address and try again.",
);
expect(el("confirm-recipient-warning").textContent).toContain(
" holds no " + symbol + ". ",
);
expect(el("confirm-contract-warning").textContent).toContain(
" Sending " + symbol + " or tokens ",
);
expect(el("confirm-from").innerHTML).toContain(
`href="${explorerUrl}/address/${HOLDER}"`,
);
expect(el("confirm-to").innerHTML).toContain(
`href="${explorerUrl}/address/${RECIPIENT}"`,
);
}
// show() estimates the fee and checks the recipient without awaiting either;
// this lets both finish.
async function settle() {
for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
}
@@ -208,6 +347,24 @@ test("a transfer reviewed on Sepolia and sent goes out on Sepolia", async () =>
expect(txStatus.showWait).toHaveBeenCalledTimes(1);
});
// Saving an RPC endpoint in Settings (src/popup/views/settings.js) sets only
// state.rpcUrl: the endpoint remembered for the active network keeps its old
// value until the next switch away from it.
test("a transfer reviewed after the active network's RPC endpoint was changed in Settings asks the new endpoint", async () => {
const newNode = "https://new.node.example.invalid";
await onChainSwitch("sepolia");
state.rpcUrl = newNode;
await saveState();
await reviewOnActiveNetwork();
expect(mockNode.asked).toEqual([
{ method: "getFeeData", network: "sepolia", url: newNode },
{ method: "getCode", network: "sepolia", url: newNode },
{ method: "getTransactionCount", network: "sepolia", url: newNode },
]);
});
describe.each([
["sepolia", "mainnet", "SepoliaETH"],
["mainnet", "sepolia", "ETH"],
@@ -238,6 +395,128 @@ describe.each([
},
);
describe.each([
["sepolia", "mainnet"],
["mainnet", "sepolia"],
])(
"a transfer reviewed on %s, with %s chosen in Settings, the popup reopened and Back pressed",
(reviewedOn, switchedTo) => {
const { nativeCurrency: symbol, explorerUrl } = NETWORKS[reviewedOn];
test("is described on the network it was reviewed for, and neither signed nor sent", async () => {
await review(reviewedOn);
await switchReopenAndGoBack(reviewedOn, switchedTo);
expect(state.currentView).toBe("confirm-tx");
expectNativeTransferOn(reviewedOn);
expect(mockNode.asked).toEqual([
{
method: "getFeeData",
network: reviewedOn,
url: ownNode(reviewedOn),
},
{
method: "getCode",
network: reviewedOn,
url: ownNode(reviewedOn),
},
{
method: "getTransactionCount",
network: reviewedOn,
url: ownNode(reviewedOn),
},
]);
expect(global.fetch.mock.calls.map(([url]) => url)).toEqual([
explorerUrl + "/address/" + RECIPIENT,
]);
expect(el("btn-confirm-send").disabled).toBe(false);
await signAndSend();
expect(signTransaction).not.toHaveBeenCalled();
expect(mockNode.broadcast).toEqual([]);
expect(txStatus.showWait).not.toHaveBeenCalled();
expect(txStatus.showError).not.toHaveBeenCalled();
expect(el("confirm-errors").innerHTML).toContain(
"The active network changed after this transaction was" +
" reviewed, so it was not sent.",
);
});
test("is sent on that network, as described, once it is chosen in Settings again and Back pressed", async () => {
await review(reviewedOn);
await switchReopenAndGoBack(reviewedOn, switchedTo);
// The Settings gear, the reviewed network chosen, and Back, which
// only unhides the screen as it was drawn.
pushCurrentView();
showView("settings");
await onChainSwitch(reviewedOn);
goBack();
expect(state.currentView).toBe("confirm-tx");
expectNativeTransferOn(reviewedOn);
await signAndSend();
const chainId = BigInt(NETWORKS[reviewedOn].chainId);
expect(mockNode.broadcast).toHaveLength(1);
expect(mockNode.broadcast[0].chainId).toBe(chainId);
expect(mockNode.broadcast[0].url).toBe(ownNode(reviewedOn));
expect(Transaction.from(mockNode.broadcast[0].signed).chainId).toBe(
chainId,
);
expect(txStatus.showWait).toHaveBeenCalledTimes(1);
});
test("for more than the balance, says so in that network's native currency", async () => {
await review(reviewedOn, { amount: "10" });
await switchReopenAndGoBack(reviewedOn, switchedTo);
expect(el("confirm-errors").innerHTML).toContain(
"Insufficient balance. You have 5.0000 " +
symbol +
" but are trying to send 10 " +
symbol +
".",
);
});
test("with a fee quoted by a node with no type-2 pricing, shows the fee in that network's native currency and price", async () => {
await review(reviewedOn);
mockNode.legacyFees = true;
await switchReopenAndGoBack(reviewedOn, switchedTo);
expect(el("confirm-fee-amount").textContent).toBe(
"0.0002 " + symbol + usdOn(reviewedOn, "$0.42"),
);
});
test("of an ERC-20 token, links the token contract to that network's explorer and prices the token by that network", async () => {
await review(reviewedOn, {
token: TOKEN,
tokenSymbol: "USDC",
tokenBalance: "50",
tokenDecimals: 6,
amount: "10",
});
await switchReopenAndGoBack(reviewedOn, switchedTo);
expect(el("confirm-type").textContent).toBe(
"ERC-20 token transfer (USDC)",
);
expect(el("confirm-token-contract").innerHTML).toContain(
`href="${explorerUrl}/address/${TOKEN}"`,
);
expect(el("confirm-amount").textContent).toBe(
"10 USDC" + usdOn(reviewedOn, "$10.00"),
);
expect(el("confirm-balance").textContent).toBe(
"50.0000 USDC" + usdOn(reviewedOn, "$50.00"),
);
});
},
);
// 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.
+14
View File
@@ -453,6 +453,20 @@ const CONTRACT = [
views: ["confirm-tx"],
restored: false,
},
// A chain id naming no network has no explorer to link to.
{
value: {
pendingTx: {
token: "ETH",
from: ADDRESS,
to: ADDRESS,
amount: "1",
chainId: "0x5",
},
},
views: ["confirm-tx"],
restored: false,
},
// wait-tx passes on `pendingWait.hash`; restoreWait() has checked
// the fields below it since it was written, and this is the
// regression guard.