add close_wallet

and remove filtering by tag in get_accounts

2 files changed, 134 insertions, 24 deletions

diff --git a/impls/monero.rs/src/lib.rs b/impls/monero.rs/src/lib.rs
index 77170e4..4fb9373 100644
--- a/impls/monero.rs/src/lib.rs
+++ b/impls/monero.rs/src/lib.rs
@@ -52,6 +52,12 @@ pub struct GetAccounts {
     pub accounts: Vec<Account>,
 }
 
+pub struct Wallet {
+    pub ptr: NonNull<c_void>,
+    manager: Arc<WalletManager>,
+    is_closed: bool, // New field to track if the wallet is closed
+}
+
 pub struct WalletManager {
     ptr: NonNull<c_void>,
 }
@@ -189,7 +195,11 @@ impl WalletManager {
                 return Err(WalletError::NullPointer);
             }
 
-            Ok(Wallet { ptr: NonNull::new(wallet_ptr).unwrap(), manager: Arc::clone(self) })
+            Ok(Wallet {
+                ptr: NonNull::new(wallet_ptr).unwrap(),
+                manager: Arc::clone(self),
+                is_closed: false,
+            })
         }
     }
 
@@ -247,18 +257,17 @@ impl WalletManager {
                 Err(self.get_status(wallet_ptr).unwrap_err())
             } else {
                 // Ensuring that we properly close the wallet when it's no longer needed
-                let wallet = Wallet { ptr: NonNull::new(wallet_ptr).unwrap(), manager: Arc::clone(self) };
+                let wallet = Wallet {
+                    ptr: NonNull::new(wallet_ptr).unwrap(),
+                    manager: Arc::clone(self),
+                    is_closed: false,
+                };
                 Ok(wallet)
             }
         }
     }
 }
 
-pub struct Wallet {
-    pub ptr: NonNull<c_void>,
-    manager: Arc<WalletManager>,
-}
-
 impl Wallet {
     /// Retrieves the wallet's seed with an optional offset.
     ///
@@ -512,11 +521,11 @@ impl Wallet {
         }
     }
 
-    /// Retrieves all accounts, optionally filtered by a tag.
+    /// Retrieves all accounts associated with the wallet.
     ///
     /// # Example
     ///
-    /// ```
+    /// ```rust
     /// use monero_c_rust::{WalletManager, NetworkType};
     /// use tempfile::TempDir;
     /// use std::fs;
