feat(asb): rebuild xmr lock tx on confirmed double spend - #1142
feat(asb): rebuild xmr lock tx on confirmed double spend #1142binarybaron wants to merge 6 commits into
Conversation
binarybaron
changed the title
|
@Einliterflasche Should we really rebuild the TX here? We could also just go into SafelyAborted (or a similar end state, e.g |
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 473b299. Configure here.
|
|
||
| return Ok(AliceState::BtcLocked { state3: state3.clone() }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Pool presence skips double-spend check
High Severity
Confirmed-input double-spend recovery runs only inside !is_present, but is_transaction_present is true while the lock tx is still in the mempool. If inputs are already SpentInBlockchain via another tx, the swap still advances to XmrLockTransactionSent and can wait forever for confirmations that never arrive.
Reviewed by Cursor Bugbot for commit 473b299. Configure here.
…ng the lock tx Before Alice constructs her Monero lock transaction, scan the shared wallet (the view pair Bob and Alice both know) from a recorded restore height and abort the swap if it already received any output, on-chain or in the mempool. This guards against ever locking Monero twice for the same swap, complementing the double-spend rebuild path which re-enters BtcLocked and now re-runs this check with a fresh restore height. - swap-machine: new AliceState::XmrReadyToLock carries the restore height captured in BtcLocked so a resumed swap scans the full window in which an earlier lock could have landed. - monero-wallet-ng: new empty module scans blocks in batches of 10 plus the mempool with a naive Scanner over the shared view pair. - The check runs once per construction attempt and again before falling back to BtcEarlyRefundable after the lock retry timeout.
473b299 to
089114a
Compare
Deploying eigenwallet-docs with
|
| Latest commit: |
17d7164
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bd99cab0.eigenwallet-docs.pages.dev |
| Branch Preview URL: | https://trusted-daemon-double-spend.eigenwallet-docs.pages.dev |
|
@Einliterflasche Is this good to merge ? |
|
This is not good to merge. |
|
Sharing a real-world reproduction of this on a live mainnet ASB today, in case the failure mode is useful. Two takers opened swaps ~70s apart. The maker had 43 unreserved XMR; the swaps needed 9.65 and 38.07 XMR, so together (47.7) they over-committed. Both were accepted (the quote-time reservation is defeated here: quotes are cached by {min,max} only, and a swap reserves 0 XMR until it reaches BtcLockTransactionSeen, so two concurrent same-size requests both see the full balance). The small swap reached the Monero lock phase first, constructed and relayed its 9.65 lock. The big swap then constructed a 38.07 lock, and because wallet2's balance/spend state does not yet reflect the freshly relayed (unconfirmed) lock, The takeaway: wallet2's own balance/scan lags the relay, so it can't reliably catch the over-commit before constructing — a daemon-level Where it could go from here: #1193 takes the abort-not-rebuild route raised earlier in this thread. It serializes the construct->first-relay window (so concurrent same-balance swaps pick different outputs — no double-spend when funds suffice) and routes the genuinely-unfundable swap to an early Bitcoin refund instead of building a doomed lock (no Monero is locked, so nothing to rebuild). The AI disclosure: I used Claude Fable 5 to help investigate and write this. I understand it and am responsible for it. |
Two overlapping swaps could select the same Monero output when building
their lock transactions. wallet2 only marks an output spent once its
lock tx is relayed, and monero-sys has no reserve API, so the second
swap's lock tx becomes a permanent double-spend that monerod rejects
forever -- the swap then hangs until the cancel timelock and the taker
is refunded ("xmr is refundable").
A related case: when concurrent swaps together need more XMR than the
maker holds, even a serialized swap can be forced to reselect a sibling's
freshly spent outputs, build a double-spend, and then retry the rejected
publish forever, since the publish has no deadline.
Serialize the construct-through-first-publish window with a process-wide
async mutex so at most one unpublished lock tx exists at a time;
overlapping swaps that are each fundable pick different outputs and both
succeed. A max-hold deadline releases the mutex if a swap wedges on a
rejected publish. Under the guard, before constructing, check that the
unlocked balance covers the lock and fail with a permanent error (an
early Bitcoin refund; no Monero was locked, so it is safe) if a sibling
already took the shared balance, instead of building a doomed lock.
Two mainnet wedges were then followed end to end on a live maker running
this change, and they split the failure into two distinct signatures:
Swap 5eabdea8 (2026-08-26) -- insufficient balance, missed. Wallet
13.156 XMR against a 13.963 XMR lock; the pre-check still PASSED (it
never logged its refusal), the publish was rejected with an empty-reason
TransactionRejected, and the swap retried that same transaction until
the cancel timelock, ~4.2h. The balance recovering mid-wedge (20.29 XMR
unreserved for 33 minutes) changed nothing.
Swap 0abd1dcd (2026-08-27) -- ample balance, unreachable by any balance
check. Wallet 41.3 XMR unlocked against a 6.85 XMR lock; the pre-check
was RIGHT to pass, but the constructed tx had selected outputs freshly
spent by an earlier swap that wallet2 had not yet reflected. monerod
rejected that one fixed transaction 799 times over 3.9h until the
cancel timelock. Meanwhile the max-hold deadline released the mutex and
sibling swap 4f885739 constructed with fresh outputs and completed
end-to-end DURING the wedge -- the serialization half doing exactly its
job. Neither wedge ever locked Monero; both takers were refunded.
What this pins down: the construct-time gate must operate on OUTPUTS,
not balance -- the daemon-side is_key_image_spent query, as the primary
check. And the two end-states want different exits: insufficient funds
routes to the early Bitcoin refund this patch adds, while
sufficient-funds-stale-selection would have been SAVED by rebuilding the
lock tx on rejection -- the direction eigenwallet#1142 explores, complementary to
this patch rather than superseded by it.
Verified with the concurrent_bobs_before_xmr_lock_proof_sent,
concurrent_bobs_after_xmr_lock_proof_sent, a new
concurrent_bobs_insufficient_xmr, and happy_path integration tests.
Refs eigenwallet#120
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 participants
No description provided.