Contracts: make_outputs takes nanogrin amounts (Vec<u64>), not a string

OutputSelectionArgs.make_outputs was a comma-separated grin string parsed inside
libwallet via amount_from_hr_string. Move that parsing to the CLI boundary and
carry explicit nanogrin u64 amounts through the API and Context instead:
- OutputSelectionArgs.make_outputs: Option<String> -> Option<Vec<u64>>;
  output_amounts() becomes a plain accessor, num_custom_outputs() uses len().
- The CLI (wallet_args) parses grin -> nanogrin per amount; ContractNewArgs and
  ContractSetupArgs carry Vec<u64> and copy it through.
- Update the two tests that supplied make_outputs as a string.

Addresses PR #754 review (types.rs, wallet_args.rs).

controller/src/command.rs

@@ -1636,8 +1636,8 @@ pub struct ContractNewArgs {
 	pub as_json: bool,
 	/// Use the specified inputs (comma separated input commitments)
 	pub use_inputs: Option<String>,
-	/// How to separate outputs (command separated amounts)
-	pub make_outputs: Option<String>,
+	/// Output amounts in nanogrin (one entry per output)
+	pub make_outputs: Option<Vec<u64>>,
 
 	// Future features
 	/// Custom fee contribution
@@ -1679,10 +1679,7 @@ impl ContractNewArgs {
 						Some(v) => Some(v.to_string()),
 						None => None,
 					},
-					make_outputs: match self.make_outputs.as_ref() {
-						Some(v) => Some(v.to_string()),
-						None => None,
-					},
+					make_outputs: self.make_outputs.clone(),
 					..Default::default()
 				},
 				proof_args: Default::default(),
@@ -1734,8 +1731,8 @@ pub struct ContractSetupArgs {
 	pub as_json: bool,
 	/// Use the specified inputs (comma separated input commitments)
 	pub use_inputs: Option<String>,
-	/// How to separate outputs (command separated amounts)
-	pub make_outputs: Option<String>,
+	/// Output amounts in nanogrin (one entry per output)
+	pub make_outputs: Option<Vec<u64>>,
 
 	// Future features
 	/// Whether we should automatically sign a receive of any value
@@ -1779,10 +1776,7 @@ impl ContractSetupArgs {
 					Some(v) => Some(v.to_string()),
 					None => None,
 				},
-				make_outputs: match self.make_outputs.as_ref() {
-					Some(v) => Some(v.to_string()),
-					None => None,
-				},
+				make_outputs: self.make_outputs.clone(),
 				..Default::default()
 			},
 			..Default::default()

controller/tests/contract_self_spend_custom.rs

