Skip to content

Conversation

@binarybaron
Copy link

@binarybaron binarybaron commented Dec 9, 2025

Previously Bob had to receive the transfer proof from Alice to transition from BtcLocked -> ... -> XmrLocked. This required interactivity meaning that if the peer-to-peer networking had issues, Bob might not detect that Alice locked the funds. This could cause unnecessary refunds of swaps that could have otherwise succeded.

This PR changes:

  • Bob now uses the view-key of the Monero wallet to verify that the Monero lock transaction transfers the correct amount. Previously we were using the transaction key which can only be computed by Alice.
  • Replace the wait_until_confirmed function which was based on wallet2 with an implementation that uses monero-oxide, skipping FFI completely.
  • Rename XmrLockProofReceived to XmrLockCandidateFound while remaining backwards compatible on the storage layer.
  • Split XmrLockProofReceived into two states (XmrLockCandidateFound and XmrLockTransactionSeen). XmrLockCandidateFound is entered either by receiving the transfer proof from Alice or when we detect an incoming transfer into the view-only wallet.
  • Before transitioning from XmrLockCandidateFound into XmrLockTransactionSeen we verify that that the transaction transfer the correct amount. We do this using a new verify_transfer_ng function that uses monero-oxide, skipping FFI completely. This does not rely at all on the tx_key. It only uses the view-key to decode the transaction.

We also extract some common behaviour in the state machines into external traits to make it more readable.


Note

Bob now detects and verifies the Monero lock non‑interactively via the view key and uses monero‑oxide for confirmations, with new intermediate states/UI, a new test, and a Rust toolchain bump.

  • Bob protocol/state machine:
    • Detect XMR lock via Monero view key (no transfer proof required) and verify amount using monero-oxide.
    • Replace XmrLockProofReceived with XmrLockTransactionCandidate and XmrLockTransactionSeen; keep storage compatibility.
    • Add helper traits for scanning/verification/confirmation; adjust cancel/refund paths to handle new states.
  • Monero integration:
    • Introduce monero-wallet-ng (confirmations, scanner, verify) and refactor monero-wallet to use it (remove wallet2/FFI wait_until_confirmed).
    • Add compat utilities and conversions; implement verify_transfer/wait_until_confirmed in Wallets via oxide.
  • Core types:
    • Add TransferProofMaybeWithTxKey; remove WatchRequest; add scalar compat; tweak key handling.
  • GUI:
    • Update Bob state enum/text and progress views (preflight/inflight enc-sig, XMR lock candidate/seen).
    • Improve history table loading UX and RPC swap info loading flag; minor copy tweaks.
  • Tests/CI/Tooling:
    • Add test: happy_path_alice_does_not_send_transfer_proof (Bob proceeds without transfer proof).
    • Bump Rust toolchain to 1.90 and update deps; add CHANGELOG notes.

Written by Cursor Bugbot for commit f626bbb. This will update automatically on new commits. Configure here.

@binarybaron binarybaron marked this pull request as draft December 9, 2025 11:53
@binarybaron
Copy link
Author

