diff --git a/src/popup/index.js b/src/popup/index.js
index f7cc77f..22ea27b 100644
--- a/src/popup/index.js
+++ b/src/popup/index.js
@@ -65,7 +65,14 @@ function renderWalletList() {
const pageClosed = new AbortController();
window.addEventListener("pagehide", () => pageClosed.abort());
+// While a refresh runs, a request through ctx is not dropped. The request sets
+// refreshAgain, and one more refresh runs once the running one ends. Several
+// requests in that time lead to one more refresh, not several. An import asks
+// for a refresh after its address scan. A refresh already running does not
+// have the addresses the scan found
+// (https://git.eeqj.de/sneak/AutistMask/issues/581).
let refreshInFlight = false;
+let refreshAgain = false;
// The ten-second refresh startRefresh() starts, stopped when the popup moves to
// the recovery screen: there is no profile left to refresh.
@@ -89,7 +96,10 @@ let refreshTimer = null;
// user turned "Use Blockscout" off. So the save drops every token the user has
// not tracked if the stored setting is off by then.
async function doRefreshAndRender() {
- if (refreshInFlight) return;
+ if (refreshInFlight) {
+ refreshAgain = true;
+ return;
+ }
refreshInFlight = true;
try {
await followStoredSettings();
@@ -132,28 +142,35 @@ async function doRefreshAndRender() {
});
renderWalletList();
} catch (e) {
- // Every call site fires this and walks away — the boot below, the ten
- // second interval, and eight views through ctx — so it must never
- // reject: an unhandled rejection is not a report of anything. The save
- // inside it reports its own failure through onSaveFailure() (see
- // src/shared/state.js); what is left here is a failed network round
- // trip, which the next tick retries.
+ // No caller waits for this function, so it must never reject. An
+ // unhandled rejection reports nothing. The save inside it reports its
+ // own failure through onSaveFailure() (see src/shared/state.js); what
+ // is left here is a failed network round trip, which the next tick
+ // retries.
log.errorf("popup: background refresh failed:", e);
} finally {
refreshInFlight = false;
}
+ if (refreshAgain) {
+ refreshAgain = false;
+ doRefreshAndRender();
+ }
}
// Refreshes now and every ten seconds from then on. Called once a wallet
// exists: by init() when the popup opens with one, and by AddWallet when it
// adds one, which in a popup that opened on Welcome is the first
-// (https://git.eeqj.de/sneak/AutistMask/issues/530). With the refresh already
-// running it starts no second one, and on the recovery screen it does nothing.
+// (https://git.eeqj.de/sneak/AutistMask/issues/530). With the ten-second
+// refresh already started, it starts no second one. On the recovery screen it
+// does nothing.
function startRefresh() {
if (!$("view-state-recovery").classList.contains("hidden")) return;
doRefreshAndRender();
if (refreshTimer === null) {
- refreshTimer = setInterval(doRefreshAndRender, 10000);
+ // A tick that finds a refresh running is dropped.
+ refreshTimer = setInterval(() => {
+ if (!refreshInFlight) return doRefreshAndRender();
+ }, 10000);
}
}
diff --git a/tests/refreshDuringRefresh.test.js b/tests/refreshDuringRefresh.test.js
new file mode 100644
index 0000000..bdad2d6
--- /dev/null
+++ b/tests/refreshDuringRefresh.test.js
@@ -0,0 +1,157 @@
+// While a refresh runs, a request through ctx is not dropped. One more refresh
+// runs once the running one ends, however many were asked for in that time
+// (https://git.eeqj.de/sneak/AutistMask/issues/581). A tick of the ten-second
+// timer that finds a refresh running is dropped. Each test boots the real popup
+// over a stored profile and holds the ten-second refresh open. `readChain()`
+// sets each address's balance from `CHAIN`, as the real `refreshBalances()`
+// sets it from the node.
+
+const {
+ bootPopup,
+ cleanupPopup,
+ unversionedValidProfile,
+ ADDRESS,
+} = require("./support/popupBoot");
+const {
+ hdWalletFromMnemonic,
+ deriveAddressFromXpub,
+} = require("../src/shared/wallet");
+
+const PHRASE =
+ "abandon abandon abandon abandon abandon abandon " +
+ "abandon abandon abandon abandon abandon about";
+const PASSWORD = "correct horse battery staple";
+
+// The first two addresses of PHRASE, both used on the chain.
+const { xpub, firstAddress } = hdWalletFromMnemonic(PHRASE);
+const SECOND_ADDRESS = deriveAddressFromXpub(xpub, 1);
+
+// The ETH balance the chain holds for each address, in ether.
+const CHAIN = {
+ [ADDRESS]: "1.5",
+ [firstAddress]: "2.0",
+ [SECOND_ADDRESS]: "3.0",
+};
+
+afterEach(() => {
+ cleanupPopup();
+});
+
+// The balance stand-in for a refresh that returns.
+async function readChain(wallets) {
+ for (const wallet of wallets) {
+ for (const addr of wallet.addresses) {
+ addr.balance = CHAIN[addr.address];
+ }
+ }
+}
+
+// Boots the popup over a profile with one wallet. From then on,
+// `refreshBalances()` never returns unless the test answers it. So an unwanted
+// refresh is counted once and then waits, instead of starting more.
+async function boot() {
+ const page = await bootPopup(unversionedValidProfile());
+ expect(page.pageErrors).toEqual([]);
+ const { refreshBalances } = require("../src/shared/balances");
+ refreshBalances.mockImplementation(() => new Promise(() => {}));
+ return { page, refreshBalances };
+}
+
+// Starts the ten-second refresh and holds it open until finish() is called.
+// The held refresh then writes the chain's balances. finish() resolves once
+// the held refresh has saved them and drawn Home. The one refresh after the
+// held one also returns.
+async function holdRefresh(page) {
+ const { refreshBalances } = require("../src/shared/balances");
+ let answer;
+ refreshBalances
+ .mockImplementationOnce(async (wallets) => {
+ await new Promise((resolve) => {
+ answer = resolve;
+ });
+ await readChain(wallets);
+ })
+ .mockImplementationOnce(readChain);
+ const ticking = page.tick();
+ await page.settle();
+ return {
+ finish: async () => {
+ answer();
+ await ticking;
+ },
+ };
+}
+
+// Imports PHRASE on AddWallet. Its address scan finds the phrase's first two
+// addresses.
+async function importPhrase(page) {
+ const { scanForAddresses } = require("../src/shared/balances");
+ scanForAddresses.mockImplementationOnce(async () => ({
+ addresses: [
+ { address: firstAddress, index: 0 },
+ { address: SECOND_ADDRESS, index: 1 },
+ ],
+ nextIndex: 2,
+ }));
+ await page.click("btn-add-wallet-bottom");
+ await page.click("tab-mnemonic");
+ page.node("wallet-mnemonic").value = PHRASE;
+ page.node("add-wallet-password").value = PASSWORD;
+ page.node("add-wallet-password-confirm").value = PASSWORD;
+ await page.click("btn-add-wallet-confirm");
+}
+
+// Imports the private key `key` on AddWallet.
+async function importKey(page, key) {
+ await page.click("btn-add-wallet-bottom");
+ await page.click("tab-privkey");
+ page.node("import-private-key").value = key;
+ page.node("add-wallet-password").value = PASSWORD;
+ page.node("add-wallet-password-confirm").value = PASSWORD;
+ await page.click("btn-add-wallet-confirm");
+}
+
+test("addresses that an import finds during a refresh show their balances once one more refresh returns", async () => {
+ const { page } = await boot();
+ const refresh = await holdRefresh(page);
+
+ await importPhrase(page);
+ expect(page.visibleViews()).toEqual(["main"]);
+ expect(page.text("flash-msg")).toBe("Found 2 addresses.");
+
+ await refresh.finish();
+ await page.settle();
+
+ const list = page.node("wallet-list").innerHTML;
+ expect(list).toContain("2.0000");
+ expect(list).toContain("3.0000");
+});
+
+test("several requests during a refresh lead to exactly one more refresh", async () => {
+ const { page, refreshBalances } = await boot();
+ const refresh = await holdRefresh(page);
+ const calls = refreshBalances.mock.calls.length;
+
+ await importKey(page, "0x" + "11".repeat(32));
+ await importKey(page, "0x" + "22".repeat(32));
+ await importPhrase(page);
+ expect(refreshBalances).toHaveBeenCalledTimes(calls);
+
+ await refresh.finish();
+ await page.settle();
+
+ expect(refreshBalances).toHaveBeenCalledTimes(calls + 1);
+});
+
+test("a tick of the ten-second timer during a refresh starts no other refresh", async () => {
+ const { page, refreshBalances } = await boot();
+ const refresh = await holdRefresh(page);
+ const calls = refreshBalances.mock.calls.length;
+
+ await page.tick();
+
+ await refresh.finish();
+ await page.settle();
+
+ expect(refreshBalances).toHaveBeenCalledTimes(calls);
+});