fix: the confirmation screen reads its balance on the network it is drawn for (closes #558)

ConfirmTx showed and checked the balance Send handed it, so a screen the popup reopened onto stated the balance as Send knew it, not as it is on the network the screen is drawn for.

Each time it is shown, ConfirmTx now reads the From address's ETH balance, and for a token send its token balance, from the node reviewedNetworkProvider() picks for the network the transaction was reviewed for, the node the fee estimate and recipient checks ask. The fee estimate starts once the read returns. Until then the balance line says "Loading..." and Sign & Send stays disabled; a failed read is stated as unknown. Each message the read can bring has a box reserved from the first paint, so nothing moves when it returns.

Model: opus-5-5

src/popup/index.html

@@ -664,6 +664,20 @@
                     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

@@ -2,7 +2,13 @@
 // Shows transaction details, warnings, errors. On Sign & Send,
 // reads inline password, decrypts secret, signs and broadcasts.
 
-const { parseEther, parseUnits, formatEther, Contract } = require("ethers");
+const {
+    parseEther,
+    parseUnits,
+    formatEther,
+    formatUnits,
+    Contract,
+} = require("ethers");
 const {
     $,
     showError,
@@ -35,11 +41,12 @@ const {
 const { ERC20_ABI, isBurnAddress } = require("../../shared/constants");
 const {
     displayedDecimals,
+    toDecimals,
     transferAmountUnits,
 } = require("../../shared/transferAmount");
 const { assertWithinCeilings } = require("../../shared/approvalVerify");
-// The balance lines, the fee reserve and the insufficient-balance messages go
-// through it, as the approval screen's amounts do.
+// The balance line and the fee reserve go through it, as the approval screen's
+// amounts do.
 const { truncateAmountNeverZero } = require("../../shared/amountDisplay");
 const {
     CODES,
@@ -63,6 +70,13 @@ let feeWei = null;
 // amount was derived from. Null for any other send, which ethers prices from
 // the node at signing time.
 let maxSendFees = null;
+// The From address's balances on the network this screen is drawn for, read
+// from that network's node by readBalances() each time the screen is shown,
+// so a screen the popup reopens onto states them as they are then, not as
+// Send knew them. Null until the read returns; then `eth` and `token` are
+// decimal strings, `token` null for an ETH send or a token whose scale is
+// unknown, and both null if the read failed.
+let balances = null;
 
 function restore() {
     const d = state.viewData;
@@ -101,18 +115,48 @@ function renderAmount(txInfo) {
     );
 }
 
+// The balance line, with its USD value: "Loading..." until readBalances()
+// returns, then the balance it read.
+function renderBalance(txInfo) {
+    const isErc20 = txInfo.token !== "ETH";
+    const rawSymbol = isErc20
+        ? txInfo.tokenSymbol || "?"
+        : nativeCurrencyByChainId(txInfo.chainId);
+    const symbol = displaySymbol(rawSymbol);
+    if (balances === null) {
+        $("confirm-balance").textContent = "Loading...";
+        return;
+    }
+    // A null balance is one nobody established, for a token whose scale is
+    // unknown or after a read that failed, and is stated as unknown, never as
+    // zero (https://git.eeqj.de/sneak/AutistMask/issues/349).
+    const bal = isErc20 ? balances.token : balances.eth;
+    if (bal === null) {
+        $("confirm-balance").textContent = "unknown (" + symbol + ")";
+        return;
+    }
+    const price = getPrice(isErc20 ? rawSymbol : "ETH", txInfo.chainId);
+    const balUsd = price ? parseFloat(bal) * price : null;
+    $("confirm-balance").textContent = valueWithUsd(
+        truncateAmountNeverZero(bal) + " " + symbol,
+        balUsd,
+    );
+}
+
 function show(txInfo) {
     pendingTx = txInfo;
     feeStatus = FEE_PENDING;
     feeWei = null;
     maxSendFees = null;
+    balances = null;
     // The network this screen is drawn for, and the only one it sends on: the
     // one active when Send handed the transaction over. Drawn again from the
     // saved transaction, as when the popup reopens onto this screen, it keeps
     // that network, so Sign & Send refuses once Settings has switched to
     // another. Everything this screen shows is that network's: native
-    // currency, USD values, explorer links, and the node and explorer its
-    // checks ask. The wait, success and error screens label amounts by it.
+    // currency, USD values, explorer links, the balance, and the node and
+    // explorer its checks ask. The wait, success and error screens label
+    // amounts by it.
     if (!txInfo.chainId) txInfo.chainId = currentNetwork().chainId;
 
     const isErc20 = txInfo.token !== "ETH";
@@ -165,33 +209,7 @@ function show(txInfo) {
     $("confirm-to-ens").classList.add("hidden");
 
     renderAmount(txInfo);
-
-    // Balance (with inline USD)
-    const ethPrice = getPrice("ETH", txInfo.chainId);
-    const tokenPrice = getPrice(rawSymbol, txInfo.chainId);
-    if (isErc20) {
-        // null is a balance whose scale nothing knows, not a balance of zero
-        // (https://git.eeqj.de/sneak/AutistMask/issues/349). The send is
-        // refused at encode time for the same missing scale; what this line
-        // must not do is state a quantity nobody established.
-        const bal = txInfo.tokenBalance;
-        const balUsd =
-            tokenPrice && bal != null ? parseFloat(bal) * tokenPrice : null;
-        $("confirm-balance").textContent =
-            bal == null
-                ? "unknown (" + symbol + ")"
-                : valueWithUsd(
-                      truncateAmountNeverZero(bal) + " " + symbol,
-                      balUsd,
-                  );
-    } else {
-        const bal = txInfo.balance || "0";
-        const balUsd = ethPrice ? parseFloat(bal) * ethPrice : null;
-        $("confirm-balance").textContent = valueWithUsd(
-            truncateAmountNeverZero(bal) + " " + symbol,
-            balUsd,
-        );
-    }
+    renderBalance(txInfo);
 
     // Check for warnings (synchronous local checks)
     const localWarnings = getLocalWarnings(txInfo.to, {
@@ -257,6 +275,20 @@ 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
@@ -282,69 +314,78 @@ function show(txInfo) {
         $("confirm-burn-warning").style.visibility = "visible";
     }
 
-    estimateGas(txInfo);
+    // The fee is estimated once the balances are read, because a max ETH
+    // send's amount is the ETH balance minus the estimate's reserve.
+    readBalances(txInfo).then(() => estimateGas(txInfo));
     checkRecipientHistory(txInfo);
 }
 
+// Read the From address's balances from the node of the network this
+// transaction was reviewed for: ETH, and for a token send the token, at the
+// scale the screen renders the amount with. A token whose scale is unknown has
+// no balance to read. Never rejects.
+async function readBalances(txInfo) {
+    let read;
+    try {
+        const provider = reviewedNetworkProvider(txInfo);
+        const decimals = toDecimals(txInfo.tokenDecimals);
+        const readToken = txInfo.token !== "ETH" && decimals !== null;
+        const [ethWei, tokenUnits] = await Promise.all([
+            provider.getBalance(txInfo.from),
+            readToken
+                ? new Contract(txInfo.token, ERC20_ABI, provider).balanceOf(
+                      txInfo.from,
+                  )
+                : null,
+        ]);
+        read = {
+            eth: formatEther(ethWei),
+            token: readToken ? formatUnits(tokenUnits, decimals) : null,
+        };
+    } catch (e) {
+        log.errorf("balance read failed:", e.shortMessage || e.message);
+        read = { eth: null, token: null };
+    }
+    // The user may have left this transaction while the read was in flight.
+    if (pendingTx !== txInfo) return;
+    balances = read;
+    renderBalance(txInfo);
+    renderValidation(txInfo);
+}
+
 // Render the balance check for the transaction on screen. Called once during
-// show() and again when the fee estimate resolves or fails. Every element it
-// touches already occupies its space, so re-running it never moves anything.
+// show(), again when the balances are read, and again when the fee estimate
+// resolves or fails. Every element it touches already occupies its space, so
+// 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
+    // the one saying the read failed.
+    const ethBalance = balances ? balances.eth : null;
+    const tokenBalance = balances ? balances.token : null;
+    const balanceRead = ethBalance !== null;
 
     const { canSend, codes } = validateTransfer({
         isErc20,
         amount: txInfo.amount,
         tokenDecimals: txInfo.tokenDecimals,
-        ethBalance: txInfo.balance,
-        tokenBalance: txInfo.tokenBalance,
+        ethBalance,
+        tokenBalance,
         feeStatus,
         feeWei,
     });
 
-    // Messages carrying the user's own numbers are built here; the fixed
-    // sentences live in the reserved elements in index.html, except the
-    // gas and fee-unknown ones, which show() sets.
+    // An invalid amount is known before the first paint, so its message is
+    // written here. Every message a later result brings has a reserved
+    // element, its sentence fixed in index.html or set by show(), and is only
+    // 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 (codes.includes(CODES.INSUFFICIENT_TOKEN)) {
-        messages.push(
-            txInfo.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(txInfo.tokenBalance) +
-                      " " +
-                      symbol +
-                      " but are trying to send " +
-                      txInfo.amount +
-                      " " +
-                      symbol +
-                      ".",
-        );
-    }
-    if (codes.includes(CODES.INSUFFICIENT_ETH)) {
-        messages.push(
-            "Insufficient balance. You have " +
-                truncateAmountNeverZero(txInfo.balance || "0") +
-                " " +
-                symbol +
-                " but are trying to send " +
-                txInfo.amount +
-                " " +
-                symbol +
-                ".",
-        );
-    }
 
     const errorsEl = $("confirm-errors");
     if (messages.length > 0) {
@@ -357,13 +398,20 @@ 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",
-        codes.includes(CODES.INSUFFICIENT_ETH_WITH_FEE),
+        balanceRead && codes.includes(CODES.INSUFFICIENT_ETH_WITH_FEE),
     );
     setVisible(
         "confirm-gas-error",
-        codes.includes(CODES.INSUFFICIENT_ETH_FOR_FEE),
+        balanceRead && codes.includes(CODES.INSUFFICIENT_ETH_FOR_FEE),
     );
     setVisible(
         "confirm-fee-unknown-error",
@@ -472,14 +520,14 @@ async function estimateGas(txInfo) {
         }
         feeStatus = FEE_KNOWN;
         feeWei = gasCostWei;
-        // A max ETH send is the balance minus this estimate's reserve, not the
-        // Send screen's, and is signed with this estimate's fee fields: fees
-        // fetched again at signing could exceed the reserve it leaves, and the
-        // node would refuse it for want of funds. Where the balance no longer
-        // covers the fee, the amount is left as it is and the balance check
-        // below says so.
+        // A max ETH send is the balance readBalances() read minus this
+        // estimate's reserve, not the Send screen's, and is signed with this
+        // estimate's fee fields: fees fetched again at signing could exceed
+        // the reserve it leaves, and the node would refuse it for want of
+        // funds. Where the balance is unknown or no longer covers the fee, the
+        // amount is left as it is and the balance check below says so.
         if (txInfo.max && txInfo.token === "ETH") {
-            const amount = maxEthAmount(txInfo.balance, gasCostWei);
+            const amount = maxEthAmount(balances?.eth, gasCostWei);
             if (amount !== null) {
                 txInfo.amount = amount;
                 renderAmount(txInfo);
@@ -546,8 +594,8 @@ async function populateVerifyAndSend(connectedSigner, tx, fees = null) {
 }
 
 // Show a full-sentence send failure in the reserved errors box, the same
-// element and markup renderValidation() uses for messages carrying the user's
-// own numbers, so it never moves anything on the screen.
+// element and markup renderValidation() uses for the invalid-amount message,
+// so it never moves anything on the screen.
 function showSendError(message) {
     const el = $("confirm-errors");
     el.innerHTML = `<div class="text-xs">${escapeHtml(message)}</div>`;

src/popup/views/send.js

@@ -164,8 +164,9 @@ function renderSendTokenSelect(addr) {
     }
 }
 
-// The token balance and scale the Send screen states and hands the
-// confirmation screen, so the two screens describe the holding the same way.
+// The token balance the Send screen states, and the scale it hands the
+// confirmation screen, which reads the balance itself from the network it is
+// drawn for.
 //
 // The scale is resolved the same way balances.js resolved the scale it
 // DISPLAYED this token's balance at: bundled list, then the user's tracked
@@ -188,13 +189,11 @@ function renderSendTokenSelect(addr) {
 //
 // The two resolutions can therefore differ, and where they do, the stored
 // `balance` is a quantity computed at a scale this screen has just declined to
-// stand behind. Stating it would leave validateTransfer() checking the amount
-// against a number the wallet does not vouch for, so it is withdrawn: unknown
-// scale means unknown balance. It is null rather than "0": both screens state
-// an unknown balance as unknown, and validateTransfer() treats it as no balance
-// to spend from, which is the fail-closed side of an amount nobody can check.
-// Only a stored quantity is withdrawn: the "0" for a token that has no row at
-// all is an absence of holdings, which is true at every scale.
+// stand behind, so it is withdrawn: unknown scale means unknown balance. It is
+// null rather than "0": this screen states it as unknown, as the confirmation
+// screen does, and Max fills nothing in for it. Only a stored quantity is
+// withdrawn: the "0" for a token that has no row at all is an absence of
+// holdings, which is true at every scale.
 function tokenBalanceAndDecimals(addr, token) {
     const tb = (addr.tokenBalances || []).find(
         (t) => t.address.toLowerCase() === token.toLowerCase(),
@@ -378,7 +377,6 @@ function init(_ctx) {
         const addr = currentAddress();
 
         let tokenSymbol = null;
-        let tokenBalance = null;
         let tokenDecimals = null;
         if (token !== "ETH") {
             tokenSymbol = resolveSymbol(
@@ -386,10 +384,7 @@ function init(_ctx) {
                 addr.tokenBalances,
                 state.trackedTokens,
             );
-            ({ tokenBalance, tokenDecimals } = tokenBalanceAndDecimals(
-                addr,
-                token,
-            ));
+            ({ tokenDecimals } = tokenBalanceAndDecimals(addr, token));
         }
 
         // The confirmation screen's own amount check, so it is never shown
@@ -424,9 +419,7 @@ function init(_ctx) {
             ensName: ensName,
             amount: amount,
             token: token,
-            balance: addr.balance,
             tokenSymbol: tokenSymbol,
-            tokenBalance: tokenBalance,
             tokenDecimals: tokenDecimals,
             max: amountIsMax,
         });

src/shared/transferAmount.js

@@ -2,11 +2,11 @@
 // encoded with.
 //
 // A token amount is a decimal string plus a scale, and the two come from
-// different places. The confirmation screen renders the amount, the balance and
-// the symbol from the block explorer's cached metadata (see
-// fetchTokenBalances() in balances.js); the transfer used to be encoded from
-// decimals() read off the contract at signing time, and nothing compared the
-// two. A token whose on-chain scale differs from the cached one — an
+// different places. The confirmation screen renders the amount, the symbol and
+// the balance it reads from the node with the block explorer's cached metadata
+// (see fetchTokenBalances() in balances.js); the transfer used to be encoded
+// from decimals() read off the contract at signing time, and nothing compared
+// the two. A token whose on-chain scale differs from the cached one — an
 // upgradeable or proxy token, a caller-dependent one, a stale or wrong explorer
 // entry — therefore signed an amount that was never displayed, off by a power
 // of ten for every decimal place of disagreement.

src/shared/txValidation.js

@@ -111,8 +111,8 @@ function isValidAmount(amount, tokenDecimals = null) {
 
 // The most ETH a send can carry: the exact balance minus the fee reserve from
 // feeReserveWei(), as a decimal string, so validateTransfer() passes it with
-// exactly that reserve left behind. `ethBalance` is the exact decimal string
-// balances.js stores, never a rounded one. Null when the balance does not
+// exactly that reserve left behind. `ethBalance` is an exact decimal string as
+// formatEther() writes it, never a rounded one. Null when the balance does not
 // leave anything to send once the fee is paid, or when either input is
 // unusable.
 function maxEthAmount(ethBalance, feeWei) {