fix: a send the confirmation screen refuses moves nothing on it (closes #583)

Sign & Send wrote its refusals, a network changed since review and a gas limit or fee over its bound, into the errors box above the password field. That box reserves one empty line, so a refusal grew it and pushed the password field and Sign & Send down.

Each refusal now has its own box with its sentence fixed in index.html, holding its space from the first paint and only made visible, as #558 did for the balance messages. One sentence is true of all three bounds and quotes no number; the full reason is logged. A Chrome test raises the stub node's fee only at signing and checks nothing moved; a unit test refuses a gas limit over its bound into the same box.

Model: opus-5-5

tests/confirmTxFeeBound.test.js

@@ -11,20 +11,115 @@
 //
 // The check is driven here with a fake connected signer rather than a real
 // one: populateTransaction() returns the fees the node would have produced,
-// and sendTransaction() records whether the send actually happened. The real
-// DOM path around it — reading the fee error into the reserved errors box — is
-// covered by the Chrome e2e suite.
+// and sendTransaction() records whether the send actually happened.
+//
+// The last test drives the real confirmation screen instead, with only the
+// node and the DOM stubbed: a gas limit over its own bound at an ordinary fee
+// is refused in the same box as a fee over the bound
+// (https://git.eeqj.de/sneak/AutistMask/issues/583). That the box moves
+// nothing on the screen is checked by the Chrome e2e suite.
 
-globalThis.chrome = {
-    storage: { local: { get: async () => ({}), set: async () => {} } },
+// What the stub node quotes for that test: a gas limit of 150,000,000 at a
+// maximum fee of 2 gwei per gas, 0.3 ETH in all. And the signed transactions
+// it was handed.
+const mockNode = {
+    gasLimit: 150000000n,
+    maxFeePerGas: 2000000000n,
+    broadcast: [],
 };
 
+jest.mock("ethers", () => {
+    const actual = jest.requireActual("ethers");
+    class StubProvider {
+        constructor(url, network) {
+            this.network = network;
+        }
+        async getNetwork() {
+            return this.network;
+        }
+        async getBalance() {
+            return actual.parseEther("5");
+        }
+        async getCode() {
+            return "0x";
+        }
+        async getTransactionCount() {
+            return 1;
+        }
+        async getFeeData() {
+            return {
+                maxFeePerGas: mockNode.maxFeePerGas,
+                maxPriorityFeePerGas: 1000000000n,
+                gasPrice: 1000000000n,
+            };
+        }
+        async estimateGas() {
+            return mockNode.gasLimit;
+        }
+        async broadcastTransaction(signed) {
+            mockNode.broadcast.push(signed);
+            return { hash: "0x" + "ab".repeat(32) };
+        }
+    }
+    return { ...actual, JsonRpcProvider: StubProvider };
+});
+
+// The screens a send that was not refused goes on to.
+jest.mock("../src/popup/views/txStatus", () => ({
+    showWait: jest.fn(),
+    showError: jest.fn(),
+}));
+
+const { makeStorageStub } = require("./support/storageStub");
+global.chrome = { storage: makeStorageStub(), runtime: { sendMessage() {} } };
+
 global.fetch = jest.fn(() => {
     throw new Error("tests must not perform network requests");
 });
 
-const { populateVerifyAndSend } = require("../src/popup/views/confirmTx");
+// 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: () => [],
+        remove() {},
+    };
+}
+
+global.document = {
+    getElementById(id) {
+        if (!elements.has(id)) elements.set(id, makeEl(id));
+        return elements.get(id);
+    },
+};
+
+const { Wallet } = require("ethers");
+const { encryptWithPassword } = require("../src/shared/vault");
+const { state, loadState } = require("../src/shared/state");
+const txStatus = require("../src/popup/views/txStatus");
+const confirmTx = require("../src/popup/views/confirmTx");
+const { populateVerifyAndSend } = confirmTx;
 const {
+    MAX_GAS_LIMIT,
     MAX_FEE_PER_GAS,
     MAX_TOTAL_FEE,
 } = require("../src/shared/approvalVerify");