@@ -58,7 +58,14 @@ fn contract_self_spend_custom_tx_impl(test_dir: &'static str) -> Result<(), libw
 	let selection_args = OutputSelectionArgs {
 		min_input_confirmation: 0,
 		use_inputs: Some(use_inputs.clone()), // we will use two coinbase inputs
-		make_outputs: Some(String::from("88,35,3,0.2,15")), // the sum is such that it will need to pick another input making total of 3 inputs
+		// the sum is such that it will need to pick another input making total of 3 inputs
+		make_outputs: Some(vec![
+			88 * consensus::GRIN_BASE,
+			35 * consensus::GRIN_BASE,
+			3 * consensus::GRIN_BASE,
+			(0.2 * consensus::GRIN_BASE as f64) as u64,
+			15 * consensus::GRIN_BASE,
+		]),
 	};
 	wallet::controller::owner_single_use(Some(send_wallet.clone()), send_mask, None, |api, m| {
 		// Send wallet inititates a standard transaction with --send=0

libwallet/src/contract/selection.rs

@@ -545,7 +545,7 @@ mod tests {
 			net_change: Some(-2_000_000_000),
 			selection_args: OutputSelectionArgs {
 				// there is no abc5 input
-				make_outputs: Some(String::from("1,3")),
+				make_outputs: Some(vec![1_000_000_000, 3_000_000_000]),
 				..Default::default()
 			},
 			..Default::default()

libwallet/src/contract/types.rs

@@ -25,9 +25,9 @@ pub struct OutputSelectionArgs {
 	pub min_input_confirmation: u64,
 	/// Which inputs we want to use - default to payjoin if available with Some("any")
 	pub use_inputs: Option<String>,
-	/// Change output specification (comma separated amounts which don't include fee subtraction)
-	/// e.g. "3,1,4,0,0" describes 5 outputs two of which hold 0 value
-	pub make_outputs: Option<String>,
+	/// Change output specification: explicit output amounts in nanogrin, not including
+	/// fee subtraction. e.g. [3, 1, 4, 0, 0] describes 5 outputs, two of which hold 0 value.
+	pub make_outputs: Option<Vec<u64>>,
 }
 
 impl OutputSelectionArgs {
@@ -48,34 +48,19 @@ impl OutputSelectionArgs {
 			None
 		}
 	}
-	/// Returns the outputs we have to create
-	// TODO: move consensus code outside of here. Consider turning make_outputs to Vec<u64>
+	/// Returns the output amounts (nanogrin) we have to create. Amounts arrive already
+	/// parsed by the caller (e.g. the CLI), so this is just an accessor.
 	pub fn output_amounts(&self) -> Result<Vec<u64>, Error> {
-		match self.make_outputs.as_ref() {
-			Some(s) => s
-				.split(",")
-				.map(|amt| {
-					// Exact decimal parsing (integer nanogrin), not f64.
-					crate::grin_core::core::amount_from_hr_string(amt).map_err(|e| {
-						Error::GenericError(format!("Invalid output amount '{}': {}", amt, e))
-					})
-				})
-				.collect(),
-			None => Ok(vec![]),
-		}
+		Ok(self.make_outputs.clone().unwrap_or_default())
 	}
 	/// Returns the sum of our output amounts
 	pub fn sum_output_amounts(&self) -> Result<u64, Error> {
 		Ok(self.output_amounts()?.iter().sum())
 	}
-	/// Returns the number of custom outputs (count only; does not parse the amounts)
+	/// Returns the number of custom outputs
 	pub fn num_custom_outputs(&self) -> usize {
-		self.make_outputs
-			.as_ref()
-			.map(|s| s.split(",").count())
-			.unwrap_or(0)
+		self.make_outputs.as_ref().map(|v| v.len()).unwrap_or(0)
 	}
-
 }
 
 impl Default for OutputSelectionArgs {

src/cmd/wallet_args.rs

@@ -1020,12 +1020,13 @@ pub fn parse_contract_new_args(
 	};
 	let make_outputs = match args.value_of("make-outputs") {
 		Some(v) => {
-			// Validate each comma-separated amount here so a bad value returns a
-			// ParseError instead of panicking later in output_amounts().
+			// Parse each comma-separated grin amount into nanogrin at the CLI boundary,
+			// so the API and Context carry explicit u64 amounts.
+			let mut amounts = Vec::new();
 			for amt in v.split(',') {
-				parse_grin_amount(amt, "make-outputs")?;
+				amounts.push(parse_grin_amount(amt, "make-outputs")?);
 			}
-			Some(String::from(v))
+			Some(amounts)
 		}
 		None => None,
 	};
@@ -1096,12 +1097,13 @@ pub fn parse_contract_setup_args(
 	};
 	let make_outputs = match args.value_of("make-outputs") {
 		Some(v) => {
-			// Validate each comma-separated amount here so a bad value returns a
-			// ParseError instead of panicking later in output_amounts().
+			// Parse each comma-separated grin amount into nanogrin at the CLI boundary,
+			// so the API and Context carry explicit u64 amounts.
+			let mut amounts = Vec::new();
 			for amt in v.split(',') {
-				parse_grin_amount(amt, "make-outputs")?;
+				amounts.push(parse_grin_amount(amt, "make-outputs")?);
 			}
-			Some(String::from(v))
+			Some(amounts)
 		}
 		None => None,
 	};