Add ability to compare selection strategies (#2516)

Before tx creation user can estimate fee and locked amount
with different selection strategies by providing `-e` flag for
`wallet send` command.

src/bin/cmd/wallet_args.rs

@@ -349,6 +349,9 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
 	// selection_strategy
 	let selection_strategy = parse_required(args, "selection_strategy")?;
 
+	// estimate_selection_strategies
+	let estimate_selection_strategies = args.is_present("estimate_selection_strategies");
+
 	// method
 	let method = parse_required(args, "method")?;
 
@@ -360,10 +363,18 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
 				None => "default",
 			}
 		} else {
-			parse_required(args, "dest")?
+			if !estimate_selection_strategies {
+				parse_required(args, "dest")?
+			} else {
+				""
+			}
 		}
 	};
-	if method == "http" && !dest.starts_with("http://") && !dest.starts_with("https://") {
+	if !estimate_selection_strategies
+		&& method == "http"
+		&& !dest.starts_with("http://")
+		&& !dest.starts_with("https://")
+	{
 		let msg = format!(
 			"HTTP Destination should start with http://: or https://: {}",
 			dest,
@@ -386,6 +397,7 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
 		message: message,
 		minimum_confirmations: min_c,
 		selection_strategy: selection_strategy.to_owned(),
+		estimate_selection_strategies,
 		method: method.to_owned(),
 		dest: dest.to_owned(),
 		change_outputs: change_outputs,
@@ -562,7 +574,11 @@ pub fn wallet_command(
 		}
 		("send", Some(args)) => {
 			let a = arg_parse!(parse_send_args(&args));
-			command::send(inst_wallet(), a)
+			command::send(
+				inst_wallet(),
+				a,
+				wallet_config.dark_background_color_scheme.unwrap_or(true),
+			)
 		}
 		("receive", Some(args)) => {
 			let a = arg_parse!(parse_receive_args(&args));

src/bin/grin.yml

@@ -157,6 +157,10 @@ subcommands:
                     - smallest
                   default_value: all
                   takes_value: true
+              - estimate_selection_strategies:
+                  help: Estimates all possible Coin/Output selection strategies.
+                  short: e
+                  long: estimate-selection
               - change_outputs:
                   help: Number of change outputs to generate (mainly for testing)
                   short: o

wallet/src/command.rs

@@ -200,6 +200,7 @@ pub struct SendArgs {
 	pub message: Option<String>,
 	pub minimum_confirmations: u64,
 	pub selection_strategy: String,
+	pub estimate_selection_strategies: bool,
 	pub method: String,
 	pub dest: String,
 	pub change_outputs: usize,
@@ -210,68 +211,89 @@ pub struct SendArgs {
 pub fn send(
 	wallet: Arc<Mutex<WalletInst<impl NodeClient + 'static, keychain::ExtKeychain>>>,
 	args: SendArgs,
+	dark_scheme: bool,
 ) -> Result<(), Error> {
 	controller::owner_single_use(wallet.clone(), |api| {
-		let result = api.initiate_tx(
-			None,
-			args.amount,
-			args.minimum_confirmations,
-			args.max_outputs,
-			args.change_outputs,
-			args.selection_strategy == "all",
-			args.message.clone(),
-		);
-		let (mut slate, lock_fn) = match result {
-			Ok(s) => {
-				info!(
-					"Tx created: {} grin to {} (strategy '{}')",
-					core::amount_to_hr_string(args.amount, false),
-					args.dest,
-					args.selection_strategy,
-				);
-				s
-			}
-			Err(e) => {
-				info!("Tx not created: {}", e);
-				return Err(e);
-			}
-		};
-		let adapter = match args.method.as_str() {
-			"http" => HTTPWalletCommAdapter::new(),
-			"file" => FileWalletCommAdapter::new(),
-			"keybase" => KeybaseWalletCommAdapter::new(),
-			"self" => NullWalletCommAdapter::new(),
-			_ => NullWalletCommAdapter::new(),
-		};
-		if adapter.supports_sync() {
-			slate = adapter.send_tx_sync(&args.dest, &slate)?;
-			api.tx_lock_outputs(&slate, lock_fn)?;
-			if args.method == "self" {
-				controller::foreign_single_use(wallet, |api| {
-					api.receive_tx(&mut slate, Some(&args.dest), None)?;
-					Ok(())
-				})?;
-			}
-			if let Err(e) = api.verify_slate_messages(&slate) {
-				error!("Error validating participant messages: {}", e);
-				return Err(e);
-			}
-			api.finalize_tx(&mut slate)?;
+		if args.estimate_selection_strategies {
+			let strategies = vec!["smallest", "all"]
+				.into_iter()
+				.map(|strategy| {
+					let (total, fee) = api
+						.estimate_initiate_tx(
+							None,
+							args.amount,
+							args.minimum_confirmations,
+							args.max_outputs,
+							args.change_outputs,
+							strategy == "all",
+						)
+						.unwrap();
+					(strategy, total, fee)
+				})
+				.collect();
+			display::estimate(args.amount, strategies, dark_scheme);
 		} else {
-			adapter.send_tx_async(&args.dest, &slate)?;
-			api.tx_lock_outputs(&slate, lock_fn)?;
-		}
-		if adapter.supports_sync() {
-			let result = api.post_tx(&slate.tx, args.fluff);
-			match result {
-				Ok(_) => {
-					info!("Tx sent ok",);
-					return Ok(());
+			let result = api.initiate_tx(
+				None,
+				args.amount,
+				args.minimum_confirmations,
+				args.max_outputs,
+				args.change_outputs,
+				args.selection_strategy == "all",
+				args.message.clone(),
+			);
+			let (mut slate, lock_fn) = match result {
+				Ok(s) => {
+					info!(
+						"Tx created: {} grin to {} (strategy '{}')",
+						core::amount_to_hr_string(args.amount, false),
+						args.dest,
+						args.selection_strategy,
+					);
+					s
 				}
 				Err(e) => {
-					error!("Tx sent fail: {}", e);
+					info!("Tx not created: {}", e);
 					return Err(e);
 				}
+			};
+			let adapter = match args.method.as_str() {
+				"http" => HTTPWalletCommAdapter::new(),
+				"file" => FileWalletCommAdapter::new(),
+				"keybase" => KeybaseWalletCommAdapter::new(),
+				"self" => NullWalletCommAdapter::new(),
+				_ => NullWalletCommAdapter::new(),
+			};
+			if adapter.supports_sync() {
+				slate = adapter.send_tx_sync(&args.dest, &slate)?;
+				api.tx_lock_outputs(&slate, lock_fn)?;
+				if args.method == "self" {
+					controller::foreign_single_use(wallet, |api| {
+						api.receive_tx(&mut slate, Some(&args.dest), None)?;
+						Ok(())
+					})?;
+				}
+				if let Err(e) = api.verify_slate_messages(&slate) {
+					error!("Error validating participant messages: {}", e);
+					return Err(e);
+				}
+				api.finalize_tx(&mut slate)?;
+			} else {
+				adapter.send_tx_async(&args.dest, &slate)?;
+				api.tx_lock_outputs(&slate, lock_fn)?;
+			}
+			if adapter.supports_sync() {
+				let result = api.post_tx(&slate.tx, args.fluff);
+				match result {
+					Ok(_) => {
+						info!("Tx sent ok",);
+						return Ok(());
+					}
+					Err(e) => {
+						error!("Tx sent fail: {}", e);
+						return Err(e);
+					}
+				}
 			}
 		}
 		Ok(())

wallet/src/display.rs

@@ -338,6 +338,49 @@ pub fn info(
 		);
 	}
 }
+
+/// Display summary info in a pretty way
+pub fn estimate(
+	amount: u64,
+	strategies: Vec<(
+		&str, // strategy
+		u64,  // total amount to be locked
+		u64,  // fee
+	)>,
+	dark_background_color_scheme: bool,
+) {
+	println!(
+		"\nEstimation for sending {}:\n",
+		amount_to_hr_string(amount, false)
+	);
+
+	let mut table = table!();
+
+	table.set_titles(row![
+		bMG->"Selection strategy",
+		bMG->"Fee",
+		bMG->"Will be locked",
+	]);
+
+	for (strategy, total, fee) in strategies {
+		if dark_background_color_scheme {
+			table.add_row(row![
+				bFC->strategy,
+				FR->amount_to_hr_string(fee, false),
+				FY->amount_to_hr_string(total, false),
+			]);
+		} else {
+			table.add_row(row![
+				bFD->strategy,
+				FR->amount_to_hr_string(fee, false),
+				FY->amount_to_hr_string(total, false),
+			]);
+		}
+	}
+	table.printstd();
+	println!();
+}
+
 /// Display list of wallet accounts in a pretty way
 pub fn accounts(acct_mappings: Vec<AcctPathMapping>) {
 	println!("\n____ Wallet Accounts ____\n",);

wallet/src/libwallet/api.rs

@@ -671,6 +671,74 @@ where
 		Ok((slate, lock_fn))
 	}
 
+	/// Estimates the amount to be locked and fee for the transaction without creating one
+	///
+	/// # Arguments
+	/// * `src_acct_name` - The human readable account name from which to draw outputs
+	/// for the transaction, overriding whatever the active account is as set via the
+	/// [`set_active_account`](struct.APIOwner.html#method.set_active_account) method.
+	/// If None, the transaction will use the active account.
+	/// * `amount` - The amount to send, in nanogrins. (`1 G = 1_000_000_000nG`)
+	/// * `minimum_confirmations` - The minimum number of confirmations an output
+	/// should have in order to be included in the transaction.
+	/// * `max_outputs` - By default, the wallet selects as many inputs as possible in a
+	/// transaction, to reduce the Output set and the fees. The wallet will attempt to spend
+	/// include up to `max_outputs` in a transaction, however if this is not enough to cover
+	/// the whole amount, the wallet will include more outputs. This parameter should be considered
+	/// a soft limit.
+	/// * `num_change_outputs` - The target number of change outputs to create in the transaction.
+	/// The actual number created will be `num_change_outputs` + whatever remainder is needed.
+	/// * `selection_strategy_is_use_all` - If `true`, attempt to use up as many outputs as
+	/// possible to create the transaction, up the 'soft limit' of `max_outputs`. This helps
+	/// to reduce the size of the UTXO set and the amount of data stored in the wallet, and
+	/// minimizes fees. This will generally result in many inputs and a large change output(s),
+	/// usually much larger than the amount being sent. If `false`, the transaction will include
+	/// as many outputs as are needed to meet the amount, (and no more) starting with the smallest
+	/// value outputs.
+	///
+	/// # Returns
+	/// * a result containing:
+	/// * (total, fee) - A tuple:
+	/// * Total amount to be locked.
+	/// * Transaction fee
+	pub fn estimate_initiate_tx(
+		&mut self,
+		src_acct_name: Option<&str>,
+		amount: u64,
+		minimum_confirmations: u64,
+		max_outputs: usize,
+		num_change_outputs: usize,
+		selection_strategy_is_use_all: bool,
+	) -> Result<
+		(
+			u64, // total
+			u64, // fee
+		),
+		Error,
+	> {
+		let mut w = self.wallet.lock();
+		w.open_with_credentials()?;
+		let parent_key_id = match src_acct_name {
+			Some(d) => {
+				let pm = w.get_acct_path(d.to_owned())?;
+				match pm {
+					Some(p) => p.path,
+					None => w.parent_key_id(),
+				}
+			}
+			None => w.parent_key_id(),
+		};
+		tx::estimate_send_tx(
+			&mut *w,
+			amount,
+			minimum_confirmations,
+			max_outputs,
+			num_change_outputs,
+			selection_strategy_is_use_all,
+			&parent_key_id,
+		)
+	}
+
 	/// Lock outputs associated with a given slate/transaction
 	pub fn tx_lock_outputs(
 		&mut self,

wallet/src/libwallet/internal/selection.rs

@@ -242,6 +242,52 @@ pub fn select_send_tx<T: ?Sized, C, K>(
 	),
 	Error,
 >
+where
+	T: WalletBackend<C, K>,
+	C: NodeClient,
+	K: Keychain,
+{
+	let (coins, _total, amount, fee) = select_coins_and_fee(
+		wallet,
+		amount,
+		current_height,
+		minimum_confirmations,
+		max_outputs,
+		change_outputs,
+		selection_strategy_is_use_all,
+		&parent_key_id,
+	)?;
+
+	// build transaction skeleton with inputs and change
+	let (mut parts, change_amounts_derivations) =
+		inputs_and_change(&coins, wallet, amount, fee, change_outputs)?;
+
+	// This is more proof of concept than anything but here we set lock_height
+	// on tx being sent (based on current chain height via api).
+	parts.push(build::with_lock_height(lock_height));
+
+	Ok((parts, coins, change_amounts_derivations, fee))
+}
+
+/// Select outputs and calculating fee.
+pub fn select_coins_and_fee<T: ?Sized, C, K>(
+	wallet: &mut T,
+	amount: u64,
+	current_height: u64,
+	minimum_confirmations: u64,
+	max_outputs: usize,
+	change_outputs: usize,
+	selection_strategy_is_use_all: bool,
+	parent_key_id: &Identifier,
+) -> Result<
+	(
+		Vec<OutputData>,
+		u64, // total
+		u64, // amount
+		u64, // fee
+	),
+	Error,
+>
 where
 	T: WalletBackend<C, K>,
 	C: NodeClient,
@@ -325,16 +371,7 @@ where
 			amount_with_fee = amount + fee;
 		}
 	}
-
-	// build transaction skeleton with inputs and change
-	let (mut parts, change_amounts_derivations) =
-		inputs_and_change(&coins, wallet, amount, fee, change_outputs)?;
-
-	// This is more proof of concept than anything but here we set lock_height
-	// on tx being sent (based on current chain height via api).
-	parts.push(build::with_lock_height(lock_height));
-
-	Ok((parts, coins, change_amounts_derivations, fee))
+	Ok((coins, total, amount, fee))
 }
 
 /// Selects inputs and change for a transaction

wallet/src/libwallet/internal/tx.rs

@@ -42,6 +42,52 @@ where
 	Ok(slate)
 }
 
+/// Estimates locked amount and fee for the transaction without creating one
+pub fn estimate_send_tx<T: ?Sized, C, K>(
+	wallet: &mut T,
+	amount: u64,
+	minimum_confirmations: u64,
+	max_outputs: usize,
+	num_change_outputs: usize,
+	selection_strategy_is_use_all: bool,
+	parent_key_id: &Identifier,
+) -> Result<
+	(
+		u64, // total
+		u64, // fee
+	),
+	Error,
+>
+where
+	T: WalletBackend<C, K>,
+	C: NodeClient,
+	K: Keychain,
+{
+	// Get lock height
+	let current_height = wallet.w2n_client().get_chain_height()?;
+	// ensure outputs we're selecting are up to date
+	updater::refresh_outputs(wallet, parent_key_id, false)?;
+
+	// Sender selects outputs into a new slate and save our corresponding keys in
+	// a transaction context. The secret key in our transaction context will be
+	// randomly selected. This returns the public slate, and a closure that locks
+	// our inputs and outputs once we're convinced the transaction exchange went
+	// according to plan
+	// This function is just a big helper to do all of that, in theory
+	// this process can be split up in any way
+	let (_coins, total, _amount, fee) = selection::select_coins_and_fee(
+		wallet,
+		amount,
+		current_height,
+		minimum_confirmations,
+		max_outputs,
+		num_change_outputs,
+		selection_strategy_is_use_all,
+		parent_key_id,
+	)?;
+	Ok((total, fee))
+}
+
 /// Add inputs to the slate (effectively becoming the sender)
 pub fn add_inputs_to_slate<T: ?Sized, C, K>(
 	wallet: &mut T,

wallet/tests/transaction.rs

@@ -226,6 +226,33 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> {
 		Ok(())
 	})?;
 
+	// Estimate fee and locked amount for a transaction
+	wallet::controller::owner_single_use(wallet1.clone(), |sender_api| {
+		let (total, fee) = sender_api.estimate_initiate_tx(
+			None,
+			amount * 2, // amount
+			2,          // minimum confirmations
+			500,        // max outputs
+			1,          // num change outputs
+			true,       // select all outputs
+		)?;
+		assert_eq!(total, 600_000_000_000);
+		assert_eq!(fee, 4_000_000);
+
+		let (total, fee) = sender_api.estimate_initiate_tx(
+			None,
+			amount * 2, // amount
+			2,          // minimum confirmations
+			500,        // max outputs
+			1,          // num change outputs
+			false,      // select the smallest amount of outputs
+		)?;
+		assert_eq!(total, 180_000_000_000);
+		assert_eq!(fee, 6_000_000);
+
+		Ok(())
+	})?;
+
 	// Send another transaction, but don't post to chain immediately and use
 	// the stored transaction instead
 	wallet::controller::owner_single_use(wallet1.clone(), |sender_api| {