test: drive ConfirmTx in the e2e suite, gate assertion included (closes #238)

ConfirmTx -- the screen that decides what gets signed -- had no automated
coverage of its own behaviour. The arithmetic underneath was well tested; the
wiring was not, so a mutant making the spend gate read the displayed fee
estimate instead of the reserve would have reintroduced the #154 overspend with
the suite still green.

Nine end-to-end tests now drive it for both the native and ERC-20 paths,
covering the pending, funded, over-balance and estimate-failed states, and
asserting that the gate reads the reserve rather than the estimate. Swapping the
two makes the suite fail. The view height is asserted constant across every
state transition rather than merely printed.

Reaching the screen needs a funded balance and a gas estimate, so the route
interception gains fixtures for both. Testing the estimate-failed state means
provoking the console error the code is supposed to emit, which the harness
otherwise fails a run on; an expectation mechanism consumes exactly one matching
record, is scoped to the declaring test, and fails that test if nothing matched,
so it cannot mask an unrelated error.

tests/e2e/run.js

@@ -9,6 +9,7 @@
 
 "use strict";
 
+const { formatEther } = require("ethers");
 const {
     PASSWORD,
     createWallet,
@@ -18,7 +19,13 @@ const {
     pageCompilesWasm,
     visible,
 } = require("./harness");
-const { STUB_TOKEN, STUB_TX_HASH } = require("./network");
+const {
+    FEE_ESTIMATE_WEI,
+    FEE_RESERVE_WEI,
+    STUB_COUNTERPARTY,
+    STUB_TOKEN,
+    STUB_TX_HASH,
+} = require("./network");
 const { DUST_THRESHOLD_MESSAGE } = require("../../src/popup/dustThreshold");
 
 const TEST_TIMEOUT_MS = 120000;
@@ -623,6 +630,573 @@ test("a rejected dust threshold shifts no layout (#233)", async (env) => {
     }
 });
 
+// --------------------------------------------- confirmation screen (#238)
+//
+// The screen that decides what gets signed. The arithmetic underneath it
+// lives in src/shared/txValidation.js and is unit tested there; what these
+// tests cover is the wiring — which number reaches the gate, when the gate
+// re-runs, what the fee block renders, and whether Send is enabled.
+//
+// The load-bearing one is "gates on the fee RESERVE": the confirmation
+// screen quotes the ESTIMATE and gates on the RESERVE, and issue #154 was
+// the gate reading the quoted number. Every other assertion here would
+// survive that mutation, so the funded and gap sends are deliberately sized
+// on opposite sides of the reserve while sitting on the same side of the
+// estimate.
+
+// The balance the funded fixture serves, and the amounts sent against it.
+const FUNDED_ETH_WEI = 10n ** 18n;
+const FUNDED_ETH_TEXT = "1.0";
+const COMFORTABLE_AMOUNT = "0.1";
+const OVER_BALANCE_AMOUNT = "2.0";
+
+// A send the balance covers to the wei once the ESTIMATE is added, and does
+// not cover once the RESERVE is. Sending this is allowed by a gate reading
+// the estimate and refused by a gate reading the reserve, which is the whole
+// discrimination these tests exist to make.
+const GAP_AMOUNT = formatEther(FUNDED_ETH_WEI - FEE_ESTIMATE_WEI);
+
+// The ERC-20 side. The ETH balance is set to exactly the estimate for the
+// fee test: it covers the expected cost to the wei and falls short of the
+// reserve, so the same swap flips this assertion too — through a different
+// balance and a different message than the ETH path uses.
+const TOKEN_BALANCE_TEXT = "1.5";
+const TOKEN_AMOUNT = "0.25";
+const OVER_TOKEN_AMOUNT = "9.0";
+const FEE_ONLY_ETH_WEI = FEE_ESTIMATE_WEI;
+
+function toHexWei(wei) {
+    return "0x" + wei.toString(16);
+}
+
+// A fee in wei as the confirmation screen writes it. Deliberately a second
+// implementation of formatFeeEth() from src/popup/views/confirmTx.js rather
+// than an import of it: that module pulls in the whole popup and cannot be
+// required outside a browser, and asserting against an independent rendering
+// is stronger than asserting a function equals itself.
+function feeEth(wei) {
+    const parts = formatEther(wei).split(".");
+    const dec =
+        parts.length > 1 ? parts[1].slice(0, 6).replace(/0+$/, "") || "0" : "0";
+    return parts[0] + "." + dec + " ETH";
+}
+
+// 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, and how tall the whole view is.
+async function confirmState(page) {
+    return page.evaluate(() => {
+        const el = (id) => document.getElementById(id);
+        // Both mechanisms matter. The two fee messages are dropped with
+        // display:none for the transaction type they cannot apply to, and
+        // shown or hidden with visibility for the one they can.
+        const shown = (id) => {
+            const cs = getComputedStyle(el(id));
+            return cs.display !== "none" && cs.visibility === "visible";
+        };
+        return {
+            height: el("view-confirm-tx").getBoundingClientRect().height,
+            type: el("confirm-type").textContent.trim(),
+            balance: el("confirm-balance").textContent.trim(),
+            fee: el("confirm-fee-amount").textContent.trim(),
+            reserve: el("confirm-fee-reserve").textContent.trim(),
+            reserveShown: shown("confirm-fee-reserve"),
+            errors: shown("confirm-errors")
+                ? el("confirm-errors").textContent.trim()
+                : "",
+            amountFeeError: shown("confirm-amount-fee-error"),
+            gasError: shown("confirm-gas-error"),
+            feeUnknownError: shown("confirm-fee-unknown-error"),
+            sendDisabled: el("btn-confirm-send").disabled,
+        };
+    });
+}
+
+async function waitForEstimate(page) {
+    await page.waitForFunction(
+        () =>
+            document.getElementById("confirm-fee-amount").textContent.trim() !==
+            "Estimating...",
+        null,
+        { timeout: 60000 },
+    );
+}
+
+async function backToAddress(page) {
+    if (await page.isVisible("#view-confirm-tx")) {
+        await page.click("#btn-confirm-back");
+        await visible(page, "#view-send");
+    }
+    if (await page.isVisible("#view-send")) {
+        await page.click("#btn-send-back");
+    }
+    await openAddressDetail(page);
+}
+
+// Drive the popup to the confirmation screen for one send.
+//
+// It waits for the send screen to be showing `balance` before filling
+// anything in. That figure is the exact number the spend gate compares
+// against, so waiting for it — rather than for a refresh to have probably
+// landed — is what keeps every assertion below deterministic after a
+// fixture change.
+async function goToConfirm(page, { token, balance, amount }) {
+    await backToAddress(page);
+    await page.click("#btn-send");
+    await visible(page, "#view-send");
+    await page.selectOption("#send-token", token);
+    await page.waitForFunction(
+        (want) =>
+            document.getElementById("send-balance").textContent.trim() === want,
+        "Current balance: " + balance,
+        { timeout: 60000 },
+    );
+    await page.fill("#send-to", STUB_COUNTERPARTY);
+    await page.fill("#send-amount", amount);
+    await page.click("#btn-send-review");
+    await visible(page, "#view-confirm-tx");
+}
+
+// A balance as the main view renders it: balanceLinesForAddress() writes
+// every quantity with four decimal places.
+function quantity(wei) {
+    return parseFloat(formatEther(wei)).toFixed(4);
+}
+
+// Wait on the main view until a changed balance fixture has been picked up.
+//
+// Deliberately not a reload: the popup re-refreshes on a 10-second timer by
+// itself, and reloading aborts whatever fetch the home screen has open at
+// that instant, which the extension reports through log.errorf and the
+// harness — correctly — fails the run on.
+//
+// It also deliberately settles on MAIN rather than on the address screen.
+// The address screen builds the send screen's token dropdown once, from the
+// balances it holds at that moment, and nothing rebuilds it when a later
+// refresh arrives, so entering it early leaves a dropdown with no token in
+// it and the ERC-20 path unreachable.
+async function settleOnMain(env, { ethWei, expectToken }) {
+    await backToAddress(env.page);
+    await env.page.click("#btn-address-back");
+    await visible(env.page, "#view-main");
+    await env.page.waitForFunction(
+        (want) =>
+            document.getElementById("wallet-list").textContent.includes(want),
+        quantity(ethWei),
+        { timeout: 60000 },
+    );
+    if (expectToken) {
+        await visible(
+            env.page,
+            '#wallet-list [data-token="' + STUB_TOKEN.address + '"]',
+            60000,
+        );
+    }
+}
+
+test("ConfirmTx blocks sending while the fee estimate is pending (#238)", async (env) => {
+    env.routeOpts.ethBalanceWei = toHexWei(FUNDED_ETH_WEI);
+    env.routeOpts.seedTokenBalance = true;
+    await settleOnMain(env, {
+        ethWei: FUNDED_ETH_WEI,
+        expectToken: true,
+    });
+
+    env.routeOpts.holdGasEstimate = true;
+    await goToConfirm(env.page, {
+        token: "ETH",
+        balance: FUNDED_ETH_TEXT + " ETH",
+        amount: COMFORTABLE_AMOUNT,
+    });
+
+    const st = await confirmState(env.page);
+    env.ethPendingHeight = st.height;
+    console.log("# confirm-tx ETH view height: " + st.height + "px");
+    assert(
+        st.type === "Native ETH transfer",
+        "unexpected transaction type: " + JSON.stringify(st.type),
+    );
+    assert(
+        st.fee === "Estimating...",
+        "the fee line is not showing the pending placeholder: " +
+            JSON.stringify(st.fee),
+    );
+    assert(!st.reserveShown, "the reserve line is shown before any estimate");
+    assert(
+        st.sendDisabled,
+        "Send is enabled while the fee estimate is still in flight",
+    );
+    assert(
+        !st.feeUnknownError,
+        "the estimate-failed message is shown for an estimate that is merely pending",
+    );
+    assert(
+        !st.amountFeeError && !st.gasError && st.errors === "",
+        "a balance message is shown before the fee is known",
+    );
+});
+
+test("ConfirmTx enables Send once the estimate lands, quoting both numbers (#238)", async (env) => {
+    env.routeOpts.holdGasEstimate = false;
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    assert(
+        st.balance === FUNDED_ETH_TEXT + " ETH",
+        "the confirmation screen shows the wrong balance: " +
+            JSON.stringify(st.balance),
+    );
+    assert(
+        st.fee === "~" + feeEth(FEE_ESTIMATE_WEI),
+        "the fee line does not quote the estimate: " + JSON.stringify(st.fee),
+    );
+    assert(st.reserveShown, "the reserve line is not shown once the fee lands");
+    assert(
+        st.reserve === "up to " + feeEth(FEE_RESERVE_WEI) + " reserved",
+        "the reserve line does not quote the reserve: " +
+            JSON.stringify(st.reserve),
+    );
+    assert(
+        !st.sendDisabled,
+        "Send is disabled for a comfortably funded transfer",
+    );
+    assert(
+        st.errors === "" &&
+            !st.amountFeeError &&
+            !st.gasError &&
+            !st.feeUnknownError,
+        "a balance message is shown for a comfortably funded transfer",
+    );
+    assert(
+        st.height === env.ethPendingHeight,
+        "the view changed height when the estimate landed: " +
+            env.ethPendingHeight +
+            "px -> " +
+            st.height +
+            "px",
+    );
+});
+
+// The one that closes the hole. Everything else here survives a gate that
+// reads the displayed estimate instead of the reserve; this does not.
+test("ConfirmTx gates on the fee RESERVE, not the displayed estimate (#238)", async (env) => {
+    await goToConfirm(env.page, {
+        token: "ETH",
+        balance: FUNDED_ETH_TEXT + " ETH",
+        amount: GAP_AMOUNT,
+    });
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    // Printed on every run, pass or fail: the two fee numbers and the gate's
+    // decision side by side is the measurement this test is really making.
+    console.log(
+        "# gate probe: balance=" +
+            FUNDED_ETH_TEXT +
+            " ETH amount=" +
+            GAP_AMOUNT +
+            " estimate=" +
+            feeEth(FEE_ESTIMATE_WEI) +
+            " reserve=" +
+            feeEth(FEE_RESERVE_WEI) +
+            " sendDisabled=" +
+            st.sendDisabled +
+            " amountFeeError=" +
+            st.amountFeeError,
+    );
+    assert(
+        st.fee === "~" + feeEth(FEE_ESTIMATE_WEI),
+        "the screen is not quoting the estimate, so this send is not in the gap: " +
+            JSON.stringify(st.fee),
+    );
+    assert(
+        st.sendDisabled,
+        "Send is ENABLED for a transfer the fee RESERVE does not cover — the " +
+            "spend gate is reading the displayed estimate, which is issue #154",
+    );
+    assert(
+        st.amountFeeError,
+        "the amount-plus-fee message is not shown for a send the reserve does not cover",
+    );
+    assert(
+        st.height === env.ethPendingHeight,
+        "the over-budget state is a different height than the pending state: " +
+            env.ethPendingHeight +
+            "px -> " +
+            st.height +
+            "px",
+    );
+});
+
+test("ConfirmTx refuses a send that exceeds the balance outright (#238)", async (env) => {
+    await goToConfirm(env.page, {
+        token: "ETH",
+        balance: FUNDED_ETH_TEXT + " ETH",
+        amount: OVER_BALANCE_AMOUNT,
+    });
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    const want =
+        "Insufficient balance. You have " +
+        FUNDED_ETH_TEXT +
+        " ETH but are trying to send " +
+        OVER_BALANCE_AMOUNT +
+        " ETH.";
+    assert(
+        st.errors === want,
+        "wrong over-balance message: " + JSON.stringify(st.errors),
+    );
+    assert(st.sendDisabled, "Send is enabled for a send that exceeds balance");
+    assert(
+        !st.amountFeeError,
+        "the amount-plus-fee message is shown for an amount that alone exceeds the balance",
+    );
+});
+
+test("ConfirmTx refuses to send when the fee estimate fails, with its own message (#238)", async (env) => {
+    // The refusal is logged by confirmTx via log.errorf, i.e. console.error,
+    // which fails a test on its own. Declaring it here consumes exactly that
+    // one record — and fails this test if it never arrives.
+    env.errors.expect(
+        "confirmTx logging the failed gas estimate",
+        /gas estimation failed/,
+    );
+    env.routeOpts.failGasEstimate = true;
+    env.routeOpts.holdGasEstimate = true;
+    await goToConfirm(env.page, {
+        token: "ETH",
+        balance: FUNDED_ETH_TEXT + " ETH",
+        amount: COMFORTABLE_AMOUNT,
+    });
+
+    const pending = await confirmState(env.page);
+    assert(
+        pending.fee === "Estimating..." && pending.sendDisabled,
+        "the screen is not in the pending state before the estimate fails",
+    );
+
+    env.routeOpts.holdGasEstimate = false;
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    env.routeOpts.failGasEstimate = false;
+    assert(
+        st.fee === "Unable to estimate",
+        "the fee line does not report the failure: " + JSON.stringify(st.fee),
+    );
+    assert(
+        !st.reserveShown,
+        "the reserve line is shown after a failed estimate",
+    );
+    assert(
+        st.feeUnknownError,
+        "the estimate-failed message is not shown after a failed estimate",
+    );
+    assert(
+        !st.amountFeeError && !st.gasError && st.errors === "",
+        "a balance message is shown for an estimate that simply failed",
+    );
+    assert(
+        st.sendDisabled,
+        "Send is enabled with no usable fee estimate — an unknown fee is being treated as zero",
+    );
+    assert(
+        st.height === pending.height,
+        "the view changed height when the estimate failed: " +
+            pending.height +
+            "px -> " +
+            st.height +
+            "px",
+    );
+});
+
+test("ConfirmTx drives the ERC-20 path from pending to funded (#238)", async (env) => {
+    env.routeOpts.holdGasEstimate = true;
+    await goToConfirm(env.page, {
+        token: STUB_TOKEN.address,
+        balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
+        amount: TOKEN_AMOUNT,
+    });
+
+    const pending = await confirmState(env.page);
+    env.erc20PendingHeight = pending.height;
+    console.log("# confirm-tx ERC-20 view height: " + pending.height + "px");
+    assert(
+        pending.type === "ERC-20 token transfer (" + STUB_TOKEN.symbol + ")",
+        "unexpected transaction type: " + JSON.stringify(pending.type),
+    );
+    assert(
+        pending.balance === TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
+        "the ERC-20 screen shows the wrong balance: " +
+            JSON.stringify(pending.balance),
+    );
+    assert(
+        pending.fee === "Estimating..." && pending.sendDisabled,
+        "the ERC-20 screen does not block sending while its estimate is pending",
+    );
+
+    env.routeOpts.holdGasEstimate = false;
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    assert(
+        st.fee === "~" + feeEth(FEE_ESTIMATE_WEI),
+        "the ERC-20 fee line does not quote the estimate: " +
+            JSON.stringify(st.fee),
+    );
+    assert(
+        st.reserveShown &&
+            st.reserve === "up to " + feeEth(FEE_RESERVE_WEI) + " reserved",
+        "the ERC-20 fee block does not quote the reserve: " +
+            JSON.stringify(st.reserve),
+    );
+    assert(!st.sendDisabled, "Send is disabled for a funded ERC-20 transfer");
+    assert(
+        st.height === pending.height,
+        "the ERC-20 view changed height when the estimate landed: " +
+            pending.height +
+            "px -> " +
+            st.height +
+            "px",
+    );
+});
+
+test("ConfirmTx refuses an ERC-20 send that exceeds the token balance (#238)", async (env) => {
+    await goToConfirm(env.page, {
+        token: STUB_TOKEN.address,
+        balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
+        amount: OVER_TOKEN_AMOUNT,
+    });
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    const want =
+        "Insufficient " +
+        STUB_TOKEN.symbol +
+        " balance. You have " +
+        TOKEN_BALANCE_TEXT +
+        " " +
+        STUB_TOKEN.symbol +
+        " but are trying to send " +
+        OVER_TOKEN_AMOUNT +
+        " " +
+        STUB_TOKEN.symbol +
+        ".";
+    assert(
+        st.errors === want,
+        "wrong over-token-balance message: " + JSON.stringify(st.errors),
+    );
+    assert(
+        st.sendDisabled,
+        "Send is enabled for an ERC-20 transfer that exceeds the token balance",
+    );
+    assert(
+        !st.gasError,
+        "the ERC-20 gas message is shown for an ETH balance that covers the fee",
+    );
+});
+
+// The same swap, through the other balance and the other message: here the
+// token balance is ample and it is the ETH balance that must cover the fee.
+// It is set to exactly the estimate, so an estimate-reading gate lets this
+// through and the reserve-reading gate refuses it.
+test("ConfirmTx gates the ERC-20 fee on the RESERVE, with the ERC-20 message (#238)", async (env) => {
+    env.routeOpts.ethBalanceWei = toHexWei(FEE_ONLY_ETH_WEI);
+    await settleOnMain(env, {
+        ethWei: FEE_ONLY_ETH_WEI,
+        expectToken: true,
+    });
+    await goToConfirm(env.page, {
+        token: STUB_TOKEN.address,
+        balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
+        amount: TOKEN_AMOUNT,
+    });
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    console.log(
+        "# erc-20 gate probe: ethBalance=" +
+            formatEther(FEE_ONLY_ETH_WEI) +
+            " estimate=" +
+            feeEth(FEE_ESTIMATE_WEI) +
+            " reserve=" +
+            feeEth(FEE_RESERVE_WEI) +
+            " sendDisabled=" +
+            st.sendDisabled +
+            " gasError=" +
+            st.gasError,
+    );
+    assert(
+        st.sendDisabled,
+        "Send is ENABLED for an ERC-20 transfer whose fee RESERVE exceeds the " +
+            "ETH balance — the spend gate is reading the displayed estimate (#154)",
+    );
+    assert(
+        st.gasError,
+        "the ERC-20 network-fee message is not shown when the ETH balance cannot cover the reserve",
+    );
+    assert(
+        !st.amountFeeError,
+        "the native-ETH over-budget message is shown on an ERC-20 transfer",
+    );
+    assert(
+        st.errors === "",
+        "a token-balance message is shown for a transfer the token balance covers: " +
+            JSON.stringify(st.errors),
+    );
+    assert(
+        st.height === env.erc20PendingHeight,
+        "the ERC-20 fee-error state is a different height than its pending state: " +
+            env.erc20PendingHeight +
+            "px -> " +
+            st.height +
+            "px",
+    );
+});
+
+test("ConfirmTx reports a failed ERC-20 estimate as unknown, not as a fee problem (#238)", async (env) => {
+    env.errors.expect(
+        "confirmTx logging the failed ERC-20 gas estimate",
+        /gas estimation failed/,
+    );
+    env.routeOpts.failGasEstimate = true;
+    await goToConfirm(env.page, {
+        token: STUB_TOKEN.address,
+        balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
+        amount: TOKEN_AMOUNT,
+    });
+    await waitForEstimate(env.page);
+
+    const st = await confirmState(env.page);
+    env.routeOpts.failGasEstimate = false;
+    assert(
+        st.fee === "Unable to estimate",
+        "the ERC-20 fee line does not report the failure: " +
+            JSON.stringify(st.fee),
+    );
+    assert(
+        st.feeUnknownError,
+        "the estimate-failed message is not shown on the ERC-20 path",
+    );
+    assert(
+        !st.gasError,
+        "the ERC-20 network-fee message is shown for a fee that is unknown rather than unaffordable",
+    );
+    assert(
+        st.sendDisabled,
+        "Send is enabled on the ERC-20 path with no usable fee estimate",
+    );
+    assert(
+        st.height === env.erc20PendingHeight,
+        "the ERC-20 estimate-failed state is a different height than its pending state: " +
+            env.erc20PendingHeight +
+            "px -> " +
+            st.height +
+            "px",
+    );
+});
+
 // ---------------------------------------------------------------- runner
 
 async function main() {
@@ -638,7 +1212,15 @@ async function main() {
         return;
     }
 
-    const routeOpts = { seedTokenTransfer: false };
+    // Every fixture switch the suite can flip, declared in one place so the
+    // starting state of a run is readable without hunting through tests.
+    const routeOpts = {
+        seedTokenTransfer: false,
+        seedTokenBalance: false,
+        ethBalanceWei: null,
+        failGasEstimate: false,
+        holdGasEstimate: false,
+    };
 
     let session;
     try {
@@ -660,9 +1242,16 @@ async function main() {
         popupUrl: session.popupUrl,
         routeOpts,
         page: null,
+        // The error collector, so a test that drives a failure path on
+        // purpose can declare the console.error it is about to provoke.
+        errors: session.errors,
         // The recovery phrase of the wallet created in test 2, so later
         // tests can assert on the real secret rather than its shape.
         phrase: null,
+        // Confirmation-screen heights, measured in the pending state and
+        // compared against every later state of the same screen.
+        ethPendingHeight: null,
+        erc20PendingHeight: null,
     };
 
     // Attribution of collected errors is total. session.errors has no
@@ -709,6 +1298,16 @@ async function main() {
             failure = "uncaught browser errors during this test";
         }
 
+        // A test that declared an error it meant to provoke and did not
+        // provoke it asserted nothing. Failing here is what keeps expect()
+        // from being usable as a mute.
+        const unmatched = session.errors.unmatchedExpectations();
+        if (!failure && unmatched.length > 0) {
+            failure =
+                "expected browser error(s) that never arrived: " +
+                unmatched.join("; ");
+        }
+
         if (failure) {
             failed += 1;
             console.log("not ok " + n + " - " + t.name);