fix: the confirmation screen's balance read and fee estimate stay on its network

The fee estimate starts once the balance is read, and both it and the balance
line took the active network at that moment, so a network switched in Settings
while the read was in flight put a mainnet balance under a SepoliaETH label and
estimated its fee on Sepolia.

show() now builds the node of the network it is drawn for once and passes it
to the balance read, the fee estimate and the recipient check, and every native
symbol on the screen comes from the chain id it is drawn for.

Model: opus-5-5

README.md

@@ -1742,18 +1742,16 @@ view would leave a wallet one click from deletion.
       plus four reserved warning boxes made visible by the async checks —
       recipient with no transaction history, recipient is a contract, burn
       address, and an Etherscan phishing/scam label
-    - Errors (an invalid amount), plus five reserved error boxes, so the balance
+    - Errors (insufficient balance, or a balance that could not be read, which
-      and the fee arriving later move nothing — the amount exceeds the balance
+      asks the user to go back and try again), plus three reserved error boxes —
-      (for a token whose scale is unknown it says the token's balance is
+      the amount plus the fee exceeds the balance (ETH transfers), not enough
-      unknown), the balance could not be read (it asks the user to go back and
+      ETH to pay the fee for the transfer (ERC-20 transfers), and the fee could
-      try again), the amount plus the fee exceeds the balance (ETH transfers),
+      not be estimated. The first two are mutually exclusive per transfer type,
-      not enough ETH to pay the fee for the transfer (ERC-20 transfers), and the
+      so only the applicable one holds space. The last names its cause: for a
-      fee could not be estimated. The third and fourth are mutually exclusive
+      token whose scale is unknown the fee can never be estimated, and it says
-      per transfer type, so only the applicable one holds space. The last names
+      the wallet does not know how many decimal places the token uses and that
-      its cause: for a token whose scale is unknown the fee can never be
+      the transaction cannot be sent; for any other failure it asks the user to
-      estimated, and it says the wallet does not know how many decimal places
+      go back and try again
-      the token uses and that the transaction cannot be sent; for any other
-      failure it asks the user to go back and try again
     - Password: an inline field on this screen, not a modal, with its own error
       line
     - "Sign & Send" button (disabled if errors, while the balance is being read,
@@ -2188,17 +2186,14 @@ view would leave a wallet one click from deletion.
   code `4100` and opens no prompt. Two such origins are `null`, which the
   browser reports for a page with an opaque origin, such as one its server
   sandboxes with a Content-Security-Policy, and `file://`, which Chrome reports
-  for a local file. Nor is such a page told the active address by
+  for a local file. The background script prefers the toolbar popup
-  `accountsChanged`, even at a connected site's URL: the background names the
+  (`action.openPopup()`) and falls back to a separate popup window
-  origin of the tab's URL on the event, and the content script passes the
+  (`src/background/index.js`, `requestApproval()`). Only one exists per site at
-  address on only to a page whose own origin is that one. The background script
+  a time: a further connection request from a site whose prompt is still
-  prefers the toolbar popup (`action.openPopup()`) and falls back to a separate
+  unanswered is refused with EIP-1193 code `-32002` and opens no new prompt. If
-  popup window (`src/background/index.js`, `requestApproval()`). Only one exists
+  that prompt was in a toolbar popup that closed before it connected, and the
-  per site at a time: a further connection request from a site whose prompt is
+  toolbar popup has since been set to open something else, the refused request
-  still unanswered is refused with EIP-1193 code `-32002` and opens no new
+  shows that prompt again.
-  prompt. If that prompt was in a toolbar popup that closed before it connected,
-  and the toolbar popup has since been set to open something else, the refused
-  request shows that prompt again.
 - **Elements**:
     - "Connection Request" heading
     - Phishing warning banner (shown when the hostname is on the phishing

src/background/index.js

@@ -1308,12 +1308,6 @@ async function broadcastAccountsChanged() {
         return;
     }
     for (const tab of tabs) {
-        // The tab's URL is all the background knows of the page in it, and a
-        // page at a site's URL may have another origin: a page its server
-        // sandboxes with a Content-Security-Policy has the origin "null". So
-        // the event names the origin it was judged by, and the content script
-        // passes the address on only to a page whose own origin is that one
-        // (src/content/index.js).
         const origin = tab.url ? new URL(tab.url).origin : "";
         const hasPermission =
             activeAddress &&
@@ -1325,7 +1319,6 @@ async function broadcastAccountsChanged() {
             type: "AUTISTMASK_EVENT",
             eventName: "accountsChanged",
             data: hasPermission ? [activeAddress] : [],
-            origin,
         }).catch(() => {});
     }
 }

src/content/index.js

