diff --git a/src/shared/chainSwitch.js b/src/shared/chainSwitch.js index 5c86f84..18e8610 100644 --- a/src/shared/chainSwitch.js +++ b/src/shared/chainSwitch.js @@ -2,8 +2,10 @@ // // A network switch reaches the popup's `state` in one of two ways: // -// - onChainSwitch() below makes the switch the user picks in Settings, with -// applyChainSwitchFields() (chainSwitchFields.js). +// - onChainSwitch() below makes the switch the user picks in Settings. It +// makes the switch on the stored record with applyChainSwitchFields(), and +// copies the switched fields into `state` with copyChainSwitchFields() +// (chainSwitchFields.js). // - followStoredSettings() in src/popup/index.js takes a switch the background // or another popup page saved, from the stored record, with // takeStoredNetwork() (state.js), which copies it with @@ -27,7 +29,10 @@ // against storage, and the singleton is not reachable from the background // bundle at all (enforced by the ESLint rule in eslint.config.js). -const { applyChainSwitchFields } = require("./chainSwitchFields"); +const { + applyChainSwitchFields, + copyChainSwitchFields, +} = require("./chainSwitchFields"); const { clearPrices } = require("./prices"); // The popup's own caches a network switch resets. Prices are chain-specific @@ -47,12 +52,23 @@ function clearPopupCaches() { // lock, so it is never switched while a follow of a switch saved elsewhere // waits for the lock (followStoredSettings() in src/popup/index.js): that // follow runs first, and this switch is made after it. +// +// The switch is made on the stored record that the save has just read, as the +// background makes its own switch. The switched fields are then copied from +// that record into `state`. So the switch also covers what another page stored +// since this page's last save: +// +// - A token tracked on the previous network stays tracked on that network. +// - An endpoint URL saved for the previous network stays with that network. +// - A balance from the previous network is cleared +// (https://git.eeqj.de/sneak/AutistMask/issues/653). async function onChainSwitch(newNetworkId) { const { state, saveState } = require("./state"); let net = null; - await saveState(() => { + await saveState((stored) => { if (state.networkId === newNetworkId) return false; - net = applyChainSwitchFields(state, newNetworkId); + net = applyChainSwitchFields(stored, newNetworkId); + copyChainSwitchFields(state, stored); clearPopupCaches(); }); return net; diff --git a/src/shared/chainSwitchFields.js b/src/shared/chainSwitchFields.js index cea6ef2..4f3b777 100644 --- a/src/shared/chainSwitchFields.js +++ b/src/shared/chainSwitchFields.js @@ -3,9 +3,10 @@ // // Split out of chainSwitch.js so the background can perform a chain switch // without the singleton being reachable from its bundle at all. The popup -// still goes through onChainSwitch() (chainSwitch.js), which applies this to -// the singleton and saves; the background applies it to the detached record of -// its own read-modify-write (src/background/state.js). +// still goes through onChainSwitch() (chainSwitch.js). That function applies +// this to the stored record that the switch's save reads, and copies the +// result into the singleton. The background applies it to the detached record +// of its own read-modify-write (src/background/state.js). // // Everything here is synchronous and touches nothing but the objects it is // given: no storage, no caches, no imports beyond the network table. That is diff --git a/src/shared/state.js b/src/shared/state.js index 227763c..1d75e94 100644 --- a/src/shared/state.js +++ b/src/shared/state.js @@ -591,8 +591,10 @@ function reportSaveFailure(err) { // // `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. +// `state`, and this save writes that change. It may also change the stored +// record: a field that this page did not change is written as `beforeWrite` +// left it. 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 diff --git a/tests/balanceRefreshNetworkSwitch.test.js b/tests/balanceRefreshNetworkSwitch.test.js index 94b0a4a..54195b3 100644 --- a/tests/balanceRefreshNetworkSwitch.test.js +++ b/tests/balanceRefreshNetworkSwitch.test.js @@ -6,6 +6,9 @@ // approved a site's request, which changes only the stored record. Driven // through the real popup over a stored profile on Ethereum Mainnet. The same // refresh in the background is in tests/backgroundStateIsolation.test.js. +// +// The last test covers a refresh by the background that is saved just before +// the switch in Settings (https://git.eeqj.de/sneak/AutistMask/issues/653). const { bootPopup, @@ -159,3 +162,50 @@ test("a refresh that returns after the background switched the network writes no expectNothingStored(page); }); + +// The background's refresh saves Ethereum Mainnet's balances while the switch +// in Settings waits for the lock. This page last saved a balance of "0" and no +// tokens, the same values the switch sets. +test("a background refresh saved while a switch in Settings waits for the lock leaves none of its balances stored", async () => { + const profile = unversionedValidProfile(); + profile.wallets[0].addresses[0].balance = "0"; + const page = await bootPopup(profile); + expect(page.pageErrors).toEqual([]); + const background = require("../src/background/state"); + await page.click("btn-settings"); + + // The background takes the lock first and holds it until the test calls + // `letGo()`. It then saves balances as backgroundRefresh() in + // src/background/index.js does. + let letGo; + const refreshSave = background.updateState(async (fresh) => { + await new Promise((resolve) => { + letGo = resolve; + }); + fresh.wallets[0].addresses[0].balance = "7.0"; + fresh.wallets[0].addresses[0].tokenBalances = [ + { + address: TOKEN_ADDRESS, + name: "Test Token", + symbol: "TST", + decimals: 18, + balance: "5.0", + holders: null, + }, + ]; + fresh.lastBalanceRefresh = Date.now(); + }); + await page.settle(); + + const select = page.node("settings-network"); + select.value = "sepolia"; + const switching = Promise.all(select.listeners.change.map((fn) => fn())); + await page.settle(); + + letGo(); + await refreshSave; + await switching; + await page.settle(); + + expectNothingStored(page); +}); diff --git a/tests/networkEndpoints.test.js b/tests/networkEndpoints.test.js index d403505..b632d8f 100644 --- a/tests/networkEndpoints.test.js +++ b/tests/networkEndpoints.test.js @@ -47,6 +47,7 @@ function loadModuleWith(persisted) { return { mod: require("../src/shared/state"), chainSwitch: require("../src/shared/chainSwitch"), + storage, written: () => storage.read("autistmask"), }; } @@ -105,6 +106,27 @@ describe("a custom endpoint survives a chain switch", () => { expect(mod.state.rpcUrl).toBe(CUSTOM_RPC); }); + // Another page saves the user's own node for Ethereum Mainnet after this + // page's last save. This page's switch keeps that node for Ethereum + // Mainnet. + test("an endpoint another page saved for the network being left is remembered", async () => { + const { mod, chainSwitch, storage, written } = loadModuleWith({ + wallets: walletFixture(), + networkId: "mainnet", + rpcUrl: MAINNET.defaultRpcUrl, + blockscoutUrl: MAINNET.defaultBlockscoutUrl, + networkEndpoints: {}, + }); + await mod.loadState(); + storage.write("autistmask", { ...written(), rpcUrl: CUSTOM_RPC }); + + await chainSwitch.onChainSwitch("sepolia"); + expect(written().networkEndpoints.mainnet.rpcUrl).toBe(CUSTOM_RPC); + + await chainSwitch.onChainSwitch("mainnet"); + expect(mod.state.rpcUrl).toBe(CUSTOM_RPC); + }); + test("the remembered endpoints survive an extension restart", async () => { const first = loadModuleWith({ wallets: walletFixture(), diff --git a/tests/trackedTokensPerNetwork.test.js b/tests/trackedTokensPerNetwork.test.js index 378d556..32bf020 100644 --- a/tests/trackedTokensPerNetwork.test.js +++ b/tests/trackedTokensPerNetwork.test.js @@ -177,6 +177,25 @@ test("a token added by contract address on Sepolia is tracked on Sepolia only", expect(onSepolia).not.toContain("DAI"); }); +// Another popup page tracks DAI on Ethereum Mainnet after this page's last +// save. This page's switch in Settings keeps DAI, tracked on Ethereum Mainnet. +test("a token another page tracked stays on its network through a switch in Settings", async () => { + const boot = await bootPopup(unversionedValidProfile()); + expect(boot.pageErrors).toEqual([]); + boot.storage.write("autistmask", { + ...boot.storage.read("autistmask"), + trackedTokens: [MAINNET_TOKEN], + }); + + await boot.click("btn-settings"); + await chooseNetwork(boot, "sepolia"); + + const stored = boot.storage.read("autistmask"); + expect(stored.networkId).toBe("sepolia"); + expect(stored.trackedTokens).toEqual([]); + expect(stored.networkTrackedTokens.mainnet).toEqual([MAINNET_TOKEN]); +}); + describe("Back after a switch from the Settings gear", () => { test("SettingsAddToken no longer offers the common tokens", async () => { const boot = await bootOnAddToken("mainnet");