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 key point for this PR: querying wallet2's own balance/scan is not enough to catch the over-commit before constructing, because it lags the relay. A daemon-level I also put up a smaller, prevention-side change in #1193 (a process-wide async mutex serializing the construct-to-first-relay window so overlapping swaps don't collide when funds are sufficient). It is complementary but has exactly the wallet2-lag limitation above under insufficient funds, so it is not a substitute for the key-image check here. Happy to share full logs or test against either approach. AI disclosure: I used Claude Fable 5 to help investigate and write this. I understand it and am responsible for it. |
3 participants
No description provided.