fix: two pages switching networks no longer lose tracked tokens (closes #701)

A page that had not yet taken another page's network switch could lose
tracked tokens.

- Its save wrote its network's list as the new network's list. saveState()
  now keeps that list and merges the change into its own network's list in
  networkTrackedTokens.
- Its own switch wrote its old per-network lists over the stored ones.
  onChainSwitch() now first takes the stored switch with takeStoredNetwork().
- takeStoredNetwork() dropped the page's unsaved change. It now keeps it in
  the list of the page's network. This also fixes
  #706.
- A page already on the stored network now takes the stored
  networkTrackedTokens before it switches, because another page's save may
  have changed a list this page holds.

Model: opus-5-5

src/shared/chainSwitch.js

@@ -61,20 +61,40 @@ function clearPopupCaches() {
 // (https://git.eeqj.de/sneak/AutistMask/issues/653,
 // https://git.eeqj.de/sneak/AutistMask/issues/692).
 //
+// When the stored record is on another network, a switch saved elsewhere has
+// not reached this page yet. This page first takes that switch with
+// takeStoredNetwork() (state.js), as followStoredSettings() does. The switch to
+// `newNetworkId` is then made from the stored record's network. So every
+// network's list of tracked tokens in the stored record is kept
+// (https://git.eeqj.de/sneak/AutistMask/issues/701). When the stored record is
+// already on `newNetworkId`, the switch made from there only resets the
+// balances and caches.
+//
+// When the stored record is on this page's network, this page first takes the
+// stored `networkTrackedTokens` (takeStoredNetworkTrackedTokens() in state.js),
+// because another page's save may have changed a list this page holds.
+//
 // Before the switch, `state` takes the tracked tokens that another page stored
 // since this page's last save (takeStoredTrackedTokens() in state.js). The
 // switch then puts them away under the previous network, the network they were
-// added on. When the stored record is on another network, a switch saved
-// elsewhere has not reached this page yet. The stored record's tokens are then
-// that network's, and none are taken.
+// added on.
 async function onChainSwitch(newNetworkId) {
-    const { state, saveState, takeStoredTrackedTokens } = require("./state");
+    const {
+        state,
+        saveState,
+        takeStoredNetwork,
+        takeStoredTrackedTokens,
+        takeStoredNetworkTrackedTokens,
+    } = require("./state");
     let net = null;
     await saveState((stored) => {
         if (state.networkId === newNetworkId) return false;
-        if (stored.networkId === state.networkId) {
-            takeStoredTrackedTokens(stored);
+        if (stored.networkId !== state.networkId) {
+            takeStoredNetwork(stored);
+        } else {
+            takeStoredNetworkTrackedTokens(stored);
         }
+        takeStoredTrackedTokens(stored);
         net = applyChainSwitchFields(state, newNetworkId);
         stored.lastBalanceRefresh = 0;
         stored.fraudContracts = [];

src/shared/state.js

@@ -127,10 +127,10 @@ function currentNetwork() {
 
 // `baseline` holds the persisted fields as this page held them when its last
 // loadState() finished, or when its last successful saveState() began.
-// takeStoredNetwork() and takeStoredTrackedTokens() also write into it what
-// this page has taken from the stored record since then. saveState() diffs the
-// live state against `baseline` to find only the fields THIS page changed
-// since.
+// takeStoredNetwork(), takeStoredTrackedTokens() and
+// takeStoredNetworkTrackedTokens() also write into it what this page has taken
+// from the stored record since then. saveState() diffs the live state against
+// `baseline` to find only the fields THIS page changed since.
 //
 // Deep-cloned, not a reference: callers mutate persisted objects and arrays
 // in place (state.wallets.push(...)), and a reference baseline would mutate
@@ -402,6 +402,23 @@ function mergeNetworkEndpoints(base, ours, theirs) {
     return mergeMapByKey(base, ours, theirs, mergeEndpointEntry);
 }
 
+// Returns a copy of `stored.networkTrackedTokens` for when this page has not yet
+// taken the switch in `stored`. `page` is this page's state. A token that this
+// page tracked or removed since its last save is tracked or removed in the list
+// of `page.networkId`, by contract address.
+function networkTrackedTokensWithChange(stored, page) {
+    return {
+        ...stored.networkTrackedTokens,
+        [page.networkId]: mergeListByIdentity(
+            baseline.trackedTokens,
+            page.trackedTokens,
+            stored.networkTrackedTokens[page.networkId],
+            (token) => token.address.toLowerCase(),
+            (base, ours) => ours,
+        ),
+    };
+}
+
 // Read-modify-write, merged per field, rather than one full-blob write.
 //
 // Every extension page (the toolbar popup, a dApp approval window) holds its
@@ -455,6 +472,11 @@ function mergeNetworkEndpoints(base, ours, theirs) {
 // own UI scratch space, not data another page has any reason to share
 // membership of.
 //
+// One exception: when the stored record is on another network than this
+// page's, a save keeps the stored `trackedTokens`. It merges this page's change
+// into the list in `networkTrackedTokens` for this page's network, by contract
+// address.
+//
 // This does not make two pages that both change the SAME leaf concurrently
 // safe: last write wins on that one leaf, same as before. What it removes
 // is the cross-field (and now cross-membership-vs-leaf) clobber — a page
@@ -527,6 +549,20 @@ async function saveStateOnce(beforeWrite) {
             merged[key] = current[key];
         }
     }
+    // When the merged record names a network other than this page's, this page
+    // has not yet taken a switch saved elsewhere. This page's `trackedTokens`
+    // is then the list of this page's network. The stored record keeps the
+    // list of this page's network in `networkTrackedTokens`. So a token that
+    // this page added or removed since its last save is added to or removed
+    // from that stored list. The merged record keeps the stored `trackedTokens`
+    // (https://git.eeqj.de/sneak/AutistMask/issues/701).
+    if (merged.networkId !== current.networkId) {
+        merged.trackedTokens = fresh.trackedTokens;
+        merged.networkTrackedTokens = networkTrackedTokensWithChange(
+            fresh,
+            current,
+        );
+    }
     merged.hasWallet = Boolean(merged.wallets && merged.wallets.length > 0);
     // Stamped on every write, never merged or diffed: the record that goes to
     // storage is in THIS build's shape whatever shape it was read in, which is
@@ -607,15 +643,27 @@ function saveState(beforeWrite) {
     return turn;
 }
 
-// Give this page the network switch held by `stored`, a record a save of this
-// page has just read (followStoredSettings() in src/popup/index.js calls this
-// from `beforeWrite`), in `state` and in `baseline` alike, as loadState() does
-// with a whole record. The switch is then not this page's own change, even when
+// Give this page the network switch held by `stored`, in `state` and in
+// `baseline` alike, as loadState() does with a whole record. `stored` is the
+// record that a save of this page has just read. followStoredSettings() in
+// src/popup/index.js and onChainSwitch() in src/shared/chainSwitch.js call this
+// from `beforeWrite`. The switch is then not this page's own change, even when
 // that save fails to write: a later switch back to the previous network still
 // differs from `baseline`, and is stored.
+//
+// A token that this page tracked or removed since its last save was tracked or
+// removed on this page's network before the switch. `state` keeps that change
+// in the list in `networkTrackedTokens` for that network, merged with the
+// stored list by contract address. `baseline` takes the stored list, so the
+// save stores the change (https://git.eeqj.de/sneak/AutistMask/issues/706).
 function takeStoredNetwork(stored) {
+    const networkTrackedTokens = networkTrackedTokensWithChange(
+        stored,
+        rawState,
+    );
     copyChainSwitchFields(rawState, stored);
     copyChainSwitchFields(baseline, structuredClone(stored));
+    rawState.networkTrackedTokens = networkTrackedTokens;
 }
 
 // Give this page the tracked tokens of `stored`, a record on this page's
@@ -640,6 +688,19 @@ function takeStoredTrackedTokens(stored) {
     baseline.trackedTokens = structuredClone(stored.trackedTokens);
 }
 
+// Give `state` and `baseline` each their own copy of the stored
+// `networkTrackedTokens`. `stored` is a record on this page's network that a
+// save of this page has just read. onChainSwitch() in
+// src/shared/chainSwitch.js calls this from `beforeWrite`.
+function takeStoredNetworkTrackedTokens(stored) {
+    rawState.networkTrackedTokens = structuredClone(
+        stored.networkTrackedTokens,
+    );
+    baseline.networkTrackedTokens = structuredClone(
+        stored.networkTrackedTokens,
+    );
+}
+
 // Rejects with StateUnusableError for a stored record this build cannot make
 // sense of. Nothing is assigned and `loaded` stays false in that case, so a
 // caller that ignores the rejection gets StateNotLoadedError on the first
@@ -687,6 +748,7 @@ module.exports = {
     onSaveFailure,
     takeStoredNetwork,
     takeStoredTrackedTokens,
+    takeStoredNetworkTrackedTokens,
     loadState,
     currentAddress,
     currentNetwork,

tests/trackedTokensPerNetwork.test.js

@@ -267,6 +267,190 @@ test("a token tracked on Sepolia when another page switched there is not tracked
     expect(stored.networkTrackedTokens.mainnet).toEqual([]);
 });
 
+// Another popup page saves a switch to Sepolia, where TST is tracked. Before
+// taking that switch, this page adds a token on Ethereum Mainnet. The add's
+// save adds the token to Ethereum Mainnet's list and leaves Sepolia's list as
+// it was.
+test("a token added before this page takes another page's switch is tracked on Ethereum Mainnet", async () => {
+    const boot = await bootOnAddToken("mainnet", {
+        trackedTokens: [MAINNET_TOKEN],
+        networkTrackedTokens: { sepolia: [SEPOLIA_TOKEN] },
+    });
+    expect(boot.pageErrors).toEqual([]);
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        networkId: "sepolia",
+        trackedTokens: [SEPOLIA_TOKEN],
+        networkTrackedTokens: {
+            mainnet: [MAINNET_TOKEN],
+            sepolia: [SEPOLIA_TOKEN],
+        },
+    });
+    const { lookupTokenInfo } = require("../src/shared/balances");
+    lookupTokenInfo.mockResolvedValueOnce({
+        symbol: "NEW",
+        decimals: 18,
+        name: "New Token",
+    });
+
+    boot.node("settings-addtoken-address").value = NEW_TOKEN_ADDRESS;
+    await boot.click("btn-settings-addtoken-manual");
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("sepolia");
+    expect(stored.trackedTokens).toEqual([SEPOLIA_TOKEN]);
+    expect(stored.networkTrackedTokens.mainnet).toEqual([
+        MAINNET_TOKEN,
+        {
+            address: NEW_TOKEN_ADDRESS,
+            symbol: "NEW",
+            decimals: 18,
+            name: "New Token",
+        },
+    ]);
+});
+
+// Another popup page tracks DAI on Ethereum Mainnet and then saves a switch to
+// Sepolia. This page does not track DAI. Before taking that switch, this page
+// adds a token on Ethereum Mainnet. The add's save puts both DAI and the new
+// token on Ethereum Mainnet's list.
+test("a token added before this page takes another page's switch joins that page's tokens on Ethereum Mainnet", async () => {
+    const boot = await bootOnAddToken("mainnet");
+    expect(boot.pageErrors).toEqual([]);
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        networkId: "sepolia",
+        trackedTokens: [],
+        networkTrackedTokens: { mainnet: [MAINNET_TOKEN] },
+    });
+    const { lookupTokenInfo } = require("../src/shared/balances");
+    lookupTokenInfo.mockResolvedValueOnce({
+        symbol: "NEW",
+        decimals: 18,
+        name: "New Token",
+    });
+
+    boot.node("settings-addtoken-address").value = NEW_TOKEN_ADDRESS;
+    await boot.click("btn-settings-addtoken-manual");
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("sepolia");
+    expect(stored.trackedTokens).toEqual([]);
+    expect(stored.networkTrackedTokens.mainnet).toEqual([
+        MAINNET_TOKEN,
+        {
+            address: NEW_TOKEN_ADDRESS,
+            symbol: "NEW",
+            decimals: 18,
+            name: "New Token",
+        },
+    ]);
+});
+
+// Another popup page saves a switch to Sepolia, where TST is tracked. Before
+// taking that switch, this page removes DAI on Ethereum Mainnet, as the `[x]`
+// on a tracked token in Settings does. The removal's save drops DAI from
+// Ethereum Mainnet's list and leaves Sepolia's list as it was.
+test("a token removed before this page takes another page's switch is gone from Ethereum Mainnet", async () => {
+    const boot = await bootPopup(
+        unversionedValidProfile({
+            trackedTokens: [MAINNET_TOKEN],
+            networkTrackedTokens: { sepolia: [SEPOLIA_TOKEN] },
+        }),
+    );
+    expect(boot.pageErrors).toEqual([]);
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        networkId: "sepolia",
+        trackedTokens: [SEPOLIA_TOKEN],
+        networkTrackedTokens: {
+            mainnet: [MAINNET_TOKEN],
+            sepolia: [SEPOLIA_TOKEN],
+        },
+    });
+
+    const { state, saveState } = require("../src/shared/state");
+    state.trackedTokens.splice(0, 1);
+    await saveState();
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("sepolia");
+    expect(stored.trackedTokens).toEqual([SEPOLIA_TOKEN]);
+    expect(stored.networkTrackedTokens.mainnet).toEqual([]);
+});
+
+// Another popup page tracks DAI on Ethereum Mainnet and then saves a switch to
+// Sepolia. That switch puts DAI away under Ethereum Mainnet. This page has not
+// taken the switch yet. A switch to Sepolia in this page's Settings keeps DAI
+// on Ethereum Mainnet's list.
+test("a token another page tracked before its own switch stays tracked on Ethereum Mainnet", async () => {
+    const boot = await bootPopup(unversionedValidProfile());
+    expect(boot.pageErrors).toEqual([]);
+    await boot.click("btn-settings");
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        networkId: "sepolia",
+        trackedTokens: [],
+        networkTrackedTokens: { mainnet: [MAINNET_TOKEN] },
+    });
+
+    await chooseNetwork(boot, "sepolia");
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("sepolia");
+    expect(stored.networkTrackedTokens.mainnet).toEqual([MAINNET_TOKEN]);
+});
+
+// This page switches to Sepolia. Another popup page has not taken that switch,
+// and its save adds a token to Ethereum Mainnet's list. This page switches back
+// and tracks that token.
+test("a token another page added on Ethereum Mainnet is tracked when this page switches back there", async () => {
+    const boot = await bootPopup(
+        unversionedValidProfile({ trackedTokens: [MAINNET_TOKEN] }),
+    );
+    expect(boot.pageErrors).toEqual([]);
+    await boot.click("btn-settings");
+    await chooseNetwork(boot, "sepolia");
+    const newToken = {
+        address: NEW_TOKEN_ADDRESS,
+        symbol: "NEW",
+        decimals: 18,
+        name: "New Token",
+    };
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        networkTrackedTokens: { mainnet: [MAINNET_TOKEN, newToken] },
+    });
+
+    await chooseNetwork(boot, "mainnet");
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("mainnet");
+    expect(stored.trackedTokens).toEqual([MAINNET_TOKEN, newToken]);
+});
+
+// This page tracks DAI on Ethereum Mainnet and switches to Sepolia. Another
+// popup page has not taken that switch, and its save removes DAI from Ethereum
+// Mainnet's list. This page switches back and does not track DAI again.
+test("a token another page removed on Ethereum Mainnet stays removed when this page switches back there", async () => {
+    const boot = await bootPopup(
+        unversionedValidProfile({ trackedTokens: [MAINNET_TOKEN] }),
+    );
+    expect(boot.pageErrors).toEqual([]);
+    await boot.click("btn-settings");
+    await chooseNetwork(boot, "sepolia");
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        networkTrackedTokens: { mainnet: [] },
+    });
+
+    await chooseNetwork(boot, "mainnet");
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("mainnet");
+    expect(stored.trackedTokens).toEqual([]);
+});
+
 // Another popup page records a fraud contract on Ethereum Mainnet after this
 // page's last save. The switch to Sepolia drops that fraud contract, as the
 // switch drops this page's own.
