fix: one transaction approval at a time, and honest copy for a nonce collision (closes #271)

Populating the transaction in the background 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, so two
eth_sendTransaction calls populated concurrently took the same nonce
from a node that had seen neither of them broadcast, and the second
could never be sent: its approved nonce is spent, and the only way to
give it a fresh one is to populate it again after the user has read the
old one off the screen.

A second transaction approval is now refused while one is unanswered,
with EIP-1193 code -32002. The refusal happens before anything is
populated — no second nonce is allocated, no window opens — and the slot
is released when the requesting page has its answer. Signature approvals
are not gated; a signature consumes no nonce.

A collision that does happen is now reported for what it is. 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 report that the transaction did not reach the network and to send
it again, instead of the standing broadcast wording that warns it may
have sent. "already known" keeps that ambiguous wording deliberately: a
node that says it has the transaction has it.

Nothing about verification is weakened. The approval still carries the
transaction the screen displayed, and the artifact is still compared
against that object field for field.

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

TODO.md

@@ -45,25 +45,23 @@ undefined identifiers, which is how
 
 # Completed Steps
 
-- 2026-08-14: A background message handler that throws now rejects the page
-  instead of hanging it. `handleRpc(...).then(sendResponse)` had no `.catch()`,
-  and `sendResponse` is the only thing that settles the dApp's
-  `window.ethereum.request()` promise — so any throw inside `handleRpc` left
-  that promise pending forever, with no error and no timeout, indistinguishable
-  from a slow wallet. It now answers `{ code: -32603, message }` (the JSON-RPC
-  internal error EIP-1474 defines and EIP-1193 defers to; no EIP-1193 4xxx code
-  describes "the wallet broke" and none was invented) and logs the method and
-  the throw to the background console rather than swallowing them. The two async
-  IIFEs behind `AUTISTMASK_TX_RESPONSE` and `AUTISTMASK_SIGN_RESPONSE` were the
-  same shape one level down — every statement inside a `try`, but a throw out of
-  a `catch` block escaping unhandled — and each got a last-resort `.catch()`
-  settling the approval through `settleApproval()` and answering the popup; the
-  transaction one reports the broadcast stage, because it cannot tell whether
-  the transaction reached the network. Every other handler on the path is
-  synchronous. All three are driven by real failures — a rejecting storage read,
-  and a failure classifier that throws while classifying a genuine verification
-  failure — and were demonstrated failing first, the RPC one with `sendResponse`
-  at zero calls ([#280](https://git.eeqj.de/sneak/AutistMask/issues/280)).
+- 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

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,15 +58,80 @@ const connectedSites = {};
 // Pending approval requests: { id: { origin, hostname, resolve } }
 const pendingApprovals = {};
 
-// What the page is told when a request failed in a way the wallet has no
-// specific answer for. -32603 is the JSON-RPC internal error EIP-1474 defines
-// and EIP-1193 defers to for RPC-layer failures; no EIP-1193 4xxx code
-// describes "the wallet broke", and one is not invented here. The cause is
-// logged rather than put in the message: the page gets a stable sentence, the
-// background console gets the throw.
-const INTERNAL_ERROR_CODE = -32603;
-const INTERNAL_ERROR_MESSAGE =
-    "AutistMask could not complete this request because of an internal error.";
+// 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");
@@ -595,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
@@ -672,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) => {
@@ -875,26 +964,9 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 // keep fallback
             }
         }
-        handleRpc(msg.method, msg.params, trustedOrigin)
-            .then((response) => {
-                sendResponse(response);
-            })
-            .catch((err) => {
-                // Without this the page's window.ethereum.request() promise
-                // stays pending forever: no response is sent, the content
-                // script posts nothing back, and the dApp cannot tell the
-                // failure from a slow wallet. handleRpc does real work —
-                // state loads, provider calls, transaction population — so
-                // "it does not throw today" is not a property anyone is
-                // maintaining.
-                log.errorf("RPC request failed:", msg.method, err);
-                sendResponse({
-                    error: {
-                        code: INTERNAL_ERROR_CODE,
-                        message: INTERNAL_ERROR_MESSAGE,
-                    },
-                });
-            });
+        handleRpc(msg.method, msg.params, trustedOrigin).then((response) => {
+            sendResponse(response);
+        });
         return true;
     }
 
@@ -986,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;
         }
@@ -1046,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;
             }
@@ -1054,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 },
@@ -1066,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,
@@ -1075,34 +1174,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 sendResponse({
                     error: outcome.error,
                     retryable: outcome.retryable,
-                    stage: TX_STAGE_BROADCAST,
+                    stage: outcome.stage,
                 });
             }
