diff --git a/README.md b/README.md index c0b1e62..73e3f69 100644 --- a/README.md +++ b/README.md @@ -2281,6 +2281,8 @@ view would leave a wallet one click from deletion. - Endpoint URL input + "Save" button. "Save" asks the URL for `/stats` and saves the URL only if that request succeeds. The request carries no user data and is made whether or not "Use Blockscout" is ticked. + - If the network changes before either "Save" stores its URL, the URL is not + saved and a flash message says so - Etherscan: - "Check recipients on Etherscan" checkbox, unticked until the user ticks it diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index 40da526..f1cfa9f 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -44,6 +44,12 @@ let ctx; let versionClickCount = 0; let versionClickTimer = null; +// The network that Settings was last drawn for. A follow of a switch saved +// elsewhere sets `state` to the new network first. Settings is drawn again only +// once the follow's write returns. So both "Save" buttons take this network, +// not the one in `state`. +let shownNetwork = null; + // One row per site origin, however many addresses it appears under, each with // an [x] that hands it to onRemove. function renderSiteList(containerId, origins, onRemove) { @@ -199,6 +205,7 @@ function renderWalletListSettings() { } function show() { + shownNetwork = currentNetwork(); $("settings-rpc").value = state.rpcUrl; $("settings-blockscout").value = state.blockscoutUrl; $("settings-network").value = state.networkId; @@ -306,6 +313,7 @@ function showNewSettings() { state.checkRecipientsOnEtherscan; if (state.currentView === "settings") { markViewRendered("settings"); + shownNetwork = currentNetwork(); $("settings-network").value = state.networkId; $("settings-rpc").value = state.rpcUrl; $("settings-blockscout").value = state.blockscoutUrl; @@ -319,6 +327,28 @@ function showNewSettings() { updateDebugBanner(state.currentView); } +// Saves `url` as `field` ("rpcUrl" or "blockscoutUrl") of the network +// `networkId`, the one that Settings showed when the user tapped "Save". The +// URL is set in `state` inside the save, holding the lock. So the page cannot +// follow a switch saved elsewhere (followStoredSettings() in +// src/popup/index.js) between setting the URL and writing it. If the active +// network is no longer `networkId`, on this page or in the stored record, +// nothing is saved. The flash message then says that the network changed. +async function saveEndpoint(field, url, networkId) { + let refused = false; + await saveState((stored) => { + refused = + state.networkId !== networkId || stored.networkId !== networkId; + if (refused) return false; + state[field] = url; + }); + if (refused) { + showFlash("The URL was not saved because the network changed."); + } else { + showFlash("Saved."); + } +} + function init(_ctx) { ctx = _ctx; deleteWallet.init(ctx); @@ -330,6 +360,8 @@ function init(_ctx) { showFlash("Please enter an RPC URL."); return; } + // The URL is for the network that Settings shows when "Save" is tapped. + const net = shownNetwork; showFlash("Testing endpoint..."); try { const resp = await debugFetch(url, { @@ -348,7 +380,6 @@ function init(_ctx) { showFlash("Endpoint returned an error."); return; } - const net = currentNetwork(); if (json.result !== net.chainId) { showFlash("Wrong network: expected " + net.name + "."); return; @@ -364,9 +395,7 @@ function init(_ctx) { showFlash("Could not reach endpoint."); return; } - state.rpcUrl = url; - await saveState(); - showFlash("Saved."); + await saveEndpoint("rpcUrl", url, net.id); }); $("btn-save-blockscout").addEventListener("click", async () => { @@ -375,6 +404,7 @@ function init(_ctx) { showFlash("Please enter a Blockscout API URL."); return; } + const net = shownNetwork; showFlash("Testing endpoint..."); try { const resp = await debugFetch(url + "/stats"); @@ -389,9 +419,7 @@ function init(_ctx) { showFlash("Could not reach endpoint."); return; } - state.blockscoutUrl = url; - await saveState(); - showFlash("Saved."); + await saveEndpoint("blockscoutUrl", url, net.id); }); // Each change is made inside the save, holding the lock. So diff --git a/tests/popupFollowsNetworkSwitch.test.js b/tests/popupFollowsNetworkSwitch.test.js index 9cc2345..1c37373 100644 --- a/tests/popupFollowsNetworkSwitch.test.js +++ b/tests/popupFollowsNetworkSwitch.test.js @@ -253,6 +253,271 @@ test("a switch back in Settings after a follow whose write failed is stored", as expect(told.at(-1).chainId).toBe(NETWORKS.mainnet.chainId); }); +// The user saves an endpoint URL in Settings on Ethereum Mainnet. Before the +// URL is stored, the background saves a switch to Sepolia. Another write holds +// the lock, so the popup's follow of that switch and the URL's save both wait +// for the lock. Either may take it first. The URL is stored for neither +// network. The flash message says that the network changed. +test.each([ + { button: "btn-save-rpc", field: "settings-rpc", first: "follow" }, + { button: "btn-save-rpc", field: "settings-rpc", first: "save" }, + { + button: "btn-save-blockscout", + field: "settings-blockscout", + first: "follow", + }, + { + button: "btn-save-blockscout", + field: "settings-blockscout", + first: "save", + }, +])( + "$button saves nothing when the network changes before the URL is stored ($first first)", + async ({ button, field, first }) => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + // This answer passes both endpoint checks. The RPC check reads + // Ethereum Mainnet's chain id from it. The Blockscout check reads `ok`. + globalThis.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ result: NETWORKS.mainnet.chainId }), + }); + const url = "https://own-node.example.invalid"; + + let releaseLock; + const lockHeld = navigator.locks.request( + "autistmask", + () => + new Promise((resolve) => { + releaseLock = resolve; + }), + ); + // The click handler runs until its save waits for the lock. + const tapSave = async () => { + page.node(field).value = url; + const saving = page.node(button).listeners.click.map((fn) => fn()); + await page.settle(); + return saving; + }; + let saving; + if (first === "save") saving = await tapSave(); + await backgroundSwitch(page, "sepolia"); + if (first === "follow") saving = await tapSave(); + releaseLock(); + await lockHeld; + await Promise.all(saving); + await page.settle(); + + const { state } = require("../src/shared/state"); + expect(state.networkId).toBe("sepolia"); + expect(JSON.stringify(page.storage.read("autistmask"))).not.toContain( + url, + ); + expect(page.text("flash-msg")).toBe( + "The URL was not saved because the network changed.", + ); + }, +); + +// The popup's follow of the background's switch to Sepolia has set `state` to +// Sepolia. The follow's write has not returned yet, so Settings still shows +// Ethereum Mainnet. The user taps "Save" there. The URL is for Ethereum +// Mainnet, so the save stores the URL for neither network. +test.each([ + { button: "btn-save-rpc", field: "settings-rpc" }, + { button: "btn-save-blockscout", field: "settings-blockscout" }, +])( + "$button saves nothing when tapped while the follow's write waits", + async ({ button, field }) => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + // This answer passes both endpoint checks on Ethereum Mainnet. + globalThis.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ result: NETWORKS.mainnet.chainId }), + }); + const url = "https://own-node.example.invalid"; + + const { set } = page.storage.local; + const write = set.getMockImplementation(); + let releaseWrite; + const writeHeld = new Promise((resolve) => { + releaseWrite = resolve; + }); + // The background's write lands; the next, the follow's, waits. + set.mockImplementationOnce(write).mockImplementationOnce( + async (items) => { + await writeHeld; + await write(items); + }, + ); + await backgroundSwitch(page, "sepolia"); + const { state } = require("../src/shared/state"); + expect(state.networkId).toBe("sepolia"); + expect(page.value("settings-network")).toBe("mainnet"); + + page.node(field).value = url; + const saving = page.node(button).listeners.click.map((fn) => fn()); + await page.settle(); + releaseWrite(); + await Promise.all(saving); + await page.settle(); + + expect(JSON.stringify(page.storage.read("autistmask"))).not.toContain( + url, + ); + expect(page.text("flash-msg")).toBe( + "The URL was not saved because the network changed.", + ); + }, +); + +// The user taps the RPC "Save" on Ethereum Mainnet. While the check waits for +// the node's answer, the popup follows the background's switch to Sepolia. The +// node answers with Sepolia's chain id. So the check refuses the node for +// Ethereum Mainnet, and nothing is stored. +test("the RPC check compares the answer with the network at the tap", async () => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + let answer; + globalThis.fetch.mockReturnValue( + new Promise((resolve) => { + answer = resolve; + }), + ); + const url = "https://sepolia-node.example.invalid"; + + page.node("settings-rpc").value = url; + const saving = page.node("btn-save-rpc").listeners.click.map((fn) => fn()); + await page.settle(); + await backgroundSwitch(page, "sepolia"); + const { state } = require("../src/shared/state"); + expect(state.networkId).toBe("sepolia"); + answer({ json: async () => ({ result: NETWORKS.sepolia.chainId }) }); + await Promise.all(saving); + await page.settle(); + + expect(JSON.stringify(page.storage.read("autistmask"))).not.toContain(url); + expect(page.text("flash-msg")).toBe( + "Wrong network: expected Ethereum Mainnet.", + ); +}); + +// The user taps the Blockscout "Save" on Ethereum Mainnet. While the check +// waits for Blockscout's answer, the popup follows the background's switch to +// Sepolia. The URL is for Ethereum Mainnet, so nothing is stored. +test("the Blockscout save is for the network at the tap", async () => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + let answer; + globalThis.fetch.mockReturnValue( + new Promise((resolve) => { + answer = resolve; + }), + ); + const url = "https://own-explorer.example.invalid/api/v2"; + + page.node("settings-blockscout").value = url; + const saving = page + .node("btn-save-blockscout") + .listeners.click.map((fn) => fn()); + await page.settle(); + await backgroundSwitch(page, "sepolia"); + expect(page.value("settings-network")).toBe("sepolia"); + answer({ ok: true }); + await Promise.all(saving); + await page.settle(); + + expect(JSON.stringify(page.storage.read("autistmask"))).not.toContain(url); + expect(page.text("flash-msg")).toBe( + "The URL was not saved because the network changed.", + ); +}); + +// Settings is open when the network changes to Sepolia. The popup follows the +// background's switch, or the user picks Sepolia in the network selector. Each +// "Save" after the switch stores its URL for Sepolia. +test.each([ + ["a follow", (page) => backgroundSwitch(page, "sepolia")], + [ + "the network selector", + async (page) => { + const select = page.node("settings-network"); + select.value = "sepolia"; + for (const fn of select.listeners.change) await fn(); + await page.settle(); + }, + ], +])( + "after a switch to Sepolia by %s, Settings saves each URL for Sepolia", + async (_how, switchToSepolia) => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + await switchToSepolia(page); + // This answer passes both endpoint checks on Sepolia. The RPC check + // reads Sepolia's chain id from it. The Blockscout check reads `ok`. + globalThis.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ result: NETWORKS.sepolia.chainId }), + }); + const rpcUrl = "https://own-node.example.invalid"; + const blockscoutUrl = "https://own-explorer.example.invalid/api/v2"; + + page.node("settings-rpc").value = rpcUrl; + await page.click("btn-save-rpc"); + expect(page.text("flash-msg")).toBe("Saved."); + page.node("settings-blockscout").value = blockscoutUrl; + await page.click("btn-save-blockscout"); + expect(page.text("flash-msg")).toBe("Saved."); + + const stored = page.storage.read("autistmask"); + expect(stored.networkId).toBe("sepolia"); + expect(stored.rpcUrl).toBe(rpcUrl); + expect(stored.blockscoutUrl).toBe(blockscoutUrl); + }, +); + +// The user picks Sepolia in Settings, but the switch's write fails. The page is +// then on Sepolia, while Settings was last drawn for Ethereum Mainnet. A "Save" +// after the failed switch stores nothing for either network. +test.each([ + { button: "btn-save-rpc", field: "settings-rpc" }, + { button: "btn-save-blockscout", field: "settings-blockscout" }, +])( + "$button saves nothing after a switch in Settings whose write failed", + async ({ button, field }) => { + const page = await bootOnAddress(); + await page.click("btn-settings"); + page.storage.local.set.mockRejectedValueOnce( + new Error("quota exceeded"), + ); + const select = page.node("settings-network"); + select.value = "sepolia"; + for (const fn of select.listeners.change) { + await expect(fn()).rejects.toThrow("quota exceeded"); + } + await page.settle(); + const { state } = require("../src/shared/state"); + expect(state.networkId).toBe("sepolia"); + // This answer passes both endpoint checks on Ethereum Mainnet. + globalThis.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ result: NETWORKS.mainnet.chainId }), + }); + const url = "https://own-node.example.invalid"; + + page.node(field).value = url; + await page.click(button); + + expect(JSON.stringify(page.storage.read("autistmask"))).not.toContain( + url, + ); + expect(page.text("flash-msg")).toBe( + "The URL was not saved because the network changed.", + ); + }, +); + // 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.