Simplify xmr-btc/tests Monero wallet

- Make it the same for Alice and Bob.
- Make it contain a wallet client instead of the `Monero` struct.

Also:

Remove `Container` from inside `Monero` struct. The caller of `new`
can simply ensure that `Container` is not dropped to keep the
container alive.

This makes the `Monero` struct easier to work with, as it just holds
the data necessary to create the different clients created during
`init`, and does not have any lifetime restrictions.

monero-harness/src/lib.rs

@@ -48,18 +48,17 @@ const WAIT_WALLET_SYNC_MILLIS: u64 = 1000;
 /// Wallet sub-account indices.
 const ACCOUNT_INDEX_PRIMARY: u32 = 0;
 
-#[derive(Debug)]
-pub struct Monero<'c> {
-    pub docker: Container<'c, Cli, image::Monero>,
-    pub monerod_rpc_port: u16,
-    pub miner_wallet_rpc_port: u16,
-    pub alice_wallet_rpc_port: u16,
-    pub bob_wallet_rpc_port: u16,
+#[derive(Copy, Clone, Debug)]
+pub struct Monero {
+    monerod_rpc_port: u16,
+    miner_wallet_rpc_port: u16,
+    alice_wallet_rpc_port: u16,
+    bob_wallet_rpc_port: u16,
 }
 
