fix: the swap decoder shows the ETH of a WRAP_ETH that does not set the input side (closes #721)

A later WRAP_ETH of a stated amount now counts as a take from the sender.
When it pays anyone but the router, it gets its own Moved from the sender
lines, with the recipient on Moved to. When it pays the router, it joins
the Amount line if Token In is ETH, and otherwise gets those lines too.
When the router may already hold ETH, the amount reads Up to.

A WRAP_ETH whose input this decoder cannot read gets a line saying the
amount could not be decoded.

A WRAP_ETH of CONTRACT_BALANCE states no amount and reads as before, so
Uniswap's own routes do not change.

Model: opus-5-5

README.md

@@ -397,10 +397,16 @@ is missing, a write fails.
 
 The popup's `saveState()` (`src/shared/state.js`) reads the stored record,
 merges this page's changes into it and writes it back. It writes only the fields
-this page changed since its last save. Wallets, site lists and endpoints are
+this page changed since its last save. It merges the fields below entry by
-merged entry by entry, so two pages changing different things do not overwrite
+entry, so two pages changing different things do not overwrite each other:
-each other. The background's `updateState()` reads the record, applies one
+
-change and writes it back.
+- wallets;
+- site lists;
+- endpoints;
+- tracked tokens.
+
+The background's `updateState()` reads the record, applies one change and writes
+it back.
 
 Every write stamps `schemaVersion` with `STATE_SCHEMA_VERSION` from
 `src/shared/stateSchema.js`, currently `1`. The version is raised only when the
@@ -1363,8 +1369,8 @@ when both tokens are known. The lines are:
   after the step that sets `Token In` takes the amount it states from the
   sender. When it pays anyone but the router, that amount gets these lines. When
   it pays the router, that amount joins the `Amount` line if `Token In` is ETH,
-  and gets these lines if not. A `WRAP_ETH` of `CONTRACT_BALANCE` states no
+  and gets these lines if not. A later `WRAP_ETH` of `CONTRACT_BALANCE` states
-  amount, so it gets no lines. ETH that the sender pays a V4 step or a
+  no amount, so it gets no lines. ETH that the sender pays a V4 step or a
   `WRAP_ETH` of a stated amount can get these three lines when the router may
   hold ETH from an earlier step. When the decoder knows the amount, its
   `Moved from the sender` line reads `Up to` and the amount.

src/shared/state.js

@@ -404,6 +404,36 @@ 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
@@ -411,12 +441,10 @@ function mergeNetworkEndpoints(base, ours, theirs) {
 function networkTrackedTokensWithChange(stored, page) {
     return {
         ...stored.networkTrackedTokens,
-        [page.networkId]: mergeListByIdentity(
+        [page.networkId]: mergeTrackedTokens(
             baseline.trackedTokens,
             page.trackedTokens,
-            stored.networkTrackedTokens[page.networkId],
+            stored.networkTrackedTokens[page.networkId] || [],
-            (token) => token.address.toLowerCase(),
-            (base, ours) => ours,
         ),
     };
 }
@@ -461,23 +489,27 @@ 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
+// page has changed it, so a token another page added or removed since this
+// page's last save stays added or removed. A token both lists hold keeps this
+// page's record. Two saves 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
+//   `networkTrackedTokens` for this page's network, by contract address.
+// - A save that switches networks writes this page's list whole. A switch in
+//   Settings first merges that list by contract address
+//   (takeStoredTrackedTokens()).
+//
 // Every other persisted field stays a whole-field diff:
 // `fraudContracts`/`viewStack` are arrays of scalars with no per-element
-// identity to merge by; a save diffs `trackedTokens` whole, and a switch in
+// identity to merge by; `networkTrackedTokens` is diffed whole like `networkId`,
-// Settings first merges that list by contract address
+// which a network switch moves together with it; `tokenHolderCache` is a map
-// (takeStoredTrackedTokens()); `networkTrackedTokens` is diffed whole like
+// shaped like the ones above, but nothing in src/ ever writes an entry into it —
-// `trackedTokens` and `networkId`, the two fields a network switch moves
+// it is only ever reset wholesale to `{}` (applyChainSwitchFields()) — so there
-// together with it; `tokenHolderCache` is a map shaped like the ones above,
+// is no in-place mutation for a whole-field diff to collide with; `viewData` is
-// but nothing in src/ ever writes an entry into it — it is only ever reset
+// this page's own UI scratch space, not data another page has any reason to
-// wholesale to `{}` (applyChainSwitchFields()) — so there is no in-place
+// share membership of.
-// mutation for a whole-field diff to collide with; `viewData` is this page's
-// 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
@@ -564,6 +596,20 @@ async function saveStateOnce(beforeWrite) {
             fresh,
             current,
         );
+    } else if (
+        baseline !== null &&
+        baseline.networkId === current.networkId &&
+        !deepEqual(current.trackedTokens, baseline.trackedTokens)
+    ) {
+        // This page's network is the one it last saved on, and the stored
+        // 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(
+            baseline.trackedTokens,
+            current.trackedTokens,
+            fresh.trackedTokens,
+        );
     }
     merged.hasWallet = Boolean(merged.wallets && merged.wallets.length > 0);
     // Stamped on every write, never merged or diffed: the record that goes to
@@ -680,12 +726,10 @@ 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 = mergeListByIdentity(
+    rawState.trackedTokens = mergeTrackedTokens(
         baseline.trackedTokens,
         rawState.trackedTokens,
         stored.trackedTokens,
-        (token) => token.address.toLowerCase(),
-        (base, ours) => ours,
     );
     baseline.trackedTokens = structuredClone(stored.trackedTokens);
 }

tests/trackedTokensPerNetwork.test.js

@@ -117,6 +117,17 @@ async function chooseNetwork(boot, networkId) {
     await boot.settle();
 }
 
+// Adds NEW_TOKEN_ADDRESS by contract address on SettingsAddToken, as the user
+// does. Returns the record the add tracks.
+async function addNewToken(boot) {
+    const { lookupTokenInfo } = require("../src/shared/balances");
+    const info = { symbol: "NEW", decimals: 18, name: "New Token" };
+    lookupTokenInfo.mockResolvedValueOnce(info);
+    boot.node("settings-addtoken-address").value = NEW_TOKEN_ADDRESS;
+    await boot.click("btn-settings-addtoken-manual");
+    return { address: NEW_TOKEN_ADDRESS, ...info, balanceNotFetched: true };
+}
+
 // The Settings gear, a switch to Sepolia, and Back.
 async function gearSwitchBack(boot) {
     await boot.click("btn-settings");
@@ -685,6 +696,113 @@ test("a token removed while this page waits to take another page's switch is gon
     expect(stored.networkTrackedTokens.mainnet).toEqual([]);
 });
 
+// Another popup page tracks DAI after this page's last save. This page then
+// adds a token. Its save keeps DAI.
+test("a token another page added stays when this page adds a token", async () => {
+    const boot = await bootOnAddToken("mainnet");
+    expect(boot.pageErrors).toEqual([]);
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        trackedTokens: [MAINNET_TOKEN],
+    });
+
+    const added = await addNewToken(boot);
+
+    expect(boot.storage.read("autistmask").trackedTokens).toEqual([
+        MAINNET_TOKEN,
+        added,
+    ]);
+});
+
+// Another popup page tracks a new token after this page's last save. This page
+// then removes DAI, as the `[x]` on a tracked token in Settings does. Its save
+// keeps the new token.
+test("a token another page added stays when this page removes a token", async () => {
+    const boot = await bootPopup(
+        unversionedValidProfile({ trackedTokens: [MAINNET_TOKEN] }),
+    );
+    expect(boot.pageErrors).toEqual([]);
+    const otherPageToken = {
+        address: NEW_TOKEN_ADDRESS,
+        symbol: "NEW",
+        decimals: 18,
+        name: "New Token",
+    };
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        trackedTokens: [MAINNET_TOKEN, otherPageToken],
+    });
+
+    const { state, saveState } = require("../src/shared/state");
+    state.trackedTokens.splice(0, 1);
+    await saveState();
+
+    expect(boot.storage.read("autistmask").trackedTokens).toEqual([
+        otherPageToken,
+    ]);
+});
+
+// Another popup page stops tracking DAI after this page's last save. This page
+// still holds DAI and adds a token. Its save does not store DAI again.
+test("a token another page removed stays removed when this page adds a token", async () => {
+    const boot = await bootOnAddToken("mainnet", {
+        trackedTokens: [MAINNET_TOKEN],
+    });
+    expect(boot.pageErrors).toEqual([]);
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        trackedTokens: [],
+    });
+
+    const added = await addNewToken(boot);
+
+    expect(boot.storage.read("autistmask").trackedTokens).toEqual([added]);
+});
+
+// Another popup page tracks DAI and the new token after this page's last save,
+// and its refresh has read the new token's balance. This page adds the new token
+// too, so the two pages hold different records for it. The save stores one
+// record for the new token, this page's, and keeps DAI.
+test("a token both pages added is stored once, with this page's record", async () => {
+    const boot = await bootOnAddToken("mainnet");
+    expect(boot.pageErrors).toEqual([]);
+    boot.storage.write("autistmask", {
+        ...boot.storage.read("autistmask"),
+        trackedTokens: [
+            MAINNET_TOKEN,
+            {
+                address: NEW_TOKEN_ADDRESS,
+                symbol: "NEW",
+                decimals: 18,
+                name: "New Token",
+            },
+        ],
+    });
+
+    const added = await addNewToken(boot);
+
+    expect(boot.storage.read("autistmask").trackedTokens).toEqual([
+        MAINNET_TOKEN,
+        added,
+    ]);
+});
+
+// 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");

tests/uniswap.test.js

@@ -2760,8 +2760,9 @@ describe("uniswap decoder", () => {
         // https://git.eeqj.de/sneak/AutistMask/issues/721 — a WRAP_ETH that
         // does not set the input side takes the amount it states from the
         // sender. On next no line gave that ETH. The rows marked "reads as on
-        // next" take nothing from the sender. Uniswap's own routes wrap
+        // next" state no amount to take from the sender. Uniswap's own routes
-        // CONTRACT_BALANCE after their first step, as the last two rows do.
+        // wrap CONTRACT_BALANCE after their first step, as the last two rows
+        // do.
         const CONTRACT_BALANCE = 2n ** 255n;
         const WRAP_TO_OTHER = encodeWrapEth(OTHER_ADDR, ONE_ETH);
         test.each([