From 25894735000d9a1e5f3ee82fb5b198476622f85b Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 11 Aug 2026 12:21:56 +0000 Subject: [PATCH] fix: derive hasWallet from the wallet list on load (closes #195) loadState() took hasWallet straight from storage, so any profile persisted with the flag out of step with wallets stayed broken on every subsequent load rather than only until the next write. The flag is now derived from wallets.length at load time. Deriving is only safe if no consumer wants hasWallet to mean something other than "the wallet list is non-empty" -- an "onboarding was completed" or "a wallet existed once" marker would be destroyed by it. So every occurrence was enumerated at this base (git grep -n hasWallet over tracked files; no bracket-notation, destructured or case-variant access exists). Reads: src/popup/index.js:265 -- if (!state.hasWallet) gates the welcome view against the wallet list at popup init, immediately after loadState(). Means "the user has a wallet"; the derivation is exactly that. src/popup/views/deleteWallet.js:73 -- if (!state.hasWallet) chooses the post-delete view. It reads the flag immediately after removeWalletFromState() has set it from state.wallets.length > 0, so it means "any wallets remain" -- identical to the derivation, and on the write path rather than the load path. src/shared/walletDelete.js:37,41,54 -- fallback address, selection reset and active-address reset inside removeWalletFromState(), all reads-after-write of the assignment at line 35 (state.hasWallet = state.wallets.length > 0). Same expression, same invocation. src/shared/state.js:52 -- saveState() persists the in-memory value. With the derivation in place it now writes the derived value, so an inconsistent stored blob is normalized by the next ordinary save. Writes: src/popup/views/addWallet.js:144,200,252 set it true immediately after pushing a wallet; src/shared/walletDelete.js:35 sets it from the remaining wallet count; src/shared/state.js:12 defaults it false alongside wallets: []. Every writer keeps it equal to wallets.length > 0. No consumer depends on the two disagreeing, so the derivation preserves every read and needs no write-back on load. --- TODO.md | 4 ++ src/shared/state.js | 5 ++- tests/state.test.js | 104 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 tests/state.test.js diff --git a/TODO.md b/TODO.md index 608523b..b9aa723 100644 --- a/TODO.md +++ b/TODO.md @@ -50,6 +50,10 @@ undefined identifiers, which is how - 2026-08-11: `docs/README.md` rewritten against the code: no competitor names, all five network destinations documented, password/Settings/Add Wallet sections corrected ([#163](https://git.eeqj.de/sneak/AutistMask/issues/163)). +- 2026-08-11: `loadState()` now derives `hasWallet` from the wallet list instead + of trusting the persisted flag, so a profile already saved inconsistent no + longer stays broken on every load + ([#195](https://git.eeqj.de/sneak/AutistMask/issues/195)). - 2026-08-11: Wallet deletion repairs its own state — `hasWallet` follows the remaining wallets, the selection only moves when it was deleted, and the active-address change is broadcast to connected sites diff --git a/src/shared/state.js b/src/shared/state.js index b0192d8..14e76eb 100644 --- a/src/shared/state.js +++ b/src/shared/state.js @@ -84,8 +84,11 @@ async function loadState() { const result = await storageApi.get("autistmask"); if (result.autistmask) { const saved = result.autistmask; - state.hasWallet = saved.hasWallet; 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; diff --git a/tests/state.test.js b/tests/state.test.js new file mode 100644 index 0000000..353d7e6 --- /dev/null +++ b/tests/state.test.js @@ -0,0 +1,104 @@ +const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; + +function oneWallet() { + return [{ name: "Wallet 1", type: "hd", addresses: [ADDRESS] }]; +} + +// state.js resolves the storage API at require time, so the stub has to exist +// before the module is loaded, and the module registry has to be reset between +// cases because `state` is a module-level singleton. +function loadModuleWith(persisted) { + jest.resetModules(); + const set = jest.fn(async () => {}); + global.chrome = { + storage: { + local: { + get: jest.fn(async () => + persisted ? { autistmask: persisted } : {}, + ), + set, + }, + }, + }; + return { mod: require("../src/shared/state"), set }; +} + +afterEach(() => { + delete global.chrome; +}); + +describe("loadState hasWallet reconciliation", () => { + // A profile that deleted its last wallet on a build predating the write + // path fix keeps hasWallet: true forever. It must load as no wallet, which + // is what sends the popup to the welcome view. + test("stored hasWallet true with zero wallets loads as no wallet", async () => { + const { mod } = loadModuleWith({ hasWallet: true, wallets: [] }); + await mod.loadState(); + expect(mod.state.hasWallet).toBe(false); + }); + + test("stored hasWallet true with a missing wallets key loads as no wallet", async () => { + const { mod } = loadModuleWith({ hasWallet: true }); + await mod.loadState(); + expect(mod.state.wallets).toEqual([]); + expect(mod.state.hasWallet).toBe(false); + }); + + test("stored hasWallet false with one wallet loads as having a wallet", async () => { + const { mod } = loadModuleWith({ + hasWallet: false, + wallets: oneWallet(), + }); + await mod.loadState(); + expect(mod.state.hasWallet).toBe(true); + }); + + test("absent hasWallet with wallets present loads as having a wallet", async () => { + const { mod } = loadModuleWith({ wallets: oneWallet() }); + await mod.loadState(); + expect(mod.state.hasWallet).toBe(true); + }); + + test("consistent stored states are preserved", async () => { + const withWallet = loadModuleWith({ + hasWallet: true, + wallets: oneWallet(), + }); + await withWallet.mod.loadState(); + expect(withWallet.mod.state.hasWallet).toBe(true); + + const without = loadModuleWith({ hasWallet: false, wallets: [] }); + await without.mod.loadState(); + expect(without.mod.state.hasWallet).toBe(false); + }); + + test("empty storage leaves the default no-wallet state", async () => { + const { mod } = loadModuleWith(null); + await mod.loadState(); + expect(mod.state.hasWallet).toBe(false); + expect(mod.state.wallets).toEqual([]); + }); + + // The correction is derived on every load rather than written back, so a + // load never has a storage side effect. + test("loadState does not write to storage", async () => { + const { mod, set } = loadModuleWith({ hasWallet: true, wallets: [] }); + await mod.loadState(); + expect(set).not.toHaveBeenCalled(); + }); + + // Deriving must not disturb the rest of the load. + test("other persisted fields still load", async () => { + const { mod } = loadModuleWith({ + hasWallet: false, + wallets: oneWallet(), + networkId: "sepolia", + theme: "dark", + activeAddress: ADDRESS, + }); + await mod.loadState(); + expect(mod.state.networkId).toBe("sepolia"); + expect(mod.state.theme).toBe("dark"); + expect(mod.state.activeAddress).toBe(ADDRESS); + }); +}); -- 2.49.1