fix: a popup save no longer undoes a network switch the background saved during it (closes #641)

The popup's saveState() and the background's updateState() each read the
stored record, change it and write it back, and each was ordered only within
its own context, so a write by one could land between the other's read and
write and be lost. Both now hold the browser's lock (navigator.locks, shared
by every page and the background) from the read to the write, through
withLock() in src/shared/browserApi.js; the recovery screen's reset takes it
too. The lock also orders a page's own saves, so both per-context queues are
gone. The unit tests get a small lock from a Jest setup file.

Model: opus-5-5

README.md

@@ -1390,17 +1390,15 @@ carries a `schemaVersion` — `STATE_SCHEMA_VERSION` in
 wrote last, the record says which build's shape it is in.
 
 Each write reads the record, changes it and writes it back, holding a lock from
-the read to the write: the browser's own (`navigator.locks`), taken through
+the read to the write: the browser's own (`navigator.locks`), which every page
-`withLock()` in `src/shared/browserApi.js`. Every page of the extension and its
+of the extension and its background share, taken through `withLock()` in
-background share it, except a page in a Firefox private window, which Firefox
+`src/shared/browserApi.js`. The popup's `saveState()`, the background's
-gives locks of its own (see https://git.eeqj.de/sneak/AutistMask/issues/646).
+`updateState()` and the StateRecovery reset all take it, so no write lands
-The popup's `saveState()`, the background's `updateState()` and the
+between another write's read and its write, where it would be lost. Chrome has
-StateRecovery reset all take it, so among the pages and the background that
+it from version 69, older than any Chrome that runs a Manifest V3 extension, in
-share it, no write lands between another write's read and its write, where it
+pages and in the service worker; Firefox from version 96, older than the 115
-would be lost. Chrome has it from version 69, older than any Chrome that runs a
+`manifest/firefox.json` declares. Where it is missing, a write fails rather than
-Manifest V3 extension, in pages and in the service worker; Firefox from version
+going ahead without it.
-96, older than the 115 `manifest/firefox.json` declares. Where it is missing, a
-write fails rather than going ahead without it.
 
 Version 1 is the shape that shipped before versions existed, so a stored record
 with no `schemaVersion` is version 1 rather than a defect: it loads normally and
@@ -2012,11 +2010,7 @@ view would leave a wallet one click from deletion.
     - Type: transaction classification — one of: Native ETH Transfer, ERC-20
       Token Transfer, Swap, Token Approval, Contract Call, Contract Creation. A
       transfer with a token contract is an ERC-20 Token Transfer whatever symbol
-      the token reports. A call named `execute` or `multicall` is a Swap, here
+      the token reports.
-      and on its history row, only when it was sent to one of the Universal
-      Router addresses Uniswap publishes for the network it was sent on (see
-      Transaction Decoding); sent to any other contract, its history row shows
-      the method name and this line reads Contract Call.
     - Status: "Success" or "Failed"
     - From: blockie + color dot + full address (tap to copy) + etherscan link;
       ENS name if available

src/shared/browserApi.js

@@ -31,11 +31,6 @@
 // Everything is resolved on use rather than captured at module load. The MV3
 // service worker is torn down and re-evaluated repeatedly, and the unit
 // suite installs its stubs on `global.chrome` around a require().
-//
-// The module also holds withLock(), a wrapper around the browser's
-// `navigator.locks`, which is in neither namespace. It sits beside
-// storageGet() and storageSet() because every write of the stored record
-// takes it.
 
 // The extension API namespace, preferring `browser.*` where it exists.
 //
@@ -227,10 +222,9 @@ function storageRemove(keys) {
 /**
  * Run `fn` while holding the browser's lock called `name`
  * (`navigator.locks`), and settle as `fn` settles. Every page of the extension
- * and its background share these locks, except a page in a Firefox private
+ * and its background share these locks, so while one of them holds `name`
- * window (README, "Stored state and its version"), so while one of them holds
+ * another asking for it waits, in the order they asked. The lock is let go
- * `name` another asking for it waits, in the order they asked. The lock is let
+ * when `fn` settles, whether it resolved or rejected.
- * go when `fn` settles, whether it resolved or rejected.
  *
  * Every write of the stored record holds the lock named after its key,
  * "autistmask", from its read to its write: saveState() in

src/shared/state.js

@@ -539,11 +539,12 @@ async function saveStateOnce() {
 // to say it on (src/popup/index.js).
 //
 // A save that fails must not fail silently. showView() fires saveState() on
-// every navigation without awaiting it, and `await saveState()` inside an
+// every navigation without awaiting it, so a failing save was swallowed
-// unguarded event handler is no better: either way, without this, a failure
+// entirely: no throw, no message, nothing on screen. The wallet kept running
-// would reach only the console, where the user never looks, and the wallet
+// against storage that was rejecting every write, which is the data-loss half
-// would keep running against storage that rejects every write. That is the
+// of https://git.eeqj.de/sneak/AutistMask/issues/362. The awaited callers were
-// data-loss half of https://git.eeqj.de/sneak/AutistMask/issues/362.
+// no better off: `await saveState()` inside an unguarded event handler surfaces
+// in the console and nowhere the user looks.
 //
 // This is the "tell the user" half; the other half is the floor in
 // normalizePersisted(), which stops the malformed-record cause from arising in

src/shared/transactions.js

@@ -20,7 +20,6 @@ const { toDecimals } = require("./transferAmount");
 // truncation without the approval screens' nonzero floor: the transaction
 // detail view is the authoritative record and already shows exact precision.
 const { truncateAmount: formatTxValue } = require("./amountDisplay");
-const { routerRelease } = require("./uniswap");
 
 // Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
 // over the address, not part of its identity.  Every address comparison in
@@ -52,6 +51,7 @@ function parseTx(tx, addrLower, chainId) {
         }
         // Map known DEX methods to "Swap" for cleaner display
         const SWAP_METHODS = new Set([
+            "execute",
             "swap",
             "swapExactTokensForTokens",
             "swapTokensForExactTokens",
@@ -59,16 +59,9 @@ function parseTx(tx, addrLower, chainId) {
             "swapTokensForExactETH",
             "swapExactTokensForETH",
             "swapETHForExactTokens",
+            "multicall",
         ]);
-        // Multisigs, smart accounts and many other contracts also have an
+        const label = SWAP_METHODS.has(method)
-        // `execute` or `multicall` method, so either is a swap only when sent
-        // to a Universal Router on this transaction's network: the same
-        // addresses the approval screen decodes a swap for.
-        const isSwap =
-            SWAP_METHODS.has(method) ||
-            ((method === "execute" || method === "multicall") &&
-                routerRelease(to, chainId) !== null);
-        const label = isSwap
             ? "Swap"
             : method.charAt(0).toUpperCase() + method.slice(1);
         direction = "contract";

src/shared/uniswap.js

@@ -2237,4 +2237,4 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
     }
 }
 
-module.exports = { decode, routerRelease };
+module.exports = { decode };

tests/stateWriteLock.test.js

@@ -62,34 +62,17 @@ test("a network the background switches to during a popup save stays stored", as
     expect(stored.currentView).toBe("settings");
 });
 
-test("after a save that failed, the next save still keeps a network the background switches to during it", async () => {
+test("a save that fails holding the lock lets it go", async () => {
     jest.spyOn(console, "error").mockImplementation(() => {});
     const storage = makeStorageStub({ autistmask: { networkId: "sepolia" } });
     const { popup, background, applyChainSwitchFields } = loadContexts(storage);
     await popup.loadState();
-    popup.state.currentView = "settings";
 
-    // The first save's write is refused, so it gives the lock back having
-    // failed.
     storage.local.set.mockRejectedValueOnce(new Error("quota exceeded"));
     await expect(popup.saveState()).rejects.toThrow("quota exceeded");
 
-    // The next save is between its read and its write when the background
+    await background.updateState((fresh) =>
-    // switches to mainnet, as in the test above.
+        applyChainSwitchFields(fresh, "mainnet"),
-    let backgroundSwitch;
+    );
-    const realSet = storage.local.set.getMockImplementation();
+    expect(storage.read("autistmask").networkId).toBe("mainnet");
-    storage.local.set.mockImplementationOnce(async (items) => {
-        backgroundSwitch = background.updateState((fresh) =>
-            applyChainSwitchFields(fresh, "mainnet"),
-        );
-        await nextMacrotask();
-        return realSet(items);
-    });
-
-    await popup.saveState();
-    await backgroundSwitch;
-
-    const stored = storage.read("autistmask");
-    expect(stored.networkId).toBe("mainnet");
-    expect(stored.currentView).toBe("settings");
 });

tests/uniswapRouterAddress.test.js

@@ -7,12 +7,6 @@
 // only when the transaction is sent to a Universal Router deployment on the
 // network it is for; anywhere else it is left to the raw data.
 //
-// The transaction history labelled any call named `execute` or `multicall` a
-// Swap, whatever address it went to
-// (https://git.eeqj.de/sneak/AutistMask/issues/580). It now does so only for
-// such a call sent to a Universal Router deployment on the network the
-// transaction is on; any other is labelled by its method name.
-//
 // The transaction detail screen and the approval screen are driven against a
 // minimal DOM stub in the shape tests/tokenTransferDetail.test.js uses, the
 // approval screen opened the way tests/swapStatusAmount.test.js opens it.
@@ -39,7 +33,6 @@ globalThis.chrome = {
 };
 
 const { state } = require("../src/shared/state");
-const { fetchRecentTransactions } = require("../src/shared/transactions");
 const approval = require("../src/popup/views/approval");
 const { decodeCalldata } = approval;
 const transactionDetail = require("../src/popup/views/transactionDetail");
@@ -54,8 +47,6 @@ const SEPOLIA_ROUTER = "0x3a9d48ab9751398bbfa63ad67599bb04e4bdf98b";
 const ROUTER_ON_BOTH = "0x3fC91A3afd70395Cd496C647d5a6CC9D4B2b7FAD";
 // The address with no code from the issue.
 const ORDINARY = "0x0b0050Da68435b3EcBC236c40F16dB2070bCD79A";
-// A contract that is not a router, such as a multisig.
-const OTHER_CONTRACT = "0x5afe000000000000000000000000000000005afe";
 
 const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
 const SEPOLIA_WETH = "0xfFf9976782d46CC05630D1f6eBAb18b2324d6B14";
@@ -138,13 +129,8 @@ describe("execute() calldata on the approval screen's decoder", () => {
 // The transaction the explorer reports: SWAP_DATA, sent to `mockSentTo`.
 const TX_HASH = "0x" + "5".repeat(64);
 let mockSentTo = null;
-// The transactions the explorer reports for RECIPIENT's history.
-let mockHistory = [];
 
 function mockExplorerAnswer(url) {
-    if (url.endsWith("/addresses/" + RECIPIENT + "/transactions")) {
-        return { items: mockHistory };
-    }
     if (url.endsWith("/transactions/" + TX_HASH)) {
         return {
             to: { hash: mockSentTo },
@@ -201,9 +187,10 @@ function node(id) {
 }
 
 // Open the transaction detail screen with `activeNetworkId` active, for the
-// history row `row`, and wait for its decoded details.
+// Sepolia transaction the explorer reports sent to `sentTo`, and wait for its
-async function showDetail(row, activeNetworkId) {
+// decoded details.
-    mockSentTo = row.to;
+async function showDetailSentTo(sentTo, activeNetworkId) {
+    mockSentTo = sentTo;
     globalThis.document = makeDocument();
     globalThis.window = { location: { search: "" } };
     state.networkId = activeNetworkId;
@@ -211,32 +198,22 @@ async function showDetail(row, activeNetworkId) {
     state.viewData = {};
     state.viewStack = [];
     state.currentView = null;
-    transactionDetail.show(row);
+    transactionDetail.show({
+        hash: TX_HASH,
+        from: RECIPIENT,
+        to: sentTo,
+        value: "",
+        symbol: "SepoliaETH",
+        timestamp: 1790000000,
+        isError: false,
+        direction: "contract",
+        directionLabel: "Swap",
+        method: "execute",
+        chainId: SEPOLIA,
+    });
     for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
 }
 
-// Open the transaction detail screen with `activeNetworkId` active, for the
-// Sepolia transaction the explorer reports sent to `sentTo`, and wait for its
-// decoded details.
-async function showDetailSentTo(sentTo, activeNetworkId) {
-    await showDetail(
-        {
-            hash: TX_HASH,
-            from: RECIPIENT,
-            to: sentTo,
-            value: "",
-            symbol: "SepoliaETH",
-            timestamp: 1790000000,
-            isError: false,
-            direction: "contract",
-            directionLabel: "Swap",
-            method: "execute",
-            chainId: SEPOLIA,
-        },
-        activeNetworkId,
-    );
-}
-
 describe("execute() calldata on the transaction detail screen", () => {
     test("sent to a router on the active network, it is a swap", async () => {
         await showDetailSentTo(SEPOLIA_ROUTER, "sepolia");
@@ -268,58 +245,6 @@ describe("execute() calldata on the transaction detail screen", () => {
     });
 });
 
-// The history row made from the explorer's answer for RECIPIENT's `method`
-// call sent to the contract `sentTo` on the network with `chainId`. The
-// explorer gives the address in checksummed form.
-async function historyRow(method, sentTo, chainId) {
-    mockHistory = [
-        {
-            hash: TX_HASH,
-            block_number: 7000000,
-            timestamp: "2026-09-01T00:00:00Z",
-            from: { hash: RECIPIENT },
-            to: { hash: getAddress(sentTo), is_contract: true },
-            value: "0",
-            method,
-            status: "ok",
-        },
-    ];
-    const rows = await fetchRecentTransactions(
-        RECIPIENT,
-        "https://explorer.example/api/v2",
-        chainId,
-    );
-    return rows[0];
-}
-
-// The history row's label and TransactionDetail's Type line, for a Sepolia
-// transaction. The active network does not decide either.
-describe.each([
-    ["execute", "Execute"],
-    ["multicall", "Multicall"],
-])("a %s() call in the history", (method, methodLabel) => {
-    test("sent to a router on its network, it is a Swap", async () => {
-        const row = await historyRow(method, SEPOLIA_ROUTER, SEPOLIA);
-        expect(row.directionLabel).toBe("Swap");
-        await showDetail(row, "mainnet");
-        expect(node("tx-detail-type").textContent).toBe("Swap");
-    });
-
-    test("sent to another contract, it is labelled by its method name", async () => {
-        const row = await historyRow(method, OTHER_CONTRACT, SEPOLIA);
-        expect(row.directionLabel).toBe(methodLabel);
-        await showDetail(row, "sepolia");
-        expect(node("tx-detail-type").textContent).toBe("Contract Call");
-    });
-
-    test("sent to a router of the other network only, it is labelled by its method name", async () => {
-        const row = await historyRow(method, MAINNET_ROUTER, SEPOLIA);
-        expect(row.directionLabel).toBe(methodLabel);
-        await showDetail(row, "mainnet");
-        expect(node("tx-detail-type").textContent).toBe("Contract Call");
-    });
-});
-
 // Open the approval screen with Ethereum Mainnet active, for a site's Sepolia
 // transaction that sends SWAP_DATA to `sentTo`.
 async function showApprovalSentTo(sentTo) {