fix: merge state per field instead of overwriting the whole blob (closes #304)

saveState() is now a read-modify-write that merges only the fields this page
changed, diffed against a deep-cloned per-page baseline. wallets, allowedSites,
deniedSites and networkEndpoints merge structurally by identity, so membership
comes from fresh storage except for this page's own adds and deletes.

Fixes a second extension page silently deleting a wallet, the background balance
refresh clobbering a concurrent add or resurrecting a delete, and a stale page
resurrecting a revoked site permission.

Colliding wallet identities keep both records and log rather than silently
dropping an encryptedSecret. Concurrent writers of the same leaf remain
last-writer-wins by design.

TODO.md

@@ -44,6 +44,32 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 2026-08-20: A second extension page can no longer silently delete a wallet
+  ([#304](https://git.eeqj.de/sneak/AutistMask/issues/304)). `saveState()` wrote
+  the entire state blob, and every extension page — the toolbar popup, a dApp
+  approval window, `backgroundRefresh()` — holds its own in-memory `state`,
+  loaded once, with `showView()` saving on every navigation; a second page that
+  saved after a first had written something new overwrote it, no attacker or
+  unusual input required. `saveState()` is now a read-modify-write: it re-reads
+  storage, diffs the persisted fields against a deep-cloned `baseline` snapshot
+  taken at the last `loadState()`/`saveState()` on that page, and writes only
+  the fields that actually changed — everything else is carried forward from
+  storage in its loaded-and-normalized shape (`normalizePersisted()`, shared
+  with `loadState()`), so a legacy or malformed record a load has always
+  self-healed in memory keeps getting written back even on a save that touched
+  something else entirely. `showView()` fires `saveState()` on every navigation
+  without awaiting it, so two saves from the same page can be in flight at once;
+  a FIFO queue serializes them rather than letting a slow one finish after a
+  later one and re-derive a stale answer. Deliberately not done: the live
+  `state` of a field this page does not own is not rehydrated from what another
+  page wrote, only the persisted record is — adopting a concurrently-written
+  value into `state` reintroduced the same clobber one page later, caught by
+  `tests/txStatus.test.js` red. Two writers of the same field still resolve
+  last-writer-wins, documented at the merge point. `tests/stateMerge.test.js`
+  covers the two-page save and the approval-window reproduction from the issue —
+  add a wallet in one page, force a save from a second page loaded before it,
+  both wallets survive — each demonstrated failing against the unfixed full-blob
+  write.
 - 2026-08-20: A forgotten password no longer wedges the wallet
   ([#312](https://git.eeqj.de/sneak/AutistMask/issues/312)). Deleting a wallet
   was password-gated and importing its recovery phrase again was refused as a

src/shared/state.js

@@ -6,6 +6,7 @@ const { networkById } = require("./networks");
 const { RESTORABLE_VIEWS } = require("../popup/restorableViews");
 
 const { storageGet, storageSet } = require("./browserApi");
+const { log } = require("./log");
 
 const DEFAULT_STATE = {
     hasWallet: false,
@@ -88,135 +89,529 @@ function currentNetwork() {
     return networkById(state.networkId);
 }
 
-async function saveState() {
-    const persisted = {
-        hasWallet: state.hasWallet,
-        wallets: state.wallets,
-        trackedTokens: state.trackedTokens,
-        networkId: state.networkId,
-        rpcUrl: state.rpcUrl,
-        blockscoutUrl: state.blockscoutUrl,
-        networkEndpoints: state.networkEndpoints,
-        lastBalanceRefresh: state.lastBalanceRefresh,
-        activeAddress: state.activeAddress,
-        allowedSites: state.allowedSites,
-        deniedSites: state.deniedSites,
-        rememberSiteChoice: state.rememberSiteChoice,
-        showZeroBalanceTokens: state.showZeroBalanceTokens,
-        hideSpoofedSymbols: state.hideSpoofedSymbols,
-        hideLowHolderTokens: state.hideLowHolderTokens,
-        hideFraudContracts: state.hideFraudContracts,
-        hideDustTransactions: state.hideDustTransactions,
-        dustThresholdGwei: state.dustThresholdGwei,
-        utcTimestamps: state.utcTimestamps,
-        fraudContracts: state.fraudContracts,
-        tokenHolderCache: state.tokenHolderCache,
-        theme: state.theme,
-        debugMode: state.debugMode,
-        currentView: state.currentView,
-        selectedWallet: state.selectedWallet,
-        selectedAddress: state.selectedAddress,
-        selectedToken: state.selectedToken,
-        viewData: state.viewData,
-        viewStack: state.viewStack,
-    };
-    await storageSet({ autistmask: persisted });
-}
+// Every field written to and read from the single "autistmask" storage key.
+// hasWallet is deliberately excluded from the diffing/merge logic below —
+// like loadState() does, it is always derived from `wallets`, never carried
+// as an independent value.
+const PERSISTED_FIELDS = Object.keys(DEFAULT_STATE)
+    .filter((key) => key !== "hasWallet")
+    .concat([
+        "currentView",
+        "selectedWallet",
+        "selectedAddress",
+        "selectedToken",
+        "viewData",
+        "viewStack",
+    ]);
 
-async function loadState() {
-    const result = await storageGet("autistmask");
-    if (result.autistmask) {
-        const saved = result.autistmask;
-        state.wallets = saved.wallets || [];
-        // Derived, never read from storage: a profile persisted with the flag
-        // out of step with the wallet list would otherwise stay broken on
-        // every load. Nothing depends on the two disagreeing.
-        state.hasWallet = state.wallets.length > 0;
-        state.trackedTokens = saved.trackedTokens || [];
-        state.networkId = saved.networkId || DEFAULT_STATE.networkId;
-        state.rpcUrl = saved.rpcUrl || DEFAULT_STATE.rpcUrl;
-        state.blockscoutUrl =
-            saved.blockscoutUrl || DEFAULT_STATE.blockscoutUrl;
-        // An actual object is required, not merely a truthy non-array: the
-        // code below and onChainSwitch() index and ASSIGN INTO this value,
-        // and assigning a property to a string or a number is a silent no-op
-        // in sloppy mode. A stored primitive would therefore be re-persisted
-        // unchanged forever, and every switch would fall back to the network
-        // default — the endpoint loss this map exists to prevent, with no
-        // self-healing. The allowedSites/deniedSites guards below are only
-        // read from, which is why they can be looser.
-        state.networkEndpoints =
+// Turn a raw stored (or missing) record into the full, defaulted shape
+// loadState() used to assign directly onto `state`. Pulled out as a pure
+// function so saveState() can apply it too: the fields THIS page did not
+// change still have to come from storage in their loaded-and-normalized
+// form, not as the raw bytes another page (or an old release) left there —
+// otherwise a legacy shape a load has always self-healed in memory (a
+// missing networkEndpoints map, an out-of-range flag) is dropped right back
+// into storage unfixed every time the page that DID normalize it saves
+// something unrelated, because that field's value never "changed" for that
+// page to notice.
+function normalizePersisted(saved) {
+    saved = saved || {};
+    const out = {};
+    out.wallets = saved.wallets || [];
+    // Derived, never trusted verbatim off storage — see loadState().
+    out.hasWallet = out.wallets.length > 0;
+    out.trackedTokens = saved.trackedTokens || [];
+    out.networkId = saved.networkId || DEFAULT_STATE.networkId;
+    out.rpcUrl = saved.rpcUrl || DEFAULT_STATE.rpcUrl;
+    out.blockscoutUrl = saved.blockscoutUrl || DEFAULT_STATE.blockscoutUrl;
+    // An actual object is required, not merely a truthy non-array: the code
+    // below and onChainSwitch() index and ASSIGN INTO this value, and
+    // assigning a property to a string or a number is a silent no-op in
+    // sloppy mode. Copied rather than referenced, nested pairs included, so
+    // normalizing never mutates the object a caller handed in.
+    const rawEndpoints =
         typeof saved.networkEndpoints === "object" &&
         saved.networkEndpoints !== null &&
         !Array.isArray(saved.networkEndpoints)
             ? saved.networkEndpoints
             : {};
-        // A profile written before this map existed carries exactly one pair
-        // of endpoints, belonging to whatever network it was last on. Adopt
-        // it as that network's remembered pair, so a custom endpoint set on
-        // the old build is not lost by the first switch away and back.
-        if (!state.networkEndpoints[state.networkId]) {
-            state.networkEndpoints[state.networkId] = {
-                rpcUrl: state.rpcUrl,
-                blockscoutUrl: state.blockscoutUrl,
+    out.networkEndpoints = {};
+    for (const netId of Object.keys(rawEndpoints)) {
+        out.networkEndpoints[netId] = { ...rawEndpoints[netId] };
+    }
+    // A profile written before this map existed carries exactly one pair of
+    // endpoints, belonging to whatever network it was last on. Adopt it as
+    // that network's remembered pair, so a custom endpoint set on the old
+    // build is not lost by the first switch away and back.
+    if (!out.networkEndpoints[out.networkId]) {
+        out.networkEndpoints[out.networkId] = {
+            rpcUrl: out.rpcUrl,
+            blockscoutUrl: out.blockscoutUrl,
         };
     }
-        state.lastBalanceRefresh = saved.lastBalanceRefresh || 0;
-        state.activeAddress = saved.activeAddress || null;
-        state.allowedSites =
+    out.lastBalanceRefresh = saved.lastBalanceRefresh || 0;
+    out.activeAddress = saved.activeAddress || null;
+    out.allowedSites =
         saved.allowedSites && !Array.isArray(saved.allowedSites)
             ? saved.allowedSites
             : {};
-        state.deniedSites =
+    out.deniedSites =
         saved.deniedSites && !Array.isArray(saved.deniedSites)
             ? saved.deniedSites
             : {};
-        state.rememberSiteChoice =
+    out.rememberSiteChoice =
         saved.rememberSiteChoice !== undefined
             ? saved.rememberSiteChoice
             : true;
-        state.showZeroBalanceTokens =
+    out.showZeroBalanceTokens =
         saved.showZeroBalanceTokens !== undefined
             ? saved.showZeroBalanceTokens
             : true;
-        // A profile written before this setting existed has no key for it.
-        // It is a safety filter, so absent must load as on, not as undefined.
-        state.hideSpoofedSymbols =
+    // A profile written before this setting existed has no key for it. It
+    // is a safety filter, so absent must load as on, not as undefined.
+    out.hideSpoofedSymbols =
         saved.hideSpoofedSymbols !== undefined
             ? saved.hideSpoofedSymbols
             : true;
-        state.hideLowHolderTokens =
+    out.hideLowHolderTokens =
         saved.hideLowHolderTokens !== undefined
             ? saved.hideLowHolderTokens
             : true;
-        state.hideFraudContracts =
+    out.hideFraudContracts =
         saved.hideFraudContracts !== undefined
             ? saved.hideFraudContracts
             : true;
-        state.hideDustTransactions =
+    out.hideDustTransactions =
         saved.hideDustTransactions !== undefined
             ? saved.hideDustTransactions
             : true;
-        state.dustThresholdGwei =
+    out.dustThresholdGwei =
         saved.dustThresholdGwei !== undefined
             ? saved.dustThresholdGwei
             : 100000;
-        state.utcTimestamps =
+    out.utcTimestamps =
         saved.utcTimestamps !== undefined ? saved.utcTimestamps : false;
-        state.fraudContracts = saved.fraudContracts || [];
-        state.tokenHolderCache = saved.tokenHolderCache || {};
-        state.theme = saved.theme || "system";
-        state.debugMode =
-            saved.debugMode !== undefined ? saved.debugMode : false;
-        state.currentView = saved.currentView || null;
-        state.selectedWallet =
+    out.fraudContracts = saved.fraudContracts || [];
+    out.tokenHolderCache = saved.tokenHolderCache || {};
+    out.theme = saved.theme || "system";
+    out.debugMode = saved.debugMode !== undefined ? saved.debugMode : false;
+    out.currentView = saved.currentView || null;
+    out.selectedWallet =
         saved.selectedWallet !== undefined ? saved.selectedWallet : null;
-        state.selectedAddress =
+    out.selectedAddress =
         saved.selectedAddress !== undefined ? saved.selectedAddress : null;
-        state.selectedToken = saved.selectedToken || null;
-        state.viewData = saved.viewData || {};
-        state.viewStack = restorableStack(saved.viewStack, state.currentView);
+    out.selectedToken = saved.selectedToken || null;
+    out.viewData = saved.viewData || {};
+    out.viewStack = restorableStack(saved.viewStack, out.currentView);
+    return out;
 }
+
+// The persisted fields as they stood at the end of this page's last
+// loadState() or saveState(). saveState() diffs the live state against this
+// to find only the fields THIS page actually changed.
+//
+// Deep-cloned, not a reference: callers mutate persisted objects and arrays
+// in place (state.wallets.push(...)), and a reference baseline would mutate
+// right along with `state`, so the diff would always come out empty.
+let baseline = null;
+
+function snapshotPersisted() {
+    const out = {};
+    for (const key of PERSISTED_FIELDS) out[key] = state[key];
+    return out;
+}
+
+function deepEqual(a, b) {
+    if (a === b) return true;
+    if (typeof a !== "object" || typeof b !== "object") return false;
+    if (a === null || b === null) return false;
+    if (Array.isArray(a) !== Array.isArray(b)) return false;
+    const aKeys = Object.keys(a);
+    const bKeys = Object.keys(b);
+    if (aKeys.length !== bKeys.length) return false;
+    for (const key of aKeys) {
+        if (!Object.prototype.hasOwnProperty.call(b, key)) return false;
+        if (!deepEqual(a[key], b[key])) return false;
+    }
+    return true;
+}
+
+// Stable identity for a wallet, independent of its position in the array
+// (which shifts under a concurrent add/delete elsewhere) and independent of
+// its mutable fields (name is user-editable; addresses gains/loses entries
+// via scanning and deleteAddress.js). An "hd"/"xprv" wallet's xpub never
+// changes for its lifetime and is already enforced unique
+// (findWalletByXpub() in addWallet.js). A "key" wallet has no xpub, exactly
+// one address for its whole lifetime (nothing ever adds to or removes from
+// a key wallet's address list), and that address is already enforced
+// unique (findWalletByAddress()) — so it stands in for identity there.
+// Neither invariant is enforced by this function or by
+// mergeListByIdentity() below — they hold only because every wallet-
+// creation path in addWallet.js happens to populate one or the other before
+// the wallet ever reaches state.wallets, and because canRemoveAddress() in
+// walletDelete.js never lets a wallet's address list go to zero. A wallet
+// with neither (an empty/legacy/corrupt record) falls back to the same
+// "addr:" identity as every other such record, which is a genuine
+// collision, not a proxy for one — see the collision handling in
+// mergeListByIdentity().
+function walletIdentity(wallet) {
+    if (wallet.xpub) return "xpub:" + wallet.xpub;
+    const first = wallet.addresses && wallet.addresses[0];
+    return "addr:" + (first ? String(first.address).toLowerCase() : "");
+}
+
+// Stable identity for an address within one wallet's address list. An
+// address is unique within its wallet and, once derived or imported, never
+// changes — only whether it is present.
+function addressIdentity(addr) {
+    return String(addr.address).toLowerCase();
+}
+
+// Merge one array of identity-bearing objects (wallets, or the addresses
+// inside one wallet) by identity rather than by array index — an index
+// shifts under a concurrent insert/delete elsewhere, which would merge the
+// wrong pair of objects entirely.
+//
+// `theirs` (fresh storage) sets the membership baseline and the order:
+//   - An item this page never had baseline knowledge of, but that is in
+//     `theirs`, was added by someone else — kept as-is.
+//   - An item `base` had and `ours` no longer has was deleted by THIS page
+//     — dropped even though `theirs` still has it (this page's own delete
+//     must win over a background save that only touched leaf fields).
+//   - An item present in both `ours` and `theirs` is merged leaf-by-leaf via
+//     `mergeItem`, so a leaf this page changed (e.g. a renamed wallet) lands
+//     on top of `theirs`' otherwise-current copy (e.g. a refreshed balance).
+// Anything left in `ours` that `base` never had and `theirs` does not have
+// yet is this page's own new addition — appended.
+//
+// `identityOf` is not guaranteed collision-free (walletIdentity() falls
+// back to one shared "addr:" value for any wallet with neither an xpub nor
+// a populated first address). Two records that collide under it must never
+// silently collapse into one — that is exactly how this function used to
+// drop a wallet, encryptedSecret included, with no error and no log. Two
+// defenses:
+//   - `ours` is indexed into GROUPS, not a single item per identity, so two
+//     colliding live items on this page can't overwrite each other in the
+//     index before the merge below even runs.
+//   - A matched pair with no shared `base` entry (neither page ever agreed
+//     on this identity) is only merged leaf-by-leaf when the two sides are
+//     already equal. If they differ, that is not "the same record edited
+//     twice", it is two different records that happen to share an identity
+//     — both are kept, unmerged, rather than guessing which one is real.
+function mergeListByIdentity(base, ours, theirs, identityOf, mergeItem) {
+    base = base || [];
+    ours = ours || [];
+    theirs = theirs || [];
+    const baseIndex = new Map(base.map((item) => [identityOf(item), item]));
+    const oursIndex = new Map();
+    for (const item of ours) {
+        const id = identityOf(item);
+        if (!oursIndex.has(id)) oursIndex.set(id, []);
+        oursIndex.get(id).push(item);
+    }
+
+    const result = [];
+    const seen = new Set();
+
+    for (const theirItem of theirs) {
+        const id = identityOf(theirItem);
+        seen.add(id);
+        const oursGroup = oursIndex.get(id);
+        if (baseIndex.has(id) && !oursGroup) continue;
+        if (oursGroup) {
+            const baseItem = baseIndex.get(id);
+            if (!baseItem && !deepEqual(oursGroup[0], theirItem)) {
+                log.errorf(
+                    "state: identity collision merging",
+                    JSON.stringify(id),
+                    "- keeping both records instead of dropping one",
+                );
+                result.push(theirItem, ...oursGroup);
+            } else {
+                result.push(mergeItem(baseItem, oursGroup[0], theirItem));
+                for (let i = 1; i < oursGroup.length; i++) {
+                    result.push(oursGroup[i]);
+                }
+            }
+        } else {
+            result.push(theirItem);
+        }
+    }
+
+    for (const item of ours) {
+        const id = identityOf(item);
+        if (seen.has(id)) continue;
+        if (!baseIndex.has(id)) result.push(item);
+    }
+
+    return result;
+}
+
+// Merge one wallet's scalar/leaf fields (name, encryptedSecret, nextIndex,
+// ...) against base, then recurse into its address list by identity. `base`
+// is null when this page created the wallet itself and no other page has
+// (yet) produced a same-identity record — nothing to merge in that case,
+// this page's own copy wins outright. mergeListByIdentity() only ever calls
+// this with `!base` when `ours` and `theirs` are already equal (a genuine
+// collision between two DIFFERENT same-identity records is caught and kept
+// as two separate entries before this function is reached), so returning
+// `ours` here can't discard a different wallet's data.
+function mergeWallet(base, ours, theirs) {
+    if (!base) return ours;
+    const merged = { ...theirs };
+    for (const key of Object.keys(ours)) {
+        if (key === "addresses") continue;
+        if (!deepEqual(ours[key], base[key])) merged[key] = ours[key];
+    }
+    merged.addresses = mergeListByIdentity(
+        base.addresses,
+        ours.addresses,
+        theirs.addresses,
+        addressIdentity,
+        mergeAddress,
+    );
+    return merged;
+}
+
+// Merge one address's leaf fields (balance, ensName, tokenBalances, ...).
+// tokenBalances is itself an array, but only backgroundRefresh() ever
+// writes it and always wholesale (refreshBalances() in
+// src/shared/balances.js), so there is no membership to reconcile within
+// it — it is a leaf like balance or ensName, not a list with its own
+// identity.
+function mergeAddress(base, ours, theirs) {
+    if (!base) return ours;
+    const merged = { ...theirs };
+    for (const key of Object.keys(ours)) {
+        if (!deepEqual(ours[key], base[key])) merged[key] = ours[key];
+    }
+    return merged;
+}
+
+// Merge a plain object keyed by string (allowedSites/deniedSites: address ->
+// hostname list; networkEndpoints: networkId -> {rpcUrl, blockscoutUrl}) the
+// same way mergeListByIdentity() merges an array — by key, not by whole-
+// object diff — so a key one page added or removed applies independently of
+// a key another page edited. Unlike an array's identity function, an object
+// key can't collide with a different logical entry (Object.keys() is
+// already deduplicated), so this needs no collision floor of its own.
+function mergeMapByKey(base, ours, theirs, mergeLeaf) {
+    base = base || {};
+    ours = ours || {};
+    theirs = theirs || {};
+    const result = {};
+    const seen = new Set();
+
+    for (const key of Object.keys(theirs)) {
+        seen.add(key);
+        const inBase = Object.prototype.hasOwnProperty.call(base, key);
+        const inOurs = Object.prototype.hasOwnProperty.call(ours, key);
+        if (inBase && !inOurs) continue; // this page deleted the whole entry
+        if (inOurs) {
+            result[key] = mergeLeaf(base[key], ours[key], theirs[key]);
+        } else {
+            result[key] = theirs[key];
+        }
+    }
+
+    for (const key of Object.keys(ours)) {
+        if (seen.has(key)) continue;
+        if (!Object.prototype.hasOwnProperty.call(base, key)) {
+            result[key] = ours[key];
+        }
+    }
+
+    return result;
+}
+
+// allowedSites/deniedSites: { [address]: [hostname, ...] }. The hostname
+// list is itself membership, not a leaf — src/background/index.js pushes a
+// newly approved/denied hostname onto it in place, and the Settings "revoke"
+// button (src/popup/views/settings.js) filters a hostname out of it in
+// place, from a different page. Merge it the same way wallets are merged:
+// identity is the hostname itself, so a merged pair is always equal and
+// mergeItem is a no-op pick.
+function mergeHostnameList(base, ours, theirs) {
+    return mergeListByIdentity(
+        base,
+        ours,
+        theirs,
+        (hostname) => hostname,
+        (b, o, t) => t,
+    );
+}
+
+function mergeSiteMap(base, ours, theirs) {
+    return mergeMapByKey(base, ours, theirs, mergeHostnameList);
+}
+
+// networkEndpoints: { [networkId]: {rpcUrl, blockscoutUrl} }. onChainSwitch()
+// (src/shared/chainSwitch.js) writes state.networkEndpoints[networkId] in
+// place before saving. No code path ever removes a key from this map, so the
+// membership collision that matters for allowedSites/wallets (an add on one
+// page racing a delete on another) can't happen here — but two pages
+// switching to two different networks concurrently still race a whole-field
+// diff the same way, so it gets the same per-key merge for the leaf edit
+// case (e.g. Settings saving a custom RPC URL for the active network).
+function mergeEndpointEntry(base, ours, theirs) {
+    if (!base) return ours;
+    const merged = { ...theirs };
+    for (const key of Object.keys(ours)) {
+        if (!deepEqual(ours[key], base[key])) merged[key] = ours[key];
+    }
+    return merged;
+}
+
+function mergeNetworkEndpoints(base, ours, theirs) {
+    return mergeMapByKey(base, ours, theirs, mergeEndpointEntry);
+}
+
+// Read-modify-write, merged per field, rather than one full-blob write.
+//
+// Every extension page (the toolbar popup, a dApp approval window, the
+// background's backgroundRefresh()) holds its own in-memory `state`, loaded
+// once, and showView() saves on every navigation. A full-blob write here
+// clobbers whatever a second page had written since — including, in the
+// worst case, an entire wallet and its encrypted secret with no attacker
+// and no unusual input (see the issue this fixes).
+//
+// Only the fields this page actually changed — those that differ from
+// `baseline`, captured at the last loadState()/saveState() on this page —
+// are written; every other field is carried forward from whatever is in
+// storage right now, which may already be a value another page wrote.
+//
+// `wallets` is merged structurally (mergeListByIdentity(), by wallet
+// identity and then by address identity within each wallet), not as one
+// whole field: backgroundRefresh() mutates wallets IN PLACE (addr.balance /
+// ensName / tokenBalances, via refreshBalances()), so a whole-field diff
+// would mark all of `wallets` "changed" the moment any balance moved and
+// write back background's own copy — loaded before its multi-second network
+// round trip — clobbering a wallet another page added, or resurrecting one
+// another page deleted, in that window. Merging by identity lets
+// background's leaf changes and another page's membership changes
+// (add/delete a wallet or an address) apply independently instead of
+// colliding as the same field.
+//
+// `allowedSites` and `deniedSites` get the same treatment (mergeSiteMap(),
+// by address key and then by hostname within each address's list), for the
+// identical reason: src/background/index.js pushes a newly
+// approved/denied hostname onto them in place, and the Settings "revoke"
+// button (src/popup/views/settings.js) filters one out in place, from a
+// different page. A whole-field diff here doesn't just lose data, it is a
+// security defect — a stale page's save can resurrect a just-revoked site
+// permission, or silently wipe a permission just granted elsewhere.
+//
+// `networkEndpoints` gets the same treatment too (mergeNetworkEndpoints(),
+// by network id), since onChainSwitch() writes into it in place; the value
+// per key is a small leaf object with no membership of its own; see the
+// comment at mergeEndpointEntry() for why the collision this closes is
+// milder than the other two.
+//
+// Every other persisted field stays a whole-field diff:
+// `trackedTokens`/`fraudContracts`/`viewStack` are arrays of scalars with no
+// per-element identity to merge by; `tokenHolderCache` is a map shaped like
+// the ones above, but nothing in src/ ever writes an entry into it — it is
+// only ever reset wholesale to `{}` (onChainSwitch()) — so there is no
+// in-place 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.
+//
+// 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
+// that only navigated, or only refreshed a balance, overwriting a wallet or
+// address list it never touched the membership of.
+//
+// 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
+// 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
+// exists to remove, just delayed and confined to one page instead of two
+// (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() {
+    const current = snapshotPersisted();
+    const result = await storageGet("autistmask");
+    // Normalized, not raw: a field this page did not change still has to
+    // come from storage in its loaded (self-healed) shape. See
+    // normalizePersisted() above.
+    const fresh = normalizePersisted(result.autistmask);
+
+    const merged = { ...fresh };
+    for (const key of PERSISTED_FIELDS) {
+        if (key === "wallets") {
+            merged.wallets = mergeListByIdentity(
+                baseline ? baseline.wallets : [],
+                current.wallets,
+                fresh.wallets,
+                walletIdentity,
+                mergeWallet,
+            );
+        } else if (key === "allowedSites" || key === "deniedSites") {
+            merged[key] = mergeSiteMap(
+                baseline ? baseline[key] : {},
+                current[key],
+                fresh[key],
+            );
+        } else if (key === "networkEndpoints") {
+            merged.networkEndpoints = mergeNetworkEndpoints(
+                baseline ? baseline.networkEndpoints : {},
+                current.networkEndpoints,
+                fresh.networkEndpoints,
+            );
+        } else if (
+            baseline === null ||
+            !deepEqual(current[key], baseline[key])
+        ) {
+            merged[key] = current[key];
+        }
+    }
+    merged.hasWallet = Boolean(merged.wallets && merged.wallets.length > 0);
+
+    await storageSet({ autistmask: merged });
+
+    // Derived from this page's own wallets, never adopted off the wire —
+    // see loadState(). Everything else this page did not change is left
+    // exactly as it stood; see the note above.
+    state.hasWallet = state.wallets.length > 0;
+
+    baseline = structuredClone(snapshotPersisted());
+}
+
+// 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();
+
+function saveState() {
+    const turn = saveQueue.then(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(() => {});
+    return turn;
+}
+
+async function loadState() {
+    const result = await storageGet("autistmask");
+    if (result.autistmask) {
+        Object.assign(state, normalizePersisted(result.autistmask));
+    }
+    // The point of comparison every saveState() on this page diffs against,
+    // whether storage had a profile or was empty. See PERSISTED_FIELDS above
+    // saveState() for why a reference here would be wrong.
+    baseline = structuredClone(snapshotPersisted());
 }
 
 function currentAddress() {

tests/stateMerge.test.js

@@ -0,0 +1,443 @@
+// saveState() used to write the entire state blob every time
+// (src/shared/state.js). Every extension page — the toolbar popup, a dApp
+// approval window opened by the background, backgroundRefresh() in
+// src/background/index.js — holds its own in-memory `state`, loaded once,
+// and src/popup/views/helpers.js showView() saves on EVERY navigation. So
+// any second page that saved after a first page had written something new
+// overwrote it, with no attacker and no unusual input: a whole wallet, name,
+// addresses and encrypted secret included, silently gone
+// (https://git.eeqj.de/sneak/AutistMask/issues/304).
+//
+// Both cases below drive the real state.js module through two independent
+// module registries sharing one storage backend, the way two real extension
+// pages share one chrome.storage.local. The storage stub structured-clones
+// on both get and set — a stub that hands back the object it was given
+// aliases the caller's own mutation and would make this entire defect class
+// invisible (see https://git.eeqj.de/sneak/AutistMask/issues/324).
+
+function makeStorage() {
+    let store = {};
+    return {
+        get: async (keys) => {
+            const wanted =
+                keys === undefined || keys === null
+                    ? Object.keys(store)
+                    : [].concat(keys);
+            const out = {};
+            for (const key of wanted) {
+                if (key in store) out[key] = structuredClone(store[key]);
+            }
+            return out;
+        },
+        set: async (items) => {
+            for (const [key, value] of Object.entries(items)) {
+                store[key] = structuredClone(value);
+            }
+        },
+    };
+}
+
+// One extension page: a fresh module registry over the shared storage.
+// state.js resolves the storage API at require time, so the stub has to be
+// installed before the module is loaded, and `state` is a module-level
+// singleton, so each page needs its own registry to hold its own copy.
+function loadPage(storage) {
+    jest.resetModules();
+    globalThis.chrome = { storage: { local: storage } };
+    return {
+        state: require("../src/shared/state"),
+        helpers: require("../src/popup/views/helpers"),
+    };
+}
+
+function wallet(name, secret, address) {
+    return {
+        type: "hd",
+        name,
+        xpub: "xpub-" + name,
+        encryptedSecret: secret,
+        nextIndex: 1,
+        addresses: [{ address, balance: "0", tokenBalances: [] }],
+    };
+}
+
+const W1 = wallet(
+    "Wallet 1",
+    "secret-one",
+    "0x66133E8ea0f5D1d612D2502a968757D1048c214a",
+);
+const W2 = wallet(
+    "Wallet 2",
+    "secret-two",
+    "0xdAC17F958D2ee523a2206206994597C13D831ec7",
+);
+
+// Minimal DOM: showView() toggles view elements, clears the flash line and
+// creates/removes the debug banner. Nothing here is asserted; it only has to
+// answer without throwing, the way the popup's own index.html would.
+function makeElement(id) {
+    const classes = new Set();
+    return {
+        id,
+        textContent: "",
+        style: {},
+        classList: {
+            add: (...n) => n.forEach((c) => classes.add(c)),
+            remove: (...n) => n.forEach((c) => classes.delete(c)),
+            toggle: (c, force) => {
+                const on = force === undefined ? !classes.has(c) : force;
+                if (on) classes.add(c);
+                else classes.delete(c);
+                return on;
+            },
+        },
+        remove: () => {},
+    };
+}
+
+function makeDocument() {
+    const els = new Map();
+    return {
+        getElementById(id) {
+            if (id === "debug-banner") return null;
+            if (!els.has(id)) els.set(id, makeElement(id));
+            return els.get(id);
+        },
+        createElement: () => makeElement("created"),
+        body: { prepend: () => {} },
+    };
+}
+
+afterEach(() => {
+    delete globalThis.chrome;
+    delete globalThis.document;
+});
+
+describe("a save from a page that never saw a wallet another page added", () => {
+    // The first DoD case on the issue: add a wallet in one page, then force
+    // a save from a second page loaded before that wallet existed. Both
+    // wallets must survive.
+    test("both wallets are in storage afterwards", async () => {
+        const storage = makeStorage();
+        await storage.set({ autistmask: { wallets: [W1] } });
+
+        // Loaded while storage held only Wallet 1, and never reloads —
+        // the approval window in the reproduction, or a second popup that
+        // has been open for a while.
+        const stale = loadPage(storage);
+        await stale.state.loadState();
+        expect(stale.state.state.wallets).toHaveLength(1);
+
+        // A second page, loaded after, adds a wallet — the exact sequence
+        // src/popup/views/addWallet.js uses.
+        const fresh = loadPage(storage);
+        await fresh.state.loadState();
+        fresh.state.state.wallets.push(W2);
+        fresh.state.state.hasWallet = true;
+        await fresh.state.saveState();
+
+        expect(
+            (await storage.get("autistmask")).autistmask.wallets,
+        ).toHaveLength(2);
+
+        // The stale page saves something that has nothing to do with
+        // wallets — exactly what showView() does on every navigation, and
+        // what backgroundRefresh() does after a balance poll.
+        stale.state.state.currentView = "settings";
+        await stale.state.saveState();
+
+        const persisted = (await storage.get("autistmask")).autistmask;
+        expect(persisted.wallets.map((w) => w.name)).toEqual([
+            "Wallet 1",
+            "Wallet 2",
+        ]);
+        expect(persisted.wallets.map((w) => w.encryptedSecret)).toEqual([
+            "secret-one",
+            "secret-two",
+        ]);
+    });
+});
+
+describe("the approval-window reproduction", () => {
+    // approval window open, add a wallet in the popup, confirm the approval
+    // — the exact sequence from the issue. The approval window and the
+    // popup are the same popup code with a different starting view, so
+    // showView() is the real save path in both: src/popup/views/approval.js
+    // showTxApproval() calls showView("approve-tx") when the window opens,
+    // and a successful confirm calls
+    // src/popup/views/txStatus.js showWait() -> startWait(), which calls
+    // showView("wait-tx") — the save that clobbered the second wallet in
+    // the reproduction on the issue.
+    test("the wallet added in the popup survives confirming the approval", async () => {
+        globalThis.document = makeDocument();
+
+        const storage = makeStorage();
+        await storage.set({ autistmask: { wallets: [W1] } });
+
+        // The background opens the approval window on the approve-tx
+        // screen; nothing else has happened yet.
+        const approvalWindow = loadPage(storage);
+        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,
+        // which is the black-box way to know it landed.
+        await approvalWindow.state.saveState();
+
+        // The user adds a wallet in the popup — a separate page, loaded
+        // after the approval window.
+        const popup = loadPage(storage);
+        await popup.state.loadState();
+        popup.state.state.wallets.push(W2);
+        popup.state.state.hasWallet = true;
+        await popup.state.saveState();
+        expect(
+            (await storage.get("autistmask")).autistmask.wallets,
+        ).toHaveLength(2);
+
+        // The user confirms the approval. The approval window navigates
+        // approve-tx -> wait-tx, saving again from state it loaded before
+        // Wallet 2 ever existed.
+        approvalWindow.helpers.showView("wait-tx");
+        await approvalWindow.state.saveState();
+
+        const persisted = (await storage.get("autistmask")).autistmask;
+        expect(persisted.wallets.map((w) => w.name)).toEqual([
+            "Wallet 1",
+            "Wallet 2",
+        ]);
+        expect(persisted.wallets.map((w) => w.encryptedSecret)).toEqual([
+            "secret-one",
+            "secret-two",
+        ]);
+    });
+});
+
+// backgroundRefresh() (src/background/index.js) loads state, spends seconds
+// on network I/O in refreshBalances() (src/shared/balances.js) mutating
+// addr.balance/ensName/tokenBalances IN PLACE on the wallets it already
+// knew about, then saves. Precondition 2 on the issue: that refresh window
+// overlapping a membership change (add or delete) on another page must not
+// clobber or resurrect a wallet — a whole-field diff on `wallets` failed
+// this, because "background changed a balance" and "another page changed
+// membership" collided as the same field.
+describe("background refresh racing a wallet added on another page", () => {
+    test("the wallet added elsewhere survives background's stale balance save", async () => {
+        const storage = makeStorage();
+        await storage.set({ autistmask: { wallets: [W1] } });
+
+        // "background": loads first, and its save is the one that lands
+        // last, modeling the multi-second network round trip in between.
+        const background = loadPage(storage);
+        await background.state.loadState();
+        background.state.state.wallets[0].addresses[0].balance = "1.2345";
+
+        // A second page, loaded after, adds a wallet while background's
+        // refresh is still in flight.
+        const popup = loadPage(storage);
+        await popup.state.loadState();
+        popup.state.state.wallets.push(W2);
+        popup.state.state.hasWallet = true;
+        await popup.state.saveState();
+        expect(
+            (await storage.get("autistmask")).autistmask.wallets,
+        ).toHaveLength(2);
+
+        // background's save lands last, carrying only its balance update.
+        await background.state.saveState();
+
+        const persisted = (await storage.get("autistmask")).autistmask;
+        expect(persisted.wallets.map((w) => w.name)).toEqual([
+            "Wallet 1",
+            "Wallet 2",
+        ]);
+        expect(persisted.wallets.map((w) => w.encryptedSecret)).toEqual([
+            "secret-one",
+            "secret-two",
+        ]);
+        // The balance update itself must not be lost either — this is a
+        // merge, not deletion-always-wins.
+        expect(persisted.wallets[0].addresses[0].balance).toBe("1.2345");
+    });
+});
+
+describe("background refresh racing a wallet deleted on another page", () => {
+    test("the wallet deleted elsewhere stays deleted after background's stale balance save", async () => {
+        const storage = makeStorage();
+        await storage.set({ autistmask: { wallets: [W1, W2] } });
+
+        const background = loadPage(storage);
+        await background.state.loadState();
+        background.state.state.wallets[0].addresses[0].balance = "1.2345";
+
+        // A second page deletes Wallet 2 while background's refresh is in
+        // flight — the same splice deleteWallet.js's removeWalletFromState()
+        // does.
+        const popup = loadPage(storage);
+        await popup.state.loadState();
+        popup.state.state.wallets.splice(1, 1);
+        popup.state.state.hasWallet = popup.state.state.wallets.length > 0;
+        await popup.state.saveState();
+        expect(
+            (await storage.get("autistmask")).autistmask.wallets,
+        ).toHaveLength(1);
+
+        await background.state.saveState();
+
+        const persisted = (await storage.get("autistmask")).autistmask;
+        expect(persisted.wallets.map((w) => w.name)).toEqual(["Wallet 1"]);
+        expect(persisted.wallets[0].addresses[0].balance).toBe("1.2345");
+    });
+});
+
+// allowedSites/deniedSites: { [address]: [hostname, ...] }. Mutated in place
+// from two different contexts — src/background/index.js:592-599 pushes a
+// newly approved hostname onto state.allowedSites[activeAddress], and the
+// Settings "revoke" button (src/popup/views/settings.js:55-68) filters a
+// hostname out of state[key][addr] in place, deleting the address key
+// entirely once its list is empty — the exact membership-vs-whole-field
+// pattern that made the whole-field `wallets` diff unsafe, on a
+// security-relevant field: a stale whole-field save here can resurrect a
+// revoked permission or wipe a freshly granted one.
+const ADDR1 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
+const ADDR2 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
+
+function approveSite(pageState, address, hostname) {
+    if (!pageState.allowedSites[address]) {
+        pageState.allowedSites[address] = [];
+    }
+    if (!pageState.allowedSites[address].includes(hostname)) {
+        pageState.allowedSites[address].push(hostname);
+    }
+}
+
+function revokeSite(pageState, hostname) {
+    for (const addr of Object.keys(pageState.allowedSites)) {
+        pageState.allowedSites[addr] = pageState.allowedSites[addr].filter(
+            (h) => h !== hostname,
+        );
+        if (pageState.allowedSites[addr].length === 0) {
+            delete pageState.allowedSites[addr];
+        }
+    }
+}
+
+describe("a dApp approval racing a stale Settings page's later save", () => {
+    test("the fresh approval survives Settings revoking an unrelated site", async () => {
+        const storage = makeStorage();
+        await storage.set({
+            autistmask: {
+                wallets: [W1],
+                allowedSites: { [ADDR2]: ["other.example"] },
+            },
+        });
+
+        // Settings loads first, and its save lands last — before either has
+        // any idea a dApp approval happened elsewhere in between.
+        const settings = loadPage(storage);
+        await settings.state.loadState();
+
+        // A dApp approval window, opened later, approves a new site for a
+        // different address and saves — the real sequence at
+        // src/background/index.js:592-599.
+        const approval = loadPage(storage);
+        await approval.state.loadState();
+        approveSite(approval.state.state, ADDR1, "dapp.example");
+        await approval.state.saveState();
+        expect(
+            (await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
+        ).toEqual(["dapp.example"]);
+
+        // Settings revokes its own, unrelated site — the real sequence at
+        // src/popup/views/settings.js:55-68 — and saves from state loaded
+        // before the dApp approval ever happened.
+        revokeSite(settings.state.state, "other.example");
+        await settings.state.saveState();
+
+        const persisted = (await storage.get("autistmask")).autistmask;
+        expect(persisted.allowedSites[ADDR1]).toEqual(["dapp.example"]);
+        expect(persisted.allowedSites[ADDR2]).toBeUndefined();
+    });
+});
+
+describe("a revoked site permission against a stale page's later save", () => {
+    test("the revocation holds even when the stale page approves something else", async () => {
+        const storage = makeStorage();
+        await storage.set({
+            autistmask: {
+                wallets: [W1],
+                allowedSites: { [ADDR1]: ["evil.example"] },
+            },
+        });
+
+        // A stale page loads while the permission still stands.
+        const stale = loadPage(storage);
+        await stale.state.loadState();
+
+        // Settings revokes it — src/popup/views/settings.js:55-68 — from a
+        // second page.
+        const settings = loadPage(storage);
+        await settings.state.loadState();
+        revokeSite(settings.state.state, "evil.example");
+        await settings.state.saveState();
+        expect(
+            (await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
+        ).toBeUndefined();
+
+        // The stale page, unaware of the revoke, approves an unrelated site
+        // for a different address and saves — src/background/index.js:592-599.
+        approveSite(stale.state.state, ADDR2, "good.example");
+        await stale.state.saveState();
+
+        const persisted = (await storage.get("autistmask")).autistmask;
+        expect(persisted.allowedSites[ADDR2]).toEqual(["good.example"]);
+        expect(persisted.allowedSites[ADDR1]).toBeUndefined();
+    });
+});
+
+// mergeListByIdentity()'s identity function is not guaranteed collision-free
+// — walletIdentity() falls back to one shared "addr:" value for any wallet
+// with neither an xpub nor a populated first address (a legacy or corrupt
+// record). Two such records created independently on two different pages
+// must not silently collapse into one, dropping the loser's
+// encryptedSecret with no error and no log.
+function legacyWallet(name, secret) {
+    return {
+        type: "legacy",
+        name,
+        encryptedSecret: secret,
+        nextIndex: 0,
+        addresses: [],
+    };
+}
+
+describe("two wallets independently created with a colliding identity", () => {
+    test("both survive, encryptedSecret included, instead of one silently replacing the other", async () => {
+        const storage = makeStorage();
+        await storage.set({ autistmask: { wallets: [W1] } });
+
+        // Both pages load before either has created their malformed wallet,
+        // so neither has baseline knowledge of the other's.
+        const pageA = loadPage(storage);
+        await pageA.state.loadState();
+        const pageB = loadPage(storage);
+        await pageB.state.loadState();
+
+        pageA.state.state.wallets.push(legacyWallet("Legacy A", "secret-a"));
+        pageA.state.state.hasWallet = true;
+        await pageA.state.saveState();
+        expect(
+            (await storage.get("autistmask")).autistmask.wallets,
+        ).toHaveLength(2);
+
+        pageB.state.state.wallets.push(legacyWallet("Legacy B", "secret-b"));
+        pageB.state.state.hasWallet = true;
+        await pageB.state.saveState();
+
+        const persisted = (await storage.get("autistmask")).autistmask;
+        const secrets = persisted.wallets.map((w) => w.encryptedSecret);
+        expect(secrets).toContain("secret-one");
+        expect(secrets).toContain("secret-a");
+        expect(secrets).toContain("secret-b");
+    });
+});