Prioritize and allow no change transaction (#1009)

Prioritize and allow no change transaction

wallet/src/receiver.rs

@@ -43,7 +43,7 @@ pub struct TxWrapper {
 /// Return result of part 2, Recipient Initation, to sender
 /// -Receiver receives inputs, outputs xS * G and kS * G
 /// -Receiver picks random blinding factors for all outputs being received,
-/// computes total blinding     
+/// computes total blinding
 /// excess xR
 /// -Receiver picks random nonce kR
 /// -Receiver computes Schnorr challenge e = H(M | kR * G + kS * G)
@@ -69,7 +69,7 @@ fn handle_sender_initiation(
 		tx.input_proofs_count(),
 		None,
 	);
-	if fee != tx.fee() {
+	if fee > tx.fee() {
 		return Err(ErrorKind::FeeDispute {
 			sender_fee: tx.fee(),
 			recipient_fee: fee,
@@ -89,7 +89,7 @@ fn handle_sender_initiation(
 		})?;
 	}
 
-	let out_amount = amount - fee;
+	let out_amount = amount - tx.fee();
 
 	// First step is just to get the excess sum of the outputs we're participating
 	// in Output and key needs to be stored until transaction finalisation time,
@@ -131,7 +131,12 @@ fn handle_sender_initiation(
 	keychain.aggsig_add_output(&partial_tx.id, &key_id);
 
 	let sig_part = keychain
-		.aggsig_calculate_partial_sig(&partial_tx.id, &sender_pub_nonce, fee, tx.lock_height())
+		.aggsig_calculate_partial_sig(
+			&partial_tx.id,
+			&sender_pub_nonce,
+			tx.fee(),
+			tx.lock_height(),
+		)
 		.unwrap();
 
 	// Build the response, which should contain sR, blinding excess xR * G, public
@@ -152,8 +157,8 @@ fn handle_sender_initiation(
 /// Receive Part 3 of interactive transactions from sender, Sender Confirmation
 /// Return Ok/Error
 /// -Receiver receives sS
-/// -Receiver verifies sender's sig, by verifying that 
+/// -Receiver verifies sender's sig, by verifying that
-/// kS * G + e *xS * G = sS* G 
+/// kS * G + e *xS * G = sS* G
 /// -Receiver calculates final sig as s=(sS+sR, kS * G+kR * G)
 /// -Receiver puts into TX kernel:
 ///
@@ -414,7 +419,7 @@ fn build_final_transaction(
 		tx.input_proofs_count(),
 		None,
 	);
-	if fee != tx.fee() {
+	if fee > tx.fee() {
 		return Err(ErrorKind::FeeDispute {
 			sender_fee: tx.fee(),
 			recipient_fee: fee,
@@ -434,7 +439,7 @@ fn build_final_transaction(
 		})?;
 	}
 
-	let out_amount = amount - fee;
+	let out_amount = amount - tx.fee();
 
 	// Get output we created in earlier step
 	// TODO: will just be one for now, support multiple later

wallet/src/sender.rs

@@ -104,8 +104,13 @@ pub fn issue_send_tx(
 	// failure.
 	let rollback_wallet = || {
 		WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
-			info!(LOGGER, "cleaning up unused change output from wallet");
+			match change_key.clone() {
-			wallet_data.delete_output(&change_key);
+				Some(change) => {
+					info!(LOGGER, "cleaning up unused change output from wallet");
+					wallet_data.delete_output(&change);
+				}
+				None => info!(LOGGER, "No change output to clean from wallet"),
+			}
 		})
 	};
 
@@ -120,8 +125,13 @@ pub fn issue_send_tx(
 
 	if &dest[..4] != "http" {
 		WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
-			info!(LOGGER, "cleaning up unused change output from wallet");
+			match change_key.clone() {
-			wallet_data.delete_output(&change_key);
+				Some(change) => {
+					info!(LOGGER, "cleaning up unused change output from wallet");
+					wallet_data.delete_output(&change);
+				}
+				None => info!(LOGGER, "No change output to clean from wallet"),
+			}
 		}).unwrap();
 		panic!(
 			"dest formatted as {} but send -d expected stdout or http://IP:port",
@@ -229,7 +239,7 @@ fn build_send_tx(
 		Transaction,
 		BlindingFactor,
 		Vec<OutputData>,
-		Identifier,
+		Option<Identifier>,
 		u64,
 	),
 	Error,
@@ -263,32 +273,40 @@ fn build_send_tx(
 	// sender is responsible for setting the fee on the partial tx
 	// recipient should double check the fee calculation and not blindly trust the
 	// sender
-	let mut fee = tx_fee(coins.len(), 2, coins_proof_count(&coins), None);
+	let mut fee;
+	// First attempt to spend without change
+	fee = tx_fee(coins.len(), 1, coins_proof_count(&coins), None);
 	let mut total: u64 = coins.iter().map(|c| c.value).sum();
 	let mut amount_with_fee = amount + fee;
 
-	// Here check if we have enough outputs for the amount including fee otherwise
+	// Check if we need to use a change address
-	// look for other outputs and check again
+	if total > amount_with_fee {
-	while total <= amount_with_fee {
-		// End the loop if we have selected all the outputs and still not enough funds
-		if coins.len() == max_outputs {
-			return Err(ErrorKind::NotEnoughFunds(total as u64))?;
-		}
-
-		// select some spendable coins from the wallet
-		coins = WalletData::read_wallet(&config.data_file_dir, |wallet_data| {
-			Ok(wallet_data.select_coins(
-				key_id.clone(),
-				amount_with_fee,
-				current_height,
-				minimum_confirmations,
-				max_outputs,
-				selection_strategy_is_use_all,
-			))
-		})?;
 		fee = tx_fee(coins.len(), 2, coins_proof_count(&coins), None);
-		total = coins.iter().map(|c| c.value).sum();
 		amount_with_fee = amount + fee;
+
+		// Here check if we have enough outputs for the amount including fee otherwise
+		// look for other outputs and check again
+		while total < amount_with_fee {
+			// End the loop if we have selected all the outputs and still not enough funds
+			if coins.len() == max_outputs {
+				return Err(ErrorKind::NotEnoughFunds(total as u64))?;
+			}
+
+			// select some spendable coins from the wallet
+			coins = WalletData::read_wallet(&config.data_file_dir, |wallet_data| {
+				Ok(wallet_data.select_coins(
+					key_id.clone(),
+					amount_with_fee,
+					current_height,
+					minimum_confirmations,
+					max_outputs,
+					selection_strategy_is_use_all,
+				))
+			})?;
+			fee = tx_fee(coins.len(), 2, coins_proof_count(&coins), None);
+			total = coins.iter().map(|c| c.value).sum();
+			amount_with_fee = amount + fee;
+		}
 	}
 
 	// build transaction skeleton with inputs and change
@@ -360,7 +378,7 @@ fn inputs_and_change(
 	keychain: &Keychain,
 	amount: u64,
 	fee: u64,
-) -> Result<(Vec<Box<build::Append>>, Identifier), Error> {
+) -> Result<(Vec<Box<build::Append>>, Option<Identifier>), Error> {
 	let mut parts = vec![];
 
 	// calculate the total across all inputs, and how much is left
@@ -393,30 +411,34 @@ fn inputs_and_change(
 			parts.push(build::input(coin.value, key_id));
 		}
 	}
+	let change_key;
+	if change != 0 {
+		// track the output representing our change
+		change_key = WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
+			let root_key_id = keychain.root_key_id();
+			let change_derivation = wallet_data.next_child(root_key_id.clone());
+			let change_key = keychain.derive_key_id(change_derivation).unwrap();
 
-	// track the output representing our change
+			wallet_data.add_output(OutputData {
-	let change_key = WalletData::with_wallet(&config.data_file_dir, |wallet_data| {
+				root_key_id: root_key_id.clone(),
-		let root_key_id = keychain.root_key_id();
+				key_id: change_key.clone(),
-		let change_derivation = wallet_data.next_child(root_key_id.clone());
+				n_child: change_derivation,
-		let change_key = keychain.derive_key_id(change_derivation).unwrap();
+				value: change as u64,
+				status: OutputStatus::Unconfirmed,
+				height: 0,
+				lock_height: 0,
+				is_coinbase: false,
+				block: None,
+				merkle_proof: None,
+			});
 
-		wallet_data.add_output(OutputData {
+			Some(change_key)
-			root_key_id: root_key_id.clone(),
+		})?;
-			key_id: change_key.clone(),
-			n_child: change_derivation,
-			value: change as u64,
-			status: OutputStatus::Unconfirmed,
-			height: 0,
-			lock_height: 0,
-			is_coinbase: false,
-			block: None,
-			merkle_proof: None,
-		});
 
-		change_key
+		parts.push(build::output(change, change_key.clone().unwrap()));
-	})?;
+	} else {
-
+		change_key = None
-	parts.push(build::output(change, change_key.clone()));
+	}
 
 	Ok((parts, change_key))
 }