@@ -49,23 +49,14 @@ window.addEventListener("message", (event) => {
 
 // Listen for events pushed from the background (e.g. accountsChanged)
 runtimeApi().onMessage.addListener((msg) => {
-    if (msg.type !== "AUTISTMASK_EVENT") return;
+    if (msg.type === "AUTISTMASK_EVENT") {
-    // The background chooses the addresses in accountsChanged for the site
+        window.postMessage(
-    // at the origin the event names, which is the origin of the tab's URL.
+            {
-    // This page hears them only when its own origin is that one. A page whose
+                type: "AUTISTMASK_EVENT",
-    // origin has no host is not a site: one its server sandboxes with a
+                eventName: msg.eventName,
-    // Content-Security-Policy has the origin "null" whatever its URL, and is
+                data: msg.data,
-    // told no address, as its requests are refused.
+            },
-    let data = msg.data;
+            "*",
-    if (msg.eventName === "accountsChanged" && msg.origin !== window.origin) {
+        );
-        data = [];
     }
-    window.postMessage(
-        {
-            type: "AUTISTMASK_EVENT",
-            eventName: msg.eventName,
-            data,
-        },
-        "*",
-    );
 });

src/popup/index.html

@@ -669,20 +669,6 @@
                     id="confirm-errors"
                     class="mb-2 border border-border border-dashed p-2 invisible min-h-[1.25rem]"
                 ></div>
-                <!-- Its sentence names the token, or why the token's balance
-                is unknown, so show() in confirmTx.js sets it. -->
-                <div
-                    id="confirm-balance-error"
-                    class="mb-2 border border-border border-dashed p-2 text-xs invisible"
-                ></div>
-                <div
-                    id="confirm-balance-read-error"
-                    class="mb-2 border border-border border-dashed p-2 text-xs invisible"
-                >
-                    Your balance could not be read from the network, so this
-                    transaction cannot be checked against it. Please go back and
-                    try again.
-                </div>
                 <div
                     id="confirm-amount-fee-error"
                     class="mb-2 border border-border border-dashed p-2 text-xs invisible"

src/popup/views/confirmTx.js

@@ -263,20 +263,6 @@ function show(txInfo) {
               " cannot be checked against your balance. Please go back and" +
               " try again.";
 
-    // So does the message for an amount the balance does not cover. It is
-    // set here, before the first paint, so the balance read returning only
-    // makes it visible and moves nothing, which is why it quotes no balance.
-    // Without the token's scale readBalances() reads no token balance.
-    $("confirm-balance-error").textContent =
-        isErc20 && toDecimals(txInfo.tokenDecimals) === null
-            ? "This token's balance is unknown, because nothing this wallet" +
-              " can consult reports how many decimal places it uses, so the" +
-              " amount you are trying to send cannot be checked against it."
-            : "Your " +
-              symbol +
-              " balance does not cover this amount. Please go back and send" +
-              " a smaller amount.";
-
     renderValidation(txInfo);
 
     // Reset password field and error
@@ -346,11 +332,13 @@ async function readBalances(txInfo, provider) {
 // re-running it never moves anything.
 function renderValidation(txInfo) {
     const isErc20 = txInfo.token !== "ETH";
+    const symbol = isErc20
+        ? displaySymbol(txInfo.tokenSymbol || "?")
+        : nativeCurrencyByChainId(txInfo.chainId);
 
     // Null while the balances are being read and after a read that failed.
     // validateTransfer() takes a missing balance as nothing to spend from, so
-    // sending stays blocked either way, and the only message shown for it is
+    // sending stays blocked either way, and no message below quotes one.
-    // the one saying the read failed.
     const ethBalance = balances ? balances.eth : null;
     const tokenBalance = balances ? balances.token : null;
     const balanceRead = ethBalance !== null;
@@ -365,14 +353,53 @@ function renderValidation(txInfo) {
         feeWei,
     });
 
-    // An invalid amount is known before the first paint, so its message is
+    // Messages carrying the user's own numbers are built here; the fixed
-    // written here. Every message a later result brings has a reserved
+    // sentences live in the reserved elements in index.html, except the
-    // element, its sentence fixed in index.html or set by show(), and is only
+    // gas and fee-unknown ones, which show() sets.
-    // made visible below, so nothing moves when the result arrives.
     const messages = [];
     if (codes.includes(CODES.AMOUNT_INVALID)) {
         messages.push("Please enter a valid amount to send.");
     }
+    if (balances !== null && !balanceRead) {
+        messages.push(
+            "Your balance could not be read from the network, so this" +
+                " transaction cannot be checked against it. Please go back" +
+                " and try again.",
+        );
+    }
+    if (balanceRead && codes.includes(CODES.INSUFFICIENT_TOKEN)) {
+        messages.push(
+            tokenBalance == null
+                ? "This token's balance is unknown, because nothing this" +
+                      " wallet can consult reports how many decimal places it" +
+                      " uses, so the amount you are trying to send cannot be" +
+                      " checked against it."
+                : "Insufficient " +
+                      symbol +
+                      " balance. You have " +
+                      truncateAmountNeverZero(tokenBalance) +
+                      " " +
+                      symbol +
+                      " but are trying to send " +
+                      txInfo.amount +
+                      " " +
+                      symbol +
+                      ".",
+        );
+    }
+    if (balanceRead && codes.includes(CODES.INSUFFICIENT_ETH)) {
+        messages.push(
+            "Insufficient balance. You have " +
+                truncateAmountNeverZero(ethBalance) +
+                " " +
+                symbol +
+                " but are trying to send " +
+                txInfo.amount +
+                " " +
+                symbol +
+                ".",
+        );
+    }
 
     const errorsEl = $("confirm-errors");
     if (messages.length > 0) {
@@ -385,13 +412,6 @@ function renderValidation(txInfo) {
         errorsEl.style.visibility = "hidden";
     }
 
-    setVisible("confirm-balance-read-error", balances !== null && !balanceRead);
-    setVisible(
-        "confirm-balance-error",
-        balanceRead &&
-            (codes.includes(CODES.INSUFFICIENT_TOKEN) ||
-                codes.includes(CODES.INSUFFICIENT_ETH)),
-    );
     setVisible(
         "confirm-amount-fee-error",
         balanceRead && codes.includes(CODES.INSUFFICIENT_ETH_WITH_FEE),

tests/accountsChangedOpaqueOrigin.test.js

@@ -1,167 +0,0 @@
-// A page whose origin has no host is never told an address by accountsChanged
-// (https://git.eeqj.de/sneak/AutistMask/issues/564).
-//
-// When the user switches address, the background tells each open tab the new
-// active address if the site at the tab's URL is connected. A page at a
-// connected site's URL can still have another origin: one its server
-// sandboxes with a Content-Security-Policy has the origin "null", and every
-// request it makes is refused. Each test here switches the address in the real
-// background, takes the event it sent to a tab at the site's URL, and hands
-// that event to the real content script in a page of the given origin.
-
-const { networkById } = require("../src/shared/networks");
-const { makeStorageStub } = require("./support/storageStub");
-
-const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
-const SITE = "https://dapp.example";
-
-const EXT_URL = "chrome-extension://autistmask/";
-const FROM_POPUP = { url: EXT_URL + "src/popup/index.html" };
-
-// One address, with SITE remembered as allowed for it.
-function storedProfile() {
-    const mainnet = networkById("mainnet");
-    return {
-        hasWallet: true,
-        wallets: [
-            {
-                name: "Wallet 1",
-                type: "hd",
-                addresses: [
-                    { address: ADDRESS, balance: "0", tokenBalances: [] },
-                ],
-            },
-        ],
-        activeAddress: ADDRESS,
-        networkId: "mainnet",
-        rpcUrl: mainnet.defaultRpcUrl,
-        blockscoutUrl: mainnet.defaultBlockscoutUrl,
-        allowedSites: { [ADDRESS]: [SITE] },
-        deniedSites: {},
-        trackedTokens: [],
-    };
-}
-
-async function settle() {
-    for (let i = 0; i < 50; i++) await Promise.resolve();
-}
-
-afterEach(() => {
-    delete global.chrome;
-    delete global.window;
-    jest.resetModules();
-});
-
-// The popup tells the background the active address changed, and the one
-// open tab, at SITE's URL, is sent an event. That event is returned.
-async function eventSentToSiteTab() {
-    jest.resetModules();
-    jest.doMock("../src/shared/balances", () => ({
-        getProvider: () => ({}),
-        refreshBalances: jest.fn(async () => {}),
-    }));
-    jest.doMock("../src/shared/phishingDomains", () => ({
-        isPhishingDomain: () => false,
-    }));
-    jest.doMock("../src/shared/alarms", () => ({
-        BALANCE_REFRESH_ALARM: "balance",
-        BALANCE_REFRESH_PERIOD_MINUTES: 1,
-        ensureRecurringAlarms: jest.fn(async () => {}),
-        registerAlarmHandlers: jest.fn(),
-    }));
-
-    let messageListener = null;
-    const sentToTabs = [];
-    global.chrome = {
-        storage: makeStorageStub({ autistmask: storedProfile() }),
-        runtime: {
-            getURL: (path) => EXT_URL + path,
-            onMessage: {
-                addListener: (fn) => {
-                    messageListener = fn;
-                },
-            },
-            onConnect: { addListener: () => {} },
-            getPlatformInfo: (cb) => cb({ os: "linux" }),
-            lastError: null,
-        },
-        windows: {
-            remove: (id, cb) => {
-                if (cb) cb();
-            },
-            onRemoved: { addListener: () => {} },
-        },
-        tabs: {
-            query: (q, cb) => cb([{ id: 1, url: SITE + "/app" }]),
-            sendMessage: (tabId, msg, cb) => {
-                sentToTabs.push(msg);
-                if (cb) cb();
-            },
-        },
-        action: { setPopup: () => {} },
-    };
-
-    require("../src/background/index");
-    messageListener(
-        { type: "AUTISTMASK_ACTIVE_CHANGED" },
-        FROM_POPUP,
-        () => {},
-    );
-    await settle();
-
-    expect(sentToTabs).toHaveLength(1);
-    return sentToTabs[0];
-}
-
-// The content script in a page whose origin is `pageOrigin` is handed `event`
-// from the background. What it posts to the page is returned.
-function postedToPage(event, pageOrigin) {
-    jest.resetModules();
-    let eventListener = null;
-    const posted = [];
-    global.chrome = {
-        runtime: {
-            onMessage: {
-                addListener: (fn) => {
-                    eventListener = fn;
-                },
-            },
-        },
-    };
-    global.window = {
-        origin: pageOrigin,
-        addEventListener: () => {},
-        postMessage: (data) => posted.push(data),
-    };
-
-    require("../src/content/index");
-    eventListener(event);
-
-    return posted;
-}
-
-describe("accountsChanged after an address switch, at a connected site's URL", () => {
-    test("the site's own page is told the active address", async () => {
-        const event = await eventSentToSiteTab();
-
-        expect(postedToPage(event, SITE)).toEqual([
-            {
-                type: "AUTISTMASK_EVENT",
-                eventName: "accountsChanged",
-                data: [ADDRESS],
-            },
-        ]);
-    });
-
-    test("a page whose origin has no host is told no address", async () => {
-        const event = await eventSentToSiteTab();
-
-        expect(postedToPage(event, "null")).toEqual([
-            {
-                type: "AUTISTMASK_EVENT",
-                eventName: "accountsChanged",
-                data: [],
-            },
-        ]);
-    });
-});

tests/confirmTxNetworkSwitch.test.js

@@ -259,10 +259,6 @@ function errors() {
     return el("confirm-errors").innerHTML;
 }
 
-function shown(id) {
-    return el(id).style.visibility === "visible";
-}
-
 function sendDisabled() {
     return el("btn-confirm-send").disabled;
 }
@@ -351,7 +347,6 @@ describe("a confirmation screen saved on mainnet and reopened after a switch to
         );
         expect(el("confirm-balance").textContent).toBe("5.0000 SepoliaETH");
         expect(errors()).toBe("");
-        expect(shown("confirm-balance-error")).toBe(false);
         expect(el("confirm-amount-fee-error").style.visibility).toBe("hidden");
         expect(sendDisabled()).toBe(false);
     });
@@ -367,10 +362,9 @@ describe("a confirmation screen saved on mainnet and reopened after a switch to
 
         await reopenOn("sepolia");
         expect(el("confirm-balance").textContent).toBe("0.5000 SepoliaETH");
-        expect(shown("confirm-balance-error")).toBe(true);
+        expect(errors()).toContain(
-        expect(el("confirm-balance-error").textContent).toBe(
+            "Insufficient balance. You have 0.5000 SepoliaETH but are trying" +
-            "Your SepoliaETH balance does not cover this amount. Please go" +
+                " to send 1 SepoliaETH.",
-                " back and send a smaller amount.",
         );
         expect(sendDisabled()).toBe(true);
     });
@@ -390,7 +384,6 @@ describe("a confirmation screen saved on mainnet and reopened after a switch to
         await reopenOn("sepolia");
         expect(el("confirm-balance").textContent).toBe("10.0000 TOK");
         expect(errors()).toBe("");
-        expect(shown("confirm-balance-error")).toBe(false);
         expect(el("confirm-gas-error").style.visibility).toBe("hidden");
         expect(sendDisabled()).toBe(false);
     });
@@ -403,10 +396,9 @@ describe("a confirmation screen saved on mainnet and reopened after a switch to
 
         await reopenOn("sepolia");
         expect(el("confirm-balance").textContent).toBe("0.5000 TOK");
-        expect(shown("confirm-balance-error")).toBe(true);
+        expect(errors()).toContain(
-        expect(el("confirm-balance-error").textContent).toBe(
+            "Insufficient TOK balance. You have 0.5000 TOK but are trying to" +
-            "Your TOK balance does not cover this amount. Please go back and" +
+                " send 1 TOK.",
-                " send a smaller amount.",
         );
         expect(sendDisabled()).toBe(true);
     });
@@ -426,8 +418,6 @@ describe("the balance read", () => {
         expect(el("confirm-balance").textContent).toBe("Loading...");
         expect(el("confirm-fee-amount").textContent).toBe("Estimating...");
         expect(el("confirm-errors").style.visibility).toBe("hidden");
-        expect(shown("confirm-balance-error")).toBe(false);
-        expect(shown("confirm-balance-read-error")).toBe(false);
         expect(sendDisabled()).toBe(true);
 
         answer(parseEther("5"));
@@ -436,32 +426,6 @@ describe("the balance read", () => {
         expect(sendDisabled()).toBe(false);
     });
 
-    // The Chrome suite measures that nothing moves; this checks the message
-    // is already written, only hidden, before the read returns.
-    test("that finds the amount not covered only makes a message written before it visible", async () => {
-        let answer;
-        const getBalance = jest
-            .spyOn(JsonRpcProvider.prototype, "getBalance")
-            .mockImplementationOnce(() => new Promise((r) => (answer = r)));
-        try {
-            await review("sepolia");
-        } finally {
-            getBalance.mockRestore();
-        }
-        const message =
-            "Your SepoliaETH balance does not cover this amount. Please go" +
-            " back and send a smaller amount.";
-        expect(el("confirm-balance-error").textContent).toBe(message);
-        expect(shown("confirm-balance-error")).toBe(false);
-
-        answer(parseEther("0.5"));
-        await settle();
-        expect(el("confirm-balance-error").textContent).toBe(message);
-        expect(shown("confirm-balance-error")).toBe(true);
-        expect(errors()).toBe("");
-        expect(sendDisabled()).toBe(true);
-    });
-
     // The fee is estimated once the read returns, so a node chosen then would
     // be the new network's.
     test("still in flight when Sepolia is chosen in Settings leaves the balance and the fee on mainnet", async () => {
@@ -499,9 +463,11 @@ describe("the balance read", () => {
             getBalance.mockRestore();
         }
         expect(el("confirm-balance").textContent).toBe("unknown (SepoliaETH)");
-        expect(shown("confirm-balance-read-error")).toBe(true);
+        expect(errors()).toBe(
-        expect(shown("confirm-balance-error")).toBe(false);
+            '<div class="text-xs">Your balance could not be read from the' +
-        expect(errors()).toBe("");
+                " network, so this transaction cannot be checked against it." +
+                " Please go back and try again.</div>",
+        );
         expect(sendDisabled()).toBe(true);
     });
 });

tests/e2e/network.js

@@ -428,10 +428,9 @@ const HOLD_MAX_MS = 30000;
 
 // Hold a reply open for as long as the test asks: until opts[name] is false.
 //
-// The switch (holdBalance, holdGasEstimate, holdTransactionCount or
+// The switch (holdGasEstimate, holdTransactionCount or holdBlockscout) is read
-// holdBlockscout) is read here rather than captured, so a test flips it on the
+// here rather than captured, so a test flips it on the same options object the
-// same options object the route was registered with — the same pattern as
+// route was registered with — the same pattern as seedTokenTransfer. This is
-// seedTokenTransfer. This is
 // the only way to observe a screen while its request is genuinely in flight;
 // sampling the screen and hoping to win a race against the network would
 // assert nothing on a slow machine.
@@ -566,9 +565,6 @@ async function handleRpc(route, postData, opts, report) {
         report("unstubbed request: POST " + route.request().url());
         return route.abort();
     }
-    if (batch.some((req) => req.method === "eth_getBalance")) {
-        await awaitRelease(opts, "holdBalance", report);
-    }
     if (batch.some((req) => req.method === "eth_estimateGas")) {
         await awaitRelease(opts, "holdGasEstimate", report);
     }
@@ -627,8 +623,6 @@ function traceEnabled(raw) {
  *   fixture expects.
  * @param {boolean} [opts.failGasEstimate]  answer eth_estimateGas with a
  *   node-side refusal.
- * @param {boolean} [opts.holdBalance]  hold every batch containing an
- *   eth_getBalance until this is cleared again.
  * @param {boolean} [opts.holdGasEstimate]  hold every batch containing an
  *   eth_estimateGas until this is cleared again.
  * @param {boolean} [opts.holdTransactionCount]  hold every batch containing an

tests/e2e/run.js

@@ -2232,7 +2232,7 @@ 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, and how tall the whole view is.
 async function confirmState(page) {
     return page.evaluate(() => {
         const el = (id) => document.getElementById(id);
@@ -2245,9 +2245,6 @@ async function confirmState(page) {
         };
         return {
             height: el("view-confirm-tx").getBoundingClientRect().height,
-            sendTop:
-                el("btn-confirm-send").getBoundingClientRect().top -
-                el("view-confirm-tx").getBoundingClientRect().top,
             type: el("confirm-type").textContent.trim(),
             balance: el("confirm-balance").textContent.trim(),
             fee: el("confirm-fee-amount").textContent.trim(),
@@ -2256,9 +2253,6 @@ async function confirmState(page) {
             errors: shown("confirm-errors")
                 ? el("confirm-errors").textContent.trim()
                 : "",
-            balanceError: shown("confirm-balance-error")
-                ? el("confirm-balance-error").textContent.trim()
-                : "",
             amountFeeError: shown("confirm-amount-fee-error"),
             gasError: shown("confirm-gas-error"),
             feeUnknownError: shown("confirm-fee-unknown-error"),
@@ -2277,16 +2271,6 @@ async function waitForEstimate(page) {
     );
 }
 
-async function waitForBalance(page) {
-    await page.waitForFunction(
-        () =>
-            document.getElementById("confirm-balance").textContent.trim() !==
-            "Loading...",
-        null,
-        { timeout: 60000 },
-    );
-}
-
 async function backToAddress(page) {
     if (await page.isVisible("#view-confirm-tx")) {
         await page.click("#btn-confirm-back");
@@ -2298,14 +2282,13 @@ async function backToAddress(page) {
     await openAddressDetail(page);
 }
 
-// Drive the popup to the confirmation screen for one send, and return as soon
+// Drive the popup to the confirmation screen for one send.
-// as the screen is shown, its balance read still in flight.
 //
 // It waits for the send screen to be showing `balance`, the fixture's balance
 // as that screen displays it, before filling anything in. 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 openConfirm(page, { token, balance, amount }) {
+async function goToConfirm(page, { token, balance, amount }) {
     await backToAddress(page);
     await page.click("#btn-send");
     await visible(page, "#view-send");
@@ -2320,14 +2303,16 @@ async function openConfirm(page, { token, balance, amount }) {
     await page.fill("#send-amount", amount);
     await page.click("#btn-send-review");
     await visible(page, "#view-confirm-tx");
-}
+    // The screen reads the balance itself before it estimates the fee, so
-
+    // waiting for it here means every state below is read with the balance
-// The same, returning once the screen has read the balance. It reads it
+    // in place, the pending fee included.
-// before it estimates the fee, so every state below is read with the balance
+    await page.waitForFunction(
-// in place, the pending fee included.
+        () =>
-async function goToConfirm(page, send) {
+            document.getElementById("confirm-balance").textContent.trim() !==
-    await openConfirm(page, send);
+            "Loading...",
-    await waitForBalance(page);
+        null,
+        { timeout: 60000 },
+    );
     await assertAddressesFit(page, "the confirmation screen");
 }
 
@@ -2403,10 +2388,7 @@ test("ConfirmTx blocks sending while the fee estimate is pending (#238)", async
         "the estimate-failed message is shown for an estimate that is merely pending",
     );
     assert(
-        !st.amountFeeError &&
+        !st.amountFeeError && !st.gasError && st.errors === "",
-            !st.gasError &&
-            st.errors === "" &&
-            st.balanceError === "",
         "a balance message is shown before the fee is known",
     );
 });
@@ -2437,7 +2419,6 @@ test("ConfirmTx enables Send once the estimate lands, quoting both numbers (#238
     );
     assert(
         st.errors === "" &&
-            st.balanceError === "" &&
             !st.amountFeeError &&
             !st.gasError &&
             !st.feeUnknownError,
@@ -2504,54 +2485,30 @@ test("ConfirmTx gates on the fee RESERVE, not the displayed estimate (#238)", as
     );
 });
 
-// The screen is first measured with its balance read held, so the message the
+test("ConfirmTx refuses a send that exceeds the balance outright (#238)", async (env) => {
-// read brings must already hold its space when the read returns (#558).
+    await goToConfirm(env.page, {
-test("ConfirmTx refuses a send that exceeds the balance outright, and nothing moves when the balance arrives (#238)", async (env) => {
+        token: "ETH",
-    env.routeOpts.holdBalance = true;
+        balance: FUNDED_ETH_TEXT + " ETH",
-    let reading;
+        amount: OVER_BALANCE_AMOUNT,
-    try {
+    });
-        await openConfirm(env.page, {
-            token: "ETH",
-            balance: FUNDED_ETH_TEXT + " ETH",
-            amount: OVER_BALANCE_AMOUNT,
-        });
-        reading = await confirmState(env.page);
-    } finally {
-        env.routeOpts.holdBalance = false;
-    }
-    assert(
-        reading.balance === "Loading...",
-        "the balance read was not held: " + JSON.stringify(reading.balance),
-    );
-    await waitForBalance(env.page);
     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),
-    );
     const want =
-        "Your ETH balance does not cover this amount. Please go back and" +
+        "Insufficient balance. You have " +
-        " send a smaller amount.";
+        FUNDED_ETH_TEXT +
+        " ETH but are trying to send " +
+        OVER_BALANCE_AMOUNT +
+        " ETH.";
     assert(
-        st.balanceError === want,
+        st.errors === want,
-        "wrong over-balance message: " + JSON.stringify(st.balanceError),
+        "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",
     );
-    assert(
-        st.sendTop === reading.sendTop,
-        "Sign & Send moved when the balance read returned: " +
-            reading.sendTop +
-            "px -> " +
-            st.sendTop +
-            "px",
-    );
 });
 
 test("ConfirmTx refuses to send when the fee estimate fails, with its own message (#238)", async (env) => {
@@ -2595,10 +2552,7 @@ test("ConfirmTx refuses to send when the fee estimate fails, with its own messag
             "the estimate-failed message is not shown after a failed estimate",
         );
         assert(
-            !st.amountFeeError &&
+            !st.amountFeeError && !st.gasError && st.errors === "",
-                !st.gasError &&
-                st.errors === "" &&
-                st.balanceError === "",
             "a balance message is shown for an estimate that simply failed",
         );
         assert(
@@ -2689,19 +2643,21 @@ test("ConfirmTx refuses an ERC-20 send that exceeds the token balance (#238)", a
     await waitForEstimate(env.page);
 
     const st = await confirmState(env.page);
-    assert(
-        st.balance === TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
-        "the confirmation screen shows the wrong token balance: " +
-            JSON.stringify(st.balance),
-    );
     const want =
-        "Your " +
+        "Insufficient " +
         STUB_TOKEN.symbol +
-        " balance does not cover this amount. Please go back and send a" +
+        " balance. You have " +
-        " smaller amount.";
+        TOKEN_BALANCE_TEXT +
+        " " +
+        STUB_TOKEN.symbol +
+        " but are trying to send " +
+        OVER_TOKEN_AMOUNT +
+        " " +
+        STUB_TOKEN.symbol +
+        ".";
     assert(
-        st.balanceError === want,
+        st.errors === want,
-        "wrong over-token-balance message: " + JSON.stringify(st.balanceError),
+        "wrong over-token-balance message: " + JSON.stringify(st.errors),
     );
     assert(
         st.sendDisabled,
@@ -2757,9 +2713,9 @@ test("ConfirmTx gates the ERC-20 fee on the RESERVE, with the ERC-20 message (#2
         "the native-ETH over-budget message is shown on an ERC-20 transfer",
     );
     assert(
-        st.errors === "" && st.balanceError === "",
+        st.errors === "",
         "a token-balance message is shown for a transfer the token balance covers: " +
-            JSON.stringify(st.errors + st.balanceError),
+            JSON.stringify(st.errors),
     );
     assert(
         st.height === env.erc20PendingHeight,
@@ -4913,9 +4869,6 @@ async function main() {
         seedTokenBalance: false,
         ethBalanceWei: null,
         failGasEstimate: false,
-        // Whether a JSON-RPC batch asking for a balance is held unanswered:
-        // the confirmation screen's own balance read (#558).
-        holdBalance: false,
         holdGasEstimate: false,
         // Whether a JSON-RPC batch asking for a transaction count is held
         // unanswered: the address scan after creating a wallet (#475).

tests/nativeCurrency.test.js

@@ -336,11 +336,14 @@ describe.each([
 
     test("the insufficient-balance error", async () => {
         await confirmSend("2");
-        expect(text("confirm-balance-error")).toBe(
+        expect(
-            "Your " +
+            global.document.getElementById("confirm-errors").innerHTML,
+        ).toContain(
+            "You have 1.5000 " +
                 symbol +
-                " balance does not cover this amount. Please go back and send" +
+                " but are trying to send 2 " +
-                " a smaller amount.",
+                symbol +
+                ".",
         );
     });
 });

tests/sendDisplayFloor.test.js

@@ -203,12 +203,6 @@ function errors() {
     return global.document.getElementById("confirm-errors").innerHTML;
 }
 
-// The message for an amount the balance does not cover, while it is shown.
-function balanceError() {
-    const box = global.document.getElementById("confirm-balance-error");
-    return box.style.visibility === "visible" ? box.textContent : "";
-}
-
 // The ETH balance the node reports and the token rows the explorer reports,
 // fetched and stored exactly where the popup stores them. The node reports the
 // token holding the explorer does.
@@ -317,10 +311,9 @@ describe("a tracked token holding below 0.000001 never renders as zero", () => {
         await confirmSend("0.0000005", TOKEN);
         expect(text("confirm-balance")).toBe("0.0000005 TOK");
         expect(errors()).toBe("");
-        expect(balanceError()).toBe("");
         await confirmSend("0.0000006", TOKEN);
-        expect(balanceError()).toContain(
+        expect(errors()).toContain(
-            "Your TOK balance does not cover this amount.",
+            "You have 0.0000005 TOK but are trying to send 0.0000006 TOK.",
         );
     });
 
@@ -334,7 +327,6 @@ describe("a tracked token holding below 0.000001 never renders as zero", () => {
         await confirmSend("1.5", TOKEN);
         expect(text("confirm-balance")).toBe("1.5000 TOK");
         expect(errors()).toBe("");
-        expect(balanceError()).toBe("");
     });
 
     test("with more than 18 decimals and a holding below 10^-18", async () => {
@@ -348,8 +340,9 @@ describe("a tracked token holding below 0.000001 never renders as zero", () => {
         );
         await confirmSend("0.000000000000000001", TOKEN);
         expect(text("confirm-balance")).toBe("0.000000000000000000000001 TOK");
-        expect(balanceError()).toContain(
+        expect(errors()).toContain(
-            "Your TOK balance does not cover this amount.",
+            "You have 0.000000000000000000000001 TOK but are trying to send" +
+                " 0.000000000000000001 TOK.",
         );
     });
 });
@@ -374,10 +367,9 @@ describe("an untracked token holding below 0.000001 never renders as zero", () =
         await confirmSend("0.0000005", TOKEN);
         expect(text("confirm-balance")).toBe("0.0000005 TOK");
         expect(errors()).toBe("");
-        expect(balanceError()).toBe("");
         await confirmSend("0.0000006", TOKEN);
-        expect(balanceError()).toContain(
+        expect(errors()).toContain(
-            "Your TOK balance does not cover this amount.",
+            "You have 0.0000005 TOK but are trying to send 0.0000006 TOK.",
         );
     });
 });

tests/unknownScaleSend.test.js

@@ -249,12 +249,6 @@ function errors() {
     return el("confirm-errors").innerHTML;
 }
 
-// The message for an amount the balance does not cover, while it is shown.
-function balanceError() {
-    const box = el("confirm-balance-error");
-    return box.style.visibility === "visible" ? box.textContent : "";
-}
-
 function sendDisabled() {
     return el("btn-confirm-send").disabled;
 }
@@ -325,7 +319,7 @@ describe("the Send screen resolves the scale rather than reading the stored one"
         confirmTx.show(txInfo);
         await settle();
         expect(text("confirm-balance")).toBe("unknown (NOVEL)");
-        expect(balanceError()).toContain("This token's balance is unknown");
+        expect(errors()).toContain("This token's balance is unknown");
         expect(sendDisabled()).toBe(true);
     });
 });
@@ -382,7 +376,8 @@ describe("a scale the explorer's own rows disagree about", () => {
         // confirm-fee-unknown-error — "the network fee could not be
         // estimated... please go back and try again" — as the only explanation
         // for a screen that can never proceed.
-        expect(balanceError()).toContain("This token's balance is unknown");
+        expect(errors()).not.toBe("");
+        expect(errors()).toContain("This token's balance is unknown");
         expect(text("confirm-balance")).toBe("unknown (NOVEL)");
         expect(sendDisabled()).toBe(true);
         // The fee line still reports the estimate as unavailable, because it
@@ -452,7 +447,6 @@ describe("a scale the explorer's own rows disagree about", () => {
         await settle();
         expect(text("confirm-balance")).toBe("5.0000 NOVEL");
         expect(errors()).toBe("");
-        expect(balanceError()).toBe("");
         expect(sendDisabled()).toBe(false);
     });
 });
@@ -482,7 +476,7 @@ describe("the confirmation screen tells an unknown balance from a zero one", ()
             tokenDecimals,
         });
         await settle();
-        return { balance: text("confirm-balance"), message: balanceError() };
+        return { balance: text("confirm-balance"), errors: errors() };
     }
 
     test("the balance line states unknown rather than a quantity of zero", async () => {
@@ -494,18 +488,16 @@ describe("the confirmation screen tells an unknown balance from a zero one", ()
     });
 
     // Both hit INSUFFICIENT_TOKEN — an unknown balance is treated as nothing to
-    // spend from, which is the fail-closed side — but "your balance does not
+    // spend from, which is the fail-closed side — but "you have 0.0" is a claim
-    // cover this amount" is a claim about the holding, and this one has no
+    // about the holding, and this one has no established quantity to claim.
-    // established quantity to claim.
+    test("the insufficient-balance message names the reason, not a figure", async () => {
-    test("the insufficient-balance message names the reason, not a shortfall", async () => {
         const unknown = await render(null);
         const zero = await render(18);
-        expect(unknown.message).toContain("This token's balance is unknown");
+        expect(unknown.errors).not.toBe(zero.errors);
-        expect(unknown.message).not.toContain("does not cover");
+        expect(unknown.errors).toContain("This token's balance is unknown");
-        expect(zero.message).toBe(
+        expect(unknown.errors).not.toContain("You have");
-            "Your NOVEL balance does not cover this amount. Please go back and" +
+        expect(zero.errors).toContain("You have 0.0000 NOVEL");
-                " send a smaller amount.",
+        expect(zero.errors).not.toContain("balance is unknown");
-        );
     });
 });