fix: two concurrent dApp transactions are populated with the same nonce, and the second fails terminally #271

New Issue

clawbot · 2026-08-12T12:04:57+02:00

clawbot commented

2026-08-12 12:04:57 +02:00

#269 moves transaction population into the background, before the approval window opens, so that the signed artifact can be verified against the object the user was actually shown. That is the right structure and it closed a real gap — but it moves when the nonce is chosen.

Previously the popup called populateTransaction() on Confirm, so a second transaction picked up a fresh nonce at the moment the user confirmed it. Now both are populated before either window opens. Two concurrent eth_sendTransaction calls — or one approval window left open while another transaction goes out — get the same nonce, and the second broadcast fails terminally with the "may still have reached the network" wording.

Found by the independent review of that PR. It fails CLOSED and is inherent to the option the issue recommended, so it was not blocked there. It is undocumented, and the wording the user sees is wrong for this case: the second transaction did not reach the network and never will, but the copy tells them it might have.

Implementation requirements

Decide deliberately how concurrent approvals should behave and say why in the PR body. The options are not equivalent: re-populate the nonce at Confirm (restores the old behaviour but reintroduces the gap #216 closed, since the verified nonce would no longer be the displayed one); allocate nonces accounting for other in-flight approvals; or refuse a second approval while one is pending, which is the simplest and arguably correct for a wallet.
Whatever is chosen, the failure copy must be accurate for this case. A nonce collision detected before broadcast has NOT reached the network, and the user should be told to retry rather than warned it may have sent.
Do not weaken the property that the verified object is the displayed object.

Definition of done

Two concurrent eth_sendTransaction requests behave as decided, not by accident of population order.
A transaction that failed on a nonce collision before broadcast reports copy that says so, not "may still have reached the network".
The verified object is still the displayed object for every field.
Tests cover two overlapping approvals and the copy each failure produces.
TODO.md updated in the same commit.
make check passes.

https://git.eeqj.de/sneak/AutistMask/pulls/269 moves transaction population into the background, before the approval window opens, so that the signed artifact can be verified against the object the user was actually shown. That is the right structure and it closed a real gap — but it moves when the nonce is chosen. Previously the popup called `populateTransaction()` on Confirm, so a second transaction picked up a fresh nonce at the moment the user confirmed it. Now both are populated before either window opens. Two concurrent `eth_sendTransaction` calls — or one approval window left open while another transaction goes out — get the **same nonce**, and the second broadcast fails terminally with the "may still have reached the network" wording. Found by the independent review of that PR. It fails CLOSED and is inherent to the option the issue recommended, so it was not blocked there. It is undocumented, and the wording the user sees is wrong for this case: the second transaction did not reach the network and never will, but the copy tells them it might have. ## Implementation requirements - Decide deliberately how concurrent approvals should behave and say why in the PR body. The options are not equivalent: re-populate the nonce at Confirm (restores the old behaviour but reintroduces the gap https://git.eeqj.de/sneak/AutistMask/issues/216 closed, since the verified nonce would no longer be the displayed one); allocate nonces accounting for other in-flight approvals; or refuse a second approval while one is pending, which is the simplest and arguably correct for a wallet. - Whatever is chosen, the failure copy must be accurate for this case. A nonce collision detected before broadcast has NOT reached the network, and the user should be told to retry rather than warned it may have sent. - Do not weaken the property that the verified object is the displayed object. ## Definition of done - [ ] Two concurrent `eth_sendTransaction` requests behave as decided, not by accident of population order. - [ ] A transaction that failed on a nonce collision before broadcast reports copy that says so, not "may still have reached the network". - [ ] The verified object is still the displayed object for every field. - [ ] Tests cover two overlapping approvals and the copy each failure produces. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot added this to the 1.0.0 milestone 2026-08-12 12:05:00 +02:00

clawbot referenced this issue from a commit

2026-08-12 12:15:27 +02:00

harden: verify the signed transaction against what the popup displayed (closes #216)

clawbot commented

2026-08-14 06:03:38 +02:00

Plan: option three — refuse a second transaction approval while one is pending.

Why not the other two. Re-populating at Confirm reintroduces exactly the gap #216 closed: the verified nonce would no longer be the displayed one. Allocating around in-flight approvals makes the wallet's own bookkeeping the authority on a nonce the network has not accepted, and an approval the user abandons then leaves a hole that stalls every later transaction until the worker restarts. Refusing keeps the displayed object the verified object, holds no state the network can contradict, and fails the second request while the page is still waiting and nothing has been shown.

Mechanism:

A single transaction-approval reservation, taken synchronously in the eth_sendTransaction branch before its first await and released when the request is answered (approved and broadcast, rejected, or window closed). The second request is refused with EIP-1193 code -32002 and never reaches prepareApprovalTx(), so no second nonce is allocated. Sign approvals are unaffected — they consume no nonce.
Copy: a broadcast the node refused for a nonce collision (nonce too low, replacement transaction underpriced, ethers NONCE_EXPIRED / REPLACEMENT_UNDERPRICED) gets its own stage and its own sentences — it did not reach the network, send it again. already known deliberately keeps the existing wording: a node saying it has the transaction means it did reach the network.
A true pre-broadcast case as well: the background records the nonces it has broadcast per address for the life of the worker and refuses an approval carrying one of them without contacting the node, with the same copy.

Proof: tests over two overlapping eth_sendTransaction calls (asserting the node is asked for a nonce once, one window opens, the first still settles, the slot frees afterwards), over the local pre-broadcast refusal (asserting broadcastTransaction is never called for the second), and over the copy each failure produces including the already known distinction. Each new test demonstrated failing against the unfixed code first, with that output in the PR body.

Plan: option three — refuse a second transaction approval while one is pending. Why not the other two. Re-populating at Confirm reintroduces exactly the gap [#216](https://git.eeqj.de/sneak/AutistMask/issues/216) closed: the verified nonce would no longer be the displayed one. Allocating around in-flight approvals makes the wallet's own bookkeeping the authority on a nonce the network has not accepted, and an approval the user abandons then leaves a hole that stalls every later transaction until the worker restarts. Refusing keeps the displayed object the verified object, holds no state the network can contradict, and fails the second request while the page is still waiting and nothing has been shown. Mechanism: - A single transaction-approval reservation, taken synchronously in the `eth_sendTransaction` branch before its first `await` and released when the request is answered (approved and broadcast, rejected, or window closed). The second request is refused with EIP-1193 code `-32002` and never reaches `prepareApprovalTx()`, so no second nonce is allocated. Sign approvals are unaffected — they consume no nonce. - Copy: a broadcast the node refused for a nonce collision (`nonce too low`, `replacement transaction underpriced`, ethers `NONCE_EXPIRED` / `REPLACEMENT_UNDERPRICED`) gets its own stage and its own sentences — it did not reach the network, send it again. `already known` deliberately keeps the existing wording: a node saying it has the transaction means it did reach the network. - A true pre-broadcast case as well: the background records the nonces it has broadcast per address for the life of the worker and refuses an approval carrying one of them without contacting the node, with the same copy. Proof: tests over two overlapping `eth_sendTransaction` calls (asserting the node is asked for a nonce once, one window opens, the first still settles, the slot frees afterwards), over the local pre-broadcast refusal (asserting `broadcastTransaction` is never called for the second), and over the copy each failure produces including the `already known` distinction. Each new test demonstrated failing against the unfixed code first, with that output in the PR body.

clawbot referenced this issue from a commit

2026-08-14 06:12:27 +02:00

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

clawbot referenced a pull request that will close this issue

2026-08-14 06:12:57 +02:00

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

clawbot referenced this issue

2026-08-14 06:25:10 +02:00

fix: answer the page when a background handler throws (closes #280) #282

Sign in to join this conversation.