Wallet amounts (#241)

* allow selecting a commit while providing a key index

* misnamed variable

* added static reference to libsecp that can be called throughout

* don't serialise rangeproof to json if it's not desired

* forgotten new file

* amounts input and displayed in wallet are now in full grins, with optional decimal place

* rustfmt

* merge branch

* better acknowledgement of transaction being sent

core/src/core/block.rs

@@ -344,8 +344,8 @@ impl Block {
 						..time::now_utc()
 					},
 					previous: prev.hash(),
-					total_difficulty: prev.pow.clone().to_difficulty()
+					total_difficulty: prev.pow.clone().to_difficulty() +
-						+ prev.total_difficulty.clone(),
+						prev.total_difficulty.clone(),
 					..Default::default()
 				},
 				inputs: inputs,
@@ -466,9 +466,7 @@ impl Block {
 			}
 
 			if k.lock_height > self.header.height {
-				return Err(Error::KernelLockHeight {
+				return Err(Error::KernelLockHeight { lock_height: k.lock_height });
-					lock_height: k.lock_height,
-				});
 			}
 		}
 
@@ -499,12 +497,13 @@ impl Block {
 	// * That the sum of blinding factors for all coinbase-marked outputs match
 	//   the coinbase-marked kernels.
 	fn verify_coinbase(&self, secp: &Secp256k1) -> Result<(), Error> {
-		let cb_outs = filter_map_vec!(self.outputs, |out| {
+		let cb_outs = filter_map_vec!(self.outputs, |out| if out.features.contains(
-			if out.features.contains(COINBASE_OUTPUT) {
+			COINBASE_OUTPUT,
+		)
+		{
 			Some(out.commitment())
 		} else {
 			None
-			}
 		});
 		let cb_kerns = filter_map_vec!(self.kernels, |k| if k.features.contains(COINBASE_KERNEL) {
 			Some(k.excess)

core/src/core/build.rs

@@ -40,8 +40,7 @@ pub struct Context<'a> {
 
 /// Function type returned by the transaction combinators. Transforms a
 /// (Transaction, BlindSum) pair into another, provided some context.
-pub type Append = for<'a> Fn(&'a mut Context, (Transaction, BlindSum))
+pub type Append = for<'a> Fn(&'a mut Context, (Transaction, BlindSum)) -> (Transaction, BlindSum);
-	-> (Transaction, BlindSum);
 
 /// Adds an input with the provided value and blinding key to the transaction
 /// being built.
@@ -133,11 +132,10 @@ pub fn transaction(
 	keychain: &keychain::Keychain,
 ) -> Result<(Transaction, BlindingFactor), keychain::Error> {
 	let mut ctx = Context { keychain };
-	let (mut tx, sum) = elems
+	let (mut tx, sum) = elems.iter().fold(
-		.iter()
+		(Transaction::empty(), BlindSum::new()),
-		.fold((Transaction::empty(), BlindSum::new()), |acc, elem| {
+		|acc, elem| elem(&mut ctx, acc),
-			elem(&mut ctx, acc)
+	);
-		});
 	let blind_sum = ctx.keychain.blind_sum(&sum)?;
 	let msg = secp::Message::from_slice(&kernel_sig_msg(tx.fee, tx.lock_height))?;
 	let sig = ctx.keychain.sign_with_blinding(&msg, &blind_sum)?;

core/src/core/hash.rs

@@ -153,9 +153,7 @@ impl HashWriter {
 
 impl Default for HashWriter {
 	fn default() -> HashWriter {
-		HashWriter {
+		HashWriter { state: Blake2b::new(32) }
-			state: Blake2b::new(32),
-		}
 	}
 }
 
@@ -204,8 +202,7 @@ impl<T: Writeable> VerifySortOrder<T> for Vec<T> {
 			.map(|item| item.hash())
 			.collect::<Vec<_>>()
 			.windows(2)
-			.any(|pair| pair[0] > pair[1])
+			.any(|pair| pair[0] > pair[1]) {
-		{
 			true => Err(ser::Error::BadlySorted),
 			false => Ok(()),
 		}

core/src/core/mod.rs

@@ -25,6 +25,8 @@ pub mod transaction;
 
 use std::fmt;
 use std::cmp::Ordering;
+use std::num::ParseFloatError;
+use consensus::GRIN_BASE;
 
 use util::secp::{self, Secp256k1};
 use util::secp::pedersen::*;
@@ -182,6 +184,22 @@ impl Writeable for Proof {
 	}
 }
 
+/// Common method for parsing an amount from human-readable, and converting
+/// to internally-compatible u64
+
+pub fn amount_from_hr_string(amount: &str) -> Result<u64, ParseFloatError> {
+	let amount = amount.parse::<f64>()?;
+	Ok((amount * GRIN_BASE as f64) as u64)
+}
+
+/// Common method for converting an amount to a human-readable string
+
+pub fn amount_to_hr_string(amount: u64) -> String {
+	let amount = (amount as f64 / GRIN_BASE as f64) as f64;
+	let places = (GRIN_BASE as f64).log(10.0) as usize + 1;
+	String::from(format!("{:.*}", places, amount))
+}
+
 #[cfg(test)]
 mod test {
 	use super::*;
@@ -192,6 +210,27 @@ mod test {
 	use keychain;
 	use keychain::{BlindingFactor, Keychain};
 
+	#[test]
+	pub fn test_amount_to_hr() {
+		assert!(50123456789 == amount_from_hr_string("50.123456789").unwrap());
+		assert!(50 == amount_from_hr_string(".000000050").unwrap());
+		assert!(1 == amount_from_hr_string(".000000001").unwrap());
+		assert!(0 == amount_from_hr_string(".0000000009").unwrap());
+		assert!(500_000_000_000 == amount_from_hr_string("500").unwrap());
+		assert!(
+			5_000_000_000_000_000_000 == amount_from_hr_string("5000000000.00000000000").unwrap()
+		);
+	}
+
+	#[test]
+	pub fn test_hr_to_amount() {
+		assert!("50.123456789" == amount_to_hr_string(50123456789));
+		assert!("0.000000050" == amount_to_hr_string(50));
+		assert!("0.000000001" == amount_to_hr_string(1));
+		assert!("500.000000000" == amount_to_hr_string(500_000_000_000));
+		assert!("5000000000.000000000" == amount_to_hr_string(5_000_000_000_000_000_000));
+	}
+
 	#[test]
 	#[should_panic(expected = "InvalidSecretKey")]
 	fn test_zero_commit_fails() {
@@ -421,9 +460,7 @@ mod test {
 			&key_id3.clone(),
 		).unwrap();
 		match b.validate(keychain.secp()) {
-			Err(KernelLockHeight {
+			Err(KernelLockHeight { lock_height: height }) => {
-				lock_height: height,
-			}) => {
 				assert_eq!(height, 2);
 			}
 			_ => panic!("expecting KernelLockHeight error here"),

core/src/core/pmmr.rs

@@ -272,9 +272,9 @@ where
 		// creation of another parent.
 		while bintree_postorder_height(pos + 1) > height {
 			let left_sibling = bintree_jump_left_sibling(pos);
-			let left_hashsum = self.backend
+			let left_hashsum = self.backend.get(left_sibling).expect(
-				.get(left_sibling)
+				"missing left sibling in tree, should not have been pruned",
-				.expect("missing left sibling in tree, should not have been pruned");
+			);
 			current_hashsum = left_hashsum + current_hashsum;
 
 			to_append.push(current_hashsum.clone());
@@ -503,9 +503,7 @@ pub struct PruneList {
 impl PruneList {
 	/// Instantiate a new empty prune list
 	pub fn new() -> PruneList {
-		PruneList {
+		PruneList { pruned_nodes: vec![] }
-			pruned_nodes: vec![],
-		}
 	}
 
 	/// Computes by how many positions a node at pos should be shifted given the
@@ -846,8 +844,8 @@ mod test {
 		fn sum(&self) -> u64 {
 			// sums are not allowed to overflow, so we use this simple
 			// non-injective "sum" function that will still be homomorphic
-			self.0[0] as u64 * 0x1000 + self.0[1] as u64 * 0x100 + self.0[2] as u64 * 0x10
+			self.0[0] as u64 * 0x1000 + self.0[1] as u64 * 0x100 + self.0[2] as u64 * 0x10 +
-				+ self.0[3] as u64
+				self.0[3] as u64
 		}
 		fn sum_len() -> usize {
 			8
@@ -897,8 +895,8 @@ mod test {
 
 		// two elements
 		pmmr.push(elems[1], None::<TestElem>).unwrap();
-		let sum2 = HashSum::from_summable(1, &elems[0], None::<TestElem>)
+		let sum2 = HashSum::from_summable(1, &elems[0], None::<TestElem>) +
-			+ HashSum::from_summable(2, &elems[1], None::<TestElem>);
+			HashSum::from_summable(2, &elems[1], None::<TestElem>);
 		assert_eq!(pmmr.root(), sum2);
 		assert_eq!(pmmr.unpruned_size(), 3);
 
@@ -910,9 +908,9 @@ mod test {
 
 		// four elements
 		pmmr.push(elems[3], None::<TestElem>).unwrap();
-		let sum4 = sum2
+		let sum4 = sum2 +
-			+ (HashSum::from_summable(4, &elems[2], None::<TestElem>)
+			(HashSum::from_summable(4, &elems[2], None::<TestElem>) +
-				+ HashSum::from_summable(5, &elems[3], None::<TestElem>));
+				 HashSum::from_summable(5, &elems[3], None::<TestElem>));
 		assert_eq!(pmmr.root(), sum4);
 		assert_eq!(pmmr.unpruned_size(), 7);
 
@@ -924,9 +922,9 @@ mod test {
 
 		// six elements
 		pmmr.push(elems[5], None::<TestElem>).unwrap();
-		let sum6 = sum4.clone()
+		let sum6 = sum4.clone() +
-			+ (HashSum::from_summable(8, &elems[4], None::<TestElem>)
+			(HashSum::from_summable(8, &elems[4], None::<TestElem>) +
-				+ HashSum::from_summable(9, &elems[5], None::<TestElem>));
+				 HashSum::from_summable(9, &elems[5], None::<TestElem>));
 		assert_eq!(pmmr.root(), sum6.clone());
 		assert_eq!(pmmr.unpruned_size(), 10);
 
@@ -938,11 +936,11 @@ mod test {
 
 		// eight elements
 		pmmr.push(elems[7], None::<TestElem>).unwrap();
-		let sum8 = sum4
+		let sum8 = sum4 +
-			+ ((HashSum::from_summable(8, &elems[4], None::<TestElem>)
+			((HashSum::from_summable(8, &elems[4], None::<TestElem>) +
-				+ HashSum::from_summable(9, &elems[5], None::<TestElem>))
+				  HashSum::from_summable(9, &elems[5], None::<TestElem>)) +
-				+ (HashSum::from_summable(11, &elems[6], None::<TestElem>)
+				 (HashSum::from_summable(11, &elems[6], None::<TestElem>) +
-					+ HashSum::from_summable(12, &elems[7], None::<TestElem>)));
+					  HashSum::from_summable(12, &elems[7], None::<TestElem>)));
 		assert_eq!(pmmr.root(), sum8);
 		assert_eq!(pmmr.unpruned_size(), 15);
 
@@ -991,8 +989,7 @@ mod test {
 
 		let res = pmmr.get_last_n_insertions(19);
 		assert!(
-			res[0].sum == 4 && res[1].sum == 3 && res[2].sum == 2 && res[3].sum == 1
+			res[0].sum == 4 && res[1].sum == 3 && res[2].sum == 2 && res[3].sum == 1 && res.len() == 4
-				&& res.len() == 4
 		);
 
 		pmmr.push(elems[5], None::<TestElem>).unwrap();
@@ -1002,8 +999,7 @@ mod test {
 
 		let res = pmmr.get_last_n_insertions(7);
 		assert!(
-			res[0].sum == 9 && res[1].sum == 8 && res[2].sum == 7 && res[3].sum == 6
+			res[0].sum == 9 && res[1].sum == 8 && res[2].sum == 7 && res[3].sum == 6 && res.len() == 7
-				&& res.len() == 7
 		);
 	}
 

core/src/core/target.rs

@@ -63,9 +63,7 @@ impl Difficulty {
 		let mut in_vec = h.to_vec();
 		in_vec.truncate(8);
 		let num = BigEndian::read_u64(&in_vec);
-		Difficulty {
+		Difficulty { num: max_target / num }
-			num: max_target / num,
-		}
 	}
 
 	/// Converts the difficulty into a u64
@@ -83,36 +81,28 @@ impl fmt::Display for Difficulty {
 impl Add<Difficulty> for Difficulty {
 	type Output = Difficulty;
 	fn add(self, other: Difficulty) -> Difficulty {
-		Difficulty {
+		Difficulty { num: self.num + other.num }
-			num: self.num + other.num,
-		}
 	}
 }
 
 impl Sub<Difficulty> for Difficulty {
 	type Output = Difficulty;
 	fn sub(self, other: Difficulty) -> Difficulty {
-		Difficulty {
+		Difficulty { num: self.num - other.num }
-			num: self.num - other.num,
-		}
 	}
 }
 
 impl Mul<Difficulty> for Difficulty {
 	type Output = Difficulty;
 	fn mul(self, other: Difficulty) -> Difficulty {
-		Difficulty {
+		Difficulty { num: self.num * other.num }
-			num: self.num * other.num,
-		}
 	}
 }
 
 impl Div<Difficulty> for Difficulty {
 	type Output = Difficulty;
 	fn div(self, other: Difficulty) -> Difficulty {
-		Difficulty {
+		Difficulty { num: self.num / other.num }
-			num: self.num / other.num,
-		}
 	}
 }
 
@@ -167,8 +157,6 @@ impl<'de> de::Visitor<'de> for DiffVisitor {
 				&"a value number",
 			));
 		};
-		Ok(Difficulty {
+		Ok(Difficulty { num: num_in.unwrap() })
-			num: num_in.unwrap(),
-		})
 	}
 }

core/src/core/transaction.rs

@@ -129,8 +129,9 @@ impl Writeable for TxKernel {
 
 impl Readable for TxKernel {
 	fn read(reader: &mut Reader) -> Result<TxKernel, ser::Error> {
-		let features =
+		let features = KernelFeatures::from_bits(reader.read_u8()?).ok_or(
-			KernelFeatures::from_bits(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
+			ser::Error::CorruptedData,
+		)?;
 
 		Ok(TxKernel {
 			features: features,
@@ -483,8 +484,9 @@ impl Writeable for Output {
 /// an Output from a binary stream.
 impl Readable for Output {
 	fn read(reader: &mut Reader) -> Result<Output, ser::Error> {
-		let features =
+		let features = OutputFeatures::from_bits(reader.read_u8()?).ok_or(
-			OutputFeatures::from_bits(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
+			ser::Error::CorruptedData,
+		)?;
 
 		Ok(Output {
 			features: features,
@@ -520,11 +522,13 @@ impl Output {
 	/// value from the range proof and the commitment
 	pub fn recover_value(&self, keychain: &Keychain, key_id: &Identifier) -> Option<u64> {
 		match keychain.rewind_range_proof(key_id, self.commit, self.proof) {
-			Ok(proof_info) => if proof_info.success {
+			Ok(proof_info) => {
+				if proof_info.success {
 					Some(proof_info.value)
 				} else {
 					None
-			},
+				}
+			}
 			Err(_) => None,
 		}
 	}
@@ -542,9 +546,7 @@ impl Summable for SumCommit {
 	type Sum = SumCommit;
 
 	fn sum(&self) -> SumCommit {
-		SumCommit {
+		SumCommit { commit: self.commit.clone() }
-			commit: self.commit.clone(),
-		}
 	}
 
 	fn sum_len() -> usize {
@@ -563,9 +565,7 @@ impl Readable for SumCommit {
 	fn read(reader: &mut Reader) -> Result<SumCommit, ser::Error> {
 		let commit = Commitment::read(reader)?;
 
-		Ok(SumCommit {
+		Ok(SumCommit { commit: commit })
-			commit: commit,
-		})
 	}
 }
 
@@ -574,15 +574,17 @@ impl ops::Add for SumCommit {
 
 	fn add(self, other: SumCommit) -> SumCommit {
 		let secp = static_secp_instance();
-		let sum = match secp.lock().unwrap()
+		let sum = match secp.lock().unwrap().commit_sum(
-			.commit_sum(vec![self.commit.clone(), other.commit.clone()], vec![])
+			vec![
-		{
+				self.commit.clone(),
+				other.commit.clone(),
+			],
+			vec![],
+		) {
 			Ok(s) => s,
 			Err(_) => Commitment::from_vec(vec![1; 33]),
 		};
-		SumCommit {
+		SumCommit { commit: sum }
-			commit: sum,
-		}
 	}
 }
 

src/bin/grin.rs

@@ -201,7 +201,7 @@ fn main() {
 				provided, the command will attempt to contact the receiver at that \
 				address and send the transaction directly.")
 			.arg(Arg::with_name("amount")
-				.help("Amount to send in the smallest denomination")
+				.help("Number of coins to send with optional fraction, e.g. 12.423")
 				.index(1))
 			.arg(Arg::with_name("minimum_confirmations")
 				.help("Minimum number of confirmations required for an output to be spendable.")
@@ -220,7 +220,7 @@ fn main() {
 				key. Similar to send but burns an output to allow single-party \
 				transactions.")
 			.arg(Arg::with_name("amount")
-				.help("Amount to burn in the smallest denomination")
+				.help("Number of coins to burn")
 				.index(1))
 			.arg(Arg::with_name("minimum_confirmations")
 				.help("Minimum number of confirmations required for an output to be spendable.")
@@ -389,9 +389,9 @@ fn wallet_command(wallet_args: &ArgMatches) {
 		("send", Some(send_args)) => {
 			let amount = send_args
 				.value_of("amount")
-				.expect("Amount to send required")
+				.expect("Amount to send required");
-				.parse()
+			let amount = core::core::amount_from_hr_string(amount)
-				.expect("Could not parse amount as a whole number.");
+				.expect("Could not parse amount as a number with optional decimal point.");
 			let minimum_confirmations: u64 = send_args
 				.value_of("minimum_confirmations")
 				.unwrap_or("1")
@@ -401,20 +401,25 @@ fn wallet_command(wallet_args: &ArgMatches) {
 			if let Some(d) = send_args.value_of("dest") {
 				dest = d;
 			}
-			wallet::issue_send_tx(
+			let result=wallet::issue_send_tx(
 				&wallet_config,
 				&keychain,
 				amount,
 				minimum_confirmations,
 				dest.to_string(),
-			).unwrap();
+			);
+			match result {
+				Ok(_) => {}, //success messaged logged internally
+				Err(wallet::Error::NotEnoughFunds(_)) => {},
+				Err(e) => panic!(e),
+			};
 		}
 		("burn", Some(send_args)) => {
 			let amount = send_args
 				.value_of("amount")
-				.expect("Amount to burn required")
+				.expect("Amount to burn required");
-				.parse()
+			let amount = core::core::amount_from_hr_string(amount)
-				.expect("Could not parse amount as a whole number.");
+				.expect("Could not parse amount as number with optional decimal point.");
 			let minimum_confirmations: u64 = send_args
 				.value_of("minimum_confirmations")
 				.unwrap_or("1")

wallet/src/client.rs

@@ -69,7 +69,14 @@ fn single_send_partial_tx(url: &str, partial_tx: &JSONPartialTx) -> Result<(), E
 	req.set_body(json);
 
 	let work = client.request(req);
-	let _ = core.run(work)?;
+	let _ = core.run(work).and_then(|res|{
+		if res.status()==hyper::StatusCode::Ok {
+			info!(LOGGER, "Transaction sent successfully");
+		} else {
+			error!(LOGGER, "Error sending transaction - status: {}", res.status());
+		}
+		Ok(())
+	})?;
 	Ok(())
 }
 

wallet/src/info.rs

@@ -14,6 +14,7 @@
 
 use checker;
 use keychain::Keychain;
+use core::core;
 use types::{WalletConfig, WalletData};
 
 pub fn show_info(config: &WalletConfig, keychain: &Keychain) {
@@ -51,7 +52,7 @@ pub fn show_info(config: &WalletConfig, keychain: &Keychain) {
 				out.status,
 				out.is_coinbase,
 				out.num_confirmations(current_height),
-				out.value,
+				core::amount_to_hr_string(out.value),
 			);
 		}
 	});

wallet/src/sender.rs

@@ -17,7 +17,7 @@ use serde_json;
 use api;
 use client;
 use checker;
-use core::core::{build, Transaction};
+use core::core::{build, Transaction, amount_to_hr_string};
 use core::ser;
 use keychain::{BlindingFactor, Identifier, Keychain};
 use receiver::TxWrapper;
@@ -88,7 +88,15 @@ fn build_send_tx(
 	})?;
 
 	// build transaction skeleton with inputs and change
-	let mut parts = inputs_and_change(&coins, config, keychain, key_id, amount)?;
+	let parts = inputs_and_change(&coins, config, keychain, key_id, amount);
+
+	if let Err(p) = parts {
+		let total: u64 = coins.iter().map(|c| c.value).sum();
+		error!(LOGGER, "Transaction not sent - Not enough funds (Max: {})", amount_to_hr_string(total));
+		return Err(p);
+	}
+
+	let mut parts=parts.unwrap();
 
 	// 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).