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

@@ -1013,8 +1013,10 @@ rendered differently on two screens.
 
 The native token's label is a network's `nativeCurrency` in
 `src/shared/networks.js`: `ETH` on mainnet, `SepoliaETH` on Sepolia. The
-wallet's balances and the Send and confirmation screens, which send on the
+wallet's balances and the Send screen, which sends on the active network, use
-active network, use the active network's. A transaction's figures use the one of
+the active network's. The confirmation screen uses the one of the network it is
+drawn for, the network active when it was shown, even after the network is
+switched in Settings while it is open. A transaction's figures use the one of
 the network its chain id names, whichever network is active: the value and fee
 on the approval screen, the amount on the wait, success and error screens, the
 transaction history and the transaction detail screen, and the refusal of a fee
@@ -1728,12 +1730,14 @@ view would leave a wallet one click from deletion.
       failed. The amount check uses the same balances: the ETH balance, and for
       a token transfer the token balance as well
     - Network fee: "Estimating..." then two lines, or "Unable to estimate",
-      fetched async once the balance is read. The first line is what the
+      fetched async once the balance is read, from the same RPC endpoint as the
-      transfer is expected to cost, `gasLimit * gasPrice` (USD in parentheses);
+      balance, even if the network is switched in Settings while the balance is
-      the second is the `gasLimit * maxFeePerGas` reserve the node requires,
+      being read. The first line is what the transfer is expected to cost,
-      which is what the balance check gates on. The second line is omitted on a
+      `gasLimit * gasPrice` (USD in parentheses); the second is the
-      network with no type-2 pricing, where the two are the same number, but its
+      `gasLimit * maxFeePerGas` reserve the node requires, which is what the
-      space is reserved either way
+      balance check gates on. The second line is omitted on a network with no
+      type-2 pricing, where the two are the same number, but its space is
+      reserved either way
     - Warnings: inline warnings from the local checks (scam address, self-send)
       plus four reserved warning boxes made visible by the async checks —
       recipient with no transaction history, recipient is a contract, burn

src/popup/views/confirmTx.js

