fix: a network switch in Settings leaves no balance from the previous network stored (closes #653)
The switch is now made on the stored record that its save has just read. The switched fields are then copied into the page's state. A save writes only the fields that the page changed since its last save. A switch made on the page's state alone mishandled what another page stored since then: - A balance from the previous network stayed under the new network. - A token tracked on the previous network moved to the new network. - An endpoint URL saved for the previous network was replaced by the page's older one. Tests store each of these before the switch and check the stored record after it. Model: opus-5-5
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
+4
-2
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user