fix: a balance refresh that returns after a network switch writes nothing (closes #577)

The popup's doRefreshAndRender() and the background's backgroundRefresh()
each note the network a refresh starts on and write its balances only if
that network is still the active one when the requests return. Otherwise
nothing is written and lastBalanceRefresh stays as the switch left it. The
background checks inside updateState(). The popup refreshes a copy of its
wallets and copies the results onto `state` from inside its save, under the
lock, only if both its own network and the stored record's still match, so
a switch the background makes is caught too. saveState() takes an optional
function, run there on the stored record before the page's state is
compared, that may change `state` or return false to cancel the write.

Model: opus-5-5

src/background/index.js

@@ -1379,6 +1379,13 @@ const RECENT_BALANCE_REFRESH_MS = Math.floor(BALANCE_REFRESH_PERIOD_MS / 2);
 // balances that came back — by address, onto whatever storage holds NOW.
 // Applying by address rather than writing the array back is what keeps a
 // wallet or address added, renamed or deleted during the round trip.
+//
+// Nothing is applied when the network stored NOW is not the one the refresh
+// ran on. A network switch during the round trip cleared every address's
+// balances (applyChainSwitchFields()), and the previous network's must not come
+// back over them (https://git.eeqj.de/sneak/AutistMask/issues/577).
+// lastBalanceRefresh is left as the switch set it, so the next tick refreshes
+// the new network.
 async function backgroundRefresh() {
     const s = await getState();
     const now = Date.now();
@@ -1402,6 +1409,7 @@ async function backgroundRefresh() {
     }
 
     await updateState((fresh) => {
+        if (fresh.networkId !== s.networkId) return;
         for (const wallet of fresh.wallets) {
             for (const addr of wallet.addresses || []) {
                 const got = refreshed.get(String(addr.address).toLowerCase());

src/popup/index.js

@@ -68,23 +68,54 @@ let refreshInFlight = false;
 // the recovery screen: there is no profile left to refresh.
 let refreshTimer = null;
 
+// The refresh writes into a copy of the wallets. Inside the save, under the
+// lock, its results are copied onto the address records in `state` only if the
+// network it ran on is still the active one, on this page and in the stored
+// record; the background's switch, after the user approves a site's request,
+// changes only the stored record. A switch while the refresh ran cleared every
+// address's balances (applyChainSwitchFields() in
+// src/shared/chainSwitchFields.js), and the previous network's must not come
+// back over them (https://git.eeqj.de/sneak/AutistMask/issues/577). A dropped
+// refresh is not recorded as done. Results are copied by address, so an address
+// added or removed meanwhile is left as it is.
 async function doRefreshAndRender() {
     if (refreshInFlight) return;
     refreshInFlight = true;
     try {
+        const networkId = state.networkId;
+        const wallets = structuredClone(state.wallets);
         await Promise.all([
             refreshPrices(),
             refreshBalances(
-                state.wallets,
+                wallets,
                 state.rpcUrl,
                 state.blockscoutUrl,
                 state.trackedTokens,
-                state.networkId,
+                networkId,
                 pageClosed.signal,
             ),
         ]);
-        state.lastBalanceRefresh = Date.now();
-        await saveState();
+        const refreshed = new Map();
+        for (const wallet of wallets) {
+            for (const addr of wallet.addresses) {
+                refreshed.set(addr.address, addr);
+            }
+        }
+        await saveState((stored) => {
+            if (state.networkId !== networkId) return false;
+            if (stored.networkId !== networkId) return false;
+            for (const wallet of state.wallets) {
+                for (const addr of wallet.addresses) {
+                    const got = refreshed.get(addr.address);
+                    if (!got) continue;
+                    for (const key of ["balance", "ensName", "tokenBalances"]) {
+                        if (got[key] !== undefined) addr[key] = got[key];
+                    }
+                }
+            }
+            state.lastBalanceRefresh = Date.now();
+            return true;
+        });
         renderWalletList();
     } catch (e) {
         // Every call site fires this and walks away — the boot below, the ten

src/shared/state.js

@@ -467,11 +467,7 @@ function mergeNetworkEndpoints(base, ours, theirs) {
 // (caught by tests/txStatus.test.js). A page's live picture of a field it
 // does not own goes on being whatever its last loadState() saw, same as
 // before this fix; only the persisted record is guaranteed current.
-async function saveStateOnce() {
-    // A copy, so what this save compares and writes is the page's state as it
-    // stood when the save began. A change made while it waits on storage is
-    // left for the next save, which compares against this copy.
-    const current = structuredClone(snapshotPersisted());
+async function saveStateOnce(beforeWrite) {
     const result = await storageGet("autistmask");
     // The record in storage right now is about to be merged into and written
     // back, so it is validated exactly like a load validates it. Without this,
@@ -485,6 +481,12 @@ async function saveStateOnce() {
     // come from storage in its loaded (self-healed) shape. See
     // normalizePersisted() in persistedState.js.
     const fresh = normalizePersisted(result.autistmask);
+    if (beforeWrite && beforeWrite(fresh) === false) return;
+
+    // A copy, so what this save compares and writes is the page's state as it
+    // stood once the stored record was read. A change made while it waits on
+    // the write is left for the next save, which compares against this copy.
+    const current = structuredClone(snapshotPersisted());
 
     const merged = { ...fresh };
     for (const key of PERSISTED_FIELDS) {
@@ -579,10 +581,15 @@ function reportSaveFailure(err) {
 // 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 = withLock("autistmask", saveStateOnce);
+// compares the page's state, as it stands once the stored record has been read
+// and `beforeWrite` has run, against the baseline the save before it left.
+//
+// `beforeWrite`, when given, is called inside the lock with the stored record
+// as just read, before this page's state is compared against it. It may change
+// `state`, and this save writes that change. When it returns false, nothing is
+// written; any other return lets the save go ahead.
+function saveState(beforeWrite) {
+    const turn = withLock("autistmask", () => saveStateOnce(beforeWrite));
     // 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.