diff --git a/src/shared/chainSwitch.js b/src/shared/chainSwitch.js index 5c86f84..3bab5b9 100644 --- a/src/shared/chainSwitch.js +++ b/src/shared/chainSwitch.js @@ -47,12 +47,20 @@ 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 also applied to the stored record that the save has just read, +// as the background applies its own switch. A save writes only the fields that +// this page changed since its last save. A cleared balance of "0" is no change +// when this page last saved "0" too. The balance that the background stored +// since then would otherwise stay under the new network +// (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); + applyChainSwitchFields(stored, newNetworkId); clearPopupCaches(); }); return net; diff --git a/src/shared/chainSwitchFields.js b/src/shared/chainSwitchFields.js index cea6ef2..b6e327a 100644 --- a/src/shared/chainSwitchFields.js +++ b/src/shared/chainSwitchFields.js @@ -4,8 +4,9 @@ // 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). +// the singleton and to the stored record that its save reads; 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..22cc4fe 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,49 @@ 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. Once let go, the background 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); +});