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

@@ -1277,18 +1277,22 @@ see Transaction Decoding), do not always read as a number. They can read:
   step pays nothing itself and swaps whatever an earlier step sent to the pair,
   so the calldata states no quantity. A V3 exact-in `amountIn` of zero has no
   such meaning and is shown as a zero.
-- `None (no minimum guaranteed)`: the swap's `Min. received` line, when it shows
+- `None (no minimum guaranteed)`: the swap's `Min. received` line, when the
-  a V2, V3 or V4 swap's minimum of zero. The router passes a
+  minimum it shows is zero, whether a V2, V3 or V4 swap's minimum or a
-  `BALANCE_CHECK_ERC20` step whenever the balance is at least `minBalance`, so a
+  `BALANCE_CHECK_ERC20` step's `minBalance`. Before
-  zero `minBalance` guarantees nothing. A check with a zero `minBalance` sets
+  [#359](https://git.eeqj.de/sneak/AutistMask/issues/359), a zero `minBalance`
-  neither `Token Out` nor `Min. received`. A nonzero `minBalance` sets both
+  read `0.0000` when the token's scale was known. The router passes a balance
-  lines, as a swap step does. Only a check of the transaction's sender's own
+  check whenever the balance is at least `minBalance`, so a zero `minBalance`
-  balance sets either line; a check of any other address sets neither (see
+  guarantees nothing: it sets `Token Out` and `Min. received` only when the
-  Transaction Decoding). A `BALANCE_CHECK_ERC20` step that the router lets fail
+  output side holds no minimum, not even a zero one, at the point the check is
-  without reverting the transaction (see Transaction Decoding) guarantees
+  reached, and otherwise leaves the current token and figure in place. A nonzero
-  nothing either, whatever its `minBalance`, and is read as one whose
+  `minBalance` sets both lines, as a swap step does. Only a check of the
-  `minBalance` is zero. A minimum stated by a V4 swap step the router lets fail
+  transaction's sender's own balance sets either line; a check of any other
-  is likewise read as zero.
+  address sets neither (see Transaction Decoding). A `BALANCE_CHECK_ERC20` step
+  that the router lets fail without reverting the transaction (see Transaction
+  Decoding) guarantees nothing either, whatever its `minBalance`, and is read as
+  one whose `minBalance` is zero. A minimum stated by a V4 swap step the router
+  lets fail is likewise read as zero.
 
 The swap's `Token In` and `Token Out` lines name a currency, not an amount; each
 reads `Unknown (not named in the calldata)` when the decoder found no token for
@@ -3393,8 +3397,7 @@ Currently supported:
   `Balance Check (of another address)`. A check whose input does not decode,
   such as one whose owner word has a nonzero top byte (the router reads the
   owner from the word's last 20 bytes), likewise sets neither, and it is listed
-  as `Balance Check (could not be decoded)`. A check with a zero `minBalance`
+  as `Balance Check (could not be decoded)`. When a swap step, `TAKE` or
-  sets neither `Token Out` nor the minimum. When a swap step, `TAKE` or
   `TAKE_PORTION` pays the router any token, the swap's output or another, a
   `WRAP_ETH` pays it WETH, or an `UNWRAP_WETH` into the router turns WETH a step
   paid it into ETH, that token stays in the router unless a later step pays out

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
+// - onChainSwitch() below makes the switch the user picks in Settings. It
-//   applyChainSwitchFields() (chainSwitchFields.js).
+//   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
@@ -48,29 +53,22 @@ function clearPopupCaches() {
 // waits for the lock (followStoredSettings() in src/popup/index.js): that
 // follow runs first, and this switch is made after it.
 //
-// The save writes only the fields that this page changed since its last save.
+// The switch is made on the stored record that the save has just read, as the
-// When this page last saved a balance of "0", clearing that balance is no
+// background makes its own switch. The switched fields are then copied from
-// change. So the switch also resets these fields in the stored record:
+// that record into `state`. So the switch also covers what another page stored
+// since this page's last save:
 //
-// - every address's balance and token balances;
+// - A token tracked on the previous network stays tracked on that network.
-// - the time of the last balance refresh.
+// - An endpoint URL saved for the previous network stays with that network.
-//
+// - A balance from the previous network is cleared
-// A balance that the background or another page stored since this page's last
+//   (https://git.eeqj.de/sneak/AutistMask/issues/653).
-// save is then not kept under the new network
-// (https://git.eeqj.de/sneak/AutistMask/issues/653).
 async function onChainSwitch(newNetworkId) {
     const { state, saveState } = require("./state");
     let net = null;
     await saveState((stored) => {
         if (state.networkId === newNetworkId) return false;
-        net = applyChainSwitchFields(state, newNetworkId);
+        net = applyChainSwitchFields(stored, newNetworkId);
-        stored.lastBalanceRefresh = 0;
+        copyChainSwitchFields(state, stored);
-        for (const wallet of stored.wallets) {
-            for (const addr of wallet.addresses) {
-                addr.balance = "0";
-                addr.tokenBalances = [];
-            }
-        }
         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
+// still goes through onChainSwitch() (chainSwitch.js). That function applies
-// the singleton and saves; the background applies it to the detached record of
+// this to the stored record that the switch's save reads, and copies the
-// its own read-modify-write (src/background/state.js).
+// 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/uniswap.js

@@ -1629,13 +1629,13 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                     } else {
                         // The router passes this check whenever the owner
                         // holds at least minBalance, so a zero one guarantees
-                        // nothing. A check the router lets fail guarantees
+                        // nothing and does not replace a minimum an earlier
-                        // nothing either, so it is read as a zero one. A zero
+                        // step stated. A check the router lets fail guarantees
-                        // check sets neither Token Out nor the minimum. Any
+                        // nothing either, so it is read as a zero one. Any
                         // other minBalance sets the output side as a swap
-                        // does. The check pays nobody.
+                        // does. It pays nobody.
                         const minBalance = allowedToFail ? 0n : b.minBalance;
-                        if (minBalance !== 0n) {
+                        if (!(minBalance === 0n && present(minOutput))) {
                             setOutput(i, b.token, minBalance, []);
                         }
                         keepsRouterWeth = true;

tests/balanceRefreshNetworkSwitch.test.js

@@ -7,9 +7,8 @@
 // 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 balance refresh by the background that is saved while
+// The last test covers a refresh by the background that is saved just before
-// a switch in Settings waits for the lock
+// the switch in Settings (https://git.eeqj.de/sneak/AutistMask/issues/653).
-// (https://git.eeqj.de/sneak/AutistMask/issues/653).
 
 const {
     bootPopup,
@@ -166,7 +165,7 @@ test("a refresh that returns after the background switched the network writes no
 
 // 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
-// token balances, the values that the switch sets.
+// 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";

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

@@ -178,9 +178,8 @@ test("a token added by contract address on Sepolia is tracked on Sepolia only",
 });
 
 // Another popup page tracks DAI on Ethereum Mainnet after this page's last
-// save. The switch's save writes only the token lists that this page changed.
+// save. This page's switch in Settings keeps DAI, tracked on Ethereum Mainnet.
-// So DAI stays in `trackedTokens`, the active network's list.
+test("a token another page tracked stays on its network through a switch in Settings", async () => {
-test("a token another page tracked before a switch in Settings stays in trackedTokens", async () => {
     const boot = await bootPopup(unversionedValidProfile());
     expect(boot.pageErrors).toEqual([]);
     boot.storage.write("autistmask", {
@@ -191,62 +190,10 @@ test("a token another page tracked before a switch in Settings stays in trackedT
     await boot.click("btn-settings");
     await chooseNetwork(boot, "sepolia");
 
-    const stored = boot.storage.read("autistmask");
-    expect(stored.networkId).toBe("sepolia");
-    expect(stored.trackedTokens).toEqual([MAINNET_TOKEN]);
-    expect(stored.networkTrackedTokens.mainnet).toEqual([]);
-});
-
-// The contract lookup on SettingsAddToken answers on Ethereum Mainnet while the
-// switch to Sepolia waits for the lock. The page adds the token to Ethereum
-// Mainnet's list, and the switch then keeps that list for Ethereum Mainnet.
-test("a token added while a switch in Settings waits for the lock stays tracked on Ethereum Mainnet", async () => {
-    const boot = await bootOnAddToken("mainnet");
-    expect(boot.pageErrors).toEqual([]);
-    const { lookupTokenInfo } = require("../src/shared/balances");
-    let answer;
-    lookupTokenInfo.mockImplementationOnce(
-        () =>
-            new Promise((resolve) => {
-                answer = resolve;
-            }),
-    );
-    boot.node("settings-addtoken-address").value = NEW_TOKEN_ADDRESS;
-    const adding = boot.click("btn-settings-addtoken-manual");
-    await boot.click("btn-settings");
-
-    // Another write holds the lock, so the switch's save waits for it.
-    let releaseLock;
-    const lockHeld = navigator.locks.request(
-        "autistmask",
-        () =>
-            new Promise((resolve) => {
-                releaseLock = resolve;
-            }),
-    );
-    const select = boot.node("settings-network");
-    select.value = "sepolia";
-    const switching = Promise.all(select.listeners.change.map((fn) => fn()));
-    await boot.settle();
-
-    answer({ symbol: "NEW", decimals: 18, name: "New Token" });
-    await boot.settle();
-    releaseLock();
-    await lockHeld;
-    await switching;
-    await adding;
-
     const stored = boot.storage.read("autistmask");
     expect(stored.networkId).toBe("sepolia");
     expect(stored.trackedTokens).toEqual([]);
-    expect(stored.networkTrackedTokens.mainnet).toEqual([
+    expect(stored.networkTrackedTokens.mainnet).toEqual([MAINNET_TOKEN]);
-        {
-            address: NEW_TOKEN_ADDRESS,
-            symbol: "NEW",
-            decimals: 18,
-            name: "New Token",
-        },
-    ]);
 });
 
 describe("Back after a switch from the Settings gear", () => {

tests/uniswap.test.js

@@ -4380,9 +4380,10 @@ describe("uniswap decoder", () => {
         expect(detail(result, "Min. received").value).toBe("2.0000 USDC");
     });
 
-    // A zero balance check sets neither Token Out nor the minimum. The route
+    // With no minimum stated before it, a zero balance check is what sets the
-    // pays only the WRAP_ETH's WETH, to the router, where it stays.
+    // output side, and it guarantees nothing. No step pays anyone, so the
-    test("a zero balance check after a WRAP_ETH into the router names no output token and no minimum", () => {
+    // minimum is not said to be received.
+    test("a zero balance check with no earlier minimum states no minimum", () => {
         const data = buildExecute(
             solidityPacked(["uint8", "uint8"], [0x0b, 0x0e]),
             [
@@ -4395,16 +4396,9 @@ describe("uniswap decoder", () => {
         const result = uniswap.decode(data, ROUTER_ADDR, "0x1", {}, USER_ADDR);
         expect(result).not.toBeNull();
 
-        expect(detail(result, "Token Out").value).toBe(
+        expect(detail(result, "Token Out").value).toContain("USDT");
-            "Unknown (not named in the calldata)",
+        expect(detail(result, "Min. paid out").value).toBe(
-        );
+            "None (no minimum guaranteed)",
-        expect(detail(result, "Min. paid out")).toBeUndefined();
-        expect(detail(result, "Min. received")).toBeUndefined();
-        expect(detail(result, "Paid to").value).toBe(
-            "The router, where it stays (no later step pays it out)",
-        );
-        expect(detail(result, "Token moved into the router").address).toBe(
-            WETH_ADDR,
         );
     });
 

tests/uniswapRecipient.test.js

@@ -688,62 +688,6 @@ describe("an UNWRAP_WETH makes the output ETH only when the router holds it", ()
         ]);
     });
 
-    // The same route, then a balance check of the sender's USDC with a zero
-    // minimum. The check guarantees nothing, so Token Out stays ETH
-    // (https://git.eeqj.de/sneak/AutistMask/issues/649).
-    test("the same route, then a zero balance check of USDC, still reads as ETH with no minimum", () => {
-        const result = decode(
-            execute(
-                v4Swap(
-                    SWAP_USDT_FOR_WETH,
-                    SETTLE_USDT,
-                    take(WETH, MSG_SENDER, (ONE_WETH * 6n) / 10n),
-                    take(WETH, ADDRESS_THIS),
-                ),
-                unwrapWeth(MSG_SENDER),
-                balanceCheck(USDC, 0n),
-            ),
-        );
-        expect(result.name).toBe("Swap USDT → ETH");
-        expect(detail(result, "Token Out").value).toBe("ETH");
-        expect(labels(result)).toEqual([
-            "Protocol",
-            "Token In",
-            "Amount",
-            "Token Out",
-            "Steps",
-            "Deadline",
-        ]);
-    });
-
-    // A PERMIT2_TRANSFER_FROM moves the sender's WETH into the router. An
-    // UNWRAP_WETH pays that WETH to the user as ETH. A zero balance check of
-    // USDC sets neither Token Out nor the minimum, before the UNWRAP_WETH or
-    // after it (https://git.eeqj.de/sneak/AutistMask/issues/649).
-    test.each([
-        {
-            kind: "before the UNWRAP_WETH",
-            steps: [balanceCheck(USDC, 0n), unwrapWeth(MSG_SENDER)],
-        },
-        {
-            kind: "after the UNWRAP_WETH",
-            steps: [unwrapWeth(MSG_SENDER), balanceCheck(USDC, 0n)],
-        },
-    ])(
-        "WETH moved into the router and unwrapped, with a zero balance check of USDC $kind, reads as ETH with no minimum",
-        ({ steps }) => {
-            const result = decode(
-                execute(
-                    permit2TransferFrom(WETH, ADDRESS_THIS, ONE_WETH),
-                    ...steps,
-                ),
-            );
-            expect(detail(result, "Token Out").value).toBe("ETH");
-            expect(detail(result, "Min. received")).toBeUndefined();
-            expect(detail(result, "Min. paid out")).toBeUndefined();
-        },
-    );
-
     // A TAKE the decoder cannot read may pay part of the WETH to the user.
     test("one V4 step paying WETH to the router beside a TAKE that cannot be read, then an UNWRAP_WETH, reads as ETH with no minimum", () => {
         const result = decode(
@@ -2876,16 +2820,17 @@ describe("the allow-revert flag", () => {
         },
     );
 
-    test("a BALANCE_CHECK_ERC20 with the flag, and no minimum before it, names no output token and no minimum", () => {
+    test("a BALANCE_CHECK_ERC20 with the flag, and no minimum before it, states none", () => {
         const check = balanceCheck(WETH, ONE_WETH);
         expect(detail(decode(execute(check)), "Min. paid out").value).toBe(
             "1.0000 WETH",
         );
 
         const withFlag = decode(execute(flagged(check)));
-        expect(detail(withFlag, "Token Out").value).toBe(UNNAMED);
+        expect(detail(withFlag, "Token Out").address).toBe(WETH);
-        expect(detail(withFlag, "Min. paid out")).toBeUndefined();
+        expect(detail(withFlag, "Min. paid out").value).toBe(
-        expect(detail(withFlag, "Min. received")).toBeUndefined();
+            "None (no minimum guaranteed)",
+        );
     });
 
     // Only the routers from UniversalRouterV2 on let a PERMIT2_PERMIT fail.