diff --git a/README.md b/README.md index b8df696..fc98049 100644 --- a/README.md +++ b/README.md @@ -585,10 +585,17 @@ screen, including ExportPrivKey, falls back to Home. - To: color dot + full address + etherscan link - Transaction hash: full hash (tap to copy) + etherscan link - Count-up timer: "Waiting for confirmation... Ns" -- **Behavior**: Polls `getTransactionReceipt` every 10 seconds. +- **Behavior**: Polls `getTransactionReceipt` every 10 seconds. The wait is + persisted: closing and reopening the popup resumes the poll, with the elapsed + counter and the timeout deadline still measured from the original broadcast. A + lookup that fails is retried on the next tick rather than counted as a missing + receipt. - **Transitions**: - Receipt found → **SuccessTx** - - 60 seconds without confirmation → **ErrorTx** (timeout message) + - A lookup that answers "no receipt" 60 seconds or more after broadcast → + **ErrorTx** (timeout message) + - Exactly one of the two: a receipt found on the tick that crosses the + deadline wins, and neither outcome can be rendered over the other #### SuccessTx (`success-tx`) diff --git a/TODO.md b/TODO.md index 7882ec1..b03d423 100644 --- a/TODO.md +++ b/TODO.md @@ -57,6 +57,11 @@ undefined identifiers, which is how of trusting the persisted flag, so a profile already saved inconsistent no longer stays broken on every load ([#195](https://git.eeqj.de/sneak/AutistMask/issues/195)). +- 2026-08-11: WaitTx lifecycle: a receipt and the 60-second timeout can no + longer both render on one tick, no timer or in-flight lookup outlives its + wait, a failed receipt lookup no longer counts as a timeout, and the wait now + resumes after a popup close + ([#155](https://git.eeqj.de/sneak/AutistMask/issues/155)). - 2026-08-11: Wallet deletion repairs its own state — `hasWallet` follows the remaining wallets, the selection only moves when it was deleted, and the active-address change is broadcast to connected sites diff --git a/src/popup/index.js b/src/popup/index.js index e0cfd42..1ca0b4c 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -110,6 +110,7 @@ const RESTORABLE_VIEWS = new Set([ "settings-addtoken", "confirm-tx", "transaction", + "wait-tx", "success-tx", "error-tx", ]); @@ -176,6 +177,12 @@ function restoreView() { fallbackView(); } break; + case "wait-tx": + // Resumes the receipt poll from the persisted broadcast time. + if (!txStatus.restoreWait()) { + fallbackView(); + } + break; case "success-tx": if (state.viewData && state.viewData.hash) { txStatus.renderSuccess(); diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index d45a67d..7ec67e5 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -16,11 +16,26 @@ const { state, saveState, currentNetwork } = require("../../shared/state"); const { getProvider } = require("../../shared/balances"); const { log } = require("../../shared/log"); +// Receipt poll cadence and the deadline after which the wait is reported as +// a timeout. Both are documented in the WaitTx section of README.md. +const POLL_INTERVAL_MS = 10000; +const TIMEOUT_MS = 60000; + let ctx; let elapsedTimer = null; let pollTimer = null; -function clearTimers() { +// Identifies the wait currently on screen. Bumped by endWait(), so a timer +// callback or an in-flight receipt lookup that outlives its wait can tell +// that it is stale and leave the current view alone. Without it, a receipt +// resolving after the wait has ended renders over whatever view replaced it. +let waitId = 0; + +// End the wait on screen: stop its timers and invalidate its pending async +// work. Called on receipt, on timeout, when a new wait starts, and when the +// user navigates away. +function endWait() { + waitId++; if (elapsedTimer) { clearInterval(elapsedTimer); elapsedTimer = null; @@ -47,8 +62,13 @@ function blockNumberHtml(blockNumber) { return copyableHtml(num) + etherscanLinkHtml(link); } -function showWait(txInfo, txHash) { - clearTimers(); +// Render the wait view and start polling for the receipt. broadcastTime is +// when the transaction was broadcast, which is what the elapsed counter and +// the timeout deadline are both measured from; pollNow runs one lookup +// immediately instead of waiting a full poll interval. +function startWait(txInfo, txHash, broadcastTime, pollNow) { + endWait(); + const id = waitId; const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?"; $("wait-tx-summary").textContent = txInfo.amount + " " + symbol; @@ -56,41 +76,98 @@ function showWait(txInfo, txHash) { $("wait-tx-hash").innerHTML = txHashHtml(txHash); attachCopyHandlers("view-wait-tx"); - const broadcastTime = Date.now(); - $("wait-tx-status").textContent = "Waiting for confirmation... 0s"; + // Persisted so closing and reopening the popup resumes this wait + // instead of silently abandoning it. + state.viewData = { + pendingWait: { + txInfo: txInfo, + hash: txHash, + broadcastTime: broadcastTime, + }, + }; - elapsedTimer = setInterval(() => { + function renderElapsed() { const elapsed = Math.floor((Date.now() - broadcastTime) / 1000); $("wait-tx-status").textContent = "Waiting for confirmation... " + elapsed + "s"; + } + renderElapsed(); + + elapsedTimer = setInterval(() => { + if (id !== waitId) return; + renderElapsed(); }, 1000); const provider = getProvider(state.rpcUrl); - pollTimer = setInterval(async () => { + + async function poll() { + if (id !== waitId) return; + let receipt = null; + let answered = true; try { - const receipt = await provider.getTransactionReceipt(txHash); - if (receipt) { - showSuccess(txInfo, txHash, receipt.blockNumber); - } + receipt = await provider.getTransactionReceipt(txHash); } catch (e) { + // A thrown lookup means "no answer this tick", not "no + // receipt": the RPC failed, the chain said nothing. Declaring + // the timeout off it would report a confirmed transaction as + // failed — which matters most on a resumed wait, where the + // first poll is already past the deadline. + answered = false; log.errorf("poll receipt failed:", e.message); } - - const elapsed = Math.floor((Date.now() - broadcastTime) / 1000); - if (elapsed >= 60) { + // The lookup is async: the wait may have ended while it was in + // flight, in which case this result must not touch the view. + if (id !== waitId) return; + // Exactly one outcome per wait. A receipt wins even on the tick + // that crosses the deadline, because the transaction did confirm. + if (receipt) { + showSuccess(txInfo, txHash, receipt.blockNumber); + return; + } + // Keep polling until a lookup actually answers; the next tick may. + if (!answered) return; + if (Date.now() - broadcastTime >= TIMEOUT_MS) { showError( txInfo, txHash, "Transaction was not confirmed within 60 seconds. It may still confirm later \u2014 check Etherscan.", ); } - }, 10000); + } + + pollTimer = setInterval(poll, POLL_INTERVAL_MS); showView("wait-tx"); + + if (pollNow) poll(); +} + +function showWait(txInfo, txHash) { + startWait(txInfo, txHash, Date.now(), false); +} + +// Resume a wait persisted by a previous popup session. The deadline still +// runs from the original broadcast, so a wait that has already outlived it +// resolves on the immediate first poll rather than restarting the clock. +// Returns false when there is nothing resumable to resume: the whole +// payload is validated, because startWait() dereferences txInfo and does +// arithmetic on broadcastTime, and a partial one would throw out of +// restoreView() or leave an unexitable wait counting "NaNs". +function restoreWait() { + const d = state.viewData; + if (!d || !d.pendingWait) return false; + const w = d.pendingWait; + if (!w.hash) return false; + if (!w.txInfo || typeof w.txInfo !== "object") return false; + if (typeof w.broadcastTime !== "number" || !isFinite(w.broadcastTime)) { + return false; + } + startWait(w.txInfo, w.hash, w.broadcastTime, true); + return true; } function showSuccess(txInfo, txHash, blockNumber) { - clearTimers(); + endWait(); const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?"; state.viewData = { @@ -182,7 +259,7 @@ function renderSuccess() { } function showError(txInfo, txHash, message) { - clearTimers(); + endWait(); const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?"; state.viewData = { @@ -218,6 +295,9 @@ function isApprovalPopup() { } function navigateBack() { + // Nothing should still be polling by now, but leaving a view is the + // point at which its timers must be gone. + endWait(); if (isApprovalPopup()) { window.close(); return; @@ -242,4 +322,12 @@ function init(_ctx) { $("btn-error-tx-done").addEventListener("click", navigateBack); } -module.exports = { init, showWait, showError, renderSuccess, renderError }; +module.exports = { + init, + showWait, + restoreWait, + endWait, + showError, + renderSuccess, + renderError, +}; diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js new file mode 100644 index 0000000..5bcc584 --- /dev/null +++ b/tests/txStatus.test.js @@ -0,0 +1,340 @@ +// Lifecycle tests for the post-broadcast transaction status views +// (src/popup/views/txStatus.js). +// +// The bug these pin down: the receipt poll rendered both outcomes on the tick +// that crossed the 60-second deadline, so a confirmed transaction was replaced +// by "not confirmed within 60 seconds" — the user is told their transaction +// failed when it succeeded. The same shape applies to any callback that +// outlives its wait: a receipt lookup still in flight when the view is left +// must not render over whatever replaced it. +// +// Fake timers make the race deterministic: the receipt promise is already +// resolved when the deadline tick runs, so in the unfixed code showSuccess() +// is always followed by showError() on that tick. +// +// No network: getProvider is mocked at the module boundary and there is no +// jsdom in this repo, so the handful of DOM calls these views make are served +// by the stub below. + +jest.mock("../src/shared/log", () => ({ + log: { + debugf: () => {}, + infof: () => {}, + warnf: () => {}, + errorf: () => {}, + }, + debugFetch: jest.fn(), + setRuntimeDebug: () => {}, + isDebug: () => false, +})); + +const mockReceiptLookup = jest.fn(); +jest.mock("../src/shared/balances", () => ({ + getProvider: () => ({ getTransactionReceipt: mockReceiptLookup }), + refreshBalances: jest.fn(), +})); + +global.fetch = jest.fn(() => { + throw new Error("tests must not perform network requests"); +}); + +// --------------------------------------------------------------------------- +// Minimal DOM. Every element is created on demand and remembered by id, so a +// test can read back what a view wrote into it. +// --------------------------------------------------------------------------- + +const elements = new Map(); + +function makeElement(id) { + const classes = new Set(["view", "hidden"]); + const el = { + id, + textContent: "", + innerHTML: "", + style: {}, + classList: { + add: (c) => classes.add(c), + remove: (c) => classes.delete(c), + contains: (c) => classes.has(c), + toggle: (c, on) => (on ? classes.add(c) : classes.delete(c)), + }, + addEventListener: () => {}, + querySelectorAll: () => [], + remove: () => {}, + prepend: () => {}, + }; + // Views reach for .parentElement to hide whole sections. + Object.defineProperty(el, "parentElement", { + get: () => getElement(id + "-parent"), + }); + return el; +} + +function getElement(id) { + if (!elements.has(id)) elements.set(id, makeElement(id)); + return elements.get(id); +} + +global.document = { + getElementById: (id) => getElement(id), + // escapeHtml() builds a detached div; textContent in, escaped HTML out. + createElement: () => { + const el = { innerHTML: "" }; + Object.defineProperty(el, "textContent", { + set(v) { + el.innerHTML = String(v) + .replace(/&/g, "&") + .replace(//g, ">"); + }, + }); + return el; + }, + body: { prepend: () => {} }, + addEventListener: () => {}, +}; + +global.window = { location: { search: "" } }; + +const stored = {}; +global.chrome = { + storage: { + local: { + set: (obj) => { + Object.assign(stored, obj); + return Promise.resolve(); + }, + get: () => Promise.resolve(stored), + }, + }, +}; + +const txStatus = require("../src/popup/views/txStatus"); +const { state } = require("../src/shared/state"); + +const TX_HASH = + "0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84"; +const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; +const TX_INFO = { + to: RECIPIENT, + amount: "0.0050", + token: "ETH", + tokenSymbol: null, +}; + +// True when a view element is not hidden. +function visible(view) { + return !getElement("view-" + view).classList.contains("hidden"); +} + +function waitStatusText() { + return getElement("wait-tx-status").textContent; +} + +beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(new Date("2026-08-11T12:00:00Z")); + elements.clear(); + mockReceiptLookup.mockReset(); + state.wallets = []; + state.viewData = {}; + state.viewStack = []; + state.currentView = null; + txStatus.init({ doRefreshAndRender: jest.fn() }); +}); + +afterEach(() => { + txStatus.endWait(); + jest.useRealTimers(); +}); + +describe("WaitTx receipt/timeout race", () => { + test("a receipt arriving on the deadline tick leaves the user on SuccessTx", async () => { + // No receipt for the first five polls; the sixth — the tick at + // t=60s, which is also the timeout deadline — returns one. + mockReceiptLookup + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValue({ blockNumber: 21000000 }); + + txStatus.showWait(TX_INFO, TX_HASH); + expect(visible("wait-tx")).toBe(true); + + await jest.advanceTimersByTimeAsync(60000); + + expect(visible("success-tx")).toBe(true); + expect(visible("error-tx")).toBe(false); + expect(state.currentView).toBe("success-tx"); + expect(state.viewData.blockNumber).toBe(21000000); + expect(state.viewData.message).toBeUndefined(); + + // And nothing is left running to undo it. + expect(jest.getTimerCount()).toBe(0); + await jest.advanceTimersByTimeAsync(300000); + expect(state.currentView).toBe("success-tx"); + expect(mockReceiptLookup).toHaveBeenCalledTimes(6); + }); + + test("a genuine timeout still shows ErrorTx with the hash", async () => { + mockReceiptLookup.mockResolvedValue(null); + + txStatus.showWait(TX_INFO, TX_HASH); + await jest.advanceTimersByTimeAsync(60000); + + expect(visible("error-tx")).toBe(true); + expect(state.currentView).toBe("error-tx"); + expect(state.viewData.message).toMatch( + /not confirmed within 60 seconds/, + ); + expect(state.viewData.hash).toBe(TX_HASH); + // The hash section carries the hash and the etherscan link. + expect(getElement("error-tx-hash").innerHTML).toContain(TX_HASH); + expect(getElement("error-tx-hash").innerHTML).toContain( + "/tx/" + TX_HASH, + ); + expect(jest.getTimerCount()).toBe(0); + }); + + test("a receipt still in flight when the view is left does not render over it", async () => { + let resolveReceipt; + mockReceiptLookup.mockReturnValue( + new Promise((r) => { + resolveReceipt = r; + }), + ); + + txStatus.showWait(TX_INFO, TX_HASH); + await jest.advanceTimersByTimeAsync(10000); + expect(mockReceiptLookup).toHaveBeenCalledTimes(1); + + // User leaves the wait (popup navigation / teardown) while the + // lookup is outstanding, then the lookup finally answers. + txStatus.endWait(); + state.currentView = "main"; + resolveReceipt({ blockNumber: 21000000 }); + await Promise.resolve(); + await Promise.resolve(); + + expect(state.currentView).toBe("main"); + expect(visible("success-tx")).toBe(false); + }); + + test("no timer survives the view being left", async () => { + mockReceiptLookup.mockResolvedValue(null); + + txStatus.showWait(TX_INFO, TX_HASH); + expect(jest.getTimerCount()).toBeGreaterThan(0); + + txStatus.endWait(); + expect(jest.getTimerCount()).toBe(0); + + await jest.advanceTimersByTimeAsync(120000); + expect(mockReceiptLookup).not.toHaveBeenCalled(); + }); +}); + +describe("WaitTx persistence across popup close", () => { + test("restoreWait resumes the poll with the deadline running from broadcast", async () => { + mockReceiptLookup.mockResolvedValue(null); + + txStatus.showWait(TX_INFO, TX_HASH); + expect(state.viewData.pendingWait.hash).toBe(TX_HASH); + const persisted = JSON.parse(JSON.stringify(state.viewData)); + + // Popup closes: timers die with the page. + txStatus.endWait(); + + // 45 seconds pass with the popup shut, then it is reopened. + jest.advanceTimersByTime(45000); + state.viewData = persisted; + expect(txStatus.restoreWait()).toBe(true); + + expect(visible("wait-tx")).toBe(true); + // Elapsed is counted from the broadcast, not from the reopen. + expect(waitStatusText()).toBe("Waiting for confirmation... 45s"); + // The immediate poll on resume has already run. + await Promise.resolve(); + expect(mockReceiptLookup).toHaveBeenCalledTimes(1); + + // The deadline is 15 seconds away, not 60. + await jest.advanceTimersByTimeAsync(20000); + expect(state.currentView).toBe("error-tx"); + }); + + test("a rejected lookup on the resume poll keeps waiting instead of reporting failure", async () => { + // A wait resumed after the deadline has already passed: the first + // poll is immediate and past 60s, so a thrown lookup must not be + // read as "no receipt". It means "no answer this tick" — keep + // polling, because the transaction may well have confirmed. + mockReceiptLookup.mockResolvedValue(null); + txStatus.showWait(TX_INFO, TX_HASH); + const persisted = JSON.parse(JSON.stringify(state.viewData)); + txStatus.endWait(); + + // Ten minutes with the popup shut, then it is reopened and the + // first receipt lookup fails transiently. + jest.advanceTimersByTime(600000); + mockReceiptLookup.mockReset(); + mockReceiptLookup + .mockRejectedValueOnce(new Error("rpc unavailable")) + .mockResolvedValue({ blockNumber: 21000000 }); + + state.viewData = persisted; + expect(txStatus.restoreWait()).toBe(true); + await jest.advanceTimersByTimeAsync(0); + + // The wait is still alive: no timeout was declared off one error. + expect(visible("wait-tx")).toBe(true); + expect(visible("error-tx")).toBe(false); + expect(state.currentView).toBe("wait-tx"); + expect(jest.getTimerCount()).toBeGreaterThan(0); + + // And the next tick answers, so the confirmed transaction is + // reported as confirmed. + await jest.advanceTimersByTimeAsync(10000); + expect(state.currentView).toBe("success-tx"); + expect(state.viewData.blockNumber).toBe(21000000); + }); + + test("a lookup returning null past the deadline still times out", async () => { + // The counterpart to the test above: the deadline must still fire + // when the lookup actually answers "no receipt". + mockReceiptLookup.mockResolvedValue(null); + txStatus.showWait(TX_INFO, TX_HASH); + const persisted = JSON.parse(JSON.stringify(state.viewData)); + txStatus.endWait(); + + jest.advanceTimersByTime(600000); + state.viewData = persisted; + expect(txStatus.restoreWait()).toBe(true); + await jest.advanceTimersByTimeAsync(0); + + expect(state.currentView).toBe("error-tx"); + expect(state.viewData.message).toMatch( + /not confirmed within 60 seconds/, + ); + }); + + test("restoreWait reports nothing to resume when no wait is persisted", () => { + state.viewData = {}; + expect(txStatus.restoreWait()).toBe(false); + expect(jest.getTimerCount()).toBe(0); + }); + + test("restoreWait rejects a persisted wait missing its txInfo or broadcast time", () => { + for (const bad of [ + { hash: TX_HASH, broadcastTime: Date.now() }, + { hash: TX_HASH, txInfo: TX_INFO }, + { hash: TX_HASH, txInfo: TX_INFO, broadcastTime: "soon" }, + { hash: TX_HASH, txInfo: TX_INFO, broadcastTime: NaN }, + { hash: TX_HASH, txInfo: "nope", broadcastTime: Date.now() }, + ]) { + state.viewData = { pendingWait: bad }; + expect(txStatus.restoreWait()).toBe(false); + expect(jest.getTimerCount()).toBe(0); + } + }); +});