Compare commits

...

2 Commits

Author SHA1 Message Date
clawbot
27e37b9976 fix: derive hasWallet from the wallet list on load (closes #195)
All checks were successful
check / check (push) Successful in 36s
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.

The only consumer is the popup's welcome-vs-wallet-list gate in
src/popup/index.js; the only writers set it true alongside an added
wallet. Nothing reads it expecting it to differ from wallets.length, so
deriving is safe and needs no write-back on load.
2026-08-11 12:23:28 +00:00
b882cede9f fix: repair wallet state on delete (closes #156)
Some checks failed
check / check (push) Has been cancelled
2026-08-11 14:23:06 +02:00
7 changed files with 330 additions and 23 deletions

View File

@@ -980,7 +980,7 @@ Currently supported:
### Wallet Management ### Wallet Management
- [ ] Delete wallet (with confirmation) - [x] Delete wallet (with confirmation)
- [ ] Delete address from HD wallet (with confirmation) - [ ] Delete address from HD wallet (with confirmation)
- [ ] Show wallet's recovery phrase (requires password) - [ ] Show wallet's recovery phrase (requires password)

View File

@@ -44,6 +44,14 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 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
([#156](https://git.eeqj.de/sneak/AutistMask/issues/156)).
- 2026-08-11: `TODO.md` Workflow rewritten to the branch-and-PR-per-issue model - 2026-08-11: `TODO.md` Workflow rewritten to the branch-and-PR-per-issue model
on `next`, with Status and Next Step refreshed on `next`, with Status and Next Step refreshed
([#191](https://git.eeqj.de/sneak/AutistMask/issues/191)). ([#191](https://git.eeqj.de/sneak/AutistMask/issues/191)).

View File

@@ -1,6 +1,10 @@
const { $, showView, showFlash, goBack, clearViewStack } = require("./helpers"); const { $, showView, showFlash, goBack, clearViewStack } = require("./helpers");
const { state, saveState } = require("../../shared/state"); const { state, saveState } = require("../../shared/state");
const { decryptWithPassword } = require("../../shared/vault"); const { decryptWithPassword } = require("../../shared/vault");
const {
removeWalletFromState,
broadcastActiveChanged,
} = require("../../shared/walletDelete");
let deleteWalletIndex = null; let deleteWalletIndex = null;
let ctx = null; let ctx = null;
@@ -58,35 +62,24 @@ function init(_ctx) {
return; return;
} }
// Collect addresses to clean up from allowedSites/deniedSites // Remove the wallet and repair selection, permissions and hasWallet
const addresses = (wallet.addresses || []).map((a) => a.address); const { activeAddressChanged } = removeWalletFromState(
state,
// Remove wallet walletIdx,
state.wallets.splice(walletIdx, 1); );
// Clean up site permissions for deleted addresses
for (const addr of addresses) {
delete state.allowedSites[addr];
delete state.deniedSites[addr];
}
deleteWalletIndex = null; deleteWalletIndex = null;
if (state.wallets.length === 0) { if (!state.hasWallet) {
// No wallets left — reset selection and show welcome
state.selectedWallet = null;
state.selectedAddress = null;
state.activeAddress = null;
clearViewStack(); clearViewStack();
await saveState(); await saveState();
// Save before broadcasting: the background reads the active
// address back out of storage to build accountsChanged.
if (activeAddressChanged) broadcastActiveChanged();
showView("welcome"); showView("welcome");
} else { } else {
// Switch to first wallet if deleted wallet was active
state.selectedWallet = 0;
state.selectedAddress = 0;
state.activeAddress =
state.wallets[0].addresses[0]?.address || null;
await saveState(); await saveState();
if (activeAddressChanged) broadcastActiveChanged();
// Reset stack to [main] so Settings back goes home. // Reset stack to [main] so Settings back goes home.
// Use require() lazily to avoid circular dependency // Use require() lazily to avoid circular dependency
// (settings.js requires deleteWallet.js). // (settings.js requires deleteWallet.js).

View File

@@ -84,8 +84,11 @@ async function loadState() {
const result = await storageApi.get("autistmask"); const result = await storageApi.get("autistmask");
if (result.autistmask) { if (result.autistmask) {
const saved = result.autistmask; const saved = result.autistmask;
state.hasWallet = saved.hasWallet;
state.wallets = saved.wallets || []; 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.trackedTokens = saved.trackedTokens || [];
state.networkId = saved.networkId || DEFAULT_STATE.networkId; state.networkId = saved.networkId || DEFAULT_STATE.networkId;
state.rpcUrl = saved.rpcUrl || DEFAULT_STATE.rpcUrl; state.rpcUrl = saved.rpcUrl || DEFAULT_STATE.rpcUrl;

View File

@@ -0,0 +1,70 @@
// Wallet deletion state transition, kept out of the view so the selection
// and broadcast rules are testable without a DOM.
// Remove wallet `walletIdx` from `state` and repair the derived state.
//
// Rules:
// - `hasWallet` tracks whether any wallet remains.
// - Site permissions are dropped for every address of the deleted wallet.
// - `selectedWallet` follows the splice: it is decremented when a wallet
// before it was removed, and falls back to the first remaining wallet's
// first address only when the selection itself was deleted.
// - `activeAddress` is only moved when it belonged to the deleted wallet;
// the fallback is the first remaining wallet's first address, or null
// when no wallet remains.
//
// Returns whether `activeAddress` changed, so the caller can broadcast it.
function removeWalletFromState(state, walletIdx) {
const wallet = state.wallets[walletIdx];
const addresses = (wallet.addresses || []).map((a) => a.address);
const previousActive = state.activeAddress;
const activeWasDeleted =
previousActive !== null &&
previousActive !== undefined &&
addresses.some(
(a) => a.toLowerCase() === String(previousActive).toLowerCase(),
);
state.wallets.splice(walletIdx, 1);
for (const addr of addresses) {
delete state.allowedSites[addr];
delete state.deniedSites[addr];
}
state.hasWallet = state.wallets.length > 0;
const fallbackAddress = state.hasWallet
? state.wallets[0].addresses[0]?.address || null
: null;
if (!state.hasWallet) {
state.selectedWallet = null;
state.selectedAddress = null;
} else if (state.selectedWallet === walletIdx) {
state.selectedWallet = 0;
state.selectedAddress = 0;
} else if (
typeof state.selectedWallet === "number" &&
state.selectedWallet > walletIdx
) {
state.selectedWallet -= 1;
}
if (activeWasDeleted || !state.hasWallet) {
state.activeAddress = fallbackAddress;
}
return { activeAddressChanged: state.activeAddress !== previousActive };
}
// Tell the background the active address changed, so it re-emits
// accountsChanged to connected sites. Same call shape as the address
// switch in the home view.
function broadcastActiveChanged() {
const runtime =
typeof browser !== "undefined" ? browser.runtime : chrome.runtime;
runtime.sendMessage({ type: "AUTISTMASK_ACTIVE_CHANGED" });
}
module.exports = { removeWalletFromState, broadcastActiveChanged };

104
tests/state.test.js Normal file
View File

@@ -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);
});
});

129
tests/walletDelete.test.js Normal file
View File

@@ -0,0 +1,129 @@
const {
removeWalletFromState,
broadcastActiveChanged,
} = require("../src/shared/walletDelete");
// Fixed addresses — never used for anything but these tests.
const A0 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599";
const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
function wallet(name, addresses) {
return {
name,
addresses: addresses.map((address) => ({ address })),
};
}
// A three-wallet state; wallet A is an HD wallet with two addresses.
function makeState(overrides = {}) {
return {
hasWallet: true,
wallets: [wallet("A", [A0, A1]), wallet("B", [B0]), wallet("C", [C0])],
selectedWallet: 0,
selectedAddress: 0,
activeAddress: A0,
allowedSites: {
[A0]: ["a.example"],
[A1]: ["b.example"],
[B0]: ["c.example"],
},
deniedSites: { [A1]: ["d.example"], [C0]: ["e.example"] },
...overrides,
};
}
describe("removeWalletFromState", () => {
test("deleting the last wallet clears hasWallet", () => {
const state = makeState({
wallets: [wallet("A", [A0])],
allowedSites: { [A0]: ["a.example"] },
deniedSites: {},
});
const { activeAddressChanged } = removeWalletFromState(state, 0);
expect(state.hasWallet).toBe(false);
expect(state.wallets).toEqual([]);
expect(state.selectedWallet).toBeNull();
expect(state.selectedAddress).toBeNull();
expect(state.activeAddress).toBeNull();
expect(activeAddressChanged).toBe(true);
});
test("deleting a non-selected wallet leaves the selection intact", () => {
const state = makeState({
selectedWallet: 2,
selectedAddress: 0,
activeAddress: C0,
});
const { activeAddressChanged } = removeWalletFromState(state, 1);
// Wallet C moved from index 2 to index 1 by the splice.
expect(state.wallets.map((w) => w.name)).toEqual(["A", "C"]);
expect(state.selectedWallet).toBe(1);
expect(state.selectedAddress).toBe(0);
expect(state.activeAddress).toBe(C0);
expect(activeAddressChanged).toBe(false);
expect(state.hasWallet).toBe(true);
});
test("deleting a wallet after the selection does not shift it", () => {
const state = makeState({
selectedWallet: 1,
selectedAddress: 0,
activeAddress: B0,
});
const { activeAddressChanged } = removeWalletFromState(state, 2);
expect(state.selectedWallet).toBe(1);
expect(state.activeAddress).toBe(B0);
expect(activeAddressChanged).toBe(false);
});
test("deleting the active wallet falls back to the first remaining address", () => {
const state = makeState({
selectedWallet: 0,
selectedAddress: 1,
activeAddress: A1,
});
const { activeAddressChanged } = removeWalletFromState(state, 0);
expect(state.wallets.map((w) => w.name)).toEqual(["B", "C"]);
expect(state.selectedWallet).toBe(0);
expect(state.selectedAddress).toBe(0);
expect(state.activeAddress).toBe(B0);
expect(activeAddressChanged).toBe(true);
expect(state.hasWallet).toBe(true);
});
test("site permissions are dropped for every address of the wallet", () => {
const state = makeState();
removeWalletFromState(state, 0);
expect(state.allowedSites).toEqual({ [B0]: ["c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["e.example"] });
});
});
describe("broadcastActiveChanged", () => {
afterEach(() => {
delete global.chrome;
});
test("sends AUTISTMASK_ACTIVE_CHANGED to the background", () => {
const sendMessage = jest.fn();
global.chrome = { runtime: { sendMessage } };
broadcastActiveChanged();
expect(sendMessage).toHaveBeenCalledWith({
type: "AUTISTMASK_ACTIVE_CHANGED",
});
});
});