diff --git a/README.md b/README.md index edeae29..c691752 100644 --- a/README.md +++ b/README.md @@ -1163,7 +1163,11 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. opening the window, so the screen shows a complete transaction and the signed artifact can be compared with it field for field. A request that cannot be populated — unreachable node, reverting gas estimate — opens no window and is - failed back to the site. + failed back to the site. Only one transaction approval exists at a time: + populating fixes the nonce, so a second `eth_sendTransaction` arriving while + one is unanswered is refused with EIP-1193 code `-32002` rather than being + populated at the same nonce. It opens no window and takes no nonce, and the + site can send it again once the pending one is answered. - **Elements**: - "Transaction Request" heading - Phishing warning banner (shown when the hostname is on the phishing diff --git a/TODO.md b/TODO.md index d3898e5..bfc5303 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,23 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-14: One transaction approval at a time. Populating in the background + before the window opens is what makes the displayed object the verified + object, and it also fixes the nonce: two `eth_sendTransaction` calls populated + concurrently took the same nonce from a node that had seen neither broadcast, + and the second could then never be sent, because the only way to give it a + fresh nonce is to populate it again after the user has read the old one off + the screen. A second request is now refused with EIP-1193 `-32002` while one + is unanswered — before anything is populated, so no second nonce is allocated + and no second window opens — and the slot is freed when the page has its + answer. Signature approvals are not gated, consuming no nonce. A collision + that does happen is also reported accurately now: a broadcast the node refused + for the nonce, and an approval carrying a nonce this worker has already + broadcast (caught before the node is asked at all), both say the transaction + did not reach the network and to send it again, instead of warning that it may + have sent. `already known` deliberately keeps the ambiguous wording, because a + node that says it has the transaction has it + ([#271](https://git.eeqj.de/sneak/AutistMask/issues/271)). - 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js` rebuilt every failure as `new Error(error.message)`, so the code the background produced and the content script relayed intact was dropped in the diff --git a/src/background/index.js b/src/background/index.js index f89e386..768f985 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -24,6 +24,7 @@ const { TX_STAGE_VERIFY, TX_STAGE_BROADCAST, TX_STAGE_INFLIGHT, + TX_STAGE_NONCE, } = require("../shared/approvalVerify"); const { prepareApprovalTx } = require("../shared/approvalTx"); const { @@ -57,6 +58,81 @@ const connectedSites = {}; // Pending approval requests: { id: { origin, hostname, resolve } } const pendingApprovals = {}; +// One transaction approval at a time, wallet-wide. +// +// The transaction a site asks for is populated before its approval window +// opens, so that the object the user is shown is the object the signed +// artifact is verified against. Populating fixes the nonce. Two requests +// populated concurrently therefore take the SAME nonce — the node reports the +// same pending count to both, neither having been broadcast — and whichever is +// broadcast second is refused by the network for a nonce it can never be +// re-signed at, because re-signing it would mean signing something other than +// what was displayed. +// +// So the second request is refused while the first is unanswered. It is +// refused before anything is populated, so no second nonce is allocated at +// all, and while the page is still waiting with nothing on screen. The +// alternatives were considered and rejected in +// https://git.eeqj.de/sneak/AutistMask/issues/271: populating again at Confirm +// puts a nonce on screen that is not the nonce that gets signed, and +// allocating around in-flight approvals makes the wallet's own bookkeeping the +// authority on a nonce the network has not accepted, which an abandoned +// approval then leaves a hole in. +// +// Sign approvals are not gated: a signature consumes no nonce. +let txApprovalSlotHeld = false; + +// EIP-1474 "resource unavailable": the standard code for a request that is +// refused because another one is already pending. +const TX_APPROVAL_PENDING_CODE = -32002; + +const TX_APPROVAL_PENDING_MESSAGE = + "Another transaction is already waiting to be approved in AutistMask," + + " so this one was not sent. Please answer that request, then send this" + + " one again."; + +// Take the slot, or refuse. Called before the first await of the +// eth_sendTransaction handler, so two requests arriving in the same tick +// cannot both pass it. +function reserveTxApprovalSlot() { + if (txApprovalSlotHeld) return false; + txApprovalSlotHeld = true; + return true; +} + +function releaseTxApprovalSlot() { + txApprovalSlotHeld = false; +} + +// Nonces this worker has already handed to the node, per address. This is the +// wallet's own knowledge that a nonce is spent, and it is checked before a +// broadcast rather than after: a node's pending count can lag a transaction it +// has itself just accepted, and a request populated inside that window would +// otherwise be signed and sent at a nonce this wallet has already used. +// +// The record dies with the worker, which is correct rather than merely +// convenient: after a restart the node's count is the only answer available, +// and a transaction of this wallet's that the node has forgotten is one the +// user does want to be able to send again. +const broadcastNonces = {}; + +function broadcastNoncesFor(address) { + const key = String(address || "").toLowerCase(); + if (!broadcastNonces[key]) broadcastNonces[key] = new Set(); + return broadcastNonces[key]; +} + +// An approved transaction's nonce as a decimal string, or null if it cannot be +// read as a number. Verification refuses an unreadable nonce before this is +// ever reached; null here only keeps the record from holding junk. +function approvedNonce(approvedTx) { + try { + return BigInt(approvedTx.nonce).toString(); + } catch { + return null; + } +} + async function getState() { const result = await storageApi.get("autistmask"); return ( @@ -585,68 +661,24 @@ async function handleRpc(method, params, origin) { } if (method === "eth_sendTransaction") { - const s = await getState(); - const activeAddress = await getActiveAddress(); - if (!activeAddress) - return { error: { message: "No accounts available" } }; - - const hostname = extractHostname(origin); - const allowed = s.allowedSites[activeAddress] || []; - if ( - !allowed.includes(hostname) && - !connectedSites[origin + ":" + activeAddress] - ) { - return { error: { code: 4100, message: "Unauthorized" } }; - } - - const txParams = params?.[0] || {}; - if (namesAnotherAddress(txParams.from, activeAddress)) { + // Synchronous, before any await: two requests delivered in the same + // tick must not both get past this. + if (!reserveTxApprovalSlot()) { return { error: { - code: 4100, - message: - "This site asked to send from an address that is not the active one.", + code: TX_APPROVAL_PENDING_CODE, + message: TX_APPROVAL_PENDING_MESSAGE, }, }; } - - // Populate here, before any window opens, so that the transaction the - // user is shown is a complete one and is the same object the signed - // artifact is checked against. A failure raises no approval at all and - // is reported to the requesting page; see approvalTx.js. - let approvedTx; try { - approvedTx = await prepareApprovalTx( - getProvider(await getRpcUrl()), - activeAddress, - txParams, - ); - } catch (e) { - return { error: { message: e.message } }; + return await handleSendTransaction(params, origin); + } finally { + // Held until the page has its answer — the approval was broadcast, + // rejected, or retired by a closed window — because until then its + // nonce is allocated and unspent. + releaseTxApprovalSlot(); } - - // Population is a network round trip, and the user can switch address - // during it. Raising the approval anyway would put an account on the - // screen that the wallet is no longer on, and it could never be signed - // — the signing handler refuses exactly that. Refuse it here instead, - // while the page is still waiting and nothing has been displayed. - if (!sameAddress(await getActiveAddress(), activeAddress)) { - return { - error: { - message: - "The active address changed while this transaction was being prepared, so it was not sent.", - }, - }; - } - - const decision = await requestTxApproval( - origin, - hostname, - approvedTx, - activeAddress, - ); - if (decision.error) return { error: decision.error }; - return { result: decision.txHash }; } // Proxy safe read-only methods to the RPC node @@ -662,6 +694,73 @@ async function handleRpc(method, params, origin) { return { error: { message: "Unsupported method: " + method } }; } +// The body of eth_sendTransaction, from the connection check through to the +// user's decision. Its caller holds the single transaction-approval slot for +// as long as this runs. +async function handleSendTransaction(params, origin) { + const s = await getState(); + const activeAddress = await getActiveAddress(); + if (!activeAddress) return { error: { message: "No accounts available" } }; + + const hostname = extractHostname(origin); + const allowed = s.allowedSites[activeAddress] || []; + if ( + !allowed.includes(hostname) && + !connectedSites[origin + ":" + activeAddress] + ) { + return { error: { code: 4100, message: "Unauthorized" } }; + } + + const txParams = params?.[0] || {}; + if (namesAnotherAddress(txParams.from, activeAddress)) { + return { + error: { + code: 4100, + message: + "This site asked to send from an address that is not the active one.", + }, + }; + } + + // Populate here, before any window opens, so that the transaction the + // user is shown is a complete one and is the same object the signed + // artifact is checked against. A failure raises no approval at all and + // is reported to the requesting page; see approvalTx.js. + let approvedTx; + try { + approvedTx = await prepareApprovalTx( + getProvider(await getRpcUrl()), + activeAddress, + txParams, + ); + } catch (e) { + return { error: { message: e.message } }; + } + + // Population is a network round trip, and the user can switch address + // during it. Raising the approval anyway would put an account on the + // screen that the wallet is no longer on, and it could never be signed + // — the signing handler refuses exactly that. Refuse it here instead, + // while the page is still waiting and nothing has been displayed. + if (!sameAddress(await getActiveAddress(), activeAddress)) { + return { + error: { + message: + "The active address changed while this transaction was being prepared, so it was not sent.", + }, + }; + } + + const decision = await requestTxApproval( + origin, + hostname, + approvedTx, + activeAddress, + ); + if (decision.error) return { error: decision.error }; + return { result: decision.txHash }; +} + // Broadcast chainChanged to all tabs when the network is switched. function broadcastChainChanged(chainId) { tabsApi.query({}, (tabs) => { @@ -959,7 +1058,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { sendResponse({ error: outcome.error, retryable: outcome.retryable, - stage: TX_STAGE_SIGN, + stage: outcome.stage, }); return false; } @@ -1019,7 +1118,28 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { sendResponse({ error: outcome.error, retryable: outcome.retryable, - stage: TX_STAGE_VERIFY, + stage: outcome.stage, + }); + return; + } + + // A nonce this worker has already broadcast for this address. The + // node is not asked: it has answered once already, and the wallet + // holding the receipt of that answer is what makes this failure + // one the user can be told did not reach the network. + const nonce = approvedNonce(approval.approvedTx); + const spent = broadcastNoncesFor(approval.approvedFrom); + if (nonce !== null && spent.has(nonce)) { + const outcome = describeTxFailure(TX_STAGE_NONCE, null); + settleApproval( + msg.id, + { error: { message: outcome.error } }, + { holdsClaim: true }, + ); + sendResponse({ + error: outcome.error, + retryable: outcome.retryable, + stage: outcome.stage, }); return; } @@ -1027,6 +1147,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { try { const provider = getProvider(state.rpcUrl); const tx = await provider.broadcastTransaction(msg.rawSignedTx); + if (nonce !== null) spent.add(nonce); settleApproval( msg.id, { txHash: tx.hash }, @@ -1039,6 +1160,11 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { // tell a transaction that never left from one already in the // mempool. The page has been given its outcome for this // request; a second attempt would report a second one. + // + // Unless the node blamed the nonce, which is the one answer + // that says plainly it did not take the transaction: + // describeTxFailure() reclassifies that, and the stage it + // returns is the one reported. const outcome = describeTxFailure(TX_STAGE_BROADCAST, e); settleApproval( msg.id, @@ -1048,7 +1174,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { sendResponse({ error: outcome.error, retryable: outcome.retryable, - stage: TX_STAGE_BROADCAST, + stage: outcome.stage, }); } })(); diff --git a/src/shared/approvalVerify.js b/src/shared/approvalVerify.js index d2fea3e..18d2878 100644 --- a/src/shared/approvalVerify.js +++ b/src/shared/approvalVerify.js @@ -602,6 +602,12 @@ const TX_STAGE_BROADCAST = "broadcast"; // may yet succeed, so the one thing the popup must not say is "start again // from the site". const TX_STAGE_INFLIGHT = "inflight"; +// A transaction refused for a nonce that is already spoken for, either by the +// node's own answer or by this wallet's record of what it has broadcast. It is +// the one broadcast-stage failure that is not ambiguous: the transaction was +// not taken, so the user is told it did not reach the network and to send it +// again, rather than being warned that it might already be out there. +const TX_STAGE_NONCE = "nonce"; function errorText(err) { if (typeof err === "string" && err !== "") return err; @@ -611,6 +617,59 @@ function errorText(err) { return "The transaction could not be sent."; } +// Every string a failure might carry its reason in. ethers reports the node's +// own words in `shortMessage`, but a JSON-RPC error it could not classify is +// nested under `error` or `info.error` with the node's message intact, and the +// classification below has to see that too. +function failureTexts(err) { + if (typeof err === "string") return [err]; + if (!err || typeof err !== "object") return []; + const texts = []; + for (const text of [err.shortMessage, err.message, err.reason]) { + if (text) texts.push(String(text)); + } + const nested = err.error || (err.info && err.info.error); + if (nested && nested.message) texts.push(String(nested.message)); + return texts; +} + +// What the Ethereum clients say when a transaction's nonce is already spoken +// for: either it is below the account's next nonce, or another transaction is +// sitting in the pool at that nonce and this one did not outbid it. Either way +// the node answered, and its answer was that it did not take this transaction. +// +// "already known" is deliberately absent. A node that says it knows the +// transaction has it, so that transaction did reach the network and the +// ambiguous broadcast wording is the correct one for it. +const NONCE_COLLISION_PATTERNS = [ + /nonce too low/i, + /nonce has already been used/i, + /invalid nonce/i, + /oldnonce/i, + /replacement transaction underpriced/i, + /replacement fee too low/i, +]; + +// ethers' own classification of the same two conditions. +const NONCE_COLLISION_CODES = ["NONCE_EXPIRED", "REPLACEMENT_UNDERPRICED"]; + +// Whether a failed send is a nonce collision. +function isNonceCollision(err) { + if (!err) return false; + if (err.code && NONCE_COLLISION_CODES.includes(err.code)) return true; + return failureTexts(err).some((text) => + NONCE_COLLISION_PATTERNS.some((pattern) => pattern.test(text)), + ); +} + +// What both the requesting page and the popup are told about a nonce +// collision. The node's own words ("nonce too low") are a fragment and are +// replaced rather than passed through: they are not a sentence, and they say +// less than the wallet knows. +const NONCE_COLLISION_MESSAGE = + "The transaction was not sent, because its nonce had already been used" + + " by another transaction."; + // What the background does with a pending transaction approval after a failed // attempt: what it tells the popup, and whether the approval is spent // (resolved to the requesting page as an error and deleted) or left standing @@ -627,12 +686,31 @@ function errorText(err) { // that never left from one that is already in the mempool. The approval is // spent and the requesting page has been given its outcome; a second // attempt against it would report a second outcome for one request. +// - nonce: terminal too, and the one case where the wallet does know the +// transaction never left. The approval carries a nonce that is spent, so +// the artifact signed against it can never be accepted and the user is told +// to send it again from the site. +// +// The stage comes back out because a broadcast failure the node blamed on the +// nonce is reclassified here; the caller reports the stage this returns rather +// than the one it passed in. function describeTxFailure(stage, err) { + if ( + stage === TX_STAGE_NONCE || + (stage === TX_STAGE_BROADCAST && isNonceCollision(err)) + ) { + return { + error: NONCE_COLLISION_MESSAGE, + retryable: false, + spendApproval: true, + stage: TX_STAGE_NONCE, + }; + } const error = errorText(err); const retryable = stage === TX_STAGE_SIGN || (stage === TX_STAGE_VERIFY && failureIsRetryable(err)); - return { error, retryable, spendApproval: !retryable }; + return { error, retryable, spendApproval: !retryable, stage }; } // What the popup shows and does after the background reports a failed signing @@ -642,14 +720,20 @@ function describeTxFailure(stage, err) { // // A failed broadcast gets its own wording: the transaction may already be on // the network, so telling the user to start again from the site is exactly the -// wrong instruction. +// wrong instruction. A nonce collision is the exception to that exception — +// the transaction demonstrably did not go out, and saying it might have would +// send the user hunting for a transaction that does not exist. function describeSigningFailure(response, fallbackMessage) { let message = (response && response.error) || fallbackMessage; if (!/[.!?]$/.test(message)) message += "."; const retryable = !!(response && response.retryable); const stage = response && response.stage; if (!retryable) { - if (stage === TX_STAGE_BROADCAST) { + if (stage === TX_STAGE_NONCE) { + message += + " The transaction did not reach the network." + + " Please send it again from the site."; + } else if (stage === TX_STAGE_BROADCAST) { message += " The transaction may still have reached the network." + " Check the account before sending it again."; @@ -675,9 +759,11 @@ module.exports = { assertWithinCeilings, sameAddress, failureIsRetryable, + isNonceCollision, describeTxFailure, describeSigningFailure, ApprovalMismatchError, + NONCE_COLLISION_MESSAGE, ALLOWED_TX_TYPES, SERIALIZED_FIELDS, FORBIDDEN_FIELDS, @@ -686,6 +772,7 @@ module.exports = { TX_STAGE_VERIFY, TX_STAGE_BROADCAST, TX_STAGE_INFLIGHT, + TX_STAGE_NONCE, MAX_GAS_LIMIT, MAX_FEE_PER_GAS, }; diff --git a/tests/approvalVerify.test.js b/tests/approvalVerify.test.js index 99fc106..1763b3d 100644 --- a/tests/approvalVerify.test.js +++ b/tests/approvalVerify.test.js @@ -14,8 +14,10 @@ const { assertWithinCeilings, sameAddress, failureIsRetryable, + isNonceCollision, describeTxFailure, describeSigningFailure, + NONCE_COLLISION_MESSAGE, ALLOWED_TX_TYPES, SERIALIZED_FIELDS, FORBIDDEN_FIELDS, @@ -23,6 +25,7 @@ const { TX_STAGE_SIGN, TX_STAGE_VERIFY, TX_STAGE_BROADCAST, + TX_STAGE_NONCE, MAX_GAS_LIMIT, MAX_FEE_PER_GAS, } = require("../src/shared/approvalVerify"); @@ -1191,7 +1194,6 @@ describe("signing failure and retry", () => { "already known", "timeout of 30000ms exceeded", "could not coalesce error", - "replacement transaction underpriced", ]) { const outcome = describeTxFailure( TX_STAGE_BROADCAST, @@ -1199,10 +1201,76 @@ describe("signing failure and retry", () => { ); expect(outcome.retryable).toBe(false); expect(outcome.spendApproval).toBe(true); + expect(outcome.stage).toBe(TX_STAGE_BROADCAST); expect(outcome.error).toBe(message); } }); + // The one broadcast failure that is not ambiguous. The node answered, and + // its answer was that the nonce was already spoken for, so this + // transaction is not in a mempool anywhere. + test("a nonce the node refused is classified however it was worded", () => { + for (const err of [ + new Error("nonce too low"), + new Error("replacement transaction underpriced"), + Object.assign(new Error("could not coalesce error"), { + code: "NONCE_EXPIRED", + }), + Object.assign(new Error("could not coalesce error"), { + code: "REPLACEMENT_UNDERPRICED", + }), + // The shape ethers hands up when it could not classify the node's + // error itself: the node's own words are nested underneath. + Object.assign(new Error("could not coalesce error"), { + info: { error: { code: -32000, message: "OldNonce" } }, + }), + ]) { + const outcome = describeTxFailure(TX_STAGE_BROADCAST, err); + expect( + describeSigningFailure( + outcome, + "The transaction could not be sent.", + ).message, + ).toMatch(/did not reach the network/); + expect(outcome.retryable).toBe(false); + expect(outcome.spendApproval).toBe(true); + expect(outcome.error).toBe(NONCE_COLLISION_MESSAGE); + expect(outcome.stage).toBe(TX_STAGE_NONCE); + expect(isNonceCollision(err)).toBe(true); + } + }); + + // A node that says it knows the transaction has it, so it did reach the + // network and the ambiguous wording is the correct one. + test("already known is not a nonce collision", () => { + const err = new Error("already known"); + const outcome = describeTxFailure(TX_STAGE_BROADCAST, err); + expect( + describeSigningFailure( + outcome, + "The transaction could not be sent.", + ).message, + ).toMatch(/may still have reached the network/); + expect(outcome.stage).toBe(TX_STAGE_BROADCAST); + expect(isNonceCollision(err)).toBe(false); + }); + + test("a nonce collision says the transaction did not reach the network", () => { + const outcome = describeTxFailure( + TX_STAGE_BROADCAST, + new Error("nonce too low"), + ); + const copy = describeSigningFailure( + outcome, + "The transaction could not be sent.", + ); + expect(copy.retryable).toBe(false); + expect(copy.message).toMatch(/did not reach the network/); + expect(copy.message).not.toMatch(/may still have reached the network/); + expect(copy.message).toMatch(/Please send it again from the site\.$/); + expect(copy.message).toMatch(/^[A-Z].*\.$/); + }); + test("a failed broadcast does not tell the user to send it again", () => { const outcome = describeSigningFailure( { diff --git a/tests/backgroundApproval.test.js b/tests/backgroundApproval.test.js index d326b81..b48f649 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -206,6 +206,10 @@ function loadBackground(options) { // approval id back out of the popup URL the background opened. function requestTx(txParams) { let rpcResult = null; + // The window this request opens, if it opens one. A request refused + // before an approval is raised opens none, and the window belonging to + // some other request must not be handed back as this one's. + const windowIndex = created.length; const sendResponse = jest.fn((r) => { rpcResult = r; }); @@ -219,7 +223,12 @@ function loadBackground(options) { sendResponse, ); return { - id: () => new URL(created[0].url).searchParams.get("approval"), + id: () => + created.length > windowIndex + ? new URL(created[windowIndex].url).searchParams.get( + "approval", + ) + : null, result: () => rpcResult, }; } @@ -444,6 +453,176 @@ describe("one approval, one broadcast", () => { }); }); +// Populating the transaction before the approval window opens is what makes +// the displayed object the verified object. It also fixes the nonce before the +// user has answered anything: two requests populated concurrently take the +// same nonce from a node that has seen neither of them broadcast, and the +// second can then never be sent, because the only way to give it a fresh nonce +// is to populate it again after the user has read the old one off the screen. +// So the second request is refused while the first is unanswered. +describe("one transaction approval at a time", () => { + test("a second eth_sendTransaction while one is pending is refused before it takes a nonce", async () => { + const getTransactionCount = jest.fn(async () => NONCE); + const bg = loadBackground({ provider: { getTransactionCount } }); + + const first = bg.requestTx(); + await settle(); + expect(first.id()).toBeTruthy(); + expect(getTransactionCount).toHaveBeenCalledTimes(1); + + const second = bg.requestTx(); + await settle(); + + expect(second.result()).toEqual({ + error: { + code: -32002, + message: expect.stringMatching( + /already waiting to be approved/, + ), + }, + }); + // Where the refusal happened matters as much as that it happened: no + // second window, and the node was never asked for a second nonce. + expect(bg.created).toHaveLength(1); + expect(getTransactionCount).toHaveBeenCalledTimes(1); + + // The refusal leaves the pending approval untouched, and it still + // sends. + bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" }); + bg.send( + { + type: "AUTISTMASK_TX_RESPONSE", + id: first.id(), + approved: true, + rawSignedTx: await signedAtNonce(NONCE), + }, + { url: bg.fromPopup.url }, + ); + await settle(); + expect(first.result()).toEqual({ result: "0xfeed" }); + }); + + test("an answered approval frees the next request", async () => { + const bg = loadBackground(); + const first = bg.requestTx(); + await settle(); + + // The user closes the approval window, which rejects it. + bg.closeWindow(1); + await settle(); + expect(first.result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + + const second = bg.requestTx(); + await settle(); + expect(second.id()).toBeTruthy(); + expect(bg.created).toHaveLength(2); + }); + + test("a signature request is not held up by a pending transaction", async () => { + const bg = loadBackground(); + bg.requestTx(); + await settle(); + + // A signature consumes no nonce, so it has nothing to collide with. + const signing = bg.requestSign(); + await settle(); + expect(signing.id()).toBeTruthy(); + expect(signing.result()).toBeNull(); + expect(bg.created).toHaveLength(2); + }); +}); + +// A nonce collision found before the transaction reaches the network is the +// one send failure the wallet can speak about with certainty. The user is told +// it did not go out and to send it again, rather than being warned it might +// already be on the chain — which would send them looking for a transaction +// that does not exist, and stop them retrying the one that never went. +describe("a nonce collision is reported as a transaction that did not go out", () => { + test("a broadcast the node refused for the nonce is not reported as possibly sent", async () => { + const bg = loadBackground(); + const pending = bg.requestTx(); + await settle(); + + bg.broadcastTransaction.mockRejectedValue( + Object.assign(new Error("nonce too low"), { + code: "NONCE_EXPIRED", + }), + ); + const answer = bg.send( + { + type: "AUTISTMASK_TX_RESPONSE", + id: pending.id(), + approved: true, + rawSignedTx: await signedAtNonce(NONCE), + }, + { url: bg.fromPopup.url }, + ); + await settle(); + + expect(answer.sendResponse).toHaveBeenCalledWith({ + error: expect.stringMatching(/nonce had already been used/), + retryable: false, + stage: "nonce", + }); + expect(pending.result()).toEqual({ + error: { + message: expect.stringMatching( + /transaction was not sent, because its nonce/, + ), + }, + }); + }); + + test("a nonce this wallet already broadcast is refused without asking the node again", async () => { + const bg = loadBackground(); + const first = bg.requestTx(); + await settle(); + + bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" }); + bg.send( + { + type: "AUTISTMASK_TX_RESPONSE", + id: first.id(), + approved: true, + rawSignedTx: await signedAtNonce(NONCE), + }, + { url: bg.fromPopup.url }, + ); + await settle(); + expect(first.result()).toEqual({ result: "0xfeed" }); + + // The stubbed node still reports NONCE as the next nonce — a pending + // count that lags a broadcast the node has already taken — so this + // second approval is populated at a nonce this worker has spent. + const second = bg.requestTx(); + await settle(); + const answer = bg.send( + { + type: "AUTISTMASK_TX_RESPONSE", + id: second.id(), + approved: true, + rawSignedTx: await signedAtNonce(NONCE), + }, + { url: bg.fromPopup.url }, + ); + await settle(); + + expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1); + expect(answer.sendResponse).toHaveBeenCalledWith({ + error: expect.stringMatching(/nonce had already been used/), + retryable: false, + stage: "nonce", + }); + expect(second.result()).toEqual({ + error: { + message: expect.stringMatching(/nonce had already been used/), + }, + }); + }); +}); + // The approval carries the transaction the user was shown and the address it // was raised for, and the artifact is checked against both. Every case here is // one the old comparison — against the dApp's request, for the address that is