@@ -98,3 +193,69 @@ describe("populateVerifyAndSend enforces the combined fee bound", () => {
         expect(signer.sent[0].gasLimit).toBe(under.gasLimit);
     });
 });
+
+describe("Sign & Send on the confirmation screen", () => {
+    const PRIVATE_KEY = "0x" + "11".repeat(32);
+    const PASSWORD = "correct horse battery staple";
+    let encryptedSecret;
+
+    beforeAll(async () => {
+        await loadState();
+        encryptedSecret = await encryptWithPassword(PRIVATE_KEY, PASSWORD);
+    });
+
+    function el(id) {
+        return global.document.getElementById(id);
+    }
+
+    function shown(id) {
+        return el(id).style.visibility === "visible";
+    }
+
+    // show() reads the balance and estimates the fee without awaiting
+    // either; this lets both finish.
+    async function settle() {
+        for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
+    }
+
+    test("refuses a gas limit over its bound at an ordinary fee in the box for a refused gas limit or fee, and sends nothing", async () => {
+        expect(mockNode.gasLimit).toBeGreaterThan(MAX_GAS_LIMIT);
+        expect(mockNode.gasLimit * mockNode.maxFeePerGas).toBeLessThan(
+            MAX_TOTAL_FEE,
+        );
+
+        const from = new Wallet(PRIVATE_KEY).address;
+        state.wallets = [
+            {
+                type: "key",
+                name: "Wallet 1",
+                encryptedSecret,
+                addresses: [{ address: from }],
+            },
+        ];
+        state.selectedWallet = 0;
+        state.selectedAddress = 0;
+        confirmTx.init({});
+        confirmTx.show({
+            from,
+            to: RECIPIENT,
+            ensName: null,
+            amount: "1",
+            token: "ETH",
+            tokenSymbol: null,
+            tokenDecimals: null,
+            max: false,
+        });
+        await settle();
+        expect(el("btn-confirm-send").disabled).toBe(false);
+        expect(shown("confirm-fee-bound-error")).toBe(false);
+
+        el("confirm-tx-password").value = PASSWORD;
+        await el("btn-confirm-send").handlers.get("click")();
+
+        expect(shown("confirm-fee-bound-error")).toBe(true);
+        expect(mockNode.broadcast).toEqual([]);
+        expect(txStatus.showWait).not.toHaveBeenCalled();
+        expect(txStatus.showError).not.toHaveBeenCalled();
+    });
+});

tests/confirmTxNetworkSwitch.test.js

@@ -440,12 +440,8 @@ describe.each([
             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.",
-            );
+            expect(shown("confirm-network-changed-error")).toBe(true);
+            expect(shown("confirm-fee-bound-error")).toBe(false);
         });
     },
 );