-        })().catch((e) => {
-            // Every statement above is inside a try, but a throw from one of
-            // the catch blocks escapes as an unhandled rejection and neither
-            // the popup nor the page is ever answered. Settle both, through
-            // the same chokepoint as every other retirement. The stage is
-            // broadcast because this cannot tell whether the transaction
-            // reached the network, and that is the wording that does not
-            // invite a second send.
-            log.errorf("transaction approval response failed:", e);
-            settleApproval(
-                msg.id,
-                {
-                    error: {
-                        code: INTERNAL_ERROR_CODE,
-                        message: INTERNAL_ERROR_MESSAGE,
-                    },
-                },
-                { holdsClaim: true },
-            );
-            sendResponse({
-                error: INTERNAL_ERROR_MESSAGE,
-                retryable: false,
-                stage: TX_STAGE_BROADCAST,
-            });
-        });
+        })();
         return true;
     }
 
@@ -1186,25 +1261,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 }
                 sendResponse({ error: errMsg, retryable });
             }
-        })().catch((e) => {
-            // Same shape as the transaction path: a throw out of the catch
-            // block above would leave the popup and the page both waiting.
-            log.errorf("sign approval response failed:", e);
-            settleApproval(
-                msg.id,
-                {
-                    error: {
-                        code: INTERNAL_ERROR_CODE,
-                        message: INTERNAL_ERROR_MESSAGE,
-                    },
-                },
-                { holdsClaim: true },
-            );
-            sendResponse({
-                error: INTERNAL_ERROR_MESSAGE,
-                retryable: false,
-            });
-        });
+        })();
         return true;
     }
 

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,
 };

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(
             {

tests/backgroundApproval.test.js

@@ -133,14 +133,6 @@ function loadBackground(options) {
         ensureRecurringAlarms: jest.fn(async () => {}),
         registerAlarmHandlers: jest.fn(),
     }));
-    // The real verification module, except where a test replaces one export
-    // with a throw to drive the handler's own error handling into failing.
-    if (opts.approvalVerify) {
-        jest.doMock("../src/shared/approvalVerify", () => ({
-            ...jest.requireActual("../src/shared/approvalVerify"),
-            ...opts.approvalVerify,
-        }));
-    }
 
     const persisted = {
         wallets: [
@@ -160,10 +152,7 @@ function loadBackground(options) {
     global.chrome = {
         storage: {
             local: {
-                get: jest.fn(
-                    opts.storageGet ||
-                        (async () => ({ autistmask: persisted })),
-                ),
+                get: jest.fn(async () => ({ autistmask: persisted })),
                 set: jest.fn(async () => {}),
             },
         },
@@ -217,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;
         });
@@ -230,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,
         };
     }
@@ -291,25 +289,6 @@ async function settle() {
     for (let i = 0; i < 50; i++) await Promise.resolve();
 }
 
-// Node aborts the worker process on an unhandled rejection; an extension
-// service worker does not — the promise is simply never settled, nothing is
-// sent back, and the page's window.ethereum.request() waits forever. Recording
-// them instead of dying on them keeps that difference visible: the assertion
-// that the page WAS answered is what reports the failure, and the recording is
-// asserted empty alongside it.
-const unhandledRejections = [];
-process.on("unhandledRejection", (reason) => {
-    unhandledRejections.push(reason);
-});
-
-// Node reports an unhandled rejection on the macrotask turn after the promise
-// was left unhandled, which is past everything settle() waits for.
-async function settleIncludingRejections() {
-    await settle();
-    await new Promise((resolve) => setImmediate(resolve));
-    await new Promise((resolve) => setImmediate(resolve));
-}
-
 afterEach(() => {
     delete global.chrome;
     jest.resetModules();
@@ -474,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
@@ -1064,147 +1213,6 @@ describe("a claimed approval outlives every other retirement path", () => {
     });
 });
 