@@ -529,7 +538,7 @@ impl Wallet {
     /// let wallet = manager.create_wallet(wallet_str, "password", "English", NetworkType::Mainnet).expect("Failed to create wallet");
     ///
     /// // Initially, there should be one account (the primary account).
-    /// let initial_accounts = wallet.get_accounts("").expect("Failed to retrieve accounts");
+    /// let initial_accounts = wallet.get_accounts().expect("Failed to retrieve accounts");
     /// assert_eq!(initial_accounts.accounts.len(), 1, "Initial account count mismatch");
     /// assert_eq!(initial_accounts.accounts[0].label, "Primary account", "Expected primary account label");
     ///
@@ -538,7 +547,7 @@ impl Wallet {
     /// wallet.create_account("Account 2").expect("Failed to create account 2");
     ///
     /// // Retrieve all accounts again; we should have three now.
-    /// let all_accounts = wallet.get_accounts("").expect("Failed to retrieve all accounts");
+    /// let all_accounts = wallet.get_accounts().expect("Failed to retrieve all accounts");
     /// assert_eq!(all_accounts.accounts.len(), 3, "Expected 3 accounts after creation");
     ///
     /// // Verify the labels of the accounts.
@@ -546,15 +555,11 @@ impl Wallet {
     /// assert_eq!(all_accounts.accounts[1].label, "Account 1", "Second account should be 'Account 1'");
     /// assert_eq!(all_accounts.accounts[2].label, "Account 2", "Third account should be 'Account 2'");
     ///
-    /// // TODO show filtering accounts with a tag that doesn't match any account.
-    ///
     /// // Clean up wallet files.
     /// fs::remove_file(wallet_str).expect("Failed to delete test wallet");
     /// fs::remove_file(format!("{}.keys", wallet_str)).expect("Failed to delete test wallet keys");
     /// ```
-    pub fn get_accounts(&self, tag: &str) -> WalletResult<GetAccounts> {
-        let c_tag = CString::new(tag).map_err(|_| WalletError::FfiError("Invalid tag".to_string()))?;
-
+    pub fn get_accounts(&self) -> WalletResult<GetAccounts> {
         unsafe {
             let accounts_size = bindings::MONERO_Wallet_numSubaddressAccounts(self.ptr.as_ptr());
             self.throw_if_error()?;
@@ -583,6 +588,58 @@ impl Wallet {
             Ok(GetAccounts { accounts })
         }
     }
+
+    /// Closes the wallet, releasing any resources associated with it.
+    ///
+    /// After calling this method, the `Wallet` instance should no longer be used.
+    ///
+    /// # Returns
+    ///
+    /// * `WalletResult<()>` - Returns `Ok(())` if the wallet was successfully closed,
+    ///   or a `WalletError` if an error occurred during closing.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// use monero_c_rust::{WalletManager, NetworkType, WalletResult};
+    /// use tempfile::TempDir;
+    /// use std::fs;
+    ///
+    /// let temp_dir = TempDir::new().expect("Failed to create temporary directory");
+    /// let wallet_path = temp_dir.path().join("wallet_name");
+    /// let wallet_str = wallet_path.to_str().unwrap();
+    ///
+    /// let manager = WalletManager::new().unwrap();
+    /// let mut wallet = manager.create_wallet(wallet_str, "password", "English", NetworkType::Mainnet).unwrap();
+    ///
+    /// // Use the wallet for operations...
+    ///
+    /// // Now close the wallet
+    /// let close_result = wallet.close_wallet();
+    /// assert!(close_result.is_ok(), "Failed to close wallet: {:?}", close_result.err());
+    ///
+    /// // Clean up wallet files.
+    /// fs::remove_file(wallet_str).expect("Failed to delete test wallet");
+    /// fs::remove_file(format!("{}.keys", wallet_str)).expect("Failed to delete test wallet keys");
+    /// ```
+    pub fn close_wallet(&mut self) -> WalletResult<()> {
+        if self.is_closed {
+            return Ok(());
+        }
+        unsafe {
+            let result = bindings::MONERO_WalletManager_closeWallet(
+                self.manager.ptr.as_ptr(),
+                self.ptr.as_ptr(),
+                false, // Don't save the wallet by default.
+            );
+            if result {
+                self.is_closed = true;
+                Ok(())
+            } else {
+                Err(WalletError::FfiError("Failed to close wallet".to_string()))
+            }
+        }
+    }
 }
 
 #[derive(Debug)]
@@ -593,12 +650,8 @@ pub struct GetBalance {
 
 impl Drop for Wallet {
     fn drop(&mut self) {
-        unsafe {
-            let _result = bindings::MONERO_WalletManager_closeWallet(
-                self.manager.ptr.as_ptr(),
-                self.ptr.as_ptr(),
-                false, // Don't save the wallet by default.
-            );
+        if !self.is_closed {
+            let _ = self.close_wallet();
         }
     }
 }
@@ -884,7 +937,7 @@ fn test_get_accounts() {
     wallet.create_account("Test Account 2").expect("Failed to create account 2");
 
     // Retrieve all accounts
-    let accounts = wallet.get_accounts("").expect("Failed to retrieve accounts");
+    let accounts = wallet.get_accounts().expect("Failed to retrieve accounts");
     assert_eq!(accounts.accounts.len(), 3); // Including the primary account
 
     // Check account names
@@ -894,3 +947,25 @@ fn test_get_accounts() {
 
     teardown(&temp_dir).expect("Failed to clean up after test");
 }
+
+#[test]
+fn test_close_wallet() {
+    let (manager, temp_dir) = setup().expect("Failed to set up test environment");
+
+    let wallet_path = temp_dir.path().join("wallet_name");
+    let wallet_str = wallet_path.to_str().expect("Failed to convert wallet path to string");
+
+    // Create a wallet.
+    let mut wallet = manager.create_wallet(wallet_str, "password", "English", NetworkType::Mainnet)
+        .expect("Failed to create wallet");
+
+    // Close the wallet.
+    let close_result = wallet.close_wallet();
+    assert!(close_result.is_ok(), "Failed to close wallet: {:?}", close_result.err());
+
+    // Attempt to close the wallet again.
+    let close_again_result = wallet.close_wallet();
+    assert!(close_again_result.is_ok(), "Failed to close wallet a second time: {:?}", close_again_result.err());
+
+    teardown(&temp_dir).expect("Failed to clean up after test");
+}
diff --git a/impls/monero.rs/tests/integration_tests.rs b/impls/monero.rs/tests/integration_tests.rs
index 95656a4..0310d4d 100644
--- a/impls/monero.rs/tests/integration_tests.rs
+++ b/impls/monero.rs/tests/integration_tests.rs
@@ -432,10 +432,45 @@ fn test_get_accounts_integration() {
     wallet.create_account("Integration Account 2").expect("Failed to create account");
 
     // Fetch accounts.
-    let accounts_result = wallet.get_accounts("");
+    let accounts_result = wallet.get_accounts();
     assert!(accounts_result.is_ok(), "Failed to fetch accounts: {:?}", accounts_result.err());
     let accounts = accounts_result.unwrap().accounts;
     assert_eq!(accounts.len(), 3, "Expected 3 accounts");
 
     teardown(&temp_dir).expect("Failed to clean up after test");
 }
+
+#[test]
+fn test_close_wallet_integration() {
+    println!("Running test_close_wallet_integration");
+    let (manager, temp_dir) = setup().expect("Failed to set up test environment");
+
+    // Construct the full path for the wallet within temp_dir.
+    let wallet_path = temp_dir.path().join("test_wallet");
+    let wallet_str = wallet_path.to_str().expect("Failed to convert wallet path to string");
+
+    // Create the wallet.
+    let mut wallet = manager
+        .create_wallet(wallet_str, "password", "English", NetworkType::Mainnet)
+        .expect("Failed to create wallet");
+
+    // Use the wallet for operations...
+    println!("Using the wallet for operations...");
+
+    // Close the wallet.
+    println!("Closing the wallet...");
+    let start = Instant::now();
+    let close_result = wallet.close_wallet();
+    println!("close_wallet took {:?}", start.elapsed());
+    assert!(close_result.is_ok(), "Failed to close wallet: {:?}", close_result.err());
+
+    // Attempt to close the wallet again.
+    println!("Attempting to close the wallet again...");
+    let start = Instant::now();
+    let close_again_result = wallet.close_wallet();
+    println!("Second close_wallet call took {:?}", start.elapsed());
+    assert!(close_again_result.is_ok(), "Failed to close wallet a second time: {:?}", close_again_result.err());
+
+    // Clean up.
+    teardown(&temp_dir).expect("Failed to clean up after test");
+}