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..da11f65 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 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 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 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..55a7fd8 100644 --- a/tests/popupFollowsNetworkSwitch.test.js +++ b/tests/popupFollowsNetworkSwitch.test.js @@ -253,6 +253,156 @@ 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.", + ); +}); + // 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.