diff --git a/README.md b/README.md index aef88ba..8ae51e1 100644 --- a/README.md +++ b/README.md @@ -461,13 +461,15 @@ address, the transaction assertions run against the raw signed transaction captured at `eth_sendRawTransaction` rather than against anything the extension reported, rejecting each prompt is required to return a rejection to the page rather than hang or resolve, a network switch request is required to leave the -stored network unchanged and send no `chainChanged` until it is approved, a -network switch in Settings is required to send the page `chainChanged` with the -new chain id, a prompt raised while another approval window has focus is -required to open a window of its own, and the password is required to be absent -from every message the approval window sends to the background — with the -message that would carry it required to be present, so that check cannot pass by -observing nothing. That last one is the standing floor under +stored network unchanged and send no `chainChanged` until it is approved, the +popup open all along is required to show the `[TESTNET]` banner once a switch to +Sepolia is approved and to drop it once the switch back is, a network switch in +Settings is required to send the page `chainChanged` with the new chain id, a +prompt raised while another approval window has focus is required to open a +window of its own, and the password is required to be absent from every message +the approval window sends to the background — with the message that would carry +it required to be present, so that check cannot pass by observing nothing. That +last one is the standing floor under [#157](https://git.eeqj.de/sneak/AutistMask/issues/157). The limits of that coverage and of the rest of the Chrome suite, none of them @@ -603,9 +605,10 @@ suite), wallet creation through the UI, the Add Token screen, and the dApp round trips — `eth_requestAccounts`, `personal_sign`, `wallet_switchEthereumChain` rejected and then approved, `eth_sendTransaction`, and a closed approval window rejecting with EIP-1193 4001 — driven through the real content script, -background page and approval windows. The `personal_sign` prompt is also -required to fit its window without scrolling sideways, with the extension's -pages at 170% zoom. +background page and approval windows. The popup open all along is required to +show the `[TESTNET]` banner once the switch to Sepolia is approved and to drop +it once the switch back is. The `personal_sign` prompt is also required to fit +its window without scrolling sideways, with the extension's pages at 170% zoom. The suite lives in `tests/e2e/firefox/`. Its WebDriver client (`driver.js`) has **no npm dependencies at all**: it is built on global `fetch` and @@ -1564,20 +1567,29 @@ such as an edit typed into Settings and not yet saved. Home is the one screen "Back" always re-renders, so the wallet list reflects anything that changed while the user was away from it. -A network switch in Settings leaves every screen beneath Settings showing the -previous network, so after it "Back" behaves as in a reopened popup: every -screen on the stack counts as not yet rendered, and the stack is cut at the -first screen that cannot be rendered from saved state, such as Send or AddToken, -or at AddressToken for any token but the native one, because that token was -picked on the previous network. The cut stack is saved with the switch, so a -popup closed straight after it and reopened onto Settings lands in the same -place. "Back" then renders the screen it lands on for the new network, and -anything typed there and not saved, which belonged to the previous network, is -dropped. ConfirmTx, WaitTx, SuccessTx, ErrorTx and TransactionDetail are -rendered instead for the network of the transaction they show: the network it -was reviewed on for ConfirmTx, and the network it was sent on for the others. -Without a wallet the stack is not cut, because it starts at Welcome, which shows -nothing from a network. +A network switch leaves every screen on the page showing the previous network. +The user makes it in Settings, or approves a site's request and the background +saves it. Any other popup open meanwhile takes the switch from the stored +record, even one saved while it was opening, and never switches the network +itself. A switch the user picks in its Settings while it takes one is made after +it, not overwritten by it. An approval window does not take the switch. After +the switch, whichever way the page got it, "Back" behaves as in a reopened +popup: every screen on the stack counts as not yet rendered, and the stack is +cut at the first screen that cannot be rendered from saved state, such as Send +or AddToken, or at AddressToken for any token but the native one, because that +token was picked on the previous network and is no longer selected. The cut +stack is saved with the popup's new network, so a popup closed straight after +and reopened lands in the same place. The screen on top is drawn again for the +new network: Settings in place, and any other screen as a reopened popup draws +it, so Home takes the place of one that cannot be rendered from saved state or +of AddressToken for a token no longer selected. "Back" then renders the screen +it lands on for the new network, and anything typed there and not saved, which +belonged to the previous network, is dropped. ConfirmTx, WaitTx, SuccessTx, +ErrorTx and TransactionDetail are rendered instead for the network of the +transaction they show: the network it was reviewed on for ConfirmTx, and the +network it was sent on for the others. Without a wallet the stack is not cut and +only Settings is drawn again, because the stack starts at Welcome, and Welcome +and AddWallet show nothing from a network. Every screen that holds secret material in the page registers a cleanup with `onViewLeave()` (`src/popup/views/helpers.js`), which `showView()` runs on every diff --git a/src/popup/index.js b/src/popup/index.js index 191d374..f05a031 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -5,12 +5,14 @@ const { state, saveState, onSaveFailure, + takeStoredNetwork, loadState, } = require("../shared/state"); const { StateUnusableError } = require("../shared/stateSchema"); const { log, setRuntimeDebug } = require("../shared/log"); const { refreshPrices } = require("../shared/prices"); const { refreshBalances } = require("../shared/balances"); +const { clearPopupCaches } = require("../shared/chainSwitch"); const { $, showView, @@ -22,6 +24,7 @@ const { } = require("./views/helpers"); const { applyTheme } = require("./theme"); const { + onStorageChanged, tabsGetCurrent, tabsGetZoom, windowsGetCurrent, @@ -72,12 +75,13 @@ let refreshTimer = null; // lock, its results are copied onto the address records in `state` only if the // network it ran on is still the active one, on this page and in the stored // record; the background's switch, after the user approves a site's request, -// changes only the stored record. A switch while the refresh ran cleared every -// address's balances (applyChainSwitchFields() in -// src/shared/chainSwitchFields.js), and the previous network's must not come -// back over them (https://git.eeqj.de/sneak/AutistMask/issues/577). A dropped -// refresh is not recorded as done. Results are copied by address, so an address -// added or removed meanwhile is left as it is. +// reaches this page only after the stored record (followNetworkSwitch() below). +// A switch while the refresh ran cleared every address's balances +// (applyChainSwitchFields() in src/shared/chainSwitchFields.js), and the +// previous network's must not come back over them +// (https://git.eeqj.de/sneak/AutistMask/issues/577). A dropped refresh is not +// recorded as done. Results are copied by address, so an address added or +// removed meanwhile is left as it is. async function doRefreshAndRender() { if (refreshInFlight) return; refreshInFlight = true; @@ -147,6 +151,7 @@ const ctx = { renderWalletList, doRefreshAndRender, startRefresh, + restoreView, pageClosed: pageClosed.signal, showAddWalletView: () => { pushCurrentView(); @@ -214,6 +219,28 @@ function fallbackView() { showView("main"); } +// When the background saves a network switch, once the user approves a site's +// request, or another popup page saves one, this page follows the stored +// record. It takes the network the record names as a save reads it, holding +// the lock, so no later save of this page writes the previous network back, and +// then leaves the page as a switch in Settings leaves it. This page's own +// switch in Settings is made under the lock as well (onChainSwitch() in +// src/shared/chainSwitch.js), so a record naming a network other than this +// page's always holds a switch saved elsewhere, and a switch the user picks +// here while this waits for the lock is made after it. +async function followNetworkSwitch() { + let switched = false; + // A failed save is reported by saveState() itself: see onSaveFailure(). + await saveState((stored) => { + if (stored.networkId === state.networkId) return false; + settings.cutViewStack(); + takeStoredNetwork(stored); + clearPopupCaches(); + switched = true; + }).catch(() => {}); + if (switched) settings.showNewNetwork(); +} + // The background opens the approval window as wide as the page, but the // browser counts the window's frame in that width, and a screen taller than // the window takes a vertical scrollbar out of it too. So once the screen is @@ -358,6 +385,17 @@ async function init() { settingsAddToken.init(ctx); deleteAddress.init(ctx); + // Only a record naming a network other than this page's is followed: the + // one this page's own switch in Settings saves names the network it + // already has, and a record naming none is no switch. + onStorageChanged((changes) => { + const written = changes.autistmask && changes.autistmask.newValue; + if (!written || !written.networkId) return; + if (written.networkId !== state.networkId) followNetworkSwitch(); + }); + // A switch saved since loadState() above, before there was a listener. + followNetworkSwitch(); + if (!state.hasWallet) { showView("welcome"); } else { diff --git a/src/popup/viewRouter.js b/src/popup/viewRouter.js index 3e59202..f0bf051 100644 --- a/src/popup/viewRouter.js +++ b/src/popup/viewRouter.js @@ -39,8 +39,8 @@ function markViewRendered(view) { } // Begin a fresh page-load scope. The popup gets one by being loaded, and -// asks for one when the user switches the network, because every screen -// then on the page shows the previous network (views/settings.js); the +// asks for one after a network switch, because every screen then on the page +// shows the previous network (showNewNetwork() in views/settings.js); the // unit tests, which simulate several page loads against one module // instance, ask for one too. function resetRenderedViews() { diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index c5a727d..4d0d53a 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -38,6 +38,8 @@ const { const { notify, sendMessage } = require("../../shared/browserApi"); +let ctx; + let versionClickCount = 0; let versionClickTimer = null; @@ -243,7 +245,51 @@ async function renderSiteLists() { ); } -function init(ctx) { +// A network switch, made by the user here or saved by the background once the +// user approved a site's request (src/popup/index.js), leaves every screen on +// the page showing the previous network. These two leave the page as README's +// Screen Map describes, the first before the switch is saved, so that save +// keeps the cut for a popup closed straight after, the second once `state` +// holds the new network. +// +// The stack is cut at the first screen that cannot be rendered from saved +// state, such as Send or AddToken, and at AddressToken unless it shows the +// native token: any other token there was picked on the previous network, so +// it is no longer selected either. Without a wallet the stack starts at +// Welcome, which that cut would drop, and Welcome shows nothing from a network. +function cutViewStack() { + if (!state.hasWallet) return; + if (state.selectedToken !== "ETH") { + const tokenView = state.viewStack.indexOf("address-token"); + if (tokenView !== -1) { + state.viewStack = state.viewStack.slice(0, tokenView); + } + state.selectedToken = null; + } + state.viewStack = restorableStack(state.viewStack, state.currentView); +} + +// Every screen counts as not yet rendered, so "Back" renders the one it lands +// on for the new network. Settings on top is brought up to date in place; +// any other screen is drawn again as a reopened popup draws it, which is Home +// for one that cannot be rendered from saved state. Without a wallet any other +// screen is Welcome or AddWallet, which show nothing from a network. +function showNewNetwork() { + resetRenderedViews(); + if (state.currentView === "settings") { + markViewRendered("settings"); + $("settings-network").value = state.networkId; + $("settings-rpc").value = state.rpcUrl; + $("settings-blockscout").value = state.blockscoutUrl; + renderTrackedTokens(); + } else if (state.hasWallet) { + ctx.restoreView(); + } + updateDebugBanner(state.currentView); +} + +function init(_ctx) { + ctx = _ctx; deleteWallet.init(ctx); showPhrase.init(); @@ -319,37 +365,18 @@ function init(ctx) { const networkSelect = $("settings-network"); networkSelect.addEventListener("change", async () => { + // Whether `state` already has the network picked is asked only once + // the switch holds the lock: while an earlier pick waits for it, + // `state` still has the network before that pick, and a pick back to + // that network must still be made. const newId = networkSelect.value; - if (newId === state.networkId) return; - // The screens beneath Settings show the previous network, so Back - // finds them as it does in a reopened popup: each is rendered again - // when Back lands on it, and the stack is cut at the first one that - // cannot be rendered from saved state, such as Send or AddToken. It - // is cut at AddressToken too, unless that shows the native token: any - // other token there was picked on the previous network. Without a - // wallet the stack starts at Welcome, which that cut would drop, and - // Welcome shows nothing from a network. The cut comes before the - // switch so that the switch's save keeps it for a popup closed - // straight after. - if (state.hasWallet) { - const tokenView = state.viewStack.indexOf("address-token"); - if (tokenView !== -1 && state.selectedToken !== "ETH") { - state.viewStack = state.viewStack.slice(0, tokenView); - } - state.viewStack = restorableStack(state.viewStack, "settings"); - } + cutViewStack(); const net = await onChainSwitch(newId); + if (!net) return; // Open pages are told by the background, as after a site's approved // switch request. notify({ type: "AUTISTMASK_NETWORK_CHANGED", chainId: net.chainId }); - resetRenderedViews(); - // Settings itself, and the testnet banner above it, are brought up - // to date for the new network here. - markViewRendered("settings"); - $("settings-rpc").value = state.rpcUrl; - $("settings-blockscout").value = state.blockscoutUrl; - renderTrackedTokens(); - updateDebugBanner(state.currentView); + showNewNetwork(); showFlash("Switched to " + net.name + "."); }); @@ -484,4 +511,10 @@ function init(ctx) { }); } -module.exports = { init, show, renderSiteLists }; +module.exports = { + init, + show, + renderSiteLists, + cutViewStack, + showNewNetwork, +}; diff --git a/src/shared/browserApi.js b/src/shared/browserApi.js index ad0f924..1fe990f 100644 --- a/src/shared/browserApi.js +++ b/src/shared/browserApi.js @@ -224,6 +224,19 @@ function storageRemove(keys) { return Promise.resolve(storage.remove(keys)); } +/** + * Call `listener` after every write to `storage.local`, whether this page, + * another page or the background made it, with what the write changed, as the + * browser reports it: `{ [key]: { oldValue, newValue } }`. + * + * @param {function(Object): void} listener + */ +function onStorageChanged(listener) { + namespaceMember("storage").onChanged.addListener((changes, areaName) => { + if (areaName === "local") listener(changes); + }); +} + /** * Run `fn` while holding the browser's lock called `name` * (`navigator.locks`), and settle as `fn` settles. Every page of the extension @@ -385,6 +398,7 @@ module.exports = { getPlatformInfo, hasBrowserNamespace, notify, + onStorageChanged, runtimeApi, sendMessage, sessionStorageGet, diff --git a/src/shared/chainSwitch.js b/src/shared/chainSwitch.js index 6226e65..9c54ca2 100644 --- a/src/shared/chainSwitch.js +++ b/src/shared/chainSwitch.js @@ -1,8 +1,18 @@ -// Consolidated chain-switch handler for the popup. +// The popup's network switch. // -// Every state change required when the active network changes is -// performed here so that callers (settings UI, future chain additions) all go -// through a single code path. +// 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). +// - followNetworkSwitch() 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 +// copyChainSwitchFields() (chainSwitchFields.js). +// +// Both change `state` inside a save, holding the lock, and both reset the +// popup's own caches through clearPopupCaches(), so a reset added there +// reaches both. A field applyChainSwitchFields() sets has to be copied by +// copyChainSwitchFields() too. // // Adding a new chain (e.g. ETC) requires only a new entry in // networks.js — no per-caller wiring is needed. @@ -20,22 +30,32 @@ const { applyChainSwitchFields } = require("./chainSwitchFields"); const { clearPrices } = require("./prices"); +// The popup's own caches a network switch resets. Prices are chain-specific +// (testnet tokens are worthless, ETC has different pricing, etc.). In-memory +// and per bundle, so this is the popup's own cache — the only context that +// ever fills it. +function clearPopupCaches() { + clearPrices(); +} + // Switch the active chain and reset all chain-specific cached state. -// Returns the network configuration object for the new chain. +// Returns the network configuration object for the new chain, or null when +// `state` already has that network once the save holds the lock, and nothing +// is switched. +// +// `state` is switched inside the save that stores the switch, holding the +// lock, so it is never switched while a follow of a switch saved elsewhere +// waits for the lock (followNetworkSwitch() in src/popup/index.js): that +// follow runs first, and this switch is made after it. async function onChainSwitch(newNetworkId) { const { state, saveState } = require("./state"); - - const net = applyChainSwitchFields(state, newNetworkId); - - // --- price cache --- - // Prices are chain-specific (testnet tokens are worthless, - // ETC has different pricing, etc.). In-memory and per bundle, so this is - // the popup's own cache — the only context that ever fills it. - clearPrices(); - - await saveState(); - + let net = null; + await saveState(() => { + if (state.networkId === newNetworkId) return false; + net = applyChainSwitchFields(state, newNetworkId); + clearPopupCaches(); + }); return net; } -module.exports = { onChainSwitch }; +module.exports = { onChainSwitch, clearPopupCaches }; diff --git a/src/shared/chainSwitchFields.js b/src/shared/chainSwitchFields.js index 24ed8e7..cea6ef2 100644 --- a/src/shared/chainSwitchFields.js +++ b/src/shared/chainSwitchFields.js @@ -7,7 +7,7 @@ // the singleton and saves; 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 object it is +// 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 // what makes it usable on a record that has been read fresh from storage // microseconds earlier and is about to be written back. @@ -76,4 +76,33 @@ function applyChainSwitchFields(s, newNetworkId) { return net; } -module.exports = { applyChainSwitchFields }; +// Give `s` every field applyChainSwitchFields() sets, as `switched` holds it: +// the popup takes a switch the background saved from the stored record this +// way, rather than performing it again (takeStoredNetwork() in +// src/shared/state.js). Balances are taken by address, and an address +// `switched` does not have keeps none. +function copyChainSwitchFields(s, switched) { + s.networkEndpoints = switched.networkEndpoints; + s.networkTrackedTokens = switched.networkTrackedTokens; + s.trackedTokens = switched.trackedTokens; + s.networkId = switched.networkId; + s.rpcUrl = switched.rpcUrl; + s.blockscoutUrl = switched.blockscoutUrl; + s.lastBalanceRefresh = switched.lastBalanceRefresh; + s.tokenHolderCache = switched.tokenHolderCache; + s.fraudContracts = switched.fraudContracts; + + const stored = new Map(); + for (const wallet of switched.wallets) { + for (const addr of wallet.addresses) stored.set(addr.address, addr); + } + for (const wallet of s.wallets) { + for (const addr of wallet.addresses) { + const got = stored.get(addr.address); + addr.balance = got ? got.balance : "0"; + addr.tokenBalances = got ? got.tokenBalances : []; + } + } +} + +module.exports = { applyChainSwitchFields, copyChainSwitchFields }; diff --git a/src/shared/state.js b/src/shared/state.js index 4134318..74b630a 100644 --- a/src/shared/state.js +++ b/src/shared/state.js @@ -23,6 +23,7 @@ // guarantee. const { networkById } = require("./networks"); +const { copyChainSwitchFields } = require("./chainSwitchFields"); const { DEFAULT_STATE, PERSISTED_FIELDS, @@ -125,8 +126,10 @@ function currentNetwork() { } // The persisted fields as this page held them when its last loadState() -// finished, or when its last successful saveState() began. saveState() diffs -// the live state against this to find only the fields THIS page changed since. +// finished, or when its last successful saveState() began, with any network +// switch saved elsewhere that this page has taken since (takeStoredNetwork()). +// saveState() diffs the live state against this to find only the fields THIS +// page changed since. // // Deep-cloned, not a reference: callers mutate persisted objects and arrays // in place (state.wallets.push(...)), and a reference baseline would mutate @@ -466,7 +469,9 @@ function mergeNetworkEndpoints(base, ours, theirs) { // exists to remove, just delayed and confined to one page instead of two // (caught by tests/txStatus.test.js). A page's live picture of a field it // does not own goes on being whatever its last loadState() saw, same as -// before this fix; only the persisted record is guaranteed current. +// before this fix; only the persisted record is guaranteed current. The +// network is the exception: the popup takes a switch another context saved +// through `beforeWrite` (followNetworkSwitch() in src/popup/index.js). async function saveStateOnce(beforeWrite) { const result = await storageGet("autistmask"); // The record in storage right now is about to be merged into and written @@ -597,6 +602,17 @@ function saveState(beforeWrite) { return turn; } +// Give this page the network switch held by `stored`, a record a save of this +// page has just read (followNetworkSwitch() in src/popup/index.js calls this +// from `beforeWrite`), in `state` and in `baseline` alike, as loadState() does +// with a whole record. The switch is then not this page's own change, even when +// that save fails to write: a later switch back to the previous network still +// differs from `baseline`, and is stored. +function takeStoredNetwork(stored) { + copyChainSwitchFields(rawState, stored); + copyChainSwitchFields(baseline, structuredClone(stored)); +} + // Rejects with StateUnusableError for a stored record this build cannot make // sense of. Nothing is assigned and `loaded` stays false in that case, so a // caller that ignores the rejection gets StateNotLoadedError on the first @@ -642,6 +658,7 @@ module.exports = { state, saveState, onSaveFailure, + takeStoredNetwork, loadState, currentAddress, currentNetwork, diff --git a/tests/balanceRefreshNetworkSwitch.test.js b/tests/balanceRefreshNetworkSwitch.test.js index 9208ab5..94b0a4a 100644 --- a/tests/balanceRefreshNetworkSwitch.test.js +++ b/tests/balanceRefreshNetworkSwitch.test.js @@ -97,9 +97,9 @@ test("a refresh that returns after a switch in Settings writes nothing, and the ); }); -// The stored record still holds Ethereum Mainnet while the refresh's save reads -// it, so only this page's own network shows the switch. -test("a refresh whose save is reading the stored record when the user switches in Settings writes nothing", async () => { +// The switch is made only once the refresh's save, holding the lock, has +// written Ethereum Mainnet's balances, and the switch then clears them. +test("a refresh whose save is reading the stored record when the user switches in Settings leaves none of its balances stored", async () => { const page = await bootPopup(unversionedValidProfile()); expect(page.pageErrors).toEqual([]); const { state } = require("../src/shared/state"); @@ -145,7 +145,8 @@ test("a refresh that returns after the background switched the network writes no const refresh = await holdRefresh(page); // The background writes the switch to Sepolia, as its updateState() does - // once the user approves a site's request. This page is not told. + // once the user approves a site's request. This page is not told of it + // yet. const record = page.storage.read("autistmask"); applyChainSwitchFields(record, "sepolia"); page.storage.write("autistmask", record); diff --git a/tests/chainSwitchGate.test.js b/tests/chainSwitchGate.test.js index 560ee08..9abbbec 100644 --- a/tests/chainSwitchGate.test.js +++ b/tests/chainSwitchGate.test.js @@ -446,7 +446,10 @@ describe("switching the network in Settings tells every open tab", () => { global.chrome.runtime.sendMessage = (msg) => { bg.send(msg, POPUP); }; - await require("../src/shared/state").loadState(); + const { state, loadState } = require("../src/shared/state"); + await loadState(); + // Set by showView(), which is mocked above. + state.currentView = "settings"; require("../src/popup/views/settings").init({ pageClosed: new AbortController().signal, }); diff --git a/tests/e2e/firefox/run.js b/tests/e2e/firefox/run.js index b6f1fc7..fc656e0 100644 --- a/tests/e2e/firefox/run.js +++ b/tests/e2e/firefox/run.js @@ -867,6 +867,14 @@ step( "the page was not told of the approved switch: " + JSON.stringify(events), ); + // The popup, open all along, shows the new network + // (https://git.eeqj.de/sneak/AutistMask/issues/650). + await d.switchToWindow(env.popupWindow); + await d.waitFor( + "the open popup to show the testnet banner", + `const banner = document.getElementById("debug-banner"); + return banner !== null && banner.textContent === "[TESTNET]";`, + ); await openNetworkPrompt(env, "switch-restore", sepolia, mainnet); await d.click("#btn-approve-network"); @@ -881,6 +889,10 @@ step( isDeepStrictEqual(await storedNetwork(env), before), "switching back did not restore the mainnet network and endpoints", ); + await d.waitFor( + "the open popup to drop the testnet banner", + `return document.getElementById("debug-banner") === null;`, + ); await d.switchToWindow(env.dappWindow); }, ); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index be29953..2d95c8f 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -4865,6 +4865,12 @@ test("a site's network switch changes nothing until the user approves it (#408)" "the page was not told of the approved switch: " + JSON.stringify(events), ); + // The popup, open all along, shows the new network + // (https://git.eeqj.de/sneak/AutistMask/issues/650). + await env.page.waitForFunction(() => { + const banner = document.getElementById("debug-banner"); + return banner !== null && banner.textContent === "[TESTNET]"; + }); const restored = await openNetworkPrompt( env, @@ -4886,6 +4892,9 @@ test("a site's network switch changes nothing until the user approves it (#408)" isDeepStrictEqual(await storedNetwork(env.page), before), "switching back did not restore the mainnet network and endpoints", ); + await env.page.waitForFunction( + () => document.getElementById("debug-banner") === null, + ); }); // Switching the network in Settings tells the page too, as an approved site diff --git a/tests/popupFollowsNetworkSwitch.test.js b/tests/popupFollowsNetworkSwitch.test.js new file mode 100644 index 0000000..9cc2345 --- /dev/null +++ b/tests/popupFollowsNetworkSwitch.test.js @@ -0,0 +1,300 @@ +// An open popup follows a network switch the background saves once the user +// approves a site's request: it takes the new network from the stored record +// and draws the screen it is on for it, as a switch in Settings does +// (https://git.eeqj.de/sneak/AutistMask/issues/650). Driven through the real +// popup over a stored profile on Ethereum Mainnet. The background's write is +// the storage stub's set, which the stub reports to the page as the browser +// does. + +const { + bootPopup, + cleanupPopup, + unversionedValidProfile, + TOKEN_ADDRESS, +} = require("./support/popupBoot"); +const { makeStorageStub } = require("./support/storageStub"); +const { + applyChainSwitchFields, + copyChainSwitchFields, +} = require("../src/shared/chainSwitchFields"); +const { NETWORKS } = require("../src/shared/networks"); +const { STATE_SCHEMA_VERSION } = require("../src/shared/stateSchema"); + +afterEach(() => { + cleanupPopup(); +}); + +const SEPOLIA_TOKEN = { + address: TOKEN_ADDRESS, + symbol: "TST", + decimals: 18, + name: "Test Token", +}; +const MAINNET_TOKEN = { + address: "0x6B175474E89094C44Da98b954EedeAC495271d0F", + symbol: "DAI", + decimals: 18, + name: "Dai Stablecoin", +}; + +// A profile on Ethereum Mainnet that reopens the popup onto AddressDetail, +// with DAI tracked there and TST tracked on Sepolia. +function profileOnAddress() { + return unversionedValidProfile({ + currentView: "address", + viewStack: ["main"], + selectedWallet: 0, + selectedAddress: 0, + trackedTokens: [MAINNET_TOKEN], + networkTrackedTokens: { sepolia: [SEPOLIA_TOKEN] }, + }); +} + +// The popup reopened onto it. +function bootOnAddress() { + return bootPopup(profileOnAddress()); +} + +// The background saves a switch to `networkId`, as its updateState() does +// once the user approves a site's request. +async function backgroundSwitch(page, networkId) { + const record = page.storage.read("autistmask"); + applyChainSwitchFields(record, networkId); + await page.storage.local.set({ autistmask: record }); + await page.settle(); + await page.settle(); +} + +test("the popup open on AddressDetail shows the new network and its balances", async () => { + const page = await bootOnAddress(); + expect(page.pageErrors).toEqual([]); + expect(page.visibleViews()).toEqual(["address"]); + expect(page.node("address-balances").innerHTML).toContain("DAI"); + expect(page.node("debug-banner")).toBeNull(); + + await backgroundSwitch(page, "sepolia"); + + const { state } = require("../src/shared/state"); + expect(state.networkId).toBe("sepolia"); + expect(state.rpcUrl).toBe(NETWORKS.sepolia.defaultRpcUrl); + expect(page.visibleViews()).toEqual(["address"]); + const balances = page.node("address-balances").innerHTML; + expect(balances).toContain("TST"); + expect(balances).toContain("SepoliaETH"); + expect(balances).not.toContain("DAI"); + expect(page.text("debug-banner")).toBe("[TESTNET]"); + + // Its next refresh runs on Sepolia, and what it fetched is stored. + await page.tick(); + const { refreshBalances } = require("../src/shared/balances"); + expect(refreshBalances.mock.lastCall[4]).toBe("sepolia"); + const stored = page.storage.read("autistmask"); + expect(stored.networkId).toBe("sepolia"); + expect(stored.lastBalanceRefresh).toBeGreaterThan(0); +}); + +test("the popup open on Settings shows the new network, and Back draws the screen beneath for it", async () => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + expect(page.visibleViews()).toEqual(["settings"]); + + await backgroundSwitch(page, "sepolia"); + + expect(page.visibleViews()).toEqual(["settings"]); + expect(page.value("settings-network")).toBe("sepolia"); + expect(page.value("settings-rpc")).toBe(NETWORKS.sepolia.defaultRpcUrl); + expect(page.node("settings-tracked-tokens").innerHTML).toContain("TST"); + expect(page.text("debug-banner")).toBe("[TESTNET]"); + + await page.click("btn-settings-back"); + expect(page.visibleViews()).toEqual(["address"]); + expect(page.node("address-balances").innerHTML).not.toContain("DAI"); +}); + +// Neither can be drawn again for the new network: Send cannot be rendered from +// saved state, and DAI was picked on Ethereum Mainnet, where alone it is +// tracked. A reopened popup goes to Home in their place. +test.each([ + ["Send", (page) => page.click("btn-send")], + [ + "AddressToken for DAI", + async (page) => { + const { state } = require("../src/shared/state"); + state.selectedToken = MAINNET_TOKEN.address; + require("../src/popup/views/helpers").pushCurrentView(); + require("../src/popup/views/addressToken").show(); + await page.settle(); + }, + ], +])("the popup open on %s goes to Home", async (_view, open) => { + const page = await bootOnAddress(); + await open(page); + expect(page.visibleViews()).not.toEqual(["address"]); + + await backgroundSwitch(page, "sepolia"); + + expect(page.visibleViews()).toEqual(["main"]); + expect(page.text("total-value")).toBe("0.0000 SepoliaETH"); +}); + +// The report of the switch's own save names the network the page already has. +test("a switch in Settings is not taken a second time when its save is reported", async () => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + const { get, set } = page.storage.local; + const reads = get.mock.calls.length; + const writes = set.mock.calls.length; + + const select = page.node("settings-network"); + select.value = "sepolia"; + for (const fn of select.listeners.change) await fn(); + await page.settle(); + await page.settle(); + + expect(get).toHaveBeenCalledTimes(reads + 1); + expect(set).toHaveBeenCalledTimes(writes + 1); + expect(page.text("flash-msg")).toBe("Switched to Sepolia Testnet."); +}); + +// The background saves the switch once the popup has read the stored record +// and before it listens for changes to it, so the browser tells the page +// nothing of it. Nor is any later write reported here: Chrome reports none +// that leaves the record as it was, such as the popup's own save at start-up. +test("a switch saved while the popup opens, before it listens for one, is followed", async () => { + const storage = makeStorageStub({ autistmask: profileOnAddress() }); + storage.onChanged = { addListener: () => {} }; + const read = storage.local.get.getMockImplementation(); + storage.local.get.mockImplementationOnce(async (key) => { + const loaded = await read(key); + const record = storage.read("autistmask"); + applyChainSwitchFields(record, "sepolia"); + storage.write("autistmask", record); + return loaded; + }); + const page = await bootPopup(undefined, { storage }); + await page.settle(); + + const { state } = require("../src/shared/state"); + expect(page.pageErrors).toEqual([]); + expect(state.networkId).toBe("sepolia"); + expect(page.visibleViews()).toEqual(["address"]); + expect(page.node("address-balances").innerHTML).toContain("SepoliaETH"); + expect(page.text("debug-banner")).toBe("[TESTNET]"); +}); + +// While the popup's follow of the background's switch to Sepolia waits for the +// lock, the user picks Sepolia in Settings and then Ethereum Mainnet again. The +// last pick is the network stored, and the last one sites are told of. +test("a switch in Settings made while a follow waits for the lock is not overwritten by it", async () => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + + // Another write holds the lock, so every save of the page waits for it. + let releaseLock; + const lockHeld = navigator.locks.request( + "autistmask", + () => + new Promise((resolve) => { + releaseLock = resolve; + }), + ); + await backgroundSwitch(page, "sepolia"); + const select = page.node("settings-network"); + const picks = []; + for (const networkId of ["sepolia", "mainnet"]) { + select.value = networkId; + for (const fn of select.listeners.change) picks.push(fn()); + } + releaseLock(); + await lockHeld; + await Promise.all(picks); + await page.settle(); + + const { state } = require("../src/shared/state"); + expect(state.networkId).toBe("mainnet"); + expect(page.storage.read("autistmask").networkId).toBe("mainnet"); + expect(page.value("settings-network")).toBe("mainnet"); + const told = globalThis.chrome.runtime.sendMessage.mock.calls + .map(([message]) => message) + .filter((message) => message.type === "AUTISTMASK_NETWORK_CHANGED"); + expect(told.at(-1).chainId).toBe(NETWORKS.mainnet.chainId); +}); + +// The popup follows the background's switch to Sepolia, but its own write in +// that save fails. The user then picks Ethereum Mainnet in Settings, the +// network the popup was on before: that switch is stored, and only then +// flashed and sent to sites. +test("a switch back in Settings after a follow whose write failed is stored", async () => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + const { set } = page.storage.local; + const write = set.getMockImplementation(); + // The background's write lands; the next, the follow's, fails. + set.mockImplementationOnce(write).mockRejectedValueOnce( + new Error("quota exceeded"), + ); + await backgroundSwitch(page, "sepolia"); + const { state } = require("../src/shared/state"); + expect(state.networkId).toBe("sepolia"); + + const select = page.node("settings-network"); + select.value = "mainnet"; + for (const fn of select.listeners.change) await fn(); + await page.settle(); + + const stored = page.storage.read("autistmask"); + expect(stored.networkId).toBe("mainnet"); + expect(stored.rpcUrl).toBe(NETWORKS.mainnet.defaultRpcUrl); + expect(stored.trackedTokens).toEqual([MAINNET_TOKEN]); + expect(page.text("flash-msg")).toBe("Switched to Ethereum Mainnet."); + const told = globalThis.chrome.runtime.sendMessage.mock.calls + .map(([message]) => message) + .filter((message) => message.type === "AUTISTMASK_NETWORK_CHANGED"); + expect(told.at(-1).chainId).toBe(NETWORKS.mainnet.chainId); +}); + +// Such as one a newer build wrote, which this build cannot read. Nothing reads +// or writes the stored record for it: the popup's next save finds it, as +// before. +test("a record that names no network is not followed", async () => { + const page = await bootOnAddress(); + const { get, set } = page.storage.local; + const reads = get.mock.calls.length; + + await set({ + autistmask: { + schemaVersion: STATE_SCHEMA_VERSION + 1, + wallets: [{ type: "hd", name: "Main", addresses: [] }], + }, + }); + await page.settle(); + + expect(get).toHaveBeenCalledTimes(reads); + expect(page.visibleViews()).toEqual(["address"]); +}); + +// A record that takes the switch from the switched record ends up as that +// record, balances the background fetched since included. +test("copyChainSwitchFields() takes every field applyChainSwitchFields() sets", () => { + const before = unversionedValidProfile({ + trackedTokens: [MAINNET_TOKEN], + networkTrackedTokens: { sepolia: [SEPOLIA_TOKEN] }, + networkEndpoints: { + sepolia: { + rpcUrl: "http://127.0.0.1:8545", + blockscoutUrl: "http://127.0.0.1:4000/api/v2", + }, + }, + lastBalanceRefresh: 1, + tokenHolderCache: { [MAINNET_TOKEN.address]: 5000 }, + fraudContracts: ["0x1111111111111111111111111111111111111111"], + }); + const switched = structuredClone(before); + applyChainSwitchFields(switched, "sepolia"); + switched.wallets[0].addresses[0].balance = "2.0"; + + const follower = structuredClone(before); + copyChainSwitchFields(follower, switched); + + expect(follower).toEqual(switched); +}); diff --git a/tests/support/popupBoot.js b/tests/support/popupBoot.js index 294f338..165e84e 100644 --- a/tests/support/popupBoot.js +++ b/tests/support/popupBoot.js @@ -311,7 +311,7 @@ async function bootPopup(stored, options) { const windowListeners = {}; globalThis.chrome = { - storage: { local: storage.local }, + storage: { local: storage.local, onChanged: storage.onChanged }, runtime: { sendMessage: jest.fn(async () => ({})), getURL: (p) => "chrome-extension://autistmask/" + p, diff --git a/tests/support/storageStub.js b/tests/support/storageStub.js index 5eb9004..0771292 100644 --- a/tests/support/storageStub.js +++ b/tests/support/storageStub.js @@ -18,7 +18,9 @@ // a storage stub by hand. // One storage area: get, set and remove over its own store, cloning both ways. -function makeArea(store, tick) { +// `changed` is handed what each set or remove changed, in the shape +// storage.onChanged reports it. +function makeArea(store, tick, changed) { const get = jest.fn(async (key) => { tick(); if (key === undefined || key === null) return structuredClone(store); @@ -34,14 +36,25 @@ function makeArea(store, tick) { const set = jest.fn(async (items) => { tick(); + const changes = {}; for (const k of Object.keys(items)) { + changes[k] = structuredClone({ + oldValue: store[k], + newValue: items[k], + }); store[k] = structuredClone(items[k]); } + changed(changes); }); const remove = jest.fn(async (key) => { tick(); - for (const k of Array.isArray(key) ? key : [key]) delete store[k]; + const changes = {}; + for (const k of Array.isArray(key) ? key : [key]) { + changes[k] = { oldValue: store[k] }; + delete store[k]; + } + changed(changes); }); return { get, set, remove }; @@ -54,12 +67,22 @@ function makeArea(store, tick) { function makeStorageStub(initial, onOp) { const store = initial ? structuredClone(initial) : {}; const tick = onOp || (() => {}); - const local = makeArea(store, tick); + // storage.onChanged: after a set or remove has finished, every listener is + // called with what it changed and the area's name, as the browser calls + // them for a write by any page or the background. + const listeners = []; + const announce = (areaName) => (changes) => { + Promise.resolve().then(() => { + for (const fn of listeners) fn(structuredClone(changes), areaName); + }); + }; + const local = makeArea(store, tick, announce("local")); return { // Drop this straight in as chrome.storage. local, - session: makeArea({}, tick), + session: makeArea({}, tick, announce("session")), + onChanged: { addListener: (fn) => listeners.push(fn) }, get: local.get, set: local.set, remove: local.remove, @@ -71,7 +94,9 @@ function makeStorageStub(initial, onOp) { ? structuredClone(store) : structuredClone(store[key]), // Seed or replace a record without going through the module under - // test — for standing in as "another page wrote this". + // test — for standing in as "another page wrote this". Nothing is + // announced to storage.onChanged, so it also stands in for the time + // before the browser tells this page of such a write. write: (key, value) => { store[key] = structuredClone(value); }, diff --git a/tests/trackedTokensPerNetwork.test.js b/tests/trackedTokensPerNetwork.test.js index 97b03b8..4ffd016 100644 --- a/tests/trackedTokensPerNetwork.test.js +++ b/tests/trackedTokensPerNetwork.test.js @@ -161,6 +161,7 @@ test("a token added by contract address on Sepolia is tracked on Sepolia only", ); expect(stored().trackedTokens).toEqual([SEPOLIA_TOKEN]); + await boot.click("btn-settings"); await chooseNetwork(boot, "mainnet"); expect(stored().networkId).toBe("mainnet"); expect(stored().trackedTokens).toEqual([MAINNET_TOKEN]);