-impl<'c> Monero<'c> {
+impl<'c> Monero {
     /// Starts a new regtest monero container.
-    pub fn new(cli: &'c Cli) -> Self {
+    pub fn new(cli: &'c Cli) -> (Self, Container<'c, Cli, image::Monero>) {
         let mut rng = rand::thread_rng();
         let monerod_rpc_port: u16 = rng.gen_range(1024, u16::MAX);
         let miner_wallet_rpc_port: u16 = rng.gen_range(1024, u16::MAX);
@@ -91,13 +90,15 @@ impl<'c> Monero<'c> {
         let docker = cli.run(image);
         println!("image ran");
 
-        Self {
+        (
+            Self {
+                monerod_rpc_port,
+                miner_wallet_rpc_port,
+                alice_wallet_rpc_port,
+                bob_wallet_rpc_port,
+            },
             docker,
-            monerod_rpc_port,
-            miner_wallet_rpc_port,
-            alice_wallet_rpc_port,
-            bob_wallet_rpc_port,
-        }
+        )
     }
 
     pub fn miner_wallet_rpc_client(&self) -> wallet::Client {
@@ -156,21 +157,6 @@ impl<'c> Monero<'c> {
         Ok(())
     }
 
-    /// Just create a wallet and start mining (you probably want `init()`).
-    pub async fn init_just_miner(&self, blocks: u32) -> Result<()> {
-        let wallet = self.miner_wallet_rpc_client();
-        let monerod = self.monerod_rpc_client();
-
-        wallet.create_wallet("miner_wallet").await?;
-        let miner = self.get_address_miner().await?.address;
-
-        let _ = monerod.generate_blocks(blocks, &miner).await?;
-
-        let _ = tokio::spawn(mine(monerod.clone(), miner));
-
-        Ok(())
-    }
-
     async fn fund_account(&self, address: &str, miner: &str, funding: u64) -> Result<()> {
         let monerod = self.monerod_rpc_client();
 
@@ -208,64 +194,42 @@ impl<'c> Monero<'c> {
     }
 
     /// Get addresses for the primary account.
-    pub async fn get_address_miner(&self) -> Result<GetAddress> {
+    async fn get_address_miner(&self) -> Result<GetAddress> {
         let wallet = self.miner_wallet_rpc_client();
         wallet.get_address(ACCOUNT_INDEX_PRIMARY).await
     }
 
     /// Get addresses for the Alice's account.
-    pub async fn get_address_alice(&self) -> Result<GetAddress> {
+    async fn get_address_alice(&self) -> Result<GetAddress> {
         let wallet = self.alice_wallet_rpc_client();
         wallet.get_address(ACCOUNT_INDEX_PRIMARY).await
     }
 
     /// Get addresses for the Bob's account.
-    pub async fn get_address_bob(&self) -> Result<GetAddress> {
+    async fn get_address_bob(&self) -> Result<GetAddress> {
         let wallet = self.bob_wallet_rpc_client();
         wallet.get_address(ACCOUNT_INDEX_PRIMARY).await
     }
 
-    /// Gets the balance of the wallet primary account.
-    pub async fn get_balance_primary(&self) -> Result<u64> {
-        let wallet = self.miner_wallet_rpc_client();
-        wallet.get_balance(ACCOUNT_INDEX_PRIMARY).await
-    }
-
     /// Gets the balance of Alice's account.
-    pub async fn get_balance_alice(&self) -> Result<u64> {
+    async fn get_balance_alice(&self) -> Result<u64> {
         let wallet = self.alice_wallet_rpc_client();
         wallet.get_balance(ACCOUNT_INDEX_PRIMARY).await
     }
 
     /// Gets the balance of Bob's account.
-    pub async fn get_balance_bob(&self) -> Result<u64> {
+    async fn get_balance_bob(&self) -> Result<u64> {
         let wallet = self.bob_wallet_rpc_client();
         wallet.get_balance(ACCOUNT_INDEX_PRIMARY).await
     }
 
     /// Transfers moneroj from the primary account.
-    pub async fn transfer_from_primary(&self, amount: u64, address: &str) -> Result<Transfer> {
+    async fn transfer_from_primary(&self, amount: u64, address: &str) -> Result<Transfer> {
         let wallet = self.miner_wallet_rpc_client();
         wallet
             .transfer(ACCOUNT_INDEX_PRIMARY, amount, address)
             .await
     }
-
-    /// Transfers moneroj from Alice's account.
-    pub async fn transfer_from_alice(&self, amount: u64, address: &str) -> Result<Transfer> {
-        let wallet = self.alice_wallet_rpc_client();
-        wallet
-            .transfer(ACCOUNT_INDEX_PRIMARY, amount, address)
-            .await
-    }
-
-    /// Transfers moneroj from Bob's account.
-    pub async fn transfer_from_bob(&self, amount: u64, address: &str) -> Result<Transfer> {
-        let wallet = self.bob_wallet_rpc_client();
-        wallet
-            .transfer(ACCOUNT_INDEX_PRIMARY, amount, address)
-            .await
-    }
 }
 
 /// Mine a block ever BLOCK_TIME_SECS seconds.

monero-harness/tests/client.rs

@@ -5,29 +5,24 @@ use testcontainers::clients::Cli;
 const ALICE_FUND_AMOUNT: u64 = 1_000_000_000_000;
 const BOB_FUND_AMOUNT: u64 = 0;
 
-fn init_cli() -> Cli {
-    Cli::default()
-}
-
-async fn init_monero(tc: &'_ Cli) -> Monero<'_> {
-    let monero = Monero::new(tc);
-    let _ = monero.init(ALICE_FUND_AMOUNT, BOB_FUND_AMOUNT).await;
-
-    monero
-}
-
 #[tokio::test]
 async fn init_accounts_for_alice_and_bob() {
-    let cli = init_cli();
-    let monero = init_monero(&cli).await;
+    let tc = Cli::default();
+    let (monero, _container) = Monero::new(&tc);
+    monero
+        .init(ALICE_FUND_AMOUNT, BOB_FUND_AMOUNT)
+        .await
+        .unwrap();
 
     let got_balance_alice = monero
-        .get_balance_alice()
+        .alice_wallet_rpc_client()
+        .get_balance(0)
         .await
         .expect("failed to get alice's balance");
 
     let got_balance_bob = monero
-        .get_balance_bob()
+        .bob_wallet_rpc_client()
+        .get_balance(0)
         .await
         .expect("failed to get bob's balance");
 

monero-harness/tests/monerod.rs

@@ -1,8 +1,6 @@
-use monero_harness::{rpc::monerod::Client, Monero};
+use monero_harness::Monero;
 use spectral::prelude::*;
-use std::time::Duration;
 use testcontainers::clients::Cli;
-use tokio::time;
 
 fn init_cli() -> Cli {
     Cli::default()
@@ -11,8 +9,8 @@ fn init_cli() -> Cli {
 #[tokio::test]
 async fn connect_to_monerod() {
     let tc = init_cli();
-    let monero = Monero::new(&tc);
-    let cli = Client::localhost(monero.monerod_rpc_port);
+    let (monero, _container) = Monero::new(&tc);
+    let cli = monero.monerod_rpc_client();
 
     let header = cli
         .get_block_header_by_height(0)
@@ -21,27 +19,3 @@ async fn connect_to_monerod() {
 
     assert_that!(header.height).is_equal_to(0);
 }
-
-#[tokio::test]
-async fn miner_is_running_and_producing_blocks() {
-    let tc = init_cli();
-    let monero = Monero::new(&tc);
-    let cli = Client::localhost(monero.monerod_rpc_port);
-
-    monero
-        .init_just_miner(2)
-        .await
-        .expect("Failed to initialize");
-
-    // Only need 3 seconds since we mine a block every second but
-    // give it 5 just for good measure.
-    time::delay_for(Duration::from_secs(5)).await;
-
-    // We should have at least 5 blocks by now.
-    let header = cli
-        .get_block_header_by_height(5)
-        .await
-        .expect("failed to get block");
-
-    assert_that!(header.height).is_equal_to(5);
-}

monero-harness/tests/wallet.rs

@@ -1,24 +1,21 @@
-use monero_harness::{rpc::wallet::Client, Monero};
+use monero_harness::Monero;
 use spectral::prelude::*;
 use testcontainers::clients::Cli;
 
 #[tokio::test]
 async fn wallet_and_accounts() {
     let tc = Cli::default();
-    let monero = Monero::new(&tc);
-    let miner_wallet = Client::localhost(monero.miner_wallet_rpc_port);
+    let (monero, _container) = Monero::new(&tc);
+    let cli = monero.miner_wallet_rpc_client();
 
     println!("creating wallet ...");
 
-    let _ = miner_wallet
+    let _ = cli
         .create_wallet("wallet")
         .await
         .expect("failed to create wallet");
 
-    let got = miner_wallet
-        .get_balance(0)
-        .await
-        .expect("failed to get balance");
+    let got = cli.get_balance(0).await.expect("failed to get balance");
     let want = 0;
 
     assert_that!(got).is_equal_to(want);
@@ -27,8 +24,8 @@ async fn wallet_and_accounts() {
 #[tokio::test]
 async fn create_account_and_retrieve_it() {
     let tc = Cli::default();
-    let monero = Monero::new(&tc);
-    let cli = Client::localhost(monero.miner_wallet_rpc_port);
+    let (monero, _container) = Monero::new(&tc);
+    let cli = monero.miner_wallet_rpc_client();
 
     let label = "Iron Man"; // This is intentionally _not_ Alice or Bob.
 
@@ -61,18 +58,20 @@ async fn transfer_and_check_tx_key() {
     let fund_bob = 0;
 
     let tc = Cli::default();
-    let monero = Monero::new(&tc);
+    let (monero, _container) = Monero::new(&tc);
     let _ = monero.init(fund_alice, fund_bob).await;
 
     let address_bob = monero
-        .get_address_bob()
+        .bob_wallet_rpc_client()
+        .get_address(0)
         .await
         .expect("failed to get Bob's address")
         .address;
 
     let transfer_amount = 100;
     let transfer = monero
-        .transfer_from_alice(transfer_amount, &address_bob)
+        .alice_wallet_rpc_client()
+        .transfer(0, transfer_amount, &address_bob)
         .await
         .expect("transfer failed");
 

xmr-btc/tests/e2e.rs

@@ -59,14 +59,14 @@ pub fn init_alice_and_bob_transports() -> (
     (a_transport, b_transport)
 }
 
-pub async fn init_test<'a>(
-    monero: &'a Monero<'a>,
+pub async fn init_test(
+    monero: &Monero,
     bitcoind: &Bitcoind<'_>,
 ) -> (
     alice::State0,
     bob::State0,
-    AliceNode<'a>,
-    BobNode<'a>,
+    AliceNode,
+    BobNode,
     InitialBalances,
     SwapAmounts,
 ) {
@@ -83,8 +83,8 @@ pub async fn init_test<'a>(
     let fund_bob = 0;
     monero.init(fund_alice, fund_bob).await.unwrap();
 
-    let alice_monero_wallet = wallet::monero::AliceWallet(&monero);
-    let bob_monero_wallet = wallet::monero::BobWallet(&monero);
+    let alice_monero_wallet = wallet::monero::Wallet(monero.alice_wallet_rpc_client());
+    let bob_monero_wallet = wallet::monero::Wallet(monero.bob_wallet_rpc_client());
 
     let alice_btc_wallet = wallet::bitcoin::Wallet::new("alice", &bitcoind.node_url)
         .await
@@ -101,8 +101,8 @@ pub async fn init_test<'a>(
     let alice_initial_btc_balance = alice.bitcoin_wallet.balance().await.unwrap();
     let bob_initial_btc_balance = bob.bitcoin_wallet.balance().await.unwrap();
 
-    let alice_initial_xmr_balance = alice.monero_wallet.0.get_balance_alice().await.unwrap();
-    let bob_initial_xmr_balance = bob.monero_wallet.0.get_balance_bob().await.unwrap();
+    let alice_initial_xmr_balance = alice.monero_wallet.0.get_balance(0).await.unwrap();
+    let bob_initial_xmr_balance = bob.monero_wallet.0.get_balance(0).await.unwrap();
 
     let redeem_address = alice.bitcoin_wallet.new_address().await.unwrap();
     let punish_address = redeem_address.clone();
@@ -166,7 +166,7 @@ mod tests {
             .set_default();
 
         let cli = Cli::default();
-        let monero = Monero::new(&cli);
+        let (monero, _container) = Monero::new(&cli);
         let bitcoind = init_bitcoind(&cli).await;
 
         let (
@@ -207,21 +207,11 @@ mod tests {
             .await
             .unwrap();
 
-        let alice_final_xmr_balance = alice_node
-            .monero_wallet
-            .0
-            .get_balance_alice()
-            .await
-            .unwrap();
+        let alice_final_xmr_balance = alice_node.monero_wallet.0.get_balance(0).await.unwrap();
 
-        bob_node
-            .monero_wallet
-            .0
-            .wait_for_bob_wallet_block_height()
-            .await
-            .unwrap();
+        monero.wait_for_bob_wallet_block_height().await.unwrap();
 
-        let bob_final_xmr_balance = bob_node.monero_wallet.0.get_balance_bob().await.unwrap();
+        let bob_final_xmr_balance = bob_node.monero_wallet.0.get_balance(0).await.unwrap();
 
         assert_eq!(
             alice_final_btc_balance,
@@ -252,7 +242,7 @@ mod tests {
             .set_default();
 
         let cli = Cli::default();
-        let monero = Monero::new(&cli);
+        let (monero, _container) = Monero::new(&cli);
         let bitcoind = init_bitcoind(&cli).await;
 
         let (
@@ -304,19 +294,9 @@ mod tests {
             .await
             .unwrap();
 
-        alice_node
-            .monero_wallet
-            .0
-            .wait_for_alice_wallet_block_height()
-            .await
-            .unwrap();
-        let alice_final_xmr_balance = alice_node
-            .monero_wallet
-            .0
-            .get_balance_alice()
-            .await
-            .unwrap();
-        let bob_final_xmr_balance = bob_node.monero_wallet.0.get_balance_bob().await.unwrap();
+        monero.wait_for_alice_wallet_block_height().await.unwrap();
+        let alice_final_xmr_balance = alice_node.monero_wallet.0.get_balance(0).await.unwrap();
+        let bob_final_xmr_balance = bob_node.monero_wallet.0.get_balance(0).await.unwrap();
 
         assert_eq!(alice_final_btc_balance, initial_balances.alice_btc);
         assert_eq!(
@@ -338,7 +318,7 @@ mod tests {
             .set_default();
 
         let cli = Cli::default();
-        let monero = Monero::new(&cli);
+        let (monero, _container) = Monero::new(&cli);
         let bitcoind = init_bitcoind(&cli).await;
 
         let (

xmr-btc/tests/harness/node.rs

@@ -5,18 +5,18 @@ use xmr_btc::{alice, bob};
 
 // TODO: merge this with bob node
 // This struct is responsible for I/O
-pub struct AliceNode<'a> {
+pub struct AliceNode {
     transport: Transport<alice::Message, bob::Message>,
     pub bitcoin_wallet: wallet::bitcoin::Wallet,
-    pub monero_wallet: wallet::monero::AliceWallet<'a>,
+    pub monero_wallet: wallet::monero::Wallet,
 }
 
-impl<'a> AliceNode<'a> {
+impl AliceNode {
     pub fn new(
         transport: Transport<alice::Message, bob::Message>,
         bitcoin_wallet: wallet::bitcoin::Wallet,
-        monero_wallet: wallet::monero::AliceWallet<'a>,
-    ) -> AliceNode<'a> {
+        monero_wallet: wallet::monero::Wallet,
+    ) -> AliceNode {
         Self {
             transport,
             bitcoin_wallet,
@@ -25,8 +25,8 @@ impl<'a> AliceNode<'a> {
     }
 }
 
-pub async fn run_alice_until<'a, R: RngCore + CryptoRng>(
-    alice: &mut AliceNode<'a>,
+pub async fn run_alice_until<R: RngCore + CryptoRng>(
+    alice: &mut AliceNode,
     initial_state: alice::State,
     is_state: fn(&alice::State) -> bool,
     rng: &mut R,
@@ -49,18 +49,18 @@ pub async fn run_alice_until<'a, R: RngCore + CryptoRng>(
 
 // TODO: merge this with alice node
 // This struct is responsible for I/O
-pub struct BobNode<'a> {
+pub struct BobNode {
     transport: Transport<bob::Message, alice::Message>,
     pub bitcoin_wallet: wallet::bitcoin::Wallet,
-    pub monero_wallet: wallet::monero::BobWallet<'a>,
+    pub monero_wallet: wallet::monero::Wallet,
 }
 
-impl<'a> BobNode<'a> {
+impl BobNode {
     pub fn new(
         transport: Transport<bob::Message, alice::Message>,
         bitcoin_wallet: wallet::bitcoin::Wallet,
-        monero_wallet: wallet::monero::BobWallet<'a>,
-    ) -> BobNode<'a> {
+        monero_wallet: wallet::monero::Wallet,
+    ) -> BobNode {
         Self {
             transport,
             bitcoin_wallet,
@@ -69,8 +69,8 @@ impl<'a> BobNode<'a> {
     }
 }
 
-pub async fn run_bob_until<'a, R: RngCore + CryptoRng>(
-    bob: &mut BobNode<'a>,
+pub async fn run_bob_until<R: RngCore + CryptoRng>(
+    bob: &mut BobNode,
     initial_state: bob::State,
     is_state: fn(&bob::State) -> bool,
     rng: &mut R,

xmr-btc/tests/harness/wallet/monero.rs

@@ -2,18 +2,17 @@ use anyhow::Result;
 use async_trait::async_trait;
 use backoff::{future::FutureOperation as _, ExponentialBackoff};
 use monero::{Address, Network, PrivateKey};
-use monero_harness::Monero;
+use monero_harness::rpc::wallet;
 use std::str::FromStr;
 use xmr_btc::monero::{
     Amount, CreateWalletForOutput, InsufficientFunds, PrivateViewKey, PublicKey, PublicViewKey,
     Transfer, TransferProof, TxHash, WatchForTransfer,
 };
 
-#[derive(Debug)]
-pub struct AliceWallet<'c>(pub &'c Monero<'c>);
+pub struct Wallet(pub wallet::Client);
 
 #[async_trait]
-impl Transfer for AliceWallet<'_> {
+impl Transfer for Wallet {
     async fn transfer(
         &self,
         public_spend_key: PublicKey,
@@ -25,7 +24,7 @@ impl Transfer for AliceWallet<'_> {
 
         let res = self
             .0
-            .transfer_from_alice(amount.as_piconero(), &destination_address.to_string())
+            .transfer(0, amount.as_piconero(), &destination_address.to_string())
             .await?;
 
         let tx_hash = TxHash(res.tx_hash);
@@ -38,7 +37,7 @@ impl Transfer for AliceWallet<'_> {
 }
 
 #[async_trait]
-impl CreateWalletForOutput for AliceWallet<'_> {
+impl CreateWalletForOutput for Wallet {
     async fn create_and_load_wallet_for_output(
         &self,
         private_spend_key: PrivateKey,
@@ -51,7 +50,6 @@ impl CreateWalletForOutput for AliceWallet<'_> {
 
         let _ = self
             .0
-            .alice_wallet_rpc_client()
             .generate_from_keys(
                 &address.to_string(),
                 &private_spend_key.to_string(),
@@ -63,11 +61,8 @@ impl CreateWalletForOutput for AliceWallet<'_> {
     }
 }
 
-#[derive(Debug)]
-pub struct BobWallet<'c>(pub &'c Monero<'c>);
-
 #[async_trait]
-impl WatchForTransfer for BobWallet<'_> {
+impl WatchForTransfer for Wallet {
     async fn watch_for_transfer(
         &self,
         public_spend_key: PublicKey,
@@ -82,14 +77,14 @@ impl WatchForTransfer for BobWallet<'_> {
             InsufficientFunds { expected: Amount, actual: Amount },
         }
 
-        let wallet = self.0.bob_wallet_rpc_client();
         let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into());
 
         let res = (|| async {
             // NOTE: Currently, this is conflating IO errors with the transaction not being
             // in the blockchain yet, or not having enough confirmations on it. All these
             // errors warrant a retry, but the strategy should probably differ per case
-            let proof = wallet
+            let proof = self
+                .0
                 .check_tx_key(
                     &String::from(transfer_proof.tx_hash()),
                     &transfer_proof.tx_key().to_string(),
@@ -124,29 +119,3 @@ impl WatchForTransfer for BobWallet<'_> {
         Ok(())
     }
 }
-
-#[async_trait]
-impl CreateWalletForOutput for BobWallet<'_> {
-    async fn create_and_load_wallet_for_output(
-        &self,
-        private_spend_key: PrivateKey,
-        private_view_key: PrivateViewKey,
-    ) -> Result<()> {
-        let public_spend_key = PublicKey::from_private_key(&private_spend_key);
-        let public_view_key = PublicKey::from_private_key(&private_view_key.into());
-
-        let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key);
-
-        let _ = self
-            .0
-            .bob_wallet_rpc_client()
-            .generate_from_keys(
-                &address.to_string(),
-                &private_spend_key.to_string(),
-                &PrivateKey::from(private_view_key).to_string(),
-            )
-            .await?;
-
-        Ok(())
-    }
-}

xmr-btc/tests/on_chain.rs

@@ -45,7 +45,7 @@ async fn swap_as_alice(
     // FIXME: It would be more intuitive to have a single network/transport struct instead of
     // splitting into two, but Rust ownership rules make this tedious
     mut sender: Sender<TransferProof>,
-    monero_wallet: &'static monero::AliceWallet<'static>,
+    monero_wallet: &'static monero::Wallet,
     bitcoin_wallet: &'static bitcoin::Wallet,
     state: alice::State3,
 ) -> Result<()> {
@@ -86,7 +86,7 @@ async fn swap_as_alice(
 async fn swap_as_bob(
     network: &'static mut BobNetwork,
     mut sender: Sender<EncryptedSignature>,
-    monero_wallet: &'static monero::BobWallet<'static>,
+    monero_wallet: &'static monero::Wallet,
     bitcoin_wallet: &'static bitcoin::Wallet,
     state: bob::State2,
 ) -> Result<()> {