fix: monero wallet refresh (#1596)

This PR changes the following behaviour in the refresh functionality of the monero wallet
- Allows for multiple retries because in some cases users have experienced an issue where the wallet rpc returns `no connection to daemon` even though the daemon is available. I'm not 100% sure why this happens but retrying often fixes the issue
- Print the current sync height after each failed attempt at syncing to see how far we've come
- The `monero-wallet-rpc` is started with the `--no-initial-sync` flag which ensures that as soon as it's started, it's ready to respond to requests
- The `monero-wallet-rpc` was upgraded to `v0.18.3.1` because this PR https://github.com/monero-project/monero/pull/8941 has improved some of the issues mentioned above


This PR is part of a larger effort to fix this issue https://github.com/comit-network/xmr-btc-swap/issues/1432

CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- Add retry logic to monero-wallet-rpc wallet refresh
+
 ## [0.13.0] - 2024-05-29
 
 - Minimum Supported Rust Version (MSRV) bumped to 1.74

monero-rpc/src/wallet.rs

@@ -162,6 +162,12 @@ pub struct BlockHeight {
     pub height: u32,
 }
 
+impl fmt::Display for BlockHeight {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.height)
+    }
+}
+
 #[derive(Clone, Copy, Debug, Deserialize)]
 #[serde(from = "CheckTxKeyResponse")]
 pub struct CheckTxKey {

swap/src/monero/wallet.rs

@@ -45,6 +45,7 @@ impl Wallet {
     pub async fn connect(client: wallet::Client, name: String, env_config: Config) -> Result<Self> {
         let main_address =
             monero::Address::from_str(client.get_address(0).await?.address.as_str())?;
+
         Ok(Self {
             inner: Mutex::new(client),
             network: env_config.monero_network,
@@ -125,13 +126,14 @@ impl Wallet {
         let temp_wallet_address =
             Address::standard(self.network, public_spend_key, public_view_key);
 
-        let wallet = self.inner.lock().await;
-
         // Close the default wallet before generating the other wallet to ensure that
         // it saves its state correctly
-        let _ = wallet.close_wallet().await?;
+        let _ = self.inner.lock().await.close_wallet().await?;
 
-        let _ = wallet
+        let _ = self
+            .inner
+            .lock()
+            .await
             .generate_from_keys(
                 file_name,
                 temp_wallet_address.to_string(),
@@ -144,8 +146,14 @@ impl Wallet {
             .await?;
 
         // Try to send all the funds from the generated wallet to the default wallet
-        match wallet.refresh().await {
-            Ok(_) => match wallet.sweep_all(self.main_address.to_string()).await {
+        match self.refresh(3).await {
+            Ok(_) => match self
+                .inner
+                .lock()
+                .await
+                .sweep_all(self.main_address.to_string())
+                .await
+            {
                 Ok(sweep_all) => {
                     for tx in sweep_all.tx_hash_list {
                         tracing::info!(
@@ -166,7 +174,12 @@ impl Wallet {
             }
         }
 
-        let _ = wallet.open_wallet(self.name.clone()).await?;
+        let _ = self
+            .inner
+            .lock()
+            .await
+            .open_wallet(self.name.clone())
+            .await?;
 
         Ok(())
     }
@@ -261,8 +274,44 @@ impl Wallet {
         self.main_address
     }
 
-    pub async fn refresh(&self) -> Result<Refreshed> {
-        Ok(self.inner.lock().await.refresh().await?)
+    pub async fn refresh(&self, max_attempts: usize) -> Result<Refreshed> {
+        const RETRY_INTERVAL: Duration = Duration::from_secs(1);
+
+        for i in 1..=max_attempts {
+            tracing::info!(name = %self.name, attempt=i, "Syncing Monero wallet");
+
+            let result = self.inner.lock().await.refresh().await;
+
+            match result {
+                Ok(refreshed) => {
+                    tracing::info!(name = %self.name, "Monero wallet synced");
+                    return Ok(refreshed);
+                }
+                Err(error) => {
+                    let attempts_left = max_attempts - i;
+
+                    // We would not want to fail here if the height is not available
+                    // as it is not critical for the operation of the wallet.
+                    // We can just log a warning and continue.
+                    let height = match self.inner.lock().await.get_height().await {
+                        Ok(height) => height.to_string(),
+                        Err(_) => {
+                            tracing::warn!(name = %self.name, "Failed to fetch Monero wallet height during sync");
+                            "unknown".to_string()
+                        }
+                    };
+
+                    tracing::warn!(attempt=i, %height, %attempts_left, name = %self.name, %error, "Failed to sync Monero wallet");
+
+                    if attempts_left == 0 {
+                        return Err(error.into());
+                    }
+                }
+            }
+
+            tokio::time::sleep(RETRY_INTERVAL).await;
+        }
+        unreachable!("Loop should have returned by now");
     }
 }
 

swap/src/monero/wallet_rpc.rs

@@ -352,6 +352,7 @@ impl WalletRpc {
             .arg("--disable-rpc-login")
             .arg("--wallet-dir")
             .arg(self.working_dir.join("monero-data"))
+            .arg("--no-initial-sync")
             .spawn()?;
 
         let stdout = child

swap/src/protocol/bob/swap.rs

@@ -264,7 +264,7 @@ async fn next_state(
             }
 
             // Ensure that the generated wallet is synced so we have a proper balance
-            monero_wallet.refresh().await?;
+            monero_wallet.refresh(20).await?;
             // Sweep (transfer all funds) to the given address
             let tx_hashes = monero_wallet.sweep_all(monero_receive_address).await?;
 

swap/tests/harness/mod.rs

@@ -891,7 +891,7 @@ impl Wallet for monero::Wallet {
     type Amount = monero::Amount;
 
     async fn refresh(&self) -> Result<()> {
-        self.refresh().await?;
+        self.refresh(1).await?;
 
         Ok(())
     }