From f5b71b41908e4724763888972ce85cf2d58d2612 Mon Sep 17 00:00:00 2001 From: hashmap Date: Sun, 11 Nov 2018 12:56:42 +0100 Subject: [PATCH] Instantiate wallet once in wallet cli (#1927) Also remove extra Box to simplify type --- servers/tests/framework/mod.rs | 45 +++++++++++++------------- servers/tests/simulnet.rs | 10 +++--- src/bin/cmd/wallet.rs | 45 ++++++++++++-------------- wallet/src/libwallet/api.rs | 52 +++++++++++++++--------------- wallet/src/libwallet/controller.rs | 21 ++++++------ wallet/tests/common/mod.rs | 17 ++++------ wallet/tests/common/testclient.rs | 4 +-- 7 files changed, 91 insertions(+), 103 deletions(-) diff --git a/servers/tests/framework/mod.rs b/servers/tests/framework/mod.rs index 0df8f417f..a8c111a1d 100644 --- a/servers/tests/framework/mod.rs +++ b/servers/tests/framework/mod.rs @@ -280,7 +280,7 @@ impl LocalServerContainer { }); wallet::controller::foreign_listener( - Box::new(wallet), + Arc::new(Mutex::new(wallet)), &self.wallet_config.api_listen_addr(), None, ).unwrap_or_else(|e| { @@ -343,29 +343,28 @@ impl LocalServerContainer { let mut wallet = LMDBBackend::new(config.clone(), "", client) .unwrap_or_else(|e| panic!("Error creating wallet: {:?} Config: {:?}", e, config)); wallet.keychain = Some(keychain); - let _ = - wallet::controller::owner_single_use(Arc::new(Mutex::new(Box::new(wallet))), |api| { - let result = api.issue_send_tx( - amount, - minimum_confirmations, + let _ = wallet::controller::owner_single_use(Arc::new(Mutex::new(wallet)), |api| { + let result = api.issue_send_tx( + amount, + minimum_confirmations, + dest, + max_outputs, + change_outputs, + selection_strategy == "all", + ); + match result { + Ok(_) => println!( + "Tx sent: {} grin to {} (strategy '{}')", + core::core::amount_to_hr_string(amount, false), dest, - max_outputs, - change_outputs, - selection_strategy == "all", - ); - match result { - Ok(_) => println!( - "Tx sent: {} grin to {} (strategy '{}')", - core::core::amount_to_hr_string(amount, false), - dest, - selection_strategy, - ), - Err(e) => { - println!("Tx not sent to {}: {:?}", dest, e); - } - }; - Ok(()) - }).unwrap_or_else(|e| panic!("Error creating wallet: {:?} Config: {:?}", e, config)); + selection_strategy, + ), + Err(e) => { + println!("Tx not sent to {}: {:?}", dest, e); + } + }; + Ok(()) + }).unwrap_or_else(|e| panic!("Error creating wallet: {:?} Config: {:?}", e, config)); } /// Stops the running wallet server diff --git a/servers/tests/simulnet.rs b/servers/tests/simulnet.rs index 5be41f6b1..34c663e07 100644 --- a/servers/tests/simulnet.rs +++ b/servers/tests/simulnet.rs @@ -881,7 +881,7 @@ fn long_fork_test_case_6(s: &Vec) { pub fn create_wallet( dir: &str, client: HTTPWalletClient, -) -> Box> { +) -> Arc>> { let mut wallet_config = WalletConfig::default(); wallet_config.data_file_dir = String::from(dir); let _ = wallet::WalletSeed::init_file(&wallet_config); @@ -895,7 +895,7 @@ pub fn create_wallet( e, wallet_config ) }); - Box::new(wallet) + Arc::new(Mutex::new(wallet)) } /// Intended to replicate https://github.com/mimblewimble/grin/issues/1325 @@ -958,12 +958,11 @@ fn replicate_tx_fluff_failure() { // get another instance of wallet1 (to update contents and perform a send) let wallet1 = create_wallet("target/tmp/tx_fluff/wallet1", client1.clone()); - let wallet1 = Arc::new(Mutex::new(wallet1)); let amount = 30_000_000_000; let mut slate = Slate::blank(1); - wallet::controller::owner_single_use(wallet1.clone(), |api| { + wallet::controller::owner_single_use(wallet1, |api| { slate = api .issue_send_tx( amount, // amount @@ -983,8 +982,7 @@ fn replicate_tx_fluff_failure() { // get another instance of wallet (to check contents) let wallet2 = create_wallet("target/tmp/tx_fluff/wallet2", client2.clone()); - let wallet2 = Arc::new(Mutex::new(wallet2)); - wallet::controller::owner_single_use(wallet2.clone(), |api| { + wallet::controller::owner_single_use(wallet2, |api| { let res = api.retrieve_summary_info(true).unwrap(); assert_eq!(res.1.amount_currently_spendable, amount); Ok(()) diff --git a/src/bin/cmd/wallet.rs b/src/bin/cmd/wallet.rs index c981ed89f..8d5b026e4 100644 --- a/src/bin/cmd/wallet.rs +++ b/src/bin/cmd/wallet.rs @@ -56,7 +56,7 @@ pub fn instantiate_wallet( passphrase: &str, account: &str, node_api_secret: Option, -) -> Box> { +) -> Arc>> { let client = HTTPWalletClient::new(&wallet_config.check_node_api_http_addr, node_api_secret); let mut db_wallet = LMDBBackend::new(wallet_config.clone(), passphrase, client).unwrap_or_else(|e| { @@ -71,7 +71,7 @@ pub fn instantiate_wallet( panic!("Error starting wallet: {}", e); }); info!("Using LMDB Backend for wallet"); - Box::new(db_wallet) + Arc::new(Mutex::new(db_wallet)) } pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i32 { @@ -137,14 +137,15 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i Some(p) => p, }; + let wallet = instantiate_wallet( + wallet_config.clone(), + passphrase, + account, + node_api_secret.clone(), + ); + // Handle listener startup commands { - let wallet = instantiate_wallet( - wallet_config.clone(), - passphrase, - account, - node_api_secret.clone(), - ); let api_secret = get_first_line(wallet_config.api_secret_path.clone()); let tls_conf = match wallet_config.tls_certificate_file.clone() { @@ -164,17 +165,20 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i if let Some(port) = listen_args.value_of("port") { wallet_config.api_listen_port = port.parse().unwrap(); } - controller::foreign_listener(wallet, &wallet_config.api_listen_addr(), tls_conf) - .unwrap_or_else(|e| { - panic!( - "Error creating wallet listener: {:?} Config: {:?}", - e, wallet_config - ); - }); + controller::foreign_listener( + wallet.clone(), + &wallet_config.api_listen_addr(), + tls_conf, + ).unwrap_or_else(|e| { + panic!( + "Error creating wallet listener: {:?} Config: {:?}", + e, wallet_config + ); + }); } ("owner_api", Some(_api_args)) => { // TLS is disabled because we bind to localhost - controller::owner_listener(wallet, "127.0.0.1:13420", api_secret, None) + controller::owner_listener(wallet.clone(), "127.0.0.1:13420", api_secret, None) .unwrap_or_else(|e| { panic!( "Error creating wallet api listener: {:?} Config: {:?}", @@ -185,7 +189,7 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i ("web", Some(_api_args)) => { // start owner listener and run static file server start_webwallet_server(); - controller::owner_listener(wallet, "127.0.0.1:13420", api_secret, tls_conf) + controller::owner_listener(wallet.clone(), "127.0.0.1:13420", api_secret, tls_conf) .unwrap_or_else(|e| { panic!( "Error creating wallet api listener: {:?} Config: {:?}", @@ -197,13 +201,6 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i }; } - // Handle single-use (command line) owner commands - let wallet = Arc::new(Mutex::new(instantiate_wallet( - wallet_config.clone(), - passphrase, - account, - node_api_secret, - ))); let res = controller::owner_single_use(wallet.clone(), |api| { match wallet_args.subcommand() { ("account", Some(acct_args)) => { diff --git a/wallet/src/libwallet/api.rs b/wallet/src/libwallet/api.rs index 0a2728ce3..19e95acc0 100644 --- a/wallet/src/libwallet/api.rs +++ b/wallet/src/libwallet/api.rs @@ -49,7 +49,7 @@ where { /// Wallet, contains its keychain (TODO: Split these up into 2 traits /// perhaps) - pub wallet: Arc>>, + pub wallet: Arc>, phantom: PhantomData, phantom_c: PhantomData, } @@ -61,7 +61,7 @@ where K: Keychain, { /// Create new API instance - pub fn new(wallet_in: Arc>>) -> Self { + pub fn new(wallet_in: Arc>) -> Self { APIOwner { wallet: wallet_in, phantom: PhantomData, @@ -89,7 +89,7 @@ where let res = Ok(( validated, - updater::retrieve_outputs(&mut **w, include_spent, tx_id, &parent_key_id)?, + updater::retrieve_outputs(&mut *w, include_spent, tx_id, &parent_key_id)?, )); w.close()?; @@ -114,7 +114,7 @@ where let res = Ok(( validated, - updater::retrieve_txs(&mut **w, tx_id, &parent_key_id)?, + updater::retrieve_txs(&mut *w, tx_id, &parent_key_id)?, )); w.close()?; @@ -135,7 +135,7 @@ where validated = self.update_outputs(&mut w); } - let wallet_info = updater::retrieve_info(&mut **w, &parent_key_id)?; + let wallet_info = updater::retrieve_info(&mut *w, &parent_key_id)?; let res = Ok((validated, wallet_info)); w.close()?; @@ -145,13 +145,13 @@ where /// Return list of existing account -> Path mappings pub fn accounts(&mut self) -> Result, Error> { let mut w = self.wallet.lock(); - keys::accounts(&mut **w) + keys::accounts(&mut *w) } /// Create a new account path pub fn new_account_path(&mut self, label: &str) -> Result { let mut w = self.wallet.lock(); - keys::new_acct_path(&mut **w, label) + keys::new_acct_path(&mut *w, label) } /// Issues a send transaction and sends to recipient @@ -174,7 +174,7 @@ where client = w.client().clone(); let (slate, context, lock_fn) = tx::create_send_tx( - &mut **w, + &mut *w, amount, minimum_confirmations, max_outputs, @@ -196,11 +196,11 @@ where } }; - tx::complete_tx(&mut **w, &mut slate_out, &context)?; + tx::complete_tx(&mut *w, &mut slate_out, &context)?; let tx_hex = util::to_hex(ser::ser_vec(&slate_out.tx).unwrap()); // lock our inputs - lock_fn_out(&mut **w, &tx_hex)?; + lock_fn_out(&mut *w, &tx_hex)?; w.close()?; Ok(slate_out) } @@ -225,7 +225,7 @@ where let parent_key_id = w.parent_key_id(); let (mut slate, context, lock_fn) = tx::create_send_tx( - &mut **w, + &mut *w, amount, minimum_confirmations, max_outputs, @@ -237,13 +237,13 @@ where w.set_parent_key_id_by_name(dest_acct_name)?; let parent_key_id = w.parent_key_id(); - tx::receive_tx(&mut **w, &mut slate, &parent_key_id, true)?; + tx::receive_tx(&mut *w, &mut slate, &parent_key_id, true)?; - tx::complete_tx(&mut **w, &mut slate, &context)?; + tx::complete_tx(&mut *w, &mut slate, &context)?; let tx_hex = util::to_hex(ser::ser_vec(&slate.tx).unwrap()); // lock our inputs - lock_fn(&mut **w, &tx_hex)?; + lock_fn(&mut *w, &tx_hex)?; w.set_parent_key_id(orig_parent_key_id); w.close()?; Ok(slate) @@ -266,7 +266,7 @@ where let parent_key_id = w.parent_key_id(); let (slate, context, lock_fn) = tx::create_send_tx( - &mut **w, + &mut *w, amount, minimum_confirmations, max_outputs, @@ -290,7 +290,7 @@ where let tx_hex = util::to_hex(ser::ser_vec(&slate.tx).unwrap()); // lock our inputs - lock_fn(&mut **w, &tx_hex)?; + lock_fn(&mut *w, &tx_hex)?; w.close()?; Ok(slate) } @@ -304,7 +304,7 @@ where w.open_with_credentials()?; let context = w.get_private_context(slate.id.as_bytes())?; - tx::complete_tx(&mut **w, slate, &context)?; + tx::complete_tx(&mut *w, slate, &context)?; { let mut batch = w.batch()?; batch.delete_private_context(slate.id.as_bytes())?; @@ -329,7 +329,7 @@ where "Can't contact running Grin node. Not Cancelling.", ))?; } - tx::cancel_tx(&mut **w, &parent_key_id, tx_id)?; + tx::cancel_tx(&mut *w, &parent_key_id, tx_id)?; w.close()?; Ok(()) } @@ -345,7 +345,7 @@ where w.open_with_credentials()?; let parent_key_id = w.parent_key_id(); let tx_burn = tx::issue_burn_tx( - &mut **w, + &mut *w, amount, minimum_confirmations, max_outputs, @@ -389,7 +389,7 @@ where let mut w = self.wallet.lock(); w.open_with_credentials()?; let parent_key_id = w.parent_key_id(); - let res = tx::retrieve_tx_hex(&mut **w, &parent_key_id, tx_id)?; + let res = tx::retrieve_tx_hex(&mut *w, &parent_key_id, tx_id)?; w.close()?; res }; @@ -424,7 +424,7 @@ where w.open_with_credentials()?; let parent_key_id = w.parent_key_id(); client = w.client().clone(); - let res = tx::retrieve_tx_hex(&mut **w, &parent_key_id, tx_id)?; + let res = tx::retrieve_tx_hex(&mut *w, &parent_key_id, tx_id)?; w.close()?; res }; @@ -510,7 +510,7 @@ where { /// Wallet, contains its keychain (TODO: Split these up into 2 traits /// perhaps) - pub wallet: Arc>>, + pub wallet: Arc>, phantom: PhantomData, phantom_c: PhantomData, } @@ -522,7 +522,7 @@ where K: Keychain, { /// Create new API instance - pub fn new(wallet_in: Arc>>) -> Box { + pub fn new(wallet_in: Arc>) -> Box { Box::new(APIForeign { wallet: wallet_in, phantom: PhantomData, @@ -534,7 +534,7 @@ where pub fn build_coinbase(&mut self, block_fees: &BlockFees) -> Result { let mut w = self.wallet.lock(); w.open_with_credentials()?; - let res = updater::build_coinbase(&mut **w, block_fees); + let res = updater::build_coinbase(&mut *w, block_fees); w.close()?; res } @@ -554,7 +554,7 @@ where // create an output using the amount in the slate let (_, mut context, receiver_create_fn) = selection::build_recipient_output_with_slate( - &mut **wallet, + &mut *wallet, &mut slate, parent_key_id, false, @@ -585,7 +585,7 @@ where let mut w = self.wallet.lock(); w.open_with_credentials()?; let parent_key_id = w.parent_key_id(); - let res = tx::receive_tx(&mut **w, slate, &parent_key_id, false); + let res = tx::receive_tx(&mut *w, slate, &parent_key_id, false); w.close()?; if let Err(e) = res { diff --git a/wallet/src/libwallet/controller.rs b/wallet/src/libwallet/controller.rs index 72aad3dcd..dc9b87504 100644 --- a/wallet/src/libwallet/controller.rs +++ b/wallet/src/libwallet/controller.rs @@ -41,7 +41,7 @@ use util::Mutex; /// Instantiate wallet Owner API for a single-use (command line) call /// Return a function containing a loaded API context to call -pub fn owner_single_use(wallet: Arc>>, f: F) -> Result<(), Error> +pub fn owner_single_use(wallet: Arc>, f: F) -> Result<(), Error> where T: WalletBackend, F: FnOnce(&mut APIOwner) -> Result<(), Error>, @@ -54,7 +54,7 @@ where /// Instantiate wallet Foreign API for a single-use (command line) call /// Return a function containing a loaded API context to call -pub fn foreign_single_use(wallet: Arc>>, f: F) -> Result<(), Error> +pub fn foreign_single_use(wallet: Arc>, f: F) -> Result<(), Error> where T: WalletBackend, F: FnOnce(&mut APIForeign) -> Result<(), Error>, @@ -68,7 +68,7 @@ where /// Listener version, providing same API but listening for requests on a /// port and wrapping the calls pub fn owner_listener( - wallet: Box, + wallet: Arc>, addr: &str, api_secret: Option, tls_config: Option, @@ -79,8 +79,7 @@ where C: WalletClient + 'static, K: Keychain + 'static, { - let wallet_arc = Arc::new(Mutex::new(wallet)); - let api_handler = OwnerAPIHandler::new(wallet_arc); + let api_handler = OwnerAPIHandler::new(wallet); let mut router = Router::new(); if api_secret.is_some() { @@ -110,7 +109,7 @@ where /// Listener version, providing same API but listening for requests on a /// port and wrapping the calls pub fn foreign_listener( - wallet: Box, + wallet: Arc>, addr: &str, tls_config: Option, ) -> Result<(), Error> @@ -119,7 +118,7 @@ where C: WalletClient + 'static, K: Keychain + 'static, { - let api_handler = ForeignAPIHandler::new(Arc::new(Mutex::new(wallet))); + let api_handler = ForeignAPIHandler::new(wallet); let mut router = Router::new(); router @@ -150,7 +149,7 @@ where K: Keychain + 'static, { /// Wallet instance - pub wallet: Arc>>, + pub wallet: Arc>, phantom: PhantomData, phantom_c: PhantomData, } @@ -162,7 +161,7 @@ where K: Keychain + 'static, { /// Create a new owner API handler for GET methods - pub fn new(wallet: Arc>>) -> OwnerAPIHandler { + pub fn new(wallet: Arc>) -> OwnerAPIHandler { OwnerAPIHandler { wallet, phantom: PhantomData, @@ -474,7 +473,7 @@ where K: Keychain + 'static, { /// Wallet instance - pub wallet: Arc>>, + pub wallet: Arc>, phantom: PhantomData, phantom_c: PhantomData, } @@ -486,7 +485,7 @@ where K: Keychain + 'static, { /// create a new api handler - pub fn new(wallet: Arc>>) -> ForeignAPIHandler { + pub fn new(wallet: Arc>) -> ForeignAPIHandler { ForeignAPIHandler { wallet, phantom: PhantomData, diff --git a/wallet/tests/common/mod.rs b/wallet/tests/common/mod.rs index a7b2a06ea..f41a5167c 100644 --- a/wallet/tests/common/mod.rs +++ b/wallet/tests/common/mod.rs @@ -31,7 +31,7 @@ use core::{consensus, global, pow, ser}; use wallet::libwallet; use wallet::libwallet::types::{BlockFees, CbData, WalletClient, WalletInst}; use wallet::lmdb_wallet::LMDBBackend; -use wallet::WalletConfig; +use wallet::{WalletBackend, WalletConfig}; use util; use util::secp::pedersen; @@ -116,7 +116,7 @@ pub fn add_block_with_reward(chain: &Chain, txs: Vec<&Transaction>, reward: CbDa pub fn award_block_to_wallet( chain: &Chain, txs: Vec<&Transaction>, - wallet: Arc>>>, + wallet: Arc>>, ) -> Result<(), libwallet::Error> where C: WalletClient, @@ -142,7 +142,7 @@ where /// Award a blocks to a wallet directly pub fn award_blocks_to_wallet( chain: &Chain, - wallet: Arc>>>, + wallet: Arc>>, number: usize, ) -> Result<(), libwallet::Error> where @@ -156,7 +156,7 @@ where } /// dispatch a db wallet -pub fn create_wallet(dir: &str, client: C) -> Arc>>> +pub fn create_wallet(dir: &str, client: C) -> Arc>> where C: WalletClient + 'static, K: keychain::Keychain + 'static, @@ -164,13 +164,8 @@ where let mut wallet_config = WalletConfig::default(); wallet_config.data_file_dir = String::from(dir); let _ = wallet::WalletSeed::init_file(&wallet_config); - let mut wallet: Box> = { - let mut wallet: LMDBBackend = LMDBBackend::new(wallet_config.clone(), "", client) - .unwrap_or_else(|e| { - panic!("Error creating wallet: {:?} Config: {:?}", e, wallet_config) - }); - Box::new(wallet) - }; + let mut wallet = LMDBBackend::new(wallet_config.clone(), "", client) + .unwrap_or_else(|e| panic!("Error creating wallet: {:?} Config: {:?}", e, wallet_config)); wallet.open_with_credentials().unwrap_or_else(|e| { panic!( "Error initializing wallet: {:?} Config: {:?}", diff --git a/wallet/tests/common/testclient.rs b/wallet/tests/common/testclient.rs index 2994fb068..82ee32f8a 100644 --- a/wallet/tests/common/testclient.rs +++ b/wallet/tests/common/testclient.rs @@ -77,7 +77,7 @@ where String, ( Sender, - Arc>>>, + Arc>>, ), >, /// simulate json send to another client @@ -133,7 +133,7 @@ where &mut self, addr: &str, tx: Sender, - wallet: Arc>>>, + wallet: Arc>>, ) { self.wallets.insert(addr.to_owned(), (tx, wallet)); }