fix: a network switch in Settings leaves no balance from the previous network stored (closes #653)
A network switch in Settings could leave the previous network's balance in the stored record. The switch clears the page's balances inside its save, but the save writes only fields the page changed since its last save. When the page had last saved "0" for an address, the cleared "0" was no change, so a balance the background's refresh stored meanwhile stayed under the new network. The switch's save now always writes the cleared balances and the time of the last balance refresh. Every other field merges as before. Model: opus-5-5
This commit was merged in pull request #689.
This commit is contained in:
@@ -47,12 +47,30 @@ 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 save writes only the fields that differ from what this page last saved.
|
||||
// When this page last saved a balance of "0", clearing that balance is no
|
||||
// change. So the switch also resets these fields in the stored record:
|
||||
//
|
||||
// - every address's balance and token balances;
|
||||
// - the time of the last balance refresh.
|
||||
//
|
||||
// A balance that the background or another page stored since this page's last
|
||||
// save is then not kept 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);
|
||||
stored.lastBalanceRefresh = 0;
|
||||
for (const wallet of stored.wallets) {
|
||||
for (const addr of wallet.addresses) {
|
||||
addr.balance = "0";
|
||||
addr.tokenBalances = [];
|
||||
}
|
||||
}
|
||||
clearPopupCaches();
|
||||
});
|
||||
return net;
|
||||
|
||||
+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 then 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,10 @@
|
||||
// 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 balance refresh by the background that is saved while
|
||||
// a switch in Settings waits for the lock
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/653).
|
||||
|
||||
const {
|
||||
bootPopup,
|
||||
@@ -159,3 +163,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
|
||||
// token balances, the values that 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);
|
||||
});
|
||||
|
||||
@@ -177,6 +177,78 @@ 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. The switch's save writes only the token lists that this page changed.
|
||||
// So DAI stays in `trackedTokens`, the active network's list.
|
||||
test("a token another page tracked before a switch in Settings stays in trackedTokens", 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([MAINNET_TOKEN]);
|
||||
expect(stored.networkTrackedTokens.mainnet).toEqual([]);
|
||||
});
|
||||
|
||||
// The contract lookup on SettingsAddToken answers on Ethereum Mainnet while the
|
||||
// switch to Sepolia waits for the lock. The page adds the token to Ethereum
|
||||
// Mainnet's list. The switch then keeps that list for Ethereum Mainnet.
|
||||
test("a token added while a switch in Settings waits for the lock stays tracked on Ethereum Mainnet", async () => {
|
||||
const boot = await bootOnAddToken("mainnet");
|
||||
expect(boot.pageErrors).toEqual([]);
|
||||
const { lookupTokenInfo } = require("../src/shared/balances");
|
||||
let answer;
|
||||
lookupTokenInfo.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
answer = resolve;
|
||||
}),
|
||||
);
|
||||
boot.node("settings-addtoken-address").value = NEW_TOKEN_ADDRESS;
|
||||
const adding = boot.click("btn-settings-addtoken-manual");
|
||||
await boot.click("btn-settings");
|
||||
|
||||
// Another write holds the lock, so the switch's save waits for it.
|
||||
let releaseLock;
|
||||
const lockHeld = navigator.locks.request(
|
||||
"autistmask",
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
releaseLock = resolve;
|
||||
}),
|
||||
);
|
||||
const select = boot.node("settings-network");
|
||||
select.value = "sepolia";
|
||||
const switching = Promise.all(select.listeners.change.map((fn) => fn()));
|
||||
await boot.settle();
|
||||
|
||||
answer({ symbol: "NEW", decimals: 18, name: "New Token" });
|
||||
await boot.settle();
|
||||
releaseLock();
|
||||
await lockHeld;
|
||||
await switching;
|
||||
await adding;
|
||||
|
||||
const stored = boot.storage.read("autistmask");
|
||||
expect(stored.networkId).toBe("sepolia");
|
||||
expect(stored.trackedTokens).toEqual([]);
|
||||
expect(stored.networkTrackedTokens.mainnet).toEqual([
|
||||
{
|
||||
address: NEW_TOKEN_ADDRESS,
|
||||
symbol: "NEW",
|
||||
decimals: 18,
|
||||
name: "New 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