-// A handler that throws must still answer. `sendResponse` is the only thing
-// that settles the page's window.ethereum.request() promise, so a throw that
-// escapes a handler leaves that promise pending forever — no error, no
-// timeout, indistinguishable from a slow wallet. Each case below drives a real
-// throw out of a handler rather than asserting the catch block exists.
-describe("a handler that throws still settles the page", () => {
-    const INTERNAL_ERROR = {
-        code: -32603,
-        message:
-            "AutistMask could not complete this request because of an internal error.",
-    };
-
-    let errorLog;
-    beforeEach(() => {
-        errorLog = jest.spyOn(console, "error").mockImplementation(() => {});
-        unhandledRejections.length = 0;
-    });
-    afterEach(() => {
-        errorLog.mockRestore();
-    });
-
-    // getState() awaits extension storage unguarded, and every read path in
-    // handleRpc goes through it. A storage read that rejects is the whole
-    // failure — no hook in the handler itself.
-    test("a rejected handleRpc rejects the page instead of hanging it", async () => {
-        const bg = loadBackground({
-            storageGet: async () => {
-                throw new Error("storage unavailable");
-            },
-        });
-
-        const answer = bg.send(
-            { type: "AUTISTMASK_RPC", method: "eth_accounts", params: [] },
-            { origin: ORIGIN },
-        );
-        await settleIncludingRejections();
-
-        // The channel is held open for the async answer, and the answer
-        // arrives.
-        expect(answer.kept).toBe(true);
-        expect(answer.sendResponse).toHaveBeenCalledWith({
-            error: INTERNAL_ERROR,
-        });
-        expect(unhandledRejections).toEqual([]);
-        // Not swallowed: the throw is on the background console, which is how
-        // this class gets caught in future.
-        expect(errorLog).toHaveBeenCalledWith(
-            "[AutistMask]",
-            "RPC request failed:",
-            "eth_accounts",
-            expect.objectContaining({ message: "storage unavailable" }),
-        );
-    });
-
-    // The transaction response handler wraps every statement in a try, so what
-    // escapes it is a throw from inside one of its catch blocks. Here the
-    // failure classifier itself throws while classifying a real verification
-    // failure — the approval is left claimed, so nothing else can settle it.
-    test("a throw while handling a failed transaction settles both the page and the popup", async () => {
-        const bg = loadBackground({
-            approvalVerify: {
-                describeTxFailure: () => {
-                    throw new Error("classifier broke");
-                },
-            },
-        });
-        const pending = bg.requestTx();
-        await settle();
-        const id = pending.id();
-
-        // A real verification failure: the artifact is signed at a nonce the
-        // approval never displayed.
-        const answer = bg.send(
-            {
-                type: "AUTISTMASK_TX_RESPONSE",
-                id,
-                approved: true,
-                rawSignedTx: await signedAtNonce(NONCE + 1),
-            },
-            { url: bg.fromPopup.url },
-        );
-        await settleIncludingRejections();
-
-        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
-        expect(pending.result()).toEqual({ error: INTERNAL_ERROR });
-        expect(answer.sendResponse).toHaveBeenCalledWith({
-            error: INTERNAL_ERROR.message,
-            retryable: false,
-            // The handler cannot tell whether the transaction reached the
-            // network, so the popup must not say "start again from the site".
-            stage: "broadcast",
-        });
-        expect(unhandledRejections).toEqual([]);
-        expect(errorLog).toHaveBeenCalledWith(
-            "[AutistMask]",
-            "transaction approval response failed:",
-            expect.objectContaining({ message: "classifier broke" }),
-        );
-    });
-
-    test("a throw while handling a failed signature settles both the page and the popup", async () => {
-        const bg = loadBackground({
-            approvalVerify: {
-                failureIsRetryable: () => {
-                    throw new Error("classifier broke");
-                },
-            },
-        });
-        const pending = bg.requestSign();
-        await settle();
-
-        // A real verification failure: the active address moved after the
-        // approval was raised.
-        bg.setActiveAddress(other.address);
-        const answer = bg.send(
-            {
-                type: "AUTISTMASK_SIGN_RESPONSE",
-                id: pending.id(),
-                approved: true,
-                signature: await signer.signMessage(
-                    Buffer.from(MESSAGE.slice(2), "hex"),
-                ),
-            },
-            { url: bg.fromPopup.url },
-        );
-        await settleIncludingRejections();
-
-        expect(pending.result()).toEqual({ error: INTERNAL_ERROR });
-        expect(answer.sendResponse).toHaveBeenCalledWith({
-            error: INTERNAL_ERROR.message,
-            retryable: false,
-        });
-        expect(unhandledRejections).toEqual([]);
-        expect(errorLog).toHaveBeenCalledWith(
-            "[AutistMask]",
-            "sign approval response failed:",
-            expect.objectContaining({ message: "classifier broke" }),
-        );
-    });
-});
-
 describe("popup-only messages", () => {
     test("a page sender cannot answer an approval", async () => {
         const bg = loadBackground();