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 popup reopened onto a confirmation saved before a network switch stated the other network's balance and checked the amount against it.

Each time it is shown, ConfirmTx now reads the From address's ETH balance, and for a token send its token balance at the scale the amount is shown with, from the node of the network it is drawn for. Until the read returns the balance line says "Loading..." and Sign & Send stays disabled; a read that fails is stated as unknown and blocks the send. The fee estimate starts once the read is done, since a max ETH send's amount comes from the balance. Send no longer hands balances over.

Model: opus-5-5

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,
@@ -32,6 +38,7 @@ const {
 const { ERC20_ABI, isBurnAddress } = require("../../shared/constants");
 const {
     displayedDecimals,
+    toDecimals,
     transferAmountUnits,
 } = require("../../shared/transferAmount");
 const { assertWithinCeilings } = require("../../shared/approvalVerify");
@@ -60,6 +67,14 @@ 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.
+// The popup can reopen onto this screen after the network was switched, so
+// any balance Send knew may be another network's. 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;
@@ -96,11 +111,38 @@ 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 || "?" : nativeCurrency();
+    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 = isErc20 ? getPrice(rawSymbol) : getPrice("ETH");
+    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:
     // Settings can switch the active network while the screen is open. The
     // wait, success and error screens label the amount by it too.
@@ -152,33 +194,7 @@ function show(txInfo) {
     $("confirm-to-ens").classList.add("hidden");
 
     renderAmount(txInfo);
-
-    // Balance (with inline USD)
-    const ethPrice = getPrice("ETH");
-    const tokenPrice = getPrice(rawSymbol);
-    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, {
@@ -262,25 +278,68 @@ 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 screen
+// is drawn for, the node the fee estimate asks: 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 = getProvider(state.rpcUrl, state.networkId);
+        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 || "?")
         : nativeCurrency();
 
+    // 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 no message below quotes one.
+    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,
     });
@@ -292,9 +351,16 @@ function renderValidation(txInfo) {
     if (codes.includes(CODES.AMOUNT_INVALID)) {
         messages.push("Please enter a valid amount to send.");
     }
-    if (codes.includes(CODES.INSUFFICIENT_TOKEN)) {
+    if (balances !== null && !balanceRead) {
         messages.push(
-            txInfo.tokenBalance == null
+            "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" +
@@ -302,7 +368,7 @@ function renderValidation(txInfo) {
                 : "Insufficient " +
                       symbol +
                       " balance. You have " +
-                      truncateAmountNeverZero(txInfo.tokenBalance) +
+                      truncateAmountNeverZero(tokenBalance) +
                       " " +
                       symbol +
                       " but are trying to send " +
@@ -312,10 +378,10 @@ function renderValidation(txInfo) {
                       ".",
         );
     }
-    if (codes.includes(CODES.INSUFFICIENT_ETH)) {
+    if (balanceRead && codes.includes(CODES.INSUFFICIENT_ETH)) {
         messages.push(
             "Insufficient balance. You have " +
-                truncateAmountNeverZero(txInfo.balance || "0") +
+                truncateAmountNeverZero(ethBalance) +
                 " " +
                 symbol +
                 " but are trying to send " +
@@ -339,11 +405,11 @@ function renderValidation(txInfo) {
 
     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",
@@ -432,14 +498,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);

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,
         });