fix: a token lookup that answers after the user left AddToken no longer changes the screen (closes #603)

When the contract lookup on AddToken answered, the popup always went to AddressDetail, even after the user had opened another screen. The token is still added when the network has not changed. The popup now goes to AddressDetail only while AddToken is the screen showing. It tells this from state.currentView, as Send does. The status line is cleared once the token is added, so Back onto AddToken no longer shows "Looking up token...".

Model: opus-5-5

README.md

@@ -2233,8 +2233,8 @@ view would leave a wallet one click from deletion.
       **AddressDetail**
     - "Add" with a token already tracked, a scam-listed address, or a failed
       contract lookup → flash message, no screen change
-    - "Add", then the contract lookup answers while another screen shows, or
-      after AddToken was opened again → tracks the token, no screen change
+    - "Add", then leaving AddToken before the contract lookup answers → tracks
+      the token, no screen change
     - "Add", then a network switch in Settings before the contract lookup
       answers → nothing is tracked; "The active network changed before the
       lookup finished, so the token was not added." flash message, no screen

src/popup/views/addToken.js

@@ -5,12 +5,7 @@ const { lookupTokenInfo } = require("../../shared/balances");
 const { isScamAddress } = require("../../shared/scamlist");
 const { log } = require("../../shared/log");
 
-// Counts the times AddToken has opened. A contract lookup started before
-// AddToken was last opened changes neither the screen nor the status line.
-let addTokenScreenOpenings = 0;
-
 function show() {
-    addTokenScreenOpenings++;
     $("add-token-address").value = "";
     $("add-token-info").textContent = "";
     $("add-token-info").style.visibility = "hidden";
@@ -60,7 +55,6 @@ function init(ctx) {
         // The network the lookup asks, which Settings may switch away from
         // before it answers, as for a transaction in confirmTx.js.
         const chainId = currentNetwork().chainId;
-        const opening = addTokenScreenOpenings;
         try {
             const info = await lookupTokenInfo(
                 contractAddr,
@@ -86,15 +80,11 @@ function init(ctx) {
             });
             await saveState();
             ctx.doRefreshAndRender();
-            // If the lookup answers after AddToken was opened again, the
-            // token is still added, but the screen and the status line stay
-            // as they are.
-            if (addTokenScreenOpenings !== opening) return;
             // The lookup is over, so a later Back onto AddToken shows no
             // status.
             infoEl.textContent = "";
             infoEl.style.visibility = "hidden";
-            // If the lookup answers while another screen shows, the token is
+            // If the user left AddToken while the lookup ran, the token is
             // still added but the screen does not change.
             if (state.currentView !== "add-token") return;
             // Pop the stack (back to address detail) and re-render it
@@ -117,7 +107,6 @@ function init(ctx) {
                     ? detail
                     : "Could not add the token.",
             );
-            if (addTokenScreenOpenings !== opening) return;
             infoEl.textContent = "";
             infoEl.style.visibility = "hidden";
         }

src/shared/chainSwitch.js

@@ -61,31 +61,20 @@ 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.
-//
 // 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.
+// 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.
 async function onChainSwitch(newNetworkId) {
-    const {
-        state,
-        saveState,
-        takeStoredNetwork,
-        takeStoredTrackedTokens,
-    } = require("./state");
+    const { state, saveState, takeStoredTrackedTokens } = require("./state");
     let net = null;
     await saveState((stored) => {
         if (state.networkId === newNetworkId) return false;
-        if (stored.networkId !== state.networkId) takeStoredNetwork(stored);
-        takeStoredTrackedTokens(stored);
+        if (stored.networkId === state.networkId) {
+            takeStoredTrackedTokens(stored);
+        }
         net = applyChainSwitchFields(state, newNetworkId);
         stored.lastBalanceRefresh = 0;
         stored.fraudContracts = [];

src/shared/state.js

@@ -455,11 +455,6 @@ 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
@@ -532,26 +527,6 @@ 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 = {
-            ...fresh.networkTrackedTokens,
-            [current.networkId]: mergeListByIdentity(
-                baseline.trackedTokens,
-                current.trackedTokens,
-                fresh.networkTrackedTokens[current.networkId],
-                (token) => token.address.toLowerCase(),
-                (base, ours) => ours,
-            ),
-        };
-    }
     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
@@ -632,33 +607,15 @@ function saveState(beforeWrite) {
     return turn;
 }
 
-// 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
+// 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
 // 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 = {
-        ...stored.networkTrackedTokens,
-        [rawState.networkId]: mergeListByIdentity(
-            baseline.trackedTokens,
-            rawState.trackedTokens,
-            stored.networkTrackedTokens[rawState.networkId],
-            (token) => token.address.toLowerCase(),
-            (base, ours) => ours,
-        ),
-    };
     copyChainSwitchFields(rawState, stored);
     copyChainSwitchFields(baseline, structuredClone(stored));
-    rawState.networkTrackedTokens = networkTrackedTokens;
 }
 
 // Give this page the tracked tokens of `stored`, a record on this page's

tests/addTokenLateAnswer.test.js

@@ -18,9 +18,6 @@ const TOKEN = {
     name: "Test Token",
 };
 
-// A second contract address, typed after AddToken was opened again.
-const OTHER_TOKEN_ADDRESS = "0xBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
-
 afterEach(() => {
     cleanupPopup();
 });
@@ -75,7 +72,7 @@ test("an answer after the user opened Settings adds the token and Settings stays
     expect(boot.storage.read("autistmask").trackedTokens).toEqual([TOKEN]);
 });
 
-test("Back from Settings onto AddToken after the lookup answered shows no lookup status", async () => {
+test("Back onto AddToken after that answer shows no lookup status", async () => {
     const boot = await openAddToken();
     const answerLookup = addWithPendingLookup(boot);
     expect(boot.text("add-token-info")).toBe("Looking up token...");
@@ -88,44 +85,6 @@ test("Back from Settings onto AddToken after the lookup answered shows no lookup
     expect(boot.node("add-token-info").style.visibility).toBe("hidden");
 });
 
-test("an answer after AddToken was opened again adds the token and leaves AddToken and its status line as they are", async () => {
-    const boot = await openAddToken();
-    const answerFirstLookup = addWithPendingLookup(boot);
-    await boot.click("btn-add-token-back");
-    await boot.click("btn-add-token");
-    expect(boot.visibleViews()).toEqual(["add-token"]);
-    boot.node("add-token-address").value = OTHER_TOKEN_ADDRESS;
-    // The second lookup is never answered.
-    addWithPendingLookup(boot);
-
-    await answerFirstLookup();
-    expect(boot.visibleViews()).toEqual(["add-token"]);
-    expect(boot.text("add-token-info")).toBe("Looking up token...");
-    expect(boot.storage.read("autistmask").trackedTokens).toEqual([TOKEN]);
-});
-
-test("a lookup that fails after AddToken was opened again leaves the status line as it is", async () => {
-    const boot = await openAddToken();
-    const { lookupTokenInfo } = require("../src/shared/balances");
-    let failFirstLookup;
-    lookupTokenInfo.mockImplementationOnce(
-        () =>
-            new Promise((_, reject) => {
-                failFirstLookup = reject;
-            }),
-    );
-    const firstAdd = boot.click("btn-add-token-confirm");
-    await boot.click("btn-add-token-back");
-    await boot.click("btn-add-token");
-    boot.node("add-token-address").value = OTHER_TOKEN_ADDRESS;
-    // The second lookup is never answered.
-    addWithPendingLookup(boot);
-
-    failFirstLookup(new Error("lookup failed"));
-    await firstAdd;
-    expect(boot.text("add-token-info")).toBe("Looking up token...");
-});
-
 test("an answer while AddToken still shows goes to AddressDetail with the token listed", async () => {
     const boot = await openAddToken();
     const answerLookup = addWithPendingLookup(boot);

tests/trackedTokensPerNetwork.test.js

@@ -257,7 +257,6 @@ test("a token tracked on Sepolia when another page switched there is not tracked
         ...boot.storage.read("autistmask"),
         networkId: "sepolia",
         trackedTokens: [SEPOLIA_TOKEN],
-        networkTrackedTokens: { mainnet: [] },
     });
 
     await chooseNetwork(boot, "sepolia");
@@ -268,140 +267,6 @@ 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]);
-});
-
 // 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.
@@ -473,120 +338,6 @@ 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",
-        },
-    ]);
-});
-
 describe("Back after a switch from the Settings gear", () => {
     test("SettingsAddToken no longer offers the common tokens", async () => {
         const boot = await bootOnAddToken("mainnet");