fix: a network switch in Settings leaves no balance from the previous network stored (closes #653)

The switch is now made on the stored record that its save has just read. The switched fields are then copied into the page's state. A save writes only the fields that the page changed since its last save. A switch made on the page's state alone mishandled what another page stored since then:

- A balance from the previous network stayed under the new network.
- A token tracked on the previous network moved to the new network.
- An endpoint URL saved for the previous network was replaced by the page's older one.

Tests store each of these before the switch and check the stored record after it.

Model: opus-5-5

README.md

@@ -3566,10 +3566,14 @@ Currently supported:
   on every router but `UniversalRouterV1`, and otherwise gives the address. A
   move into the router needs none of them, because its `Paid to` line and
   `Token moved into the router` line already give the amount, the token and
-  where it goes, and so its amount is shown once. Every `PERMIT2_PERMIT` whose
-  amount does not stand as the `Amount` figure, before the step that sets the
-  input side or after it, gives three lines, in step order, after those for each
-  `PERMIT2_TRANSFER_FROM`: `Permit2 allowance`, its amount, for example
+  where it goes, and so its amount is shown once. For a move of the zero
+  address, `Moved from the sender` gives the amount in base units and
+  `Token moved` gives the zero address. For such a move into the router, the
+  `Paid to` line gives the amount in base units and
+  `Token moved into the router` gives the zero address. Every `PERMIT2_PERMIT`
+  whose amount does not stand as the `Amount` figure, before the step that sets
+  the input side or after it, gives three lines, in step order, after those for
+  each `PERMIT2_TRANSFER_FROM`: `Permit2 allowance`, its amount, for example
   `5000.0000 DAI`, or `Unlimited` for Permit2's maximum; `Allowance token`,
   giving the token's address as the `Token In` line does; and
   `Allowance spender`, the address it lets take the token. Only a permit whose

src/shared/chainSwitch.js

@@ -2,8 +2,10 @@
 //
 // A network switch reaches the popup's `state` in one of two ways:
 //
-// - onChainSwitch() below makes the switch the user picks in Settings, with
-//   applyChainSwitchFields() (chainSwitchFields.js).
+// - onChainSwitch() below makes the switch the user picks in Settings. It
+//   makes the switch on the stored record with applyChainSwitchFields(), and
+//   copies the switched fields into `state` with copyChainSwitchFields()
+//   (chainSwitchFields.js).
 // - followStoredSettings() in src/popup/index.js takes a switch the background
 //   or another popup page saved, from the stored record, with
 //   takeStoredNetwork() (state.js), which copies it with
@@ -27,7 +29,10 @@
 // against storage, and the singleton is not reachable from the background
 // bundle at all (enforced by the ESLint rule in eslint.config.js).
 
-const { applyChainSwitchFields } = require("./chainSwitchFields");
+const {
+    applyChainSwitchFields,
+    copyChainSwitchFields,
+} = require("./chainSwitchFields");
 const { clearPrices } = require("./prices");
 
 // The popup's own caches a network switch resets. Prices are chain-specific
@@ -47,12 +52,23 @@ function clearPopupCaches() {
 // lock, so it is never switched while a follow of a switch saved elsewhere
 // waits for the lock (followStoredSettings() in src/popup/index.js): that
 // follow runs first, and this switch is made after it.
+//
+// The switch is made on the stored record that the save has just read, as the
+// background makes its own switch. The switched fields are then copied from
+// that record into `state`. So the switch also covers what another page stored
+// since this page's last save:
+//
+// - A token tracked on the previous network stays tracked on that network.
+// - An endpoint URL saved for the previous network stays with that network.
+// - A balance from the previous network is cleared
+//   (https://git.eeqj.de/sneak/AutistMask/issues/653).
 async function onChainSwitch(newNetworkId) {
     const { state, saveState } = require("./state");
     let net = null;
-    await saveState(() => {
+    await saveState((stored) => {
         if (state.networkId === newNetworkId) return false;
-        net = applyChainSwitchFields(state, newNetworkId);
+        net = applyChainSwitchFields(stored, newNetworkId);
+        copyChainSwitchFields(state, stored);
         clearPopupCaches();
     });
     return net;

src/shared/chainSwitchFields.js

@@ -3,9 +3,10 @@
 //
 // Split out of chainSwitch.js so the background can perform a chain switch
 // without the singleton being reachable from its bundle at all. The popup
-// still goes through onChainSwitch() (chainSwitch.js), which applies this to
-// the singleton and saves; the background applies it to the detached record of
-// its own read-modify-write (src/background/state.js).
+// still goes through onChainSwitch() (chainSwitch.js). That function applies
+// this to the stored record that the switch's save reads, and copies the
+// result into the singleton. The background applies it to the detached record
+// of its own read-modify-write (src/background/state.js).
 //
 // Everything here is synchronous and touches nothing but the objects it is
 // given: no storage, no caches, no imports beyond the network table. That is

src/shared/state.js

@@ -591,8 +591,10 @@ function reportSaveFailure(err) {
 //
 // `beforeWrite`, when given, is called inside the lock with the stored record
 // as just read, before this page's state is compared against it. It may change
-// `state`, and this save writes that change. When it returns false, nothing is
-// written; any other return lets the save go ahead.
+// `state`, and this save writes that change. It may also change the stored
+// record: a field that this page did not change is written as `beforeWrite`
+// left it. When it returns false, nothing is written; any other return lets the
+// save go ahead.
 function saveState(beforeWrite) {
     const turn = withLock("autistmask", () => saveStateOnce(beforeWrite));
     // Every failed save is reported, whether or not the caller awaited this

src/shared/uniswap.js

@@ -479,6 +479,17 @@ function tokenInfo(address, sources) {
     };
 }
 
+// The token of a Permit2 allowance or of a PERMIT2_TRANSFER_FROM, in the
+// shape tokenInfo() gives. Permit2 has no allowance of ETH and moves no ETH.
+// The zero address there reads as a token nothing knows: its amount in base
+// units, and the zero address as its token.
+function permit2TokenInfo(address, sources) {
+    if (address === "0x0000000000000000000000000000000000000000") {
+        return { symbol: null, decimals: null, address };
+    }
+    return tokenInfo(address, sources);
+}
+
 // A swap amount line. With a scale it is the token quantity; with none it is
 // the base-unit integer with the unknown scale stated, the same refusal the
 // ERC-20 amount line makes, so the screen has one way of saying it. `display`
@@ -497,16 +508,18 @@ function amountText(raw, info) {
 
 // The line giving a moved token, or the token of a permit, under `label`, in
 // the shape of the Token In line, so the screens show it as they show that
-// one. ETH has no address.
-function tokenMovedLine(label, token, sources) {
-    const info = tokenInfo(token, sources);
+// one. `info` is what tokenInfo() or permit2TokenInfo() gives for the token.
+// ETH has no address.
+function tokenMovedLine(label, info) {
     if (!present(info.address)) {
         return { label, value: info.symbol };
     }
     return {
         label,
-        value: info.symbol ? info.symbol + " (" + token + ")" : token,
-        address: token,
+        value: info.symbol
+            ? info.symbol + " (" + info.address + ")"
+            : info.address,
+        address: info.address,
         isToken: true,
     };
 }
@@ -2329,7 +2342,9 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                     : UNDECODED_CURRENCY,
             });
             if (present(s.token)) {
-                details.push(tokenMovedLine("Token moved", s.token, sources));
+                details.push(
+                    tokenMovedLine("Token moved", tokenInfo(s.token, sources)),
+                );
                 details.push({
                     label: "Moved to",
                     value: "The swap step that spends it",
@@ -2340,12 +2355,12 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
         // What each PERMIT2_TRANSFER_FROM moves: its amount, its token and
         // where it goes, an address on a line of its own.
         for (const m of movedFromSender) {
+            const info = permit2TokenInfo(m.token, sources);
             details.push({
                 label: "Moved from the sender",
-                value: amountText(m.amount, tokenInfo(m.token, sources))
-                    .display,
+                value: amountText(m.amount, info).display,
             });
-            details.push(tokenMovedLine("Token moved", m.token, sources));
+            details.push(tokenMovedLine("Token moved", info));
             details.push(
                 present(m.to)
                     ? { label: "Moved to", value: m.to }
@@ -2375,36 +2390,15 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                 });
                 continue;
             }
-            // Permit2 has no allowance of ETH. An allowance of the zero
-            // address reads as one of a token nothing knows: its amount in
-            // base units, and the zero address as its token.
-            if (p.token === "0x0000000000000000000000000000000000000000") {
-                details.push({
-                    label: "Permit2 allowance",
-                    value:
-                        p.amount >= MAX_UINT160
-                            ? "Unlimited"
-                            : unknownDecimalsAmount(p.amount),
-                });
-                details.push({
-                    label: "Allowance token",
-                    value: p.token,
-                    address: p.token,
-                    isToken: true,
-                });
-            } else {
-                details.push({
-                    label: "Permit2 allowance",
-                    value:
-                        p.amount >= MAX_UINT160
-                            ? "Unlimited"
-                            : amountText(p.amount, tokenInfo(p.token, sources))
-                                  .display,
-                });
-                details.push(
-                    tokenMovedLine("Allowance token", p.token, sources),
-                );
-            }
+            const info = permit2TokenInfo(p.token, sources);
+            details.push({
+                label: "Permit2 allowance",
+                value:
+                    p.amount >= MAX_UINT160
+                        ? "Unlimited"
+                        : amountText(p.amount, info).display,
+            });
+            details.push(tokenMovedLine("Allowance token", info));
             details.push({
                 label: "Allowance spender",
                 value: p.spender,
@@ -2462,7 +2456,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
             paidTo.push({ label: "Paid to", value: PAID_TO_BRIDGE });
         }
         for (const m of movedIntoRouter) {
-            const info = tokenInfo(m.token, sources);
+            const info = permit2TokenInfo(m.token, sources);
             paidTo.push({
                 label: "Paid to",
                 value:
@@ -2470,9 +2464,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                     amountText(m.amount, info).display +
                     " from the sender (later steps may not spend all of it)",
             });
-            paidTo.push(
-                tokenMovedLine("Token moved into the router", m.token, sources),
-            );
+            paidTo.push(tokenMovedLine("Token moved into the router", info));
         }
         if (outputHeld) {
             paidTo.push({ label: "Paid to", value: STAYS_IN_ROUTER });
@@ -2489,8 +2481,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
             paidTo.push(
                 tokenMovedLine(
                     "Token moved into the router",
-                    getAddress(token),
-                    sources,
+                    tokenInfo(getAddress(token), sources),
                 ),
             );
         }

tests/balanceRefreshNetworkSwitch.test.js

@@ -6,6 +6,9 @@
 // approved a site's request, which changes only the stored record. Driven
 // through the real popup over a stored profile on Ethereum Mainnet. The same
 // refresh in the background is in tests/backgroundStateIsolation.test.js.
+//
+// The last test covers a refresh by the background that is saved just before
+// the switch in Settings (https://git.eeqj.de/sneak/AutistMask/issues/653).
 
 const {
     bootPopup,
@@ -159,3 +162,50 @@ test("a refresh that returns after the background switched the network writes no
 
     expectNothingStored(page);
 });
+
+// The background's refresh saves Ethereum Mainnet's balances while the switch
+// in Settings waits for the lock. This page last saved a balance of "0" and no
+// tokens, the same values the switch sets.
+test("a background refresh saved while a switch in Settings waits for the lock leaves none of its balances stored", async () => {
+    const profile = unversionedValidProfile();
+    profile.wallets[0].addresses[0].balance = "0";
+    const page = await bootPopup(profile);
+    expect(page.pageErrors).toEqual([]);
+    const background = require("../src/background/state");
+    await page.click("btn-settings");
+
+    // The background takes the lock first and holds it until the test calls
+    // `letGo()`. It then saves balances as backgroundRefresh() in
+    // src/background/index.js does.
+    let letGo;
+    const refreshSave = background.updateState(async (fresh) => {
+        await new Promise((resolve) => {
+            letGo = resolve;
+        });
+        fresh.wallets[0].addresses[0].balance = "7.0";
+        fresh.wallets[0].addresses[0].tokenBalances = [
+            {
+                address: TOKEN_ADDRESS,
+                name: "Test Token",
+                symbol: "TST",
+                decimals: 18,
+                balance: "5.0",
+                holders: null,
+            },
+        ];
+        fresh.lastBalanceRefresh = Date.now();
+    });
+    await page.settle();
+
+    const select = page.node("settings-network");
+    select.value = "sepolia";
+    const switching = Promise.all(select.listeners.change.map((fn) => fn()));
+    await page.settle();
+
+    letGo();
+    await refreshSave;
+    await switching;
+    await page.settle();
+
+    expectNothingStored(page);
+});

tests/networkEndpoints.test.js

@@ -47,6 +47,7 @@ function loadModuleWith(persisted) {
     return {
         mod: require("../src/shared/state"),
         chainSwitch: require("../src/shared/chainSwitch"),
+        storage,
         written: () => storage.read("autistmask"),
     };
 }
@@ -105,6 +106,27 @@ describe("a custom endpoint survives a chain switch", () => {
         expect(mod.state.rpcUrl).toBe(CUSTOM_RPC);
     });
 
+    // Another page saves the user's own node for Ethereum Mainnet after this
+    // page's last save. This page's switch keeps that node for Ethereum
+    // Mainnet.
+    test("an endpoint another page saved for the network being left is remembered", async () => {
+        const { mod, chainSwitch, storage, written } = loadModuleWith({
+            wallets: walletFixture(),
+            networkId: "mainnet",
+            rpcUrl: MAINNET.defaultRpcUrl,
+            blockscoutUrl: MAINNET.defaultBlockscoutUrl,
+            networkEndpoints: {},
+        });
+        await mod.loadState();
+        storage.write("autistmask", { ...written(), rpcUrl: CUSTOM_RPC });
+
+        await chainSwitch.onChainSwitch("sepolia");
+        expect(written().networkEndpoints.mainnet.rpcUrl).toBe(CUSTOM_RPC);
+
+        await chainSwitch.onChainSwitch("mainnet");
+        expect(mod.state.rpcUrl).toBe(CUSTOM_RPC);
+    });
+
     test("the remembered endpoints survive an extension restart", async () => {
         const first = loadModuleWith({
             wallets: walletFixture(),

tests/trackedTokensPerNetwork.test.js

@@ -177,6 +177,25 @@ test("a token added by contract address on Sepolia is tracked on Sepolia only",
     expect(onSepolia).not.toContain("DAI");
 });
 
+// Another popup page tracks DAI on Ethereum Mainnet after this page's last
+// save. This page's switch in Settings keeps DAI, tracked on Ethereum Mainnet.
+test("a token another page tracked stays on its network through a switch in Settings", async () => {
+    const boot = await bootPopup(unversionedValidProfile());
+    expect(boot.pageErrors).toEqual([]);
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        trackedTokens: [MAINNET_TOKEN],
+    });
+
+    await boot.click("btn-settings");
+    await chooseNetwork(boot, "sepolia");
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("sepolia");
+    expect(stored.trackedTokens).toEqual([]);
+    expect(stored.networkTrackedTokens.mainnet).toEqual([MAINNET_TOKEN]);
+});
+
 describe("Back after a switch from the Settings gear", () => {
     test("SettingsAddToken no longer offers the common tokens", async () => {
         const boot = await bootOnAddToken("mainnet");

tests/uniswap.test.js

@@ -3440,6 +3440,82 @@ describe("uniswap decoder", () => {
             });
         });
 
+        // https://git.eeqj.de/sneak/AutistMask/issues/685 — Permit2 moves no
+        // ETH, so a PERMIT2_TRANSFER_FROM of the zero address reads as a move
+        // of a token nothing knows. On next at ec6b6b5 its lines read ETH,
+        // with an ETH figure.
+        describe("a PERMIT2_TRANSFER_FROM of the zero address", () => {
+            const MOVED_AMOUNT =
+                "1000000000000000000 base units (decimals unknown)";
+
+            test("to another address names the address", () => {
+                const result = decodeSteps(
+                    [0x00, 0x02],
+                    [V3_600, transfer(ZERO_ADDR, OTHER_ADDR, ONE_WETH)],
+                );
+                expect(linesBetweenTokens(result)).toEqual([
+                    ["Amount", "600.0000 USDT"],
+                    ["Moved from the sender", MOVED_AMOUNT],
+                    ["Token moved", ZERO_ADDR],
+                    ["Moved to", OTHER_ADDR],
+                ]);
+                expect(detail(result, "Token moved").address).toBe(ZERO_ADDR);
+            });
+
+            test("into the router names the address", () => {
+                const result = decodeSteps(
+                    [0x02, 0x00],
+                    [transfer(ZERO_ADDR, ROUTER_ADDR, ONE_WETH), V3_600],
+                );
+                expect(detail(result, "Paid to").value).toBe(
+                    "The router, " +
+                        MOVED_AMOUNT +
+                        " from the sender (later steps may not spend all of it)",
+                );
+                expect(detail(result, "Token moved into the router")).toEqual({
+                    label: "Token moved into the router",
+                    value: ZERO_ADDR,
+                    address: ZERO_ADDR,
+                    isToken: true,
+                });
+            });
+
+            // The V4 step's SETTLE names the user as payer and states no
+            // amount. The step's lines say that the ETH amount is not known.
+            test("after a V4 exact-out step of ETH, also shows that the ETH amount is not known", () => {
+                const v4EthExactOut = encodeV4Swap(
+                    new Uint8Array([
+                        V4_SWAP_EXACT_OUT_SINGLE_ID,
+                        V4_SETTLE_ID,
+                        V4_TAKE_ALL_ID,
+                    ]),
+                    [
+                        encodeV4ExactOutSingle(
+                            ZERO_ADDR,
+                            USDC_ADDR,
+                            590000000n,
+                            ONE_WETH,
+                        ),
+                        encodeV4Settle(ZERO_ADDR),
+                        encodeV4TakeAll(USDC_ADDR, 0n),
+                    ],
+                );
+                const result = decodeSteps(
+                    [0x10, 0x02],
+                    [v4EthExactOut, transfer(ZERO_ADDR, OTHER_ADDR, ONE_WETH)],
+                );
+                expect(detail(result, "Token In").value).toBe("ETH (native)");
+                expect(linesBetweenTokens(result)).toEqual([
+                    ["Moved from the sender", UNDECODED],
+                    ["Token moved", "ETH"],
+                    ["Moved to", "The swap step that spends it"],
+                    ["Moved from the sender", MOVED_AMOUNT],
+                    ["Token moved", ZERO_ADDR],
+                    ["Moved to", OTHER_ADDR],
+                ]);
+            });
+        });
+
         // https://git.eeqj.de/sneak/AutistMask/issues/682 — V4 pays a SETTLE
         // of ETH from the router's own ETH, whichever payer the SETTLE names,
         // and the router holds the transaction's value. On next at 36b2002 a