fix: a swap's WETH minimum reads as ETH only when all its WETH reaches the UNWRAP_WETH (closes #642)

Between the step that set the output side and the UNWRAP_WETH, only a
step the decoder read that is a PERMIT2_PERMIT, a BALANCE_CHECK_ERC20,
or a SWEEP, TRANSFER, PAY_PORTION or PAY_PORTION_FULL_PRECISION of a
token other than WETH, and not of ETH to a WETH contract, keeps the
swap's WETH minimum as an ETH minimum; any other step leaves ETH with no
minimum. After an UNWRAP_WETH whose recipient is the router or cannot be
read, any later step but those makes the swap read WETH with no minimum.
Only a step that sets the output side again brings back ETH or a
minimum. A WETH fee paid by PAY_PORTION before the UNWRAP_WETH now shows
no minimum. README Display Consistency states the same rule.

Model: opus-5-5

README.md

@@ -1271,10 +1271,12 @@ after a V4 step that `TAKE`s some of its WETH to the user and the rest to the
 router, or that pays the router WETH beside a payout the decoder cannot read,
 and with a WETH interface fee paid by `PAY_PORTION` before the `UNWRAP_WETH`:
 the decoder does not count how much of that step's minimum reaches the
-`UNWRAP_WETH`, nor read the `UNWRAP_WETH`'s own minimum. When that `UNWRAP_WETH`
+`UNWRAP_WETH`, nor read the `UNWRAP_WETH`'s own minimum. When an `UNWRAP_WETH`
-pays the router, or its recipient cannot be read, the router may hold the ETH,
+of that WETH pays the router, or its recipient cannot be read, the router may
-and a later step may pay it out as WETH, as a `WRAP_ETH` of it to the user does:
+hold the ETH, and a later step may pay it out as WETH, as a `WRAP_ETH` of it to
-after any later step but those listed, the swap shows WETH with no minimum. A
+the user does: after any later step but those listed, the swap shows WETH with
+no minimum. Once the swap shows no minimum, or shows WETH this way, no later
+step, a later `UNWRAP_WETH` included, brings back the minimum or ETH; only a
 later step that sets the output side sets it anew. Otherwise `Token Out` and
 `Min. received` keep the output side's own token and figure, whether the swap
 was paid in ETH or in a token: a V2 exact-out swap that buys USDC and then
@@ -1399,6 +1401,19 @@ carries a `schemaVersion` — `STATE_SCHEMA_VERSION` in
 `saveState()` and the background's `updateState()` both do, so whichever context
 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
+`withLock()` in `src/shared/browserApi.js`. Every page of the extension and its
+background share it, except a page in a Firefox private window, which Firefox
+gives locks of its own (see https://git.eeqj.de/sneak/AutistMask/issues/646).
+The popup's `saveState()`, the background's `updateState()` and the
+StateRecovery reset all take it, so among the pages and the background that
+share it, no write lands between another write's read and its write, where it
+would be lost. Chrome has it from version 69, older than any Chrome that runs a
+Manifest V3 extension, in pages and in the service worker; Firefox from version
+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
 is migrated in place by being stamped on the first write. An upgrade never shows

package.json

@@ -13,6 +13,11 @@
         "fmt": "prettier --write .",
         "fmt-check": "prettier --check ."
     },