@@ -497,10 +493,7 @@ describe.each([
             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.",
-            );
+            expect(shown("confirm-network-changed-error")).toBe(true);
         });
 
         test("is sent on that network, as described, once it is chosen in Settings again and Back pressed", async () => {
@@ -579,10 +572,7 @@ describe.each([
 
             expect(signTransaction).not.toHaveBeenCalled();
             expect(mockNode.broadcast).toEqual([]);
-            expect(errors()).toContain(
-                "The active network changed after this transaction was" +
-                    " reviewed, so it was not sent.",
-            );
+            expect(shown("confirm-network-changed-error")).toBe(true);
         });
 
         const tokenSend = {
@@ -629,10 +619,7 @@ describe.each([
 
             expect(signTransaction).not.toHaveBeenCalled();
             expect(mockNode.broadcast).toEqual([]);
-            expect(errors()).toContain(
-                "The active network changed after this transaction was" +
-                    " reviewed, so it was not sent.",
-            );
+            expect(shown("confirm-network-changed-error")).toBe(true);
         });
 
         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 () => {

tests/e2e/network.js

@@ -226,8 +226,9 @@ const RPC_RESULTS = {
 // The "latest" block, which ethers' getFeeData() reads baseFeePerGas from
 // to derive maxFeePerGas. Without it every fee is a legacy gasPrice, the
 // reserve and the estimate collapse to the same number, and the gate tests
-// stop being able to distinguish them.
-function latestBlock() {
+// stop being able to distinguish them. opts.baseFeeWei replaces its base fee,
+// read at request time like every other fixture switch.
+function latestBlock(opts) {
     return {
         hash: "0x" + "11".repeat(32),
         parentHash: "0x" + "22".repeat(32),
@@ -239,7 +240,7 @@ function latestBlock() {
         gasUsed: "0xf4240",
         miner: STUB_COUNTERPARTY,
         extraData: "0x",
-        baseFeePerGas: hex(BASE_FEE_WEI),
+        baseFeePerGas: opts.baseFeeWei || hex(BASE_FEE_WEI),
         transactions: [],
     };
 }
@@ -481,7 +482,7 @@ function rpcReply(req, opts, report) {
         });
     }
     if (req.method === "eth_getBlockByNumber") {
-        return Object.assign(envelope, { result: latestBlock() });
+        return Object.assign(envelope, { result: latestBlock(opts) });
     }
     // The end of the dApp transaction round trip: the raw signed transaction
     // the background hands to the node. It is recorded verbatim so a test can
@@ -625,6 +626,8 @@ function traceEnabled(raw) {
  * @param {string} [opts.ethBalanceWei]  hex wei answered to eth_getBalance;
  *   defaults to zero, which is what every test that predates the funded
  *   fixture expects.
+ * @param {string} [opts.baseFeeWei]  hex wei answered as the latest block's
+ *   baseFeePerGas; defaults to BASE_FEE_WEI. Read at request time.
  * @param {boolean} [opts.failGasEstimate]  answer eth_estimateGas with a
  *   node-side refusal.
  * @param {boolean} [opts.holdBalance]  hold every batch containing an

tests/e2e/run.js

@@ -2232,7 +2232,8 @@ function feeEth(wei) {
 
 // What the confirmation screen is showing right now, read out of the DOM in
 // one pass: whether sending is allowed, which reason it is giving, what the
-// fee block says, how tall the whole view is, and where Sign & Send sits in it.
+// fee block says, how tall the whole view is, and where the password field and
+// Sign & Send sit in it.
 async function confirmState(page) {
     return page.evaluate(() => {
         const el = (id) => document.getElementById(id);
@@ -2245,6 +2246,9 @@ async function confirmState(page) {
         };
         return {
             height: el("view-confirm-tx").getBoundingClientRect().height,
+            passwordTop:
+                el("confirm-tx-password").getBoundingClientRect().top -
+                el("view-confirm-tx").getBoundingClientRect().top,
             sendTop:
                 el("btn-confirm-send").getBoundingClientRect().top -
                 el("view-confirm-tx").getBoundingClientRect().top,
@@ -2262,6 +2266,12 @@ async function confirmState(page) {
             amountFeeError: shown("confirm-amount-fee-error"),
             gasError: shown("confirm-gas-error"),
             feeUnknownError: shown("confirm-fee-unknown-error"),
+            // Its sentence is wrapped across lines in index.html.
+            feeBoundError: shown("confirm-fee-bound-error")
+                ? el("confirm-fee-bound-error")
+                      .textContent.replace(/\s+/g, " ")
+                      .trim()
+                : "",
             sendDisabled: el("btn-confirm-send").disabled,
         };
     });
@@ -3057,6 +3067,91 @@ test("a token that lies about decimals() at signing time broadcasts nothing (#30
     }
 });
 
+// ------------------------- a send Sign & Send refuses moves nothing (#583)
+//
+// README, No Layout Shift. Sign & Send refuses a send on the confirmation
+// screen when the active network changed since review and when its gas limit
+// or fee is over its bound, and each refusal's sentence sits in a box that held
+// its space from the first paint. This drives the total fee refusal, the longer
+// of the two sentences.
+
+// The base fee the stub node quotes once Sign & Send is pressed. ethers asks
+// twice it plus the 1 gwei priority fee per gas, 50,001 gwei, so the 21,000-gas
+// send would allow a fee of about 1.05 ETH, over the 1 ETH this wallet signs
+// for (MAX_TOTAL_FEE in src/shared/approvalVerify.js).
+const OVER_BOUND_BASE_FEE_WEI = 25000n * 10n ** 9n;
+
+test("a send refused for its fee leaves the password field and Sign & Send where they were (#583)", async (env) => {
+    env.errors.expect("confirmTx logging the refused send", /send refused/);
+    try {
+        await goToConfirm(env.page, {
+            token: "ETH",
+            balance: FUNDED_ETH_TEXT + " ETH",
+            amount: COMFORTABLE_AMOUNT,
+        });
+        await waitForEstimate(env.page);
+        const before = await confirmState(env.page);
+        assert(
+            !before.sendDisabled && before.feeBoundError === "",
+            "the screen is not ready to send: " + JSON.stringify(before),
+        );
+
+        // Only now, with the screen drawn and its fee estimated, does the
+        // node quote a fee over the bound.
+        env.routeOpts.baseFeeWei = toHexWei(OVER_BOUND_BASE_FEE_WEI);
+        const sent = env.routeOpts.broadcastTransactions.length;
+        await fillPasswordAndSend(env.page);
+        await env.page.waitForFunction(
+            () =>
+                getComputedStyle(
+                    document.getElementById("confirm-fee-bound-error"),
+                ).visibility === "visible",
+            null,
+            { timeout: 60000 },
+        );
+        const after = await confirmState(env.page);
+
+        // Printed pass or fail, as the dust threshold test does.
+        console.log(
+            "# refused send: password field top " +
+                before.passwordTop +
+                " -> " +
+                after.passwordTop +
+                "px, Sign & Send top " +
+                before.sendTop +
+                " -> " +
+                after.sendTop +
+                "px",
+        );
+        assert(
+            env.routeOpts.broadcastTransactions.length === sent,
+            "a send over the fee bound reached the RPC",
+        );
+        assert(
+            after.feeBoundError ===
+                "The gas limit or fee the RPC endpoint quoted for this" +
+                    " transaction is more than this wallet will sign for, so" +
+                    " it was not sent. Please go back and try again.",
+            "wrong fee refusal: " + JSON.stringify(after.feeBoundError),
+        );
+        assert(
+            after.passwordTop === before.passwordTop,
+            "the password field moved " +
+                (after.passwordTop - before.passwordTop) +
+                "px when the send was refused",
+        );
+        assert(
+            after.sendTop === before.sendTop,
+            "Sign & Send moved " +
+                (after.sendTop - before.sendTop) +
+                "px when the send was refused",
+        );
+    } finally {
+        env.routeOpts.baseFeeWei = undefined;
+        await backToAddress(env.page);
+    }
+});
+
 // ------------------------------ the wait for a receipt ending in error (#315)
 //
 // README.md (WaitTx) documents two ways the wait ends on the error screen: a

tests/trackedTokensPerNetwork.test.js

@@ -313,9 +313,9 @@ describe("Back after a switch from the Settings gear", () => {
         boot.node("confirm-tx-password").value = "any password";
         await boot.click("btn-confirm-send");
         expect(boot.visibleViews()).toEqual(["confirm-tx"]);
-        expect(boot.node("confirm-errors").innerHTML).toContain(
-            "The active network changed after this transaction was reviewed",
-        );
+        expect(
+            boot.node("confirm-network-changed-error").style.visibility,
+        ).toBe("visible");
         // Refused before the password is even tried.
         expect(boot.text("confirm-tx-password-error")).toBe("");
     });