fix(swap): Split approve-and-sign and publish-lock-tx into two states (#498)

* fix(swap): Split approve-and-sign and publish-lock-tx into two states

* fix: cannot get blockchain height

* add RetrievingMoneroBlockheight, RetrievingMoneroBlockheight tauri events

* propagate daemon blcok height fetch error, treat height 0 as error

* check if tx_lock was previously published

CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- GUI + CLI: Fixed a potential race condition where if the user closed the app while the Bitcoin was in the process of being published, manual recovery would be required to get to a recoverable state.
+
 ## [3.0.0-beta.4] - 2025-08-03
 
 - GUI: The following rendezvous points have been added to the default list of rendezvous points. If you're running a maker, please add them to your config file (under `network.rendezvous_point`). They are operated by members of the community that have shown to be reliable. These rendezvous points act as bootstrapping nodes for peer discovery: `/dns4/eigen.center/tcp/8888/p2p/12D3KooWS5RaYJt4ANKMH4zczGVhNcw5W214e2DDYXnjs5Mx5zAT`, `/dns4/swapanarchy.cfd/tcp/8888/p2p/12D3KooWRtyVpmyvwzPYXuWyakFbRKhyXGrjhq6tP7RrBofpgQGp`, `/dns4/darkness.su/tcp/8888/p2p/12D3KooWFQAgVVS9t9UgL6v1sLprJVM7am5hFK7vy9iBCCoCBYmU`

justfile

@@ -53,6 +53,12 @@ gui_build:
 tests:
         cargo nextest run
 
+docker_test_happy_path:
+	cargo test --package swap --test happy_path -- --nocapture
+
+docker_test_all:
+	cargo test --package swap --test all -- --nocapture
+
 # Tests the Rust bindings for Monero
 test_monero_sys:
         cd monero-sys && cargo nextest run

monero-sys/src/bridge.rs

@@ -202,9 +202,12 @@ pub mod ffi {
         /// Set a listener to the wallet.
         unsafe fn setListener(self: Pin<&mut Wallet>, listener: *mut WalletListener) -> Result<()>;
 
-        /// Get the daemon's blockchain height.
+        /// Get the daemon's blockchain target height.
         fn daemonBlockChainTargetHeight(self: &Wallet) -> Result<u64>;
 
+        /// Get the daemon's blockchain height.
+        fn daemonBlockChainHeight(self: &Wallet) -> Result<u64>;
+
         /// Check if wallet was ever synchronized.
         fn synchronized(self: &Wallet) -> Result<bool>;
 

monero-sys/src/lib.rs

@@ -398,21 +398,28 @@ impl WalletHandle {
     /// Retries at most 5 times with a 500ms delay between attempts.
     pub async fn blockchain_height(&self) -> anyhow::Result<u64> {
         const MAX_RETRIES: u64 = 5;
+        const RETRY_DELAY: u64 = 500;
+
+        let mut last_error = None;
 
         for _ in 0..MAX_RETRIES {
-            if let Some(height) = self
+            match self
                 .call(move |wallet| wallet.daemon_blockchain_height())
                 .await
             {
-                return Ok(height);
+                Ok(0) => last_error = Some(anyhow!("Daemon blockchain height is 0")),
+                Err(e) => last_error = Some(e),
+                Ok(height) => return Ok(height),
             }
 
-            tokio::time::sleep(std::time::Duration::from_millis(500)).await;
+            tracing::warn!(error=%last_error.as_ref().unwrap_or(&anyhow!("Unknown error")), "Failed to get blockchain height, retrying in {}ms", RETRY_DELAY);
+
+            tokio::time::sleep(std::time::Duration::from_millis(RETRY_DELAY)).await;
         }
 
         self.check_wallet().await?;
 
-        bail!("Failed to get blockchain height after 5 attempts");
+        bail!("Failed to get blockchain height after 5 attempts: {last_error:?}");
     }
 
     /// Transfer funds to an address without approval.
@@ -1705,20 +1712,23 @@ impl FfiWallet {
     ///
     /// Returns the height of the blockchain, if connected.
     /// Returns None if not connected.
-    fn daemon_blockchain_height(&self) -> Option<u64> {
+    fn daemon_blockchain_height(&self) -> anyhow::Result<u64> {
         // Here we actually use the _target_ height -- incase the remote node is
         // currently catching up we want to work with the height it ends up at.
-        match self
-            .inner
-            .daemonBlockChainTargetHeight()
-            .context(
+        let target_height = self.inner.daemonBlockChainTargetHeight().context(
             "Failed to get daemon blockchain target height: FFI call failed with exception",
-            )
-            .expect("Shouldn't panic")
-        {
-            0 => None,
-            height => Some(height),
-        }
+        )?;
+
+        let height = self
+            .inner
+            .daemonBlockChainHeight()
+            .context("Failed to get daemon blockchain height: FFI call failed with exception")?;
+
+        // wallet2 is such a crazy construction that sometimes the target_height is less than the current height
+        // we therefore take the max of the two
+        let max_height = std::cmp::max(height, target_height);
+
+        Ok(max_height)
     }
 
     /// Get the total balance across all accounts.

src-gui/src/models/tauriModelExt.ts

@@ -25,6 +25,7 @@ export type TauriSwapProgressEventExt<T extends TauriSwapProgressEventType> =
 export enum BobStateName {
   Started = "quote has been requested",
   SwapSetupCompleted = "execution setup done",
+  BtcLockReadyToPublish = "btc lock ready to publish",
   BtcLocked = "btc is locked",
   XmrLockProofReceived = "XMR lock transaction transfer proof received",
   XmrLocked = "xmr is locked",
@@ -47,6 +48,8 @@ export function bobStateNameToHumanReadable(stateName: BobStateName): string {
       return "Started";
     case BobStateName.SwapSetupCompleted:
       return "Setup completed";
+    case BobStateName.BtcLockReadyToPublish:
+      return "Bitcoin lock ready to publish";
     case BobStateName.BtcLocked:
       return "Bitcoin locked";
     case BobStateName.XmrLockProofReceived:

src-gui/src/renderer/components/alert/SwapStatusAlert/SwapStatusAlert.tsx

@@ -216,6 +216,10 @@ export function StateAlert({
       }
       return <PunishTimelockExpiredAlert />;
 
+    // If the Bitcoin lock transaction has not been published yet
+    // there is no need to display an alert
+    case BobStateName.BtcLockReadyToPublish:
+      return null;
     default:
       exhaustiveGuard(swap.state_name);
   }

src-gui/src/renderer/components/pages/swap/swap/SwapStatePage.tsx

@@ -50,6 +50,14 @@ export default function SwapStatePage({ state }: { state: SwapState | null }) {
         return <SwapSetupInflightPage {...state.curr.content} />;
       }
       break;
+    case "RetrievingMoneroBlockheight":
+      return (
+        <CircularProgressWithSubtitle description="Retrieving Monero blockheight..." />
+      );
+    case "BtcLockPublishInflight":
+      return (
+        <CircularProgressWithSubtitle description="Publishing Bitcoin lock transaction..." />
+      );
     case "BtcLockTxInMempool":
       if (state.curr.type === "BtcLockTxInMempool") {
         return <BitcoinLockTxInMempoolPage {...state.curr.content} />;

swap/src/cli/api/tauri_bindings.rs

@@ -603,7 +603,8 @@ impl TauriEmitter for Option<TauriHandle> {
     ) -> Result<bool> {
         match self {
             Some(tauri) => tauri.request_bitcoin_approval(details, timeout_secs).await,
-            None => bail!("No Tauri handle available"),
+            // If no TauriHandle is available, we just approve the request
+            None => Ok(true),
         }
     }
 
@@ -886,10 +887,9 @@ pub enum TauriSwapProgressEvent {
         #[typeshare(serialized_as = "number")]
         #[serde(with = "::bitcoin::amount::serde::as_sat")]
         btc_lock_amount: bitcoin::Amount,
-        #[typeshare(serialized_as = "number")]
-        #[serde(with = "::bitcoin::amount::serde::as_sat")]
-        btc_tx_lock_fee: bitcoin::Amount,
     },
+    RetrievingMoneroBlockheight,
+    BtcLockPublishInflight,
     BtcLockTxInMempool {
         #[typeshare(serialized_as = "string")]
         btc_lock_txid: bitcoin::Txid,

swap/src/cli/cancel_and_refund.rs

@@ -42,6 +42,14 @@ pub async fn cancel(
             // We do not know the block height, so we set it to 0
             state3.cancel(BlockHeight { height: 0 })
         }
+        // Refunding in this state is not possible but we still allow it
+        // because this function is only used for recovery purposes.
+        // We can try to do a refund here so we do.
+        BobState::BtcLockReadyToPublish {
+            state3,
+            monero_wallet_restore_blockheight,
+            ..
+        } => state3.cancel(monero_wallet_restore_blockheight),
         BobState::BtcLocked {
             state3,
             monero_wallet_restore_blockheight,
@@ -144,6 +152,11 @@ pub async fn refund(
             state3,
             monero_wallet_restore_blockheight,
         } => state3.cancel(monero_wallet_restore_blockheight),
+        BobState::BtcLockReadyToPublish {
+            state3,
+            monero_wallet_restore_blockheight,
+            ..
+        } => state3.cancel(monero_wallet_restore_blockheight),
         BobState::XmrLockProofReceived {
             state,
             monero_wallet_restore_blockheight,

swap/src/database/bob.rs

@@ -17,6 +17,11 @@ pub enum Bob {
     ExecutionSetupDone {
         state2: bob::State2,
     },
+    BtcLockReadyToPublish {
+        btc_lock_tx_signed: bitcoin::Transaction,
+        state3: bob::State3,
+        monero_wallet_restore_blockheight: BlockHeight,
+    },
     BtcLocked {
         state3: bob::State3,
         monero_wallet_restore_blockheight: BlockHeight,
@@ -65,6 +70,15 @@ impl From<BobState> for Bob {
                 tx_lock_fee,
             },
             BobState::SwapSetupCompleted(state2) => Bob::ExecutionSetupDone { state2 },
+            BobState::BtcLockReadyToPublish {
+                btc_lock_tx_signed,
+                state3,
+                monero_wallet_restore_blockheight,
+            } => Bob::BtcLockReadyToPublish {
+                btc_lock_tx_signed,
+                state3,
+                monero_wallet_restore_blockheight,
+            },
             BobState::BtcLocked {
                 state3,
                 monero_wallet_restore_blockheight,
@@ -114,6 +128,15 @@ impl From<Bob> for BobState {
                 tx_lock_fee,
             },
             Bob::ExecutionSetupDone { state2 } => BobState::SwapSetupCompleted(state2),
+            Bob::BtcLockReadyToPublish {
+                btc_lock_tx_signed,
+                state3,
+                monero_wallet_restore_blockheight,
+            } => BobState::BtcLockReadyToPublish {
+                btc_lock_tx_signed,
+                state3,
+                monero_wallet_restore_blockheight,
+            },
             Bob::BtcLocked {
                 state3,
                 monero_wallet_restore_blockheight,
@@ -153,6 +176,7 @@ impl fmt::Display for Bob {
         match self {
             Bob::Started { .. } => write!(f, "Started"),
             Bob::ExecutionSetupDone { .. } => f.write_str("Execution setup done"),
+            Bob::BtcLockReadyToPublish { .. } => f.write_str("Bitcoin lock ready to publish"),
             Bob::BtcLocked { .. } => f.write_str("Bitcoin locked"),
             Bob::XmrLockProofReceived { .. } => {
                 f.write_str("XMR lock transaction transfer proof received")

swap/src/protocol/bob/state.rs

@@ -32,6 +32,11 @@ pub enum BobState {
         change_address: bitcoin::Address,
     },
     SwapSetupCompleted(State2),
+    BtcLockReadyToPublish {
+        btc_lock_tx_signed: Transaction,
+        state3: State3,
+        monero_wallet_restore_blockheight: BlockHeight,
+    },
     BtcLocked {
         state3: State3,
         monero_wallet_restore_blockheight: BlockHeight,
@@ -65,6 +70,9 @@ impl fmt::Display for BobState {
         match self {
             BobState::Started { .. } => write!(f, "quote has been requested"),
             BobState::SwapSetupCompleted(..) => write!(f, "execution setup done"),
+            BobState::BtcLockReadyToPublish { .. } => {
+                write!(f, "btc lock ready to publish")
+            }
             BobState::BtcLocked { .. } => write!(f, "btc is locked"),
             BobState::XmrLockProofReceived { .. } => {
                 write!(f, "XMR lock transaction transfer proof received")
@@ -94,6 +102,7 @@ impl BobState {
     ) -> Result<Option<ExpiredTimelocks>> {
         Ok(match self.clone() {
             BobState::Started { .. }
+            | BobState::BtcLockReadyToPublish { .. }
             | BobState::SafelyAborted
             | BobState::SwapSetupCompleted(_) => None,
             BobState::BtcLocked { state3: state, .. }

swap/src/protocol/bob/swap.rs

@@ -127,8 +127,6 @@ async fn next_state(
                 swap_id,
                 TauriSwapProgressEvent::SwapSetupInflight {
                     btc_lock_amount: btc_amount,
-                    // TODO: Replace this with the actual fee
-                    btc_tx_lock_fee: bitcoin::Amount::ZERO,
                 },
             );
 
@@ -182,7 +180,9 @@ async fn next_state(
 
             match approval_result {
                 Ok(true) => {
-                    tracing::debug!("User approved swap offer");
+                    tracing::debug!(
+                        "User approved swap offer, fetching current Monero blockheight"
+                    );
 
                     // Record the current monero wallet block height so we don't have to scan from
                     // block 0 once we create the redeem wallet.
@@ -193,15 +193,23 @@ async fn next_state(
                     // If the Monero transaction gets confirmed before Bob comes online again then
                     // Bob would record a wallet-height that is past the lock transaction height,
                     // which can lead to the wallet not detect the transaction.
+                    event_emitter.emit_swap_progress_event(
+                        swap_id,
+                        TauriSwapProgressEvent::RetrievingMoneroBlockheight,
+                    );
+
                     let monero_wallet_restore_blockheight = monero_wallet
                         .blockchain_height()
                         .await
                         .context("Failed to fetch current Monero blockheight")?;
 
-                    // Publish the signed Bitcoin lock transaction
-                    let (..) = bitcoin_wallet.broadcast(signed_tx, "lock").await?;
+                    tracing::debug!(
+                        %monero_wallet_restore_blockheight,
+                        "User approved swap offer, recording monero wallet restore blockheight",
+                    );
 
-                    BobState::BtcLocked {
+                    BobState::BtcLockReadyToPublish {
+                        btc_lock_tx_signed: signed_tx,
                         state3,
                         monero_wallet_restore_blockheight,
                     }
@@ -218,8 +226,32 @@ async fn next_state(
                 }
             }
         }
-        // Bob has locked Bitcoin
-        // Watch for Alice to lock Monero or for cancel timelock to elapse
+        // User has approved the swap
+        // Bitcoin lock transaction has been signed
+        // Monero restore height has been recorded
+        BobState::BtcLockReadyToPublish {
+            btc_lock_tx_signed,
+            state3,
+            monero_wallet_restore_blockheight,
+        } => {
+            event_emitter
+                .emit_swap_progress_event(swap_id, TauriSwapProgressEvent::BtcLockPublishInflight);
+
+            // Check if the transaction has already been broadcasted
+            // It could be that the operation was aborted after the transaction reached the Electrum server
+            // but before we transitioned to the BtcLocked state
+            if let Ok(Some(_)) = bitcoin_wallet.get_raw_transaction(state3.tx_lock_id()).await {
+                tracing::info!(txid = %state3.tx_lock_id(), "Bitcoin lock transaction already published, skipping publish");
+            }else {
+                 // Publish the signed Bitcoin lock transaction
+                let (..) = bitcoin_wallet.broadcast(btc_lock_tx_signed, "lock").await?;
+            }
+
+            BobState::BtcLocked {
+                state3,
+                monero_wallet_restore_blockheight,
+            }
+        }
         BobState::BtcLocked {
             state3,
             monero_wallet_restore_blockheight,

swap/tests/harness/mod.rs

@@ -315,6 +315,7 @@ async fn init_test_wallets(
         monero::Network::Mainnet,
         true,
         None,
+        None,
     )
     .await
     .unwrap();
@@ -1125,7 +1126,7 @@ pub struct SlowCancelConfig;
 impl GetConfig for SlowCancelConfig {
     fn get_config() -> Config {
         Config {
-            bitcoin_cancel_timelock: CancelTimelock::new(180),
+            bitcoin_cancel_timelock: CancelTimelock::new(180).into(),
             ..env::Regtest::get_config()
         }
     }
@@ -1136,7 +1137,7 @@ pub struct FastCancelConfig;
 impl GetConfig for FastCancelConfig {
     fn get_config() -> Config {
         Config {
-            bitcoin_cancel_timelock: CancelTimelock::new(10),
+            bitcoin_cancel_timelock: CancelTimelock::new(10).into(),
             ..env::Regtest::get_config()
         }
     }
@@ -1147,8 +1148,8 @@ pub struct FastPunishConfig;
 impl GetConfig for FastPunishConfig {
     fn get_config() -> Config {
         Config {
-            bitcoin_cancel_timelock: CancelTimelock::new(10),
-            bitcoin_punish_timelock: PunishTimelock::new(10),
+            bitcoin_cancel_timelock: CancelTimelock::new(10).into(),
+            bitcoin_punish_timelock: PunishTimelock::new(10).into(),
             ..env::Regtest::get_config()
         }
     }