+    "jest": {
+        "setupFiles": [
+            "<rootDir>/tests/support/navigatorLocks.js"
+        ]
+    },
     "devDependencies": {
         "@eslint/js": "10.0.1",
         "@tailwindcss/cli": "4.3.1",

src/background/state.js

@@ -16,17 +16,20 @@
 //                     across any number of awaits and no concurrent work can
 //                     move it.
 //   updateState(fn)   read fresh, apply fn to that fresh record, write it
-//                     back — all inside a queue, so two background writes
+//                     back — all holding the lock every write of the stored
-//                     never interleave, and the read is one storage round trip
+//                     record takes, so no other write lands in between, by
-//                     ahead of the write rather than a page lifetime ahead of
+//                     the background or by any page except a page in a
-//                     it (which is what made the popup's saveState() need a
+//                     Firefox private window (README, "Stored state and its
+//                     version"). The read is one storage round trip ahead of
+//                     the write rather than a page lifetime ahead of it
+//                     (which is what made the popup's saveState() need a
 //                     per-field merge against a baseline at all).
 //
 // A handler that must both read and write therefore does its network work
 // against a snapshot it owns, and applies the RESULT inside updateState().
 // It never publishes an object other in-flight work is holding.
 
-const { storageGet, storageSet } = require("../shared/browserApi");
+const { storageGet, storageSet, withLock } = require("../shared/browserApi");
 const { normalizePersisted } = require("../shared/persistedState");
 const {
     STATE_SCHEMA_VERSION,
@@ -51,11 +54,6 @@ async function getState() {
     return normalizePersisted(result.autistmask);
 }
 
-// Serializes the read-modify-write turns below. Two of them interleaved would
-// each read before the other wrote, and the second write would carry the first
-// one's fields back to their pre-turn values.
-let updateQueue = Promise.resolve();
-
 async function updateStateOnce(mutate) {
     const s = await getState();
     await mutate(s);
@@ -70,27 +68,22 @@ async function updateStateOnce(mutate) {
 
 // Apply `mutate` to a record read fresh from storage and write the result
 // back. `mutate` receives a detached, normalized profile and mutates it in
-// place; it may be async, but it must not do anything slow — the window
+// place; it may be async, but it must not do anything slow. The write is the
-// between the read and the write is the window in which another context's
+// WHOLE record, which is safe only because the lock (withLock() in
-// write is lost, and keeping it to one storage round trip is what makes a
+// src/shared/browserApi.js) is held from the read to the write: two
-// whole-record write safe here. Concretely: the write is the WHOLE record, so
+// background turns, or a turn and a page's save, never interleave
-// a popup write that lands inside that window is reverted, in every field, by
+// (https://git.eeqj.de/sneak/AutistMask/issues/641). Every page's save waits
-// the record this turn read before it. That is accepted because the window is
+// while it is held, except a page in a Firefox private window, which Firefox
-// one round trip long and the popup is not writing while the worker is;
+// gives locks of its own (README, "Stored state and its version").
-// widening it is what would make it a real hazard.
 //
 // `mutate` must also not call updateState() itself, directly or through
-// anything it awaits: the queue is strictly serial, so the inner turn waits on
+// anything it awaits: the inner turn waits for the lock the outer one holds,
-// the outer one, which is waiting on the inner one. That deadlocks the whole
+// and the outer one is waiting on the inner one. That deadlocks every write of
-// background, not just the caller. Mutate the record you were handed.
+// the stored record, not just the caller. Mutate the record you were handed.
 //
 // Resolves with the record that was written.
 function updateState(mutate) {
-    const turn = updateQueue.then(() => updateStateOnce(mutate));
+    return withLock("autistmask", () => updateStateOnce(mutate));
-    // The queue must advance even when a turn rejects, or every update after
-    // it queues behind a promise that never settles.
-    updateQueue = turn.catch(() => {});
-    return turn;
 }
 
 module.exports = { getState, updateState };

src/popup/index.js

@@ -220,7 +220,7 @@ async function widenApprovalWindow() {
 async function init() {
     // First, before anything can save: showView() saves on every navigation
     // without awaiting, so a save that fails from here on has somewhere to be
-    // reported rather than being swallowed by the save queue
+    // reported rather than reaching only the console
     // (https://git.eeqj.de/sneak/AutistMask/issues/362). Registered ahead of
     // the approval-window branch below too, since that window saves as well.
     //

src/popup/views/stateRecovery.js

@@ -24,7 +24,11 @@
 // than written again locally so that tests/popupElementIds.test.js sees these
 // lookups and holds every id below against the markup.
 const { $, VIEWS } = require("./helpers");
-const { storageGet, storageRemove } = require("../../shared/browserApi");
+const {
+    storageGet,
+    storageRemove,
+    withLock,
+} = require("../../shared/browserApi");
 const { log } = require("../../shared/log");
 
 // Typed in full before anything is erased, in the same spirit as the wallet
@@ -137,7 +141,11 @@ async function resetProfile() {
         return;
     }
     try {
-        await storageRemove("autistmask");
+        // Under the lock every write of the stored record takes, so the erase
+        // cannot land between another write's read and its write and be undone.
+        // A page in a Firefox private window is the exception: Firefox gives
+        // it locks of its own (README, "Stored state and its version").
+        await withLock("autistmask", () => storageRemove("autistmask"));
     } catch (e) {
         log.errorf("state recovery: reset failed:", e);
         setFlash("The saved data could not be erased. Nothing was changed.");

src/shared/browserApi.js

@@ -31,6 +31,11 @@
 // 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.
 //
@@ -219,6 +224,33 @@ function storageRemove(keys) {
     return Promise.resolve(storage.remove(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
+ * window (README, "Stored state and its version"), so while one of them holds
+ * `name` another asking for it waits, in the order they asked. The lock is let
+ * 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, updateState() in src/background/state.js, and the
+ * reset in src/popup/views/stateRecovery.js.
+ *
+ * @param {string} name
+ * @param {function(): Promise<*>} fn
+ * @returns {Promise<*>} what `fn` resolved with.
+ * @throws rejects, without running `fn`, where `navigator.locks` is absent.
+ */
+function withLock(name, fn) {
+    if (typeof navigator === "undefined" || !navigator.locks) {
+        return Promise.reject(
+            new Error("navigator.locks is not available in this context"),
+        );
+    }
+    return navigator.locks.request(name, () => fn());
+}
+
 // `storage.session`, the browser's session storage: what is kept there lasts
 // until the browser closes. It survives the background worker being stopped
 // and started again, which module-level variables do not (README, Background
@@ -373,4 +405,5 @@ module.exports = {
     windowsGetLastFocused,
     windowsRemove,
     windowsUpdate,
+    withLock,
 };

src/shared/state.js

@@ -35,7 +35,7 @@ const {
     migrationNeeded,
 } = require("./stateSchema");
 
-const { storageGet, storageSet } = require("./browserApi");
+const { storageGet, storageSet, withLock } = require("./browserApi");
 const { log } = require("./log");
 
 // The live record the proxy below guards. Everything inside this module reads
@@ -458,8 +458,8 @@ function mergeNetworkEndpoints(base, ours, theirs) {
 // This page's own live `state` is deliberately NOT rehydrated from a field
 // another page changed — only the record written to storage is merged.
 // showView() fires saveState() on every navigation without awaiting it,
-// which is what makes the queue above necessary in the first place, and a
+// which is why saves from one page wait for each other (see saveState()), and
-// save that is slow to come back has no way to tell whether the field it
+// a save that is slow to come back has no way to tell whether the field it
 // is about to hand back is still the current answer or has since been
 // overtaken by something this very page did in the meantime; writing it
 // into `state` regardless reintroduced exactly the clobber this function
@@ -529,35 +529,21 @@ async function saveStateOnce() {
     rawState.hasWallet = rawState.wallets.length > 0;
 
     // What this save compared and wrote, not the page's state now: a change
-    // made during the save must still differ from the baseline, or the save
+    // made during the save must still differ from the baseline, or the next
-    // queued after it finds nothing to store
+    // save finds nothing to store
     // (https://git.eeqj.de/sneak/AutistMask/issues/448).
     baseline = current;
 }
 
-// showView() calls saveState() on every navigation without awaiting it, so
-// two saves from the SAME page can be in flight at once — e.g. a screen
-// shown, then immediately replaced before the first save's storageGet()
-// round trip has come back. Left concurrent, the first save's turn would
-// finish after the second's live-state mutation and then re-hydrate `state`
-// from what IT read, stomping the second, later change back to a stale
-// value — the same clobber this function exists to prevent, just between
-// two saves on one page instead of two pages. Queuing makes every save's
-// snapshot-diff-write-rehydrate run start to finish before the next one
-// begins, so each one only ever sees the true live state at its turn.
-let saveQueue = Promise.resolve();
-
 // Where a failed save is REPORTED, set once by the context that has a screen
 // 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 the queue below has to attach a
+// every navigation without awaiting it, and `await saveState()` inside an
-// rejection handler to keep advancing — 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
@@ -583,11 +569,20 @@ function reportSaveFailure(err) {
     }
 }
 
+// Each save runs start to finish holding the lock every write of the stored
+// record takes (withLock() in src/shared/browserApi.js), so no write by
+// another page or the background lands between its read and its write and is
+// lost (https://git.eeqj.de/sneak/AutistMask/issues/641). A page in a Firefox
+// private window is the exception: Firefox gives it locks of its own (README,
+// "Stored state and its version").
+//
+// The same lock orders this page's own saves. showView() calls saveState() on
+// every navigation without awaiting it, so two saves from one page can be
+// asked for at once; they take the lock in the order they were asked, and each
+// compares the page's state as it stands when its turn begins against the
+// baseline the save before it left.
 function saveState() {
-    const turn = saveQueue.then(saveStateOnce);
+    const turn = withLock("autistmask", saveStateOnce);
-    // The queue must advance even when a save rejects, or every save after
-    // it queues behind a promise that never settles.
-    saveQueue = turn.catch(() => {});
     // Every failed save is reported, whether or not the caller awaited this
     // one. The returned promise still rejects, so a caller that DOES await
     // keeps its own error handling.

src/shared/uniswap.js

@@ -1222,9 +1222,11 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
         // Set when an UNWRAP_WETH pays out, as ETH, the output side's WETH,
         // which the router holds; cleared whenever the output side is set
         // again, or a later step may pay that ETH out as WETH (see after the
-        // loop). `unwrapIntoRouterStep` is the index of that UNWRAP_WETH when
+        // loop). `unwrapIntoRouterStep` is the index of the first such
-        // it may have paid the ETH to the router, which then holds it: its
+        // UNWRAP_WETH that may have paid the ETH to the router, which then
-        // recipient is the router or cannot be read. Otherwise it is null.
+        // holds it: its recipient is the router or cannot be read. A later
+        // UNWRAP_WETH does not replace it. It is null when there is none, and
+        // is cleared with `outputUnwrapped` when the output side is set again.
         let outputUnwrapped = false;
         let unwrapIntoRouterStep = null;
         // The index of the step that set the output side.
@@ -1284,6 +1286,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
             minOutput = present(amount) ? amount : null;
             outputEstablished = true;
             outputUnwrapped = false;
+            unwrapIntoRouterStep = null;
             outputRecipients = recipients;
             outputStep = step;
         };
@@ -1419,6 +1422,14 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                     v2PairFor(chainId, next.path[0], next.path[1]).toLowerCase()
             );
         };
+        // Whether a SWEEP, TRANSFER, PAY_PORTION or PAY_PORTION_FULL_PRECISION
+        // of `token` to `recipient` pays anyone WETH (see lastWethStep): it
+        // does when `token` is WETH, and when it pays ETH to a WETH contract,
+        // which pays the router WETH for it.
+        const paysWeth = (token, recipient) =>
+            WETH_ADDRESSES.includes(token.toLowerCase()) ||
+            (token === "0x0000000000000000000000000000000000000000" &&
+                WETH_ADDRESSES.includes(recipient.toLowerCase()));
 
         for (let i = 0; i < commandsBytes.length; i++) {
             const cmdId = commandsBytes[i] & commandTypeMask;
@@ -1624,15 +1635,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                         if (!isRouter(s.recipient)) {
                             heldByRouter.delete(s.token.toLowerCase());
                         }
-                        // ETH paid to a WETH contract pays the router WETH.
+                        keepsRouterWeth = !paysWeth(s.token, s.recipient);
-                        keepsRouterWeth = !(
-                            WETH_ADDRESSES.includes(s.token.toLowerCase()) ||
-                            (s.token ===
-                                "0x0000000000000000000000000000000000000000" &&
-                                WETH_ADDRESSES.includes(
-                                    s.recipient.toLowerCase(),
-                                ))
-                        );
                     } else {
                         payeeUnread = true;
                         stepsUnread.push(i);
@@ -1670,15 +1673,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                         ) {
                             heldByRouter.delete(t.token.toLowerCase());
                         }
-                        // ETH paid to a WETH contract pays the router WETH.
+                        keepsRouterWeth = !paysWeth(t.token, t.recipient);
-                        keepsRouterWeth = !(
-                            WETH_ADDRESSES.includes(t.token.toLowerCase()) ||
-                            (t.token ===
-                                "0x0000000000000000000000000000000000000000" &&
-                                WETH_ADDRESSES.includes(
-                                    t.recipient.toLowerCase(),
-                                ))
-                        );
                     } else {
                         payeeUnread = true;
                         stepsUnread.push(i);
@@ -1803,6 +1798,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                         minOutput = v4.amountOutMin;
                         outputEstablished = true;
                         outputUnwrapped = false;
+                        unwrapIntoRouterStep = null;
                         outputRecipients = takeRecipients;
                         outputStep = i;
                     } else if (v4) {
@@ -1862,8 +1858,12 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                         outputRecipients.some(toRouter)
                     ) {
                         outputUnwrapped = true;
-                        unwrapIntoRouterStep =
+                        if (
-                            u === null || isRouter(u.recipient) ? i : null;
+                            unwrapIntoRouterStep === null &&
+                            (u === null || isRouter(u.recipient))
+                        ) {
+                            unwrapIntoRouterStep = i;
+                        }
                         if (
                             !outputRecipients.every(toRouter) ||
                             lastWethStep > outputStep
@@ -1917,7 +1917,9 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
         // WRAP_ETH of it to the user does. This decoder treats every later
         // step but those lastWethStep names as one that may, and does not
         // count how much of that ETH each pays out: after one, the output
-        // side reads as WETH with no figure.
+        // side reads as WETH with no figure. Neither this nor the loss of the
+        // figure in the UNWRAP_WETH branch is undone by any later step, a
+        // later UNWRAP_WETH included, but one that sets the output side again.
         if (
             outputUnwrapped &&
             unwrapIntoRouterStep !== null &&

tests/confirmTxAmount.test.js

@@ -158,7 +158,7 @@ global.document = {
     body: { prepend() {}, appendChild() {} },
     addEventListener() {},
 };
-global.navigator = { clipboard: { writeText() {} } };
+global.navigator.clipboard = { writeText() {} };
 
 const {
     Interface,

tests/confirmTxNetworkSwitch.test.js

@@ -189,7 +189,7 @@ global.document = {
     body: { prepend() {}, appendChild() {} },
     addEventListener() {},
 };
-global.navigator = { clipboard: { writeText() {} } };
+global.navigator.clipboard = { writeText() {} };
 
 const {
     BaseWallet,

tests/e2e/run.js

@@ -367,11 +367,14 @@ async function leaveTransactionDetail(page) {
     await openAddressDetail(page);
 }
 
-// Back out to Home from wherever the previous test finished.
+// Back out to Home from wherever the previous test finished, and wait for the
+// save that navigation makes, so a test reading the stored stack next reads
+// the one Home left.
 async function goHome(page) {
     await leaveTransactionDetail(page);
     await page.click("#btn-address-back");
     await visible(page, "#view-main");
+    await waitForPersisted(page, "currentView", "main", "back on Home");
 }
 
 // One field of the popup's state as it was actually persisted, read out of

tests/nativeCurrency.test.js

@@ -151,7 +151,7 @@ global.document = {
     body: { prepend() {}, appendChild() {} },
     addEventListener() {},
 };
-global.navigator = { clipboard: { writeText() {} } };
+global.navigator.clipboard = { writeText() {} };
 
 const { state } = require("../src/shared/state");
 const { NETWORKS } = require("../src/shared/networks");

tests/persistedEntryFloors.test.js

@@ -364,7 +364,7 @@ describe("a save that fails", () => {
         return storage;
     }
 
-    test("is reported, not swallowed by the save queue", async () => {
+    test("is reported when the caller does not await it", async () => {
         const storage = failingStorage(unversionedValidProfile());
         const { state, loadState, saveState, onSaveFailure } =
             loadStateModule(storage);

tests/sendAmountRule.test.js

@@ -97,7 +97,7 @@ global.document = {
     body: { prepend() {}, appendChild() {} },
     addEventListener() {},
 };
-global.navigator = { clipboard: { writeText() {} } };
+global.navigator.clipboard = { writeText() {} };
 
 const { state } = require("../src/shared/state");
 const send = require("../src/popup/views/send");

tests/sendDisplayFloor.test.js

@@ -151,7 +151,7 @@ global.document = {
     body: { prepend() {}, appendChild() {} },
     addEventListener() {},
 };
-global.navigator = { clipboard: { writeText() {} } };
+global.navigator.clipboard = { writeText() {} };
 
 const { refreshBalances } = require("../src/shared/balances");
 const { state } = require("../src/shared/state");

tests/sendMax.test.js

@@ -146,7 +146,7 @@ global.document = {
     body: { prepend() {}, appendChild() {} },
     addEventListener() {},
 };
-global.navigator = { clipboard: { writeText() {} } };
+global.navigator.clipboard = { writeText() {} };
 
 const { Transaction, Wallet, formatEther } = require("ethers");
 const { refreshBalances } = require("../src/shared/balances");

tests/stateMerge.test.js

@@ -160,7 +160,7 @@ describe("the approval-window reproduction", () => {
         await approvalWindow.state.loadState();
         approvalWindow.helpers.showView("approve-tx");
         // showView() does not await its own saveState(); an extra save
-        // joins the same queue and only resolves once that one has too,
+        // waits for the lock behind that one and so resolves only after it,
         // which is the black-box way to know it landed.
         await approvalWindow.state.saveState();
 

tests/stateWriteLock.test.js

@@ -0,0 +1,96 @@
+// A popup save reads the stored record, merges this page's changes into it
+// and writes it back. The background's write of a network the user approved
+// used to be able to land between that read and that write, and the popup's
+// write then put the old network back
+// (https://git.eeqj.de/sneak/AutistMask/issues/641). Every write of the record
+// now holds a lock from its read to its write.
+//
+// The popup and the background run here as the real modules over one shared
+// storage stub, and share the lock the way the background and a page outside
+// a Firefox private window share the browser's
+// (tests/support/navigatorLocks.js).
+
+const { makeStorageStub } = require("./support/storageStub");
+
+function loadContexts(storage) {
+    jest.resetModules();
+    globalThis.chrome = { storage: { local: storage.local } };
+    return {
+        popup: require("../src/shared/state"),
+        background: require("../src/background/state"),
+        applyChainSwitchFields: require("../src/shared/chainSwitchFields")
+            .applyChainSwitchFields,
+    };
+}
+
+// Lets every pending promise callback run, and then some.
+function nextMacrotask() {
+    return new Promise((resolve) => setImmediate(resolve));
+}
+
+afterEach(() => {
+    delete globalThis.chrome;
+    jest.restoreAllMocks();
+});
+
+test("a network the background switches to during a popup save stays stored", async () => {
+    const storage = makeStorageStub({ autistmask: { networkId: "sepolia" } });
+    const { popup, background, applyChainSwitchFields } = loadContexts(storage);
+    await popup.loadState();
+
+    // The popup navigates, so its save has a change of its own to write.
+    popup.state.currentView = "settings";
+
+    // The popup's save has read the record and is about to write it back when
+    // the user's approval of a site's switch to mainnet reaches the
+    // background. Without the lock the background's whole update runs before
+    // the next macrotask; with it, the update waits for the popup's write.
+    let backgroundSwitch;
+    const realSet = storage.local.set.getMockImplementation();
+    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");
+});
+
+test("after a save that failed, the next save still keeps a network the background switches to during it", 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
+    // switches to mainnet, as in the test above.
+    let backgroundSwitch;
+    const realSet = storage.local.set.getMockImplementation();
+    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/support/navigatorLocks.js

@@ -0,0 +1,27 @@
+// `navigator.locks` for the unit tests, which run on node, where there is none.
+//
+// One lock per name: granted in the order it was asked for, and let go when
+// the callback settles, whether it resolved or rejected. That is what the
+// browser's own does for the one call the extension makes,
+// `navigator.locks.request(name, callback)`. Every module a test file loads
+// shares it, as the extension's background and its pages share the browser's,
+// except a page in a Firefox private window (README, "Stored state and its
+// version").
+//
+// Run before each test file by `setupFiles` in package.json.
+
+// For each name, a promise that settles once the last request for it has
+// let the lock go.
+const released = new Map();
+
+function request(name, callback) {
+    const previous = released.get(name) || Promise.resolve();
+    const result = previous.then(() => callback({ name, mode: "exclusive" }));
+    released.set(
+        name,
+        result.catch(() => {}),
+    );
+    return result;
+}
+
+globalThis.navigator = { locks: { request } };

tests/uniswapRecipient.test.js

@@ -848,6 +848,80 @@ describe("what reaches the UNWRAP_WETH of the swap's WETH", () => {
         expect(detail(result, "Min. received").value).toBe("1.0000 ETH");
     });
 
+    test("a V4 step that TAKEs its WETH to the router, then an UNWRAP_WETH to the user, reads as ETH with the swap's minimum", () => {
+        const result = decode(
+            execute(
+                v4Swap(
+                    SWAP_USDT_FOR_WETH,
+                    SETTLE_USDT,
+                    take(WETH, ADDRESS_THIS),
+                ),
+                unwrapWeth(MSG_SENDER),
+            ),
+        );
+        expect(result.name).toBe("Swap USDT → ETH");
+        expect(detail(result, "Min. received").value).toBe("1.0000 ETH");
+    });
+
+    test.each([
+        {
+            kind: "a TRANSFER of 100 USDC",
+            step: transfer(0x05, USDC, OTHER, 100000000n),
+        },
+        {
+            kind: "a PAY_PORTION of 25 bips of USDC",
+            step: transfer(0x06, USDC, OTHER, 25n),
+        },
+        {
+            kind: "a PAY_PORTION_FULL_PRECISION of 1e15 of USDC",
+            step: transfer(0x07, USDC, OTHER, 10n ** 15n),
+        },
+    ])(
+        "$kind to another address between them reads as ETH with the swap's minimum",
+        ({ step }) => {
+            const result = decode(
+                execute(swapToWeth(ADDRESS_THIS), step, unwrapWeth(MSG_SENDER)),
+                USER,
+                ROUTER_V2_1_2,
+            );
+            expect(result.name).toBe("Swap USDT → ETH");
+            expect(paidTo(result)).toEqual([OTHER]);
+            expect(detail(result, "Min. paid out").value).toBe("1.0000 ETH");
+        },
+    );
+
+    test.each([
+        {
+            kind: "a TRANSFER of 0.0025 ETH",
+            step: transfer(0x05, ZERO, OTHER, ONE_WETH / 400n),
+        },
+        {
+            kind: "a PAY_PORTION of 25 bips of its ETH",
+            step: transfer(0x06, ZERO, OTHER, 25n),
+        },
+        {
+            kind: "a PAY_PORTION_FULL_PRECISION of 1e15 of its ETH",
+            step: transfer(0x07, ZERO, OTHER, 10n ** 15n),
+        },
+    ])(
+        "an UNWRAP_WETH into the router, then $kind to another address, then a SWEEP of its ETH to the user, reads as ETH with the swap's minimum",
+        ({ step }) => {
+            const result = decode(
+                execute(
+                    swapToWeth(ADDRESS_THIS),
+                    unwrapWeth(ADDRESS_THIS),
+                    step,
+                    sweep(ZERO, MSG_SENDER),
+                ),
+                USER,
+                ROUTER_V2_1_2,
+            );
+            expect(result.name).toBe("Swap USDT → ETH");
+            expect(paidTo(result)).toEqual([OTHER]);
+            expect(detail(result, "Min. paid out").value).toBe("1.0000 ETH");
+        },
+    );
+
     // The SWEEP pays the user the swap's WETH, so the UNWRAP_WETH unwraps the
     // WRAP_ETH's.
     test("a SWEEP of the swap's WETH to the user, then a WRAP_ETH into the router, reads as ETH with no minimum", () => {
@@ -890,6 +964,41 @@ describe("what reaches the UNWRAP_WETH of the swap's WETH", () => {
         expect(detail(result, "Min. paid out")).toBeUndefined();
     });
 
+    // The first UNWRAP_WETH may pay the router the swap's output as ETH, and
+    // the WRAP_ETH to the user pays all of it out as WETH, so the last
+    // UNWRAP_WETH finds none of it.
+    test.each([
+        {
+            kind: "an UNWRAP_WETH whose recipient cannot be read, then a WRAP_ETH of all the router's ETH to the user",
+            steps: [
+                withDirtyWord(unwrapWeth(ADDRESS_THIS), 0),
+                wrapEth(MSG_SENDER, CONTRACT_BALANCE),
+            ],
+        },
+        {
+            kind: "an UNWRAP_WETH into the router, a WRAP_ETH of all its ETH to the user, then a WRAP_ETH of all its ETH into the router",
+            steps: [
+                unwrapWeth(ADDRESS_THIS),
+                wrapEth(MSG_SENDER, CONTRACT_BALANCE),
+                wrapEth(ADDRESS_THIS, CONTRACT_BALANCE),
+            ],
+        },
+    ])(
+        "$kind, then an UNWRAP_WETH to the user, reads as WETH with no minimum",
+        ({ steps }) => {
+            const result = decode(
+                execute(
+                    swapToWeth(ADDRESS_THIS),
+                    ...steps,
+                    unwrapWeth(MSG_SENDER),
+                ),
+            );
+            expect(result.name).toBe("Swap USDT → WETH");
+            expect(detail(result, "Min. received")).toBeUndefined();
+            expect(detail(result, "Min. paid out")).toBeUndefined();
+        },
+    );
+
     // A SWEEP of WETH pays out WETH, and ETH paid to a WETH contract pays the
     // router WETH for it. The decoder treats each as a step that may pay out
     // as WETH the swap's output, which the router holds as ETH.

tests/unknownScaleSend.test.js

@@ -120,7 +120,7 @@ global.document = {
     body: { prepend() {}, appendChild() {} },
     addEventListener() {},
 };
-global.navigator = { clipboard: { writeText() {} } };
+global.navigator.clipboard = { writeText() {} };
 
 const { AbiCoder, parseUnits } = require("ethers");
 const { fetchTokenBalances } = require("../src/shared/balances");