Implement fee inclusive transactions (#657)

* Add amount_includes_fee option in TX building

* Add --amount_includes_fee CLI option

* Implement send 'max' amount

Co-authored-by: cliik <cliik@example.com>

controller/src/command.rs

@@ -322,6 +322,8 @@ where
 #[derive(Clone)]
 pub struct SendArgs {
 	pub amount: u64,
+	pub amount_includes_fee: bool,
+	pub use_max_amount: bool,
 	pub minimum_confirmations: u64,
 	pub selection_strategy: String,
 	pub estimate_selection_strategies: bool,
@@ -353,6 +355,15 @@ where
 	K: keychain::Keychain + 'static,
 {
 	let mut slate = Slate::blank(2, false);
+	let mut amount = args.amount;
+	if args.use_max_amount {
+		controller::owner_single_use(None, keychain_mask, Some(owner_api), |api, m| {
+			let (_, wallet_info) =
+				api.retrieve_summary_info(m, true, args.minimum_confirmations)?;
+			amount = wallet_info.amount_currently_spendable;
+			Ok(())
+		})?;
+	};
 	controller::owner_single_use(None, keychain_mask, Some(owner_api), |api, m| {
 		if args.estimate_selection_strategies {
 			let strategies = vec!["smallest", "all"]
@@ -360,7 +371,8 @@ where
 				.map(|strategy| {
 					let init_args = InitTxArgs {
 						src_acct_name: None,
-						amount: args.amount,
+						amount: amount,
+						amount_includes_fee: Some(args.amount_includes_fee),
 						minimum_confirmations: args.minimum_confirmations,
 						max_outputs: args.max_outputs as u32,
 						num_change_outputs: args.change_outputs as u32,
@@ -372,12 +384,13 @@ where
 					Ok((strategy, slate.amount, slate.fee_fields))
 				})
 				.collect::<Result<Vec<_>, grin_wallet_libwallet::Error>>()?;
-			display::estimate(args.amount, strategies, dark_scheme);
+			display::estimate(amount, strategies, dark_scheme);
 			return Ok(());
 		} else {
 			let init_args = InitTxArgs {
 				src_acct_name: None,
-				amount: args.amount,
+				amount: amount,
+				amount_includes_fee: Some(args.amount_includes_fee),
 				minimum_confirmations: args.minimum_confirmations,
 				max_outputs: args.max_outputs as u32,
 				num_change_outputs: args.change_outputs as u32,
@@ -394,7 +407,7 @@ where
 				Ok(s) => {
 					info!(
 						"Tx created: {} grin to {} (strategy '{}')",
-						core::amount_to_hr_string(args.amount, false),
+						core::amount_to_hr_string(amount, false),
 						args.dest,
 						args.selection_strategy,
 					);

controller/tests/transaction.rs

@@ -354,6 +354,47 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error>
 		Ok(())
 	})?;
 
+	// try to send a transaction with amount inclusive of fees, but amount too
+	// small to cover fees. Should fail.
+	wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |sender_api, m| {
+		// note this will increment the block count as part of the transaction "Posting"
+		let args = InitTxArgs {
+			src_acct_name: None,
+			amount: 1,
+			amount_includes_fee: Some(true),
+			minimum_confirmations: 2,
+			max_outputs: 500,
+			num_change_outputs: 1,
+			selection_strategy_is_use_all: true,
+			..Default::default()
+		};
+		let res = sender_api.init_send_tx(m, args);
+		assert!(res.is_err());
+		Ok(())
+	})?;
+
+	// try to build a transaction with amount inclusive of fees. Confirm that tx
+	// amount + fee is equal to the originally specified amount
+	let amount = 60_000_000_000;
+	wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |sender_api, m| {
+		// note this will increment the block count as part of the transaction "Posting"
+		let args = InitTxArgs {
+			src_acct_name: None,
+			amount: amount,
+			amount_includes_fee: Some(true),
+			minimum_confirmations: 2,
+			max_outputs: 500,
+			num_change_outputs: 1,
+			selection_strategy_is_use_all: true,
+			..Default::default()
+		};
+		let slate_i = sender_api.init_send_tx(m, args)?;
+		assert_eq!(slate_i.state, SlateState::Standard1);
+		let total_spend: u64 = slate_i.amount + slate_i.fee_fields.fee();
+		assert_eq!(amount, total_spend);
+		Ok(())
+	})?;
+
 	// let logging finish
 	stopper.store(false, Ordering::Relaxed);
 	thread::sleep(Duration::from_millis(200));

libwallet/src/api_impl/owner.rs

@@ -508,6 +508,7 @@ where
 			&mut *w,
 			keychain_mask,
 			args.amount,
+			args.amount_includes_fee.unwrap_or(false),
 			args.minimum_confirmations,
 			args.max_outputs as usize,
 			args.num_change_outputs as usize,
@@ -543,6 +544,7 @@ where
 			&parent_key_id,
 			true,
 			use_test_rng,
+			args.amount_includes_fee.unwrap_or(false),
 		)?
 	};
 
@@ -696,6 +698,7 @@ where
 		&parent_key_id,
 		false,
 		use_test_rng,
+		false,
 	)?;
 
 	let keychain = w.keychain(keychain_mask)?;
@@ -823,6 +826,7 @@ where
 			parent_key_id.clone(),
 			false,
 			true,
+			false,
 		)?;
 
 		// Add inputs and outputs to original context

libwallet/src/api_impl/types.rs

@@ -40,6 +40,8 @@ pub struct InitTxArgs {
 	#[serde(with = "secp_ser::string_or_u64")]
 	/// The amount to send, in nanogrins. (`1 G = 1_000_000_000nG`)
 	pub amount: u64,
+	/// Does the amount include the fee, or will fees be spent in addition to the amount?
+	pub amount_includes_fee: Option<bool>,
 	#[serde(with = "secp_ser::string_or_u64")]
 	/// The minimum number of confirmations an output
 	/// should have in order to be included in the transaction.
@@ -105,6 +107,7 @@ impl Default for InitTxArgs {
 		InitTxArgs {
 			src_acct_name: None,
 			amount: 0,
+			amount_includes_fee: None,
 			minimum_confirmations: 10,
 			max_outputs: 500,
 			num_change_outputs: 1,

libwallet/src/internal/selection.rs

@@ -51,6 +51,7 @@ pub fn build_send_tx<'a, T: ?Sized, C, K>(
 	parent_key_id: Identifier,
 	use_test_nonce: bool,
 	is_initiator: bool,
+	amount_includes_fee: bool,
 ) -> Result<Context, Error>
 where
 	T: WalletBackend<'a, C, K>,
@@ -61,6 +62,7 @@ where
 		wallet,
 		keychain_mask,
 		slate.amount,
+		amount_includes_fee,
 		current_height,
 		minimum_confirmations,
 		max_outputs,
@@ -69,6 +71,14 @@ where
 		&parent_key_id,
 		false,
 	)?;
+	if amount_includes_fee {
+		slate.amount = slate
+			.amount
+			.checked_sub(fee)
+			.ok_or(ErrorKind::GenericError(
+				format!("Transaction amount is too small to include fee").into(),
+			))?;
+	};
 
 	if fixed_fee.map(|f| fee != f).unwrap_or(false) {
 		return Err(ErrorKind::Fee("The initially selected fee is not sufficient".into()).into());
@@ -312,6 +322,7 @@ pub fn select_send_tx<'a, T: ?Sized, C, K, B>(
 	wallet: &mut T,
 	keychain_mask: Option<&SecretKey>,
 	amount: u64,
+	amount_includes_fee: bool,
 	current_height: u64,
 	minimum_confirmations: u64,
 	max_outputs: usize,
@@ -337,6 +348,7 @@ where
 	let (coins, _total, amount, fee) = select_coins_and_fee(
 		wallet,
 		amount,
+		amount_includes_fee,
 		current_height,
 		minimum_confirmations,
 		max_outputs,
@@ -363,6 +375,7 @@ where
 pub fn select_coins_and_fee<'a, T: ?Sized, C, K>(
 	wallet: &mut T,
 	amount: u64,
+	amount_includes_fee: bool,
 	current_height: u64,
 	minimum_confirmations: u64,
 	max_outputs: usize,
@@ -401,7 +414,10 @@ where
 	// First attempt to spend without change
 	let mut fee = tx_fee(coins.len(), 1, 1);
 	let mut total: u64 = coins.iter().map(|c| c.value).sum();
-	let mut amount_with_fee = amount + fee;
+	let mut amount_with_fee = match amount_includes_fee {
+		true => amount,
+		false => amount + fee,
+	};
 
 	if total == 0 {
 		return Err(ErrorKind::NotEnoughFunds {
@@ -429,7 +445,10 @@ where
 	// We need to add a change address or amount with fee is more than total
 	if total != amount_with_fee {
 		fee = tx_fee(coins.len(), num_outputs, 1);
-		amount_with_fee = amount + fee;
+		amount_with_fee = match amount_includes_fee {
+			true => amount,
+			false => amount + fee,
+		};
 
 		// Here check if we have enough outputs for the amount including fee otherwise
 		// look for other outputs and check again
@@ -458,10 +477,21 @@ where
 			.1;
 			fee = tx_fee(coins.len(), num_outputs, 1);
 			total = coins.iter().map(|c| c.value).sum();
-			amount_with_fee = amount + fee;
+			amount_with_fee = match amount_includes_fee {
+				true => amount,
+				false => amount + fee,
+			};
 		}
 	}
-	Ok((coins, total, amount, fee))
+	// If original amount includes fee, the new amount should
+	// be reduced, to accommodate the fee.
+	let new_amount = match amount_includes_fee {
+		true => amount.checked_sub(fee).ok_or(ErrorKind::GenericError(
+			format!("Transaction amount is too small to include fee").into(),
+		))?,
+		false => amount,
+	};
+	Ok((coins, total, new_amount, fee))
 }
 
 /// Selects inputs and change for a transaction

libwallet/src/internal/tx.rs

@@ -96,6 +96,7 @@ pub fn estimate_send_tx<'a, T: ?Sized, C, K>(
 	wallet: &mut T,
 	keychain_mask: Option<&SecretKey>,
 	amount: u64,
+	amount_includes_fee: bool,
 	minimum_confirmations: u64,
 	max_outputs: usize,
 	num_change_outputs: usize,
@@ -128,6 +129,7 @@ where
 	let (_coins, total, _amount, fee) = selection::select_coins_and_fee(
 		wallet,
 		amount,
+		amount_includes_fee,
 		current_height,
 		minimum_confirmations,
 		max_outputs,
@@ -151,6 +153,7 @@ pub fn add_inputs_to_slate<'a, T: ?Sized, C, K>(
 	parent_key_id: &Identifier,
 	is_initiator: bool,
 	use_test_rng: bool,
+	amount_includes_fee: bool,
 ) -> Result<Context, Error>
 where
 	T: WalletBackend<'a, C, K>,
@@ -181,6 +184,7 @@ where
 		parent_key_id.clone(),
 		use_test_rng,
 		is_initiator,
+		amount_includes_fee,
 	)?;
 
 	// Generate a kernel offset and subtract from our context's secret key. Store
@@ -270,6 +274,7 @@ where
 	let (_coins, _total, _amount, fee) = selection::select_coins_and_fee(
 		wallet,
 		init_tx_args.amount,
+		init_tx_args.amount_includes_fee.unwrap_or(false),
 		current_height,
 		init_tx_args.minimum_confirmations,
 		init_tx_args.max_outputs as usize,

src/bin/grin-wallet.yml

@@ -109,7 +109,7 @@ subcommands:
       about: Builds a transaction to send coins and sends to the recipient via the Slatepack workflow
       args:
         - amount:
-            help: Number of coins to send with optional fraction, e.g. 12.423
+            help: Number of coins to send with optional fraction, e.g. 12.423. Keyword 'max' will send maximum amount.
             index: 1
         - minimum_confirmations:
             help: Minimum number of confirmations required for an output to be spendable
@@ -181,6 +181,9 @@ subcommands:
             help: Show slatepack data as QR code
             short: q
             long: slatepack_qr
+        - amount_includes_fee:
+            help: Transaction amount includes transaction fee. Recipient will receive (amount - fee).
+            long: amount_includes_fee
   - unpack:
       about: Unpack and display an armored Slatepack Message, decrypting if possible
       args:

src/cmd/wallet_args.rs

@@ -444,7 +444,11 @@ pub fn parse_account_args(account_args: &ArgMatches) -> Result<command::AccountA
 pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseError> {
 	// amount
 	let amount = parse_required(args, "amount")?;
-	let amount = core::core::amount_from_hr_string(amount);
+	let (amount, spend_max) = if amount.eq_ignore_ascii_case("max") {
+		(Ok(0), true)
+	} else {
+		(core::core::amount_from_hr_string(amount), false)
+	};
 	let amount = match amount {
 		Ok(a) => a,
 		Err(e) => {
@@ -455,6 +459,7 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
 			return Err(ParseError::ArgumentError(msg));
 		}
 	};
+	let amount_includes_fee = args.is_present("amount_includes_fee") || spend_max;
 
 	// minimum_confirmations
 	let min_c = parse_required(args, "minimum_confirmations")?;
@@ -524,6 +529,8 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
 
 	Ok(command::SendArgs {
 		amount: amount,
+		amount_includes_fee: amount_includes_fee,
+		use_max_amount: spend_max,
 		minimum_confirmations: min_c,
 		selection_strategy: selection_strategy.to_owned(),
 		estimate_selection_strategies,

tests/cmd_line_basic.rs

@@ -281,6 +281,85 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 		},
 	)?;
 
+	// Send to wallet 2 with --amount_includes_fee
+	let mut old_balance = 0;
+	grin_wallet_controller::controller::owner_single_use(
+		Some(wallet1.clone()),
+		mask1,
+		None,
+		|api, m| {
+			api.set_active_account(m, "mining")?;
+			let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?;
+			old_balance = wallet1_info.amount_currently_spendable;
+			Ok(())
+		},
+	)?;
+	let arg_vec = vec![
+		"grin-wallet",
+		"-p",
+		"password1",
+		"-a",
+		"mining",
+		"send",
+		"--amount_includes_fee",
+		"10",
+	];
+	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
+	let file_name = format!(
+		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b01.S1.slatepack",
+		test_dir
+	);
+	let arg_vec = vec![
+		"grin-wallet",
+		"-p",
+		"password2",
+		"-a",
+		"account_1",
+		"receive",
+		"-i",
+		&file_name,
+	];
+	execute_command(&app, test_dir, "wallet2", &client2, arg_vec.clone())?;
+	let file_name = format!(
+		"{}/wallet2/slatepack/0436430c-2b02-624c-2032-570501212b01.S2.slatepack",
+		test_dir
+	);
+	let arg_vec = vec![
+		"grin-wallet",
+		"-a",
+		"mining",
+		"-p",
+		"password1",
+		"finalize",
+		"-i",
+		&file_name,
+	];
+	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
+	bh += 1;
+
+	// Mine some blocks to confirm the transaction
+	let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 10, false);
+	bh += 10;
+
+	// Check the new balance of wallet 1 reduced by EXACTLY the tx amount (instead of amount + fee)
+	// This confirms that the TX amount was correctly computed to allow for the fee
+	grin_wallet_controller::controller::owner_single_use(
+		Some(wallet1.clone()),
+		mask1,
+		None,
+		|api, m| {
+			api.set_active_account(m, "mining")?;
+			let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?;
+			// make sure the new balance is exactly equal to the old balance - the tx amount + the amount mined since then
+			let amt_mined = 10 * 60_000_000_000;
+			assert_eq!(
+				wallet1_info.amount_currently_spendable + 10_000_000_000,
+				old_balance + amt_mined
+			);
+			Ok(())
+		},
+	)?;
+
 	// Send encrypted from wallet 1 to wallet 2
 	// output wallet 2's address for test creation purposes,
 	let arg_vec = vec![
@@ -308,7 +387,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
 	let file_name = format!(
-		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b01.S1.slatepack",
+		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b02.S1.slatepack",
 		test_dir
 	);
 	let arg_vec = vec![
@@ -324,7 +403,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 	execute_command(&app, test_dir, "wallet2", &client2, arg_vec.clone())?;
 
 	let file_name = format!(
-		"{}/wallet2/slatepack/0436430c-2b02-624c-2032-570501212b01.S2.slatepack",
+		"{}/wallet2/slatepack/0436430c-2b02-624c-2032-570501212b02.S2.slatepack",
 		test_dir
 	);
 
@@ -381,7 +460,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
 	let file_name = format!(
-		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b02.S1.slatepack",
+		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b03.S1.slatepack",
 		test_dir
 	);
 	let arg_vec = vec![
@@ -397,7 +476,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 	execute_command(&app, test_dir, "wallet1", &client1, arg_vec.clone())?;
 
 	let file_name = format!(
-		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b02.S2.slatepack",
+		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b03.S2.slatepack",
 		test_dir
 	);
 
@@ -475,7 +554,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 		"mining",
 		"cancel",
 		"-i",
-		"25",
+		"36",
 	];
 	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
@@ -483,7 +562,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 	let arg_vec = vec!["grin-wallet", "-p", "password2", "invoice", "65"];
 	execute_command(&app, test_dir, "wallet2", &client2, arg_vec)?;
 	let file_name = format!(
-		"{}/wallet2/slatepack/0436430c-2b02-624c-2032-570501212b05.I1.slatepack",
+		"{}/wallet2/slatepack/0436430c-2b02-624c-2032-570501212b06.I1.slatepack",
 		test_dir
 	);
 
@@ -501,7 +580,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
 	let file_name = format!(
-		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b05.I2.slatepack",
+		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b06.I2.slatepack",
 		test_dir
 	);
 
@@ -565,6 +644,73 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller::
 	let arg_vec = vec!["grin-wallet", "-p", "password2", "txs", "-t", &tx_id[..]];
 	execute_command(&app, test_dir, "wallet2", &client2, arg_vec)?;
 
+	// bit of mining
+	let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 10, false);
+
+	// Test wallet sweep
+	let arg_vec = vec![
+		"grin-wallet",
+		"-p",
+		"password1",
+		"-a",
+		"mining",
+		"send",
+		"max",
+	];
+	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
+	let file_name = format!(
+		"{}/wallet1/slatepack/0436430c-2b02-624c-2032-570501212b07.S1.slatepack",
+		test_dir
+	);
+	let arg_vec = vec![
+		"grin-wallet",
+		"-p",
+		"password2",
+		"-a",
+		"account_1",
+		"receive",
+		"-i",
+		&file_name,
+	];
+	execute_command(&app, test_dir, "wallet2", &client2, arg_vec.clone())?;
+	let file_name = format!(
+		"{}/wallet2/slatepack/0436430c-2b02-624c-2032-570501212b07.S2.slatepack",
+		test_dir
+	);
+	let arg_vec = vec![
+		"grin-wallet",
+		"-a",
+		"mining",
+		"-p",
+		"password1",
+		"finalize",
+		"-i",
+		&file_name,
+	];
+	execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
+
+	// Mine some blocks to confirm the transaction
+	let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 10, false);
+
+	// Check wallet 1 is now empty, except for immature coinbase outputs from recent mining),
+	// and recently matured coinbase outputs, which were not mature at time of spending.
+	// This confirms that the TX amount was correctly computed to allow for the fee
+	grin_wallet_controller::controller::owner_single_use(
+		Some(wallet1.clone()),
+		mask1,
+		None,
+		|api, m| {
+			api.set_active_account(m, "mining")?;
+			let (_, wallet1_info) = api.retrieve_summary_info(m, true, 10)?;
+			// Entire 'spendable' wallet balance should have been swept, except the coinbase outputs
+			// which matured in the last batch of mining. Check that the new spendable balance is
+			// exactly equal to those matured coins.
+			let amt_mined = 10 * 60_000_000_000;
+			assert_eq!(wallet1_info.amount_currently_spendable, amt_mined);
+			Ok(())
+		},
+	)?;
+
 	// let logging finish
 	thread::sleep(Duration::from_millis(200));
 	clean_output_dir(test_dir);

tests/data/v3_reqs/init_send_tx.req.json

@@ -6,6 +6,7 @@
 		"args": {
 			"src_acct_name": null,
 			"amount": "600000000000",
+			"amount_includes_fee": false,
 			"minimum_confirmations": 2,
 			"max_outputs": 500,
 			"num_change_outputs": 1,