fix: a save merges this page's tracked token change into the stored list (closes #717)

A save wrote `trackedTokens` whole, so two pages on one network dropped each
other's adds and brought back each other's removals.

A save that keeps the network now merges this page's change into the stored
list by contract address. A token both lists hold keeps this page's record.
Two pages that add one token can hold different records for it, and
`mergeListByIdentity()` alone would store both.

A save that switches networks is unchanged.

Model: opus-5-5

src/shared/state.js

@@ -404,36 +404,6 @@ function mergeNetworkEndpoints(base, ours, theirs) {
     return mergeMapByKey(base, ours, theirs, mergeEndpointEntry);
 }
 
-// Merges one network's tracked tokens by contract address:
-//
-// - A token another page added or removed since this page's last save stays
-//   added or removed.
-// - A token that this page and `theirs` both hold keeps this page's record.
-// - The result holds one record per contract address.
-//
-// mergeListByIdentity() keeps records that share an identity, on purpose, for
-// wallets. A contract address names one token, so this keeps one record for it.
-// Without that, a token that `ours` and `theirs` each hold twice comes back four
-// times, and each later merge multiplies it again.
-//
-// `theirs` takes this page's records first. mergeListByIdentity() would
-// otherwise log an error for two different records of one token.
-function mergeTrackedTokens(base, ours, theirs) {
-    const contract = (token) => token.address.toLowerCase();
-    const oursByContract = new Map(
-        ours.map((token) => [contract(token), token]),
-    );
-    const merged = mergeListByIdentity(
-        base,
-        ours,
-        theirs.map((token) => oursByContract.get(contract(token)) || token),
-        contract,
-        (baseToken, ourToken) => ourToken,
-    );
-    const byContract = new Map(merged.map((token) => [contract(token), token]));
-    return [...byContract.values()];
-}
-
 // 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
@@ -441,10 +411,12 @@ function mergeTrackedTokens(base, ours, theirs) {
 function networkTrackedTokensWithChange(stored, page) {
     return {
         ...stored.networkTrackedTokens,
-        [page.networkId]: mergeTrackedTokens(
+        [page.networkId]: mergeListByIdentity(
             baseline.trackedTokens,
             page.trackedTokens,
-            stored.networkTrackedTokens[page.networkId] || [],
+            stored.networkTrackedTokens[page.networkId],
+            (token) => token.address.toLowerCase(),
+            (base, ours) => ours,
         ),
     };
 }
@@ -489,10 +461,10 @@ function networkTrackedTokensWithChange(stored, page) {
 // comment at mergeEndpointEntry() for why the collision this closes is
 // milder than the other two.
 //
-// `trackedTokens` is merged by contract address (mergeTrackedTokens()) when this
+// `trackedTokens` is merged by contract address when this page has changed it,
-// page has changed it, so a token another page added or removed since this
+// so a token another page added or removed since this page's last save stays
-// page's last save stays added or removed. A token both lists hold keeps this
+// added or removed. A token both lists hold keeps this page's record. Two saves
-// page's record. Two saves differ:
+// differ:
 //
 // - 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
@@ -605,10 +577,23 @@ async function saveStateOnce(beforeWrite) {
         // record is on it too, so all three lists are of one network. This
         // page's change goes into the stored list, and another page's add or
         // removal stays (https://git.eeqj.de/sneak/AutistMask/issues/717).
-        merged.trackedTokens = mergeTrackedTokens(
+        //
+        // Two pages that add one token can hold different records for it, and
+        // mergeListByIdentity() keeps both of two such records. A contract
+        // address names one token, so the stored list is given this page's
+        // record for each token both hold.
+        const contract = (token) => token.address.toLowerCase();
+        const oursByContract = new Map(
+            current.trackedTokens.map((token) => [contract(token), token]),
+        );
+        merged.trackedTokens = mergeListByIdentity(
             baseline.trackedTokens,
             current.trackedTokens,
-            fresh.trackedTokens,
+            fresh.trackedTokens.map(
+                (token) => oursByContract.get(contract(token)) || token,
+            ),
+            contract,
+            (base, ours) => ours,
         );
     }
     merged.hasWallet = Boolean(merged.wallets && merged.wallets.length > 0);
@@ -726,10 +711,12 @@ function takeStoredNetwork(stored) {
 // `baseline` takes the stored list, so the save stores the list in `state`
 // whenever the two differ.
 function takeStoredTrackedTokens(stored) {
-    rawState.trackedTokens = mergeTrackedTokens(
+    rawState.trackedTokens = mergeListByIdentity(
         baseline.trackedTokens,
         rawState.trackedTokens,
         stored.trackedTokens,
+        (token) => token.address.toLowerCase(),
+        (base, ours) => ours,
     );
     baseline.trackedTokens = structuredClone(stored.trackedTokens);
 }

tests/trackedTokensPerNetwork.test.js

@@ -787,22 +787,6 @@ test("a token both pages added is stored once, with this page's record", async (
     ]);
 });
 
-// This page and the stored list each hold DAI twice. This page then adds a
-// token. Its save stores DAI once.
-test("a token held twice is stored once when this page adds a token", async () => {
-    const boot = await bootOnAddToken("mainnet", {
-        trackedTokens: [MAINNET_TOKEN, MAINNET_TOKEN],
-    });
-    expect(boot.pageErrors).toEqual([]);
-
-    const added = await addNewToken(boot);
-
-    expect(boot.storage.read("autistmask").trackedTokens).toEqual([
-        MAINNET_TOKEN,
-        added,
-    ]);
-});
-
 describe("Back after a switch from the Settings gear", () => {
     test("SettingsAddToken no longer offers the common tokens", async () => {
         const boot = await bootOnAddToken("mainnet");