@@ -338,6 +522,162 @@ test("a token added while a switch in Settings waits for the lock stays tracked
     ]);
 });
 
+// The contract lookup on SettingsAddToken answers on Ethereum Mainnet while
+// this page's switch to Sepolia waits for the lock. Another popup page holds
+// the lock and stores its own switch to Sepolia, with DAI tracked on Ethereum
+// Mainnet. This page's switch keeps DAI and the new token on Ethereum Mainnet's
+// list.
+test("a token added while a switch in Settings waits for another page's switch 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");
+
+    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();
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        networkId: "sepolia",
+        trackedTokens: [],
+        networkTrackedTokens: { mainnet: [MAINNET_TOKEN] },
+    });
+
+    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([
+        MAINNET_TOKEN,
+        {
+            address: NEW_TOKEN_ADDRESS,
+            symbol: "NEW",
+            decimals: 18,
+            name: "New Token",
+        },
+    ]);
+});
+
+// Another popup page holds the lock and stores its switch to Sepolia, with DAI
+// tracked on Ethereum Mainnet. This page waits for the lock to take that
+// switch. Meanwhile, the contract lookup on SettingsAddToken answers on
+// Ethereum Mainnet. When this page takes that switch, DAI and the new token
+// stay on Ethereum Mainnet's list.
+test("a token added while this page waits to take another page's switch stays tracked on Ethereum Mainnet", async () => {
+    const boot = await bootOnAddToken("mainnet");
+    expect(boot.pageErrors).toEqual([]);
+    let releaseLock;
+    const lockHeld = navigator.locks.request(
+        "autistmask",
+        () =>
+            new Promise((resolve) => {
+                releaseLock = resolve;
+            }),
+    );
+    await boot.storage.local.set({
+        autistmask: {
+            ...boot.storage.read("autistmask"),
+            networkId: "sepolia",
+            trackedTokens: [],
+            networkTrackedTokens: { mainnet: [MAINNET_TOKEN] },
+        },
+    });
+    await boot.settle();
+    const { lookupTokenInfo } = require("../src/shared/balances");
+    lookupTokenInfo.mockResolvedValueOnce({
+        symbol: "NEW",
+        decimals: 18,
+        name: "New Token",
+    });
+
+    boot.node("settings-addtoken-address").value = NEW_TOKEN_ADDRESS;
+    const adding = boot.click("btn-settings-addtoken-manual");
+    await boot.settle();
+    releaseLock();
+    await lockHeld;
+    await adding;
+    await boot.settle();
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("sepolia");
+    expect(stored.trackedTokens).toEqual([]);
+    expect(stored.networkTrackedTokens.mainnet).toEqual([
+        MAINNET_TOKEN,
+        {
+            address: NEW_TOKEN_ADDRESS,
+            symbol: "NEW",
+            decimals: 18,
+            name: "New Token",
+        },
+    ]);
+});
+
+// This page tracks DAI on Ethereum Mainnet. Another popup page holds the lock
+// and stores its switch to Sepolia, with DAI on Ethereum Mainnet's list. This
+// page waits for the lock to take that switch. Meanwhile, the user removes DAI,
+// as the `[x]` on a tracked token in Settings does. When this page takes that
+// switch, DAI is gone from Ethereum Mainnet's list.
+test("a token removed while this page waits to take another page's switch is gone from Ethereum Mainnet", async () => {
+    const boot = await bootPopup(
+        unversionedValidProfile({ trackedTokens: [MAINNET_TOKEN] }),
+    );
+    expect(boot.pageErrors).toEqual([]);
+    let releaseLock;
+    const lockHeld = navigator.locks.request(
+        "autistmask",
+        () =>
+            new Promise((resolve) => {
+                releaseLock = resolve;
+            }),
+    );
+    await boot.storage.local.set({
+        autistmask: {
+            ...boot.storage.read("autistmask"),
+            networkId: "sepolia",
+            trackedTokens: [],
+            networkTrackedTokens: { mainnet: [MAINNET_TOKEN] },
+        },
+    });
+    await boot.settle();
+
+    const { state, saveState } = require("../src/shared/state");
+    state.trackedTokens.splice(0, 1);
+    const removing = saveState();
+    releaseLock();
+    await lockHeld;
+    await removing;
+    await boot.settle();
+
+    const stored = boot.storage.read("autistmask");
+    expect(stored.networkId).toBe("sepolia");
+    expect(stored.trackedTokens).toEqual([]);
+    expect(stored.networkTrackedTokens.mainnet).toEqual([]);
+});
+
 describe("Back after a switch from the Settings gear", () => {
     test("SettingsAddToken no longer offers the common tokens", async () => {
         const boot = await bootOnAddToken("mainnet");