Contracts: use checked conversions for amount -> i64

A large u64 amount could wrap when cast `as i64`. Convert with i64::try_from in
the CLI net-change derivation and in contract view, returning an error instead.
get_net_change / to_api_args now return Result (the two arg panics become
errors too), and the to_api_args call moves out of the owner-session closure.

Addresses PR #754 review (command.rs, view.rs).

controller/src/command.rs

@@ -1649,21 +1649,23 @@ pub struct ContractNewArgs {
 }
 
 impl ContractNewArgs {
-	fn get_net_change(&self) -> i64 {
+	fn get_net_change(&self) -> Result<i64, Error> {
-		// TODO: could the cast 'as i64' overflow or something?
+		let to_i64 = |v: u64| {
+			i64::try_from(v).map_err(|_| Error::ArgumentError(format!("Amount {} is too large", v)))
+		};
 		match self.receive {
 			None => match self.send {
-				None => panic!("Send or receive not specified."),
+				None => Err(Error::ArgumentError("Send or receive not specified.".into())),
-				Some(v) => -(v as i64), // negative net change on send
+				Some(v) => Ok(-to_i64(v)?), // negative net change on send
 			},
-			Some(v) => v as i64, // positive net change on receive
+			Some(v) => to_i64(v), // positive net change on receive
 		}
 	}
 
 	// Create a ContractNewArgsAPI from the ContractNewArgs
-	fn to_api_args(&self) -> ContractNewArgsAPI {
+	fn to_api_args(&self) -> Result<ContractNewArgsAPI, Error> {
-		let net_change = self.get_net_change();
+		let net_change = self.get_net_change()?;
-		ContractNewArgsAPI {
+		Ok(ContractNewArgsAPI {
 			setup_args: ContractSetupArgsAPI {
 				src_acct_name: match self.src_acct_name.as_ref() {
 					Some(v) => Some(v.to_string()),
@@ -1686,7 +1688,7 @@ impl ContractNewArgs {
 				proof_args: Default::default(),
 			},
 			..Default::default()
-		}
+		})
 	}
 }
 
@@ -1700,9 +1702,8 @@ where
 	C: NodeClient + 'static,
 	K: keychain::Keychain + 'static,
 {
+	let contract_new_args = args.to_api_args()?;
 	controller::owner_single_use(None, keychain_mask, Some(owner_api), |api, m| {
-		let contract_new_args = args.to_api_args();
-
 		let slate = api.contract_new(m, &contract_new_args)?;
 
 		print_slatepack(
@@ -1748,25 +1749,27 @@ pub struct ContractSetupArgs {
 }
 
 impl ContractSetupArgs {
-	fn get_net_change(&self) -> Option<i64> {
+	fn get_net_change(&self) -> Result<Option<i64>, Error> {
-		let mut net_change: Option<i64> = None;
-		// TODO: Check bounds before casting to i64.
 		if self.receive.is_some() && self.send.is_some() {
-			panic!("Can't pass both --receive and --send parameters.");
+			return Err(Error::ArgumentError(
+				"Can't pass both --receive and --send parameters.".into(),
+			));
 		}
-		if self.receive.is_some() {
+		let to_i64 = |v: u64| {
-			net_change = Some(self.receive.unwrap() as i64);
+			i64::try_from(v).map_err(|_| Error::ArgumentError(format!("Amount {} is too large", v)))
-		}
+		};
-		if self.send.is_some() {
+		let net_change = match (self.receive, self.send) {
-			net_change = Some(-(self.send.unwrap() as i64));
+			(Some(v), _) => Some(to_i64(v)?),
-		}
+			(_, Some(v)) => Some(-to_i64(v)?),
-		net_change
+			(None, None) => None,
+		};
+		Ok(net_change)
 	}
 
 	// Create a ContractSetupArgsAPI from the ContractSetupArgs
-	fn to_api_args(&self) -> ContractSetupArgsAPI {
+	fn to_api_args(&self) -> Result<ContractSetupArgsAPI, Error> {
-		let net_change = self.get_net_change();
+		let net_change = self.get_net_change()?;
-		ContractSetupArgsAPI {
+		Ok(ContractSetupArgsAPI {
 			// TODO: num_participants is derived here. It should be an Option and read from the slate.
 			// Need to check no attack are possible regarding kernel fee contribution.
 			net_change: net_change,
@@ -1783,7 +1786,7 @@ impl ContractSetupArgs {
 				..Default::default()
 			},
 			..Default::default()
-		}
+		})
 	}
 }
 
@@ -1864,6 +1867,8 @@ where
 	C: NodeClient + 'static,
 	K: keychain::Keychain + 'static,
 {
+	// Args for signing are just setup args
+	let contract_sign_args = args.to_api_args()?;
 	controller::owner_single_use(None, keychain_mask, Some(owner_api), |api, m| {
 		// Read the slatepack from stdin
 		println!("Paste slatepack:");
@@ -1872,9 +1877,6 @@ where
 			.read_line(&mut slatepack_msg)
 			.expect("Failed to read from stdin");
 
-		// Args for signing are just setup args
-		let contract_sign_args = args.to_api_args();
-
 		// Decrypt the slate, sign it and encrypt it for the next party
 		// TODO: Make sure you get the counterparty_addr and slate with 1 call.
 		let slatepack = owner::decode_slatepack_message(

libwallet/src/contract/actions/view.rs

@@ -37,10 +37,14 @@ where
 	// Otherwise, you can't really predict who the party doing the next step should be.
 
 	// TODO: Do we need to do any slate verification here?
+	// Checked conversion so an out-of-range amount can't wrap into a bogus net change.
 	let suggested_net_change: Option<i64> = match slate.state {
-		// TODO: Check bounds against overflow/underflow
+		SlateState::Invoice1 => Some(i64::try_from(slate.amount).map_err(|_| {
-		SlateState::Invoice1 => Some(slate.amount as i64),
+			Error::GenericError(format!("Slate amount {} exceeds i64", slate.amount))
-		SlateState::Standard1 => Some(-(slate.amount as i64)),
+		})?),
+		SlateState::Standard1 => Some(-i64::try_from(slate.amount).map_err(|_| {
+			Error::GenericError(format!("Slate amount {} exceeds i64", slate.amount))
+		})?),
 		_ => None,
 	};
 	let is_executed = false;