I wanted to replace the transfer proof validation logic with monero-oxide but that is sadly currently not possible (see monero-oxide/monero-oxide#145).

@binarybaron binarybaron changed the title feat(bob): scan multi-sig while concurrently waiting for transfer proof feat(bob): Scan for incoming transfers while concurrently waiting for transfer proof Dec 9, 2025
@binarybaron binarybaron force-pushed the xmr-scan-multi-sig-lock branch from 6d296bb to d15aa9c Compare December 10, 2025 15:14
@binarybaron binarybaron force-pushed the xmr-scan-multi-sig-lock branch from 1709cbe to cd48523 Compare December 12, 2025 14:30
@binarybaron
Copy link
Author

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Frontend missing XmrLockTransactionSeen in cancellable/refundable states

The new XmrLockTransactionSeen state is added to the BobStateName enum but is not included in the BobStateNamePossiblyCancellableSwap or BobStateNamePossiblyRefundableSwap types and their corresponding isBobStateNamePossiblyCancellableSwap and isBobStateNamePossiblyRefundableSwap functions. This causes the UI to incorrectly indicate that swaps in this state cannot be cancelled or refunded, even though the backend is intended to support these operations.

src-gui/src/models/tauriModelExt.ts#L162-L196

export type BobStateNamePossiblyCancellableSwap =
| BobStateName.BtcLocked
| BobStateName.XmrLockCandidateFound
| BobStateName.XmrLocked
| BobStateName.EncSigSent
| BobStateName.CancelTimelockExpired
| BobStateName.BtcRefundPublished
| BobStateName.BtcEarlyRefundPublished;
/**
Checks if a swap is in a state where it can possibly be cancelled
The following conditions must be met:
- The bitcoin must be locked
- The bitcoin must not be redeemed
- The bitcoin must not be cancelled
- The bitcoin must not be refunded
- The bitcoin must not be punished
- The bitcoin must not be early refunded
See: https://github.com/comit-network/xmr-btc-swap/blob/7023e75bb51ab26dff4c8fcccdc855d781ca4b15/swap/src/cli/cancel.rs#L16-L35
*/
export function isBobStateNamePossiblyCancellableSwap(
state: BobStateName,
): state is BobStateNamePossiblyCancellableSwap {
return [
BobStateName.BtcLocked,
BobStateName.XmrLockCandidateFound,
BobStateName.XmrLocked,
BobStateName.EncSigSent,
BobStateName.CancelTimelockExpired,
BobStateName.BtcRefundPublished,
BobStateName.BtcEarlyRefundPublished,
].includes(state);

src-gui/src/models/tauriModelExt.ts#L198-L232

export type BobStateNamePossiblyRefundableSwap =
| BobStateName.BtcLocked
| BobStateName.XmrLockCandidateFound
| BobStateName.XmrLocked
| BobStateName.EncSigSent
| BobStateName.CancelTimelockExpired
| BobStateName.BtcCancelled
| BobStateName.BtcRefundPublished
| BobStateName.BtcEarlyRefundPublished;
/**
Checks if a swap is in a state where it can possibly be refunded (meaning it's not impossible)
The following conditions must be met:
- The bitcoin must be locked
- The bitcoin must not be redeemed
- The bitcoin must not be refunded
- The bitcoin must not be punished
See: https://github.com/comit-network/xmr-btc-swap/blob/7023e75bb51ab26dff4c8fcccdc855d781ca4b15/swap/src/cli/refund.rs#L16-L34
*/
export function isBobStateNamePossiblyRefundableSwap(
state: BobStateName,
): state is BobStateNamePossiblyRefundableSwap {
return [
BobStateName.BtcLocked,
BobStateName.XmrLockCandidateFound,
BobStateName.XmrLocked,
BobStateName.EncSigSent,
BobStateName.CancelTimelockExpired,
BobStateName.BtcCancelled,
BobStateName.BtcRefundPublished,
BobStateName.BtcEarlyRefundPublished,
].includes(state);

Fix in Cursor Fix in Web


@binarybaron binarybaron mentioned this pull request Dec 12, 2025
12 tasks
@binarybaron binarybaron changed the title feat(bob): Scan for incoming transfers while concurrently waiting for transfer proof feat(bob): Non-interactive transfer proof Dec 13, 2025
@binarybaron binarybaron force-pushed the xmr-scan-multi-sig-lock branch from 3b846f0 to 715992a Compare December 14, 2025 20:07
@binarybaron
Copy link
Author

bugbot run

@binarybaron binarybaron marked this pull request as ready for review December 15, 2025 08:58
@binarybaron
Copy link
Author

@Einliterflasche This is ready for a final review. I think it is ready to be merged.

@binarybaron
Copy link
Author

  • remove check_tx_key entirely

@binarybaron binarybaron force-pushed the xmr-scan-multi-sig-lock branch from 5f33dfe to f626bbb Compare December 15, 2025 12:00
@binarybaron
Copy link
Author

Half broken alice locks wrong Monero amount integration test:

pub mod harness;

use harness::FastCancelConfig;
use swap::asb::FixedRate;
use swap::protocol::alice::AliceState;
use swap::protocol::bob::BobState;
use swap::protocol::{alice, bob};
use swap_core::monero::Amount;

use crate::harness::SlowCancelConfig;

#[tokio::test]
async fn given_alice_locks_wrong_xmr_amount_bob_rejects() {
    harness::setup_test(SlowCancelConfig, None, |mut ctx| async move {
        // Run Bob until he gives up on the swap
        let (bob_swap, bob_join_handle) = ctx.bob_swap().await;
        let bob_swap_id = bob_swap.id;
        let bob_swap = tokio::spawn(bob::run_until(bob_swap, |s| {
            matches!(s, BobState::WaitingForCancelTimelockExpiration { .. })
        }));

        // Run until Alice detects the Bitcoin as having been locked
        let alice_swap = ctx.alice_next_swap().await;

        let alice_state = alice::run_until(
            alice_swap,
            |s| matches!(s, AliceState::BtcLocked { .. }),
            FixedRate::default(),
        )
        .await?;

        // Resume Alice such that she locks the wrong amount of Monero
        ctx.restart_alice().await;
        let mut alice_swap = ctx.alice_next_swap().await;
        
        // Modify Alice such that she locks the wrong amount of Monero
        alice_swap.state = alice_state.lower_by_one_piconero();
        let alice_swap = tokio::spawn(alice::run(alice_swap, FixedRate::default()));

        // Run until Bob detects the wrong amount of Monero and gives up
        // He gives up by waiting for the cancel timelock to expire
        let bob_state = bob_swap.await??;
        assert!(matches!(bob_state, BobState::WaitingForCancelTimelockExpiration { .. }));

        // Resume Bob and wait run until completion
        let (bob_swap, _) = ctx
            .stop_and_resume_bob_from_db(bob_join_handle, bob_swap_id)
            .await;
        let bob_state = bob::run_until(bob_swap, |s| {
            matches!(s, BobState::BtcRefunded(..))
        })
        .await?;

        // Assert that both Bob and Alice refunded
        ctx.assert_bob_refunded(bob_state).await;
        let alice_state = alice_swap.await??;
        ctx.assert_alice_refunded(alice_state).await;

        Ok(())
    })
    .await;
}

trait FakeModifyMoneroAmount {
    /// Reduces the Monero amount by one piconero
    fn lower_by_one_piconero(self) -> Self;
}

impl FakeModifyMoneroAmount for AliceState {
    fn lower_by_one_piconero(self) -> Self {
        let AliceState::BtcLocked { mut state3 } = self else {
            panic!("Expected BtcLocked state to be able to modify the Monero amount");
        };

        let one_piconero = Amount::from_piconero(1);

        state3.xmr = state3.xmr.checked_sub(one_piconero).unwrap();

        Self::BtcLocked { state3 }
    }
}

@binarybaron binarybaron merged commit 15f0ef8 into master Dec 15, 2025
52 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants