fix: Settings refuses an endpoint URL whose network changed before it was stored (closes #663)

The RPC and Blockscout "Save" buttons in Settings now set the URL
inside the save, holding the lock. The save is refused if the network
active when the user tapped "Save" is no longer active, on the page or
in the stored record. The flash message then says that the network
changed, and "Saved." is shown only for a stored URL.

Before, a popup that followed a switch saved elsewhere could replace
the URL before the write and still show "Saved.". It could also write
the previous network's URL as the new network's.

Model: opus-5-5

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

src/popup/views/settings.js

@@ -319,6 +319,28 @@ function showNewSettings() {
     updateDebugBanner(state.currentView);
 }
 
+// Saves `url` as `field` ("rpcUrl" or "blockscoutUrl") of the network
+// `networkId`, the one active 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 +352,8 @@ function init(_ctx) {
             showFlash("Please enter an RPC URL.");
             return;
         }
+        // The URL is for the network active when "Save" is tapped.
+        const net = currentNetwork();
         showFlash("Testing endpoint...");
         try {
             const resp = await debugFetch(url, {
@@ -348,7 +372,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 +387,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 +396,7 @@ function init(_ctx) {
             showFlash("Please enter a Blockscout API URL.");
             return;
         }
+        const networkId = state.networkId;
         showFlash("Testing endpoint...");
         try {
             const resp = await debugFetch(url + "/stats");
@@ -389,9 +411,7 @@ function init(_ctx) {
             showFlash("Could not reach endpoint.");
             return;
         }
-        state.blockscoutUrl = url;
-        await saveState();
-        showFlash("Saved.");
+        await saveEndpoint("blockscoutUrl", url, networkId);
     });
 
     // Each change is made inside the save, holding the lock. So

tests/popupFollowsNetworkSwitch.test.js

@@ -253,6 +253,72 @@ 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.",
+        );
+    },
+);
+
 // 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.