@@ -17,7 +17,6 @@ const {
     addressTitle,
     escapeHtml,
     displaySymbol,
-    nativeCurrency,
     renderAddressHtml,
     blockieHtml,
     attachCopyHandlers,
@@ -26,6 +25,7 @@ const {
     formatFee,
 } = require("./helpers");
 const { state, currentNetwork } = require("../../shared/state");
+const { nativeCurrencyByChainId } = require("../../shared/networks");
 const { getSignerForAddress } = require("../../shared/wallet");
 const { walletDefect } = require("../../shared/walletDefects");
 const { decryptWithPassword } = require("../../shared/vault");
@@ -102,7 +102,9 @@ function valueWithUsd(text, usdAmount) {
 // once its amount is re-derived from the fee estimate.
 function renderAmount(txInfo) {
     const isErc20 = txInfo.token !== "ETH";
-    const rawSymbol = isErc20 ? txInfo.tokenSymbol || "?" : nativeCurrency();
+    const rawSymbol = isErc20
+        ? txInfo.tokenSymbol || "?"
+        : nativeCurrencyByChainId(txInfo.chainId);
     const price = isErc20 ? getPrice(rawSymbol) : getPrice("ETH");
     const amountUsd = price ? parseFloat(txInfo.amount) * price : null;
     $("confirm-amount").textContent = valueWithUsd(
@@ -115,7 +117,9 @@ function renderAmount(txInfo) {
 // returns, then the balance it read.
 function renderBalance(txInfo) {
     const isErc20 = txInfo.token !== "ETH";
-    const rawSymbol = isErc20 ? txInfo.tokenSymbol || "?" : nativeCurrency();
+    const rawSymbol = isErc20
+        ? txInfo.tokenSymbol || "?"
+        : nativeCurrencyByChainId(txInfo.chainId);
     const symbol = displaySymbol(rawSymbol);
     if (balances === null) {
         $("confirm-balance").textContent = "Loading...";
@@ -144,15 +148,21 @@ function show(txInfo) {
     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
+    // Settings can switch the active network while the screen is open. This
-    // wait, success and error screens label the amount by it too.
+    // screen and the wait, success and error screens label amounts by it.
     txInfo.chainId = currentNetwork().chainId;
+    // That network's node, read here once and passed to every read below, so
+    // a read still in flight when the network is switched, and the fee
+    // estimate that follows the balance read, stay on it.
+    const provider = getProvider(state.rpcUrl, state.networkId);
 
     const isErc20 = txInfo.token !== "ETH";
     // The raw symbol is the price-table key; the capped one is what the
     // screen says. Truncating before the lookup would silently drop the
     // price of any token whose symbol is long enough to be capped.
-    const rawSymbol = isErc20 ? txInfo.tokenSymbol || "?" : nativeCurrency();
+    const rawSymbol = isErc20
+        ? txInfo.tokenSymbol || "?"
+        : nativeCurrencyByChainId(txInfo.chainId);
     const symbol = displaySymbol(rawSymbol);
 
     // Transaction type
@@ -228,15 +238,15 @@ function show(txInfo) {
     $("confirm-gas-error").classList.toggle("hidden", !isErc20);
     $("confirm-gas-error").textContent =
         "You do not have enough " +
-        nativeCurrency() +
+        nativeCurrencyByChainId(txInfo.chainId) +
         " to pay the network fee for this transfer. Please add " +
-        nativeCurrency() +
+        nativeCurrencyByChainId(txInfo.chainId) +
         " to this address and try again.";
 
     // Shown later, once checkRecipientHistory() finds a contract.
     $("confirm-contract-warning").textContent =
         "WARNING: The recipient is a smart contract. Sending " +
-        nativeCurrency() +
+        nativeCurrencyByChainId(txInfo.chainId) +
         " or tokens directly to a contract may result in permanent loss of" +
         " funds.";
 
@@ -280,18 +290,17 @@ function show(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));
+    readBalances(txInfo, provider).then(() => estimateGas(txInfo, provider));
-    checkRecipientHistory(txInfo);
+    checkRecipientHistory(txInfo, provider);
 }
 
-// Read the From address's balances from the node of the network this screen
+// Read the From address's balances from `provider`, the node of the network
-// is drawn for, the node the fee estimate asks: ETH, and for a token send the
+// this screen is drawn for: ETH, and for a token send the token, at the scale
-// token, at the scale the screen renders the amount with. A token whose scale
+// the screen renders the amount with. A token whose scale is unknown has no
-// is unknown has no balance to read. Never rejects.
+// balance to read. Never rejects.
-async function readBalances(txInfo) {
+async function readBalances(txInfo, provider) {
     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([
@@ -325,7 +334,7 @@ function renderValidation(txInfo) {
     const isErc20 = txInfo.token !== "ETH";
     const symbol = isErc20
         ? displaySymbol(txInfo.tokenSymbol || "?")
-        : nativeCurrency();
+        : 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
@@ -428,9 +437,10 @@ function setVisible(id, visible) {
     $(id).style.visibility = visible ? "visible" : "hidden";
 }
 
-async function estimateGas(txInfo) {
+// Estimate the network fee on `provider`, the node of the network this screen
+// is drawn for.
+async function estimateGas(txInfo, provider) {
     try {
-        const provider = getProvider(state.rpcUrl, state.networkId);
         const feeData = await provider.getFeeData();
         let gasLimit;
 
@@ -478,12 +488,13 @@ async function estimateGas(txInfo) {
         // does, so the same fee reads the same on both.
         if (estimateWei !== null && estimateWei < gasCostWei) {
             $("confirm-fee-amount").textContent =
-                "~" + formatFee(estimateWei, nativeCurrency());
+                "~" +
+                formatFee(estimateWei, nativeCurrencyByChainId(txInfo.chainId));
             $("confirm-fee-reserve").textContent =
                 "up to " +
                 truncateAmountNeverZero(formatEther(gasCostWei)) +
                 " " +
-                nativeCurrency() +
+                nativeCurrencyByChainId(txInfo.chainId) +
                 " reserved";
             setVisible("confirm-fee-reserve", true);
         } else {
@@ -492,7 +503,7 @@ async function estimateGas(txInfo) {
             // cost is not below the reserve. Show the reserve alone.
             $("confirm-fee-amount").textContent = formatFee(
                 gasCostWei,
-                nativeCurrency(),
+                nativeCurrencyByChainId(txInfo.chainId),
             );
             setVisible("confirm-fee-reserve", false);
         }
@@ -580,9 +591,8 @@ function showSendError(message) {
     el.style.visibility = "visible";
 }
 
-async function checkRecipientHistory(txInfo) {
+async function checkRecipientHistory(txInfo, provider) {
     try {
-        const provider = getProvider(state.rpcUrl, state.networkId);
         const asyncWarnings = await getFullWarnings(txInfo.to, provider, {
             fromAddress: txInfo.from,
         });

tests/confirmTxNetworkSwitch.test.js

@@ -11,7 +11,8 @@
 // (https://git.eeqj.de/sneak/AutistMask/issues/558). The popup can reopen onto
 // a confirmation screen saved before the network was switched in another
 // popup, and the screen, drawn again for the new network, showed and checked
-// the balance Send had read on the old one.
+// the balance Send had read on the old one. The balance read and the fee
+// estimate after it stay on that network when it is switched while they run.
 //
 // Driven through the real confirmation screen, signer and chain switch, with
 // only the node and the DOM stubbed. The stub node is on the network its
@@ -21,9 +22,15 @@
 "use strict";
 
 // The balances the stub node reports on each network, by network name: ETH
-// in wei and the token in its base units. And the signed transactions it was
+// in wei and the token in its base units. The networks whose node was asked
-// handed, with its network's chain id.
+// for the fee, in order. And the signed transactions it was handed, with its
-const mockNode = { balanceWei: {}, tokenUnits: {}, broadcast: [] };
+// network's chain id.
+const mockNode = {
+    balanceWei: {},
+    tokenUnits: {},
+    feeAskedOn: [],
+    broadcast: [],
+};
 
 jest.mock("ethers", () => {
     const actual = jest.requireActual("ethers");
@@ -51,6 +58,7 @@ jest.mock("ethers", () => {
             );
         }
         async getFeeData() {
+            mockNode.feeAskedOn.push(this.network.name);
             return {
                 maxFeePerGas: 20000000000n,
                 maxPriorityFeePerGas: 1000000000n,
@@ -188,6 +196,7 @@ beforeEach(() => {
         sepolia: parseEther("5"),
     };
     mockNode.tokenUnits = { mainnet: 0n, sepolia: 0n };
+    mockNode.feeAskedOn = [];
     mockNode.broadcast = [];
     txStatus.showWait.mockClear();
     txStatus.showError.mockClear();
@@ -417,6 +426,33 @@ describe("the balance read", () => {
         expect(sendDisabled()).toBe(false);
     });
 
+    // 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 () => {
+        let answer;
+        const getBalance = jest
+            .spyOn(JsonRpcProvider.prototype, "getBalance")
+            .mockImplementationOnce(() => new Promise((r) => (answer = r)));
+        try {
+            await review("mainnet");
+        } finally {
+            getBalance.mockRestore();
+        }
+        expect(mockNode.feeAskedOn).toEqual([]);
+
+        await onChainSwitch("sepolia");
+        answer(parseEther("5"));
+        await settle();
+
+        expect(mockNode.feeAskedOn).toEqual(["mainnet"]);
+        expect(el("confirm-balance").textContent).toBe("5.0000 ETH");
+        // 21000 gas at 10 gwei expected, at 20 gwei reserved.
+        expect(el("confirm-fee-amount").textContent).toBe("~0.0002 ETH");
+        expect(el("confirm-fee-reserve").textContent).toBe(
+            "up to 0.0004 ETH reserved",
+        );
+    });
+
     test("that fails states the balance as unknown, not zero, and blocks sending", async () => {
         const getBalance = jest
             .spyOn(JsonRpcProvider.prototype, "getBalance")