grin wallet outputs now displays output commitments (#1365)

* commit - wip

* rustfmt

* save output commitment when saving output in wallet

* rustfmt

* fixup wallet tests with commitments in the wallet db

* rustfmt

wallet/src/display.rs

@@ -18,9 +18,15 @@ use libwallet::Error;
 use prettytable;
 use std::io::prelude::Write;
 use term;
+use util;
+use util::secp::pedersen;
 
 /// Display outputs in a pretty way
-pub fn outputs(cur_height: u64, validated: bool, outputs: Vec<OutputData>) -> Result<(), Error> {
+pub fn outputs(
+	cur_height: u64,
+	validated: bool,
+	outputs: Vec<(OutputData, pedersen::Commitment)>,
+) -> Result<(), Error> {
 	let title = format!("Wallet Outputs - Block Height: {}", cur_height);
 	println!();
 	let mut t = term::stdout().unwrap();
@@ -31,20 +37,18 @@ pub fn outputs(cur_height: u64, validated: bool, outputs: Vec<OutputData>) -> Re
 	let mut table = table!();
 
 	table.set_titles(row![
-		bMG->"Key Id",
+		bMG->"Output Commitment",
-		bMG->"Child Key Index",
 		bMG->"Block Height",
 		bMG->"Locked Until",
 		bMG->"Status",
-		bMG->"Is Coinbase?",
+		bMG->"Coinbase?",
-		bMG->"Num. of Confirmations",
+		bMG->"# Confirms",
 		bMG->"Value",
-		bMG->"Transaction"
+		bMG->"Tx"
 	]);
 
-	for out in outputs {
+	for (out, commit) in outputs {
-		let key_id = format!("{}", out.key_id);
+		let commit = format!("{}", util::to_hex(commit.as_ref().to_vec()));
-		let n_child = format!("{}", out.n_child);
 		let height = format!("{}", out.height);
 		let lock_height = format!("{}", out.lock_height);
 		let status = format!("{:?}", out.status);
@@ -52,12 +56,11 @@ pub fn outputs(cur_height: u64, validated: bool, outputs: Vec<OutputData>) -> Re
 		let num_confirmations = format!("{}", out.num_confirmations(cur_height));
 		let value = format!("{}", core::amount_to_hr_string(out.value));
 		let tx = match out.tx_log_entry {
-			None => "None".to_owned(),
+			None => "".to_owned(),
 			Some(t) => t.to_string(),
 		};
 		table.add_row(row![
-			bFC->key_id,
+			bFC->commit,
-			bFC->n_child,
 			bFB->height,
 			bFB->lock_height,
 			bFR->status,

wallet/src/file_wallet.rs

@@ -15,6 +15,7 @@
 use std::collections::HashMap;
 use std::fs::{self, File};
 use std::io::Write;
+use std::marker;
 use std::path::{Path, MAIN_SEPARATOR};
 
 use serde_json;
@@ -26,6 +27,7 @@ use uuid::Uuid;
 use failure::ResultExt;
 
 use keychain::{self, Identifier, Keychain};
+use util::secp::pedersen;
 use util::LOGGER;
 
 use error::{Error, ErrorKind};
@@ -45,7 +47,10 @@ const BCK_FILE: &'static str = "wallet.bck";
 const LOCK_FILE: &'static str = "wallet.lock";
 
 #[derive(Debug)]
-struct FileBatch<'a> {
+struct FileBatch<'a, K: 'a>
+where
+	K: Keychain,
+{
 	/// List of outputs
 	outputs: &'a mut HashMap<String, OutputData>,
 	/// Wallet Details
@@ -56,9 +61,18 @@ struct FileBatch<'a> {
 	details_file_path: String,
 	/// lock file path
 	lock_file_path: String,
+	/// PhantomData for our K: Keychain.
+	_marker: marker::PhantomData<K>,
+}
+
+impl<'a, K> WalletOutputBatch<K> for FileBatch<'a, K>
+where
+	K: Keychain,
+{
+	fn keychain(&mut self) -> &mut K {
+		unimplemented!();
 	}
 
-impl<'a> WalletOutputBatch for FileBatch<'a> {
 	fn save(&mut self, out: OutputData) -> Result<(), libwallet::Error> {
 		let _ = self.outputs.insert(out.key_id.to_hex(), out);
 		Ok(())
@@ -134,7 +148,10 @@ impl<'a> WalletOutputBatch for FileBatch<'a> {
 	}
 }
 
-impl<'a> Drop for FileBatch<'a> {
+impl<'a, K> Drop for FileBatch<'a, K>
+where
+	K: Keychain,
+{
 	fn drop(&mut self) {
 		// delete the lock file
 		if let Err(e) = fs::remove_dir(&self.lock_file_path) {
@@ -229,7 +246,14 @@ where
 			.ok_or(libwallet::ErrorKind::Backend("not found".to_string()).into())
 	}
 
-	fn batch<'a>(&'a mut self) -> Result<Box<WalletOutputBatch + 'a>, libwallet::Error> {
+	fn get_commitment(
+		&mut self,
+		_id: &Identifier,
+	) -> Result<pedersen::Commitment, libwallet::Error> {
+		unimplemented!();
+	}
+
+	fn batch<'a>(&'a mut self) -> Result<Box<WalletOutputBatch<K> + 'a>, libwallet::Error> {
 		self.lock()?;
 
 		// We successfully acquired the lock - so do what needs to be done.
@@ -244,6 +268,7 @@ where
 			data_file_path: self.data_file_path.clone(),
 			details_file_path: self.details_file_path.clone(),
 			lock_file_path: self.lock_file_path.clone(),
+			_marker: marker::PhantomData,
 		}))
 	}
 
@@ -326,7 +351,8 @@ where
 		let mut core = reactor::Core::new().unwrap();
 		let retry_strategy = FibonacciBackoff::from_millis(1000).take(10);
 		let retry_future = Retry::spawn(core.handle(), retry_strategy, action);
-		let retry_result = core.run(retry_future)
+		let retry_result = core
+			.run(retry_future)
 			.context(libwallet::ErrorKind::CallbackImpl(
 				"Failed to acquire lock file",
 			));

wallet/src/libwallet/api.rs

@@ -27,11 +27,12 @@ use serde_json as json;
 use core::ser;
 use keychain::Keychain;
 use libtx::slate::Slate;
-use libwallet::internal::{tx, updater, selection, sigcontext};
+use libwallet::internal::{selection, sigcontext, tx, updater};
 use libwallet::types::{
 	BlockFees, CbData, OutputData, TxLogEntry, TxWrapper, WalletBackend, WalletClient, WalletInfo,
 };
 use libwallet::{Error, ErrorKind};
+use util::secp::pedersen;
 use util::{self, LOGGER};
 
 /// Wrapper around internal API functions, containing a reference to
@@ -72,7 +73,7 @@ where
 		include_spent: bool,
 		refresh_from_node: bool,
 		tx_id: Option<u32>,
-	) -> Result<(bool, Vec<OutputData>), Error> {
+	) -> Result<(bool, Vec<(OutputData, pedersen::Commitment)>), Error> {
 		let mut w = self.wallet.lock().unwrap();
 		w.open_with_credentials()?;
 
@@ -214,11 +215,7 @@ where
 	/// A sender provided a transaction file with appropriate public keys and
 	/// metadata. Complete the receivers' end of it to generate another file
 	/// to send back.
-	pub fn file_receive_tx(
+	pub fn file_receive_tx(&mut self, source: &str) -> Result<(), Error> {
-		&mut self,
-		source: &str,
-	) -> Result<(), Error> {
-
 		let mut pub_tx_f = File::open(source)?;
 		let mut content = String::new();
 		pub_tx_f.read_to_string(&mut content)?;
@@ -260,7 +257,6 @@ where
 		private_tx_file: &str,
 		receiver_file: &str,
 	) -> Result<Slate, Error> {
-
 		let mut pub_tx_f = File::open(receiver_file)?;
 		let mut content = String::new();
 		pub_tx_f.read_to_string(&mut content)?;
@@ -349,7 +345,7 @@ where
 			}
 			Err(_) => {
 				let outputs = self.retrieve_outputs(true, false, None)?;
-				let height = match outputs.1.iter().map(|out| out.height).max() {
+				let height = match outputs.1.iter().map(|(out, _)| out.height).max() {
 					Some(height) => height,
 					None => 0,
 				};

wallet/src/libwallet/controller.rs

@@ -37,6 +37,7 @@ use libwallet::types::{
 use libwallet::{Error, ErrorKind};
 use url::form_urlencoded;
 
+use util::secp::pedersen;
 use util::LOGGER;
 
 /// Instantiate wallet Owner API for a single-use (command line) call
@@ -187,7 +188,7 @@ where
 		&self,
 		req: &Request<Body>,
 		api: APIOwner<T, C, K>,
-	) -> Result<(bool, Vec<OutputData>), Error> {
+	) -> Result<(bool, Vec<(OutputData, pedersen::Commitment)>), Error> {
 		let mut update_from_node = false;
 		let mut id = None;
 		let mut show_spent = false;
@@ -247,7 +248,8 @@ where
 
 	fn handle_get_request(&self, req: &Request<Body>) -> Result<Response<Body>, Error> {
 		let api = APIOwner::new(self.wallet.clone());
-		Ok(match req.uri()
+		Ok(match req
+			.uri()
 			.path()
 			.trim_right_matches("/")
 			.rsplit("/")
@@ -292,7 +294,8 @@ where
 
 	fn handle_post_request(&self, req: Request<Body>) -> WalletResponseFuture {
 		let api = APIOwner::new(self.wallet.clone());
-		match req.uri()
+		match req
+			.uri()
 			.path()
 			.trim_right_matches("/")
 			.rsplit("/")
@@ -398,7 +401,8 @@ where
 
 	fn handle_request(&self, req: Request<Body>) -> WalletResponseFuture {
 		let api = *APIForeign::new(self.wallet.clone());
-		match req.uri()
+		match req
+			.uri()
 			.path()
 			.trim_right_matches("/")
 			.rsplit("/")

wallet/src/libwallet/internal/tx.rs

@@ -150,7 +150,8 @@ where
 		return Err(ErrorKind::TransactionNotCancellable(tx_id))?;
 	}
 	// get outputs associated with tx
-	let outputs = updater::retrieve_outputs(wallet, false, Some(tx_id))?;
+	let res = updater::retrieve_outputs(wallet, false, Some(tx_id))?;
+	let outputs = res.iter().map(|(out, _)| out).cloned().collect();
 	updater::cancel_tx_and_outputs(wallet, tx, outputs)?;
 	Ok(())
 }

wallet/src/libwallet/internal/updater.rs

@@ -38,13 +38,14 @@ pub fn retrieve_outputs<T: ?Sized, C, K>(
 	wallet: &mut T,
 	show_spent: bool,
 	tx_id: Option<u32>,
-) -> Result<Vec<OutputData>, Error>
+) -> Result<Vec<(OutputData, pedersen::Commitment)>, Error>
 where
 	T: WalletBackend<C, K>,
 	C: WalletClient,
 	K: Keychain,
 {
 	let root_key_id = wallet.keychain().clone().root_key_id();
+
 	// just read the wallet here, no need for a write lock
 	let mut outputs = wallet
 		.iter()
@@ -57,6 +58,7 @@ where
 			}
 		})
 		.collect::<Vec<_>>();
+
 	// only include outputs with a given tx_id if provided
 	if let Some(id) = tx_id {
 		outputs = outputs
@@ -64,8 +66,17 @@ where
 			.filter(|out| out.tx_log_entry == Some(id))
 			.collect::<Vec<_>>();
 	}
+
 	outputs.sort_by_key(|out| out.n_child);
-	Ok(outputs)
+
+	let res = outputs
+		.into_iter()
+		.map(|out| {
+			let commit = wallet.get_commitment(&out.key_id).unwrap();
+			(out, commit)
+		})
+		.collect();
+	Ok(res)
 }
 
 /// Retrieve all of the transaction entries, or a particular entry

wallet/src/libwallet/types.rs

@@ -47,8 +47,7 @@ where
 	T: WalletBackend<C, K> + Send + Sync + 'static,
 	C: WalletClient,
 	K: Keychain,
-{
+{}
-}
 
 /// TODO:
 /// Wallets should implement this backend for their storage. All functions
@@ -77,6 +76,9 @@ where
 	/// Get output data by id
 	fn get(&self, id: &Identifier) -> Result<OutputData, Error>;
 
+	/// Get associated output commitment by id.
+	fn get_commitment(&mut self, id: &Identifier) -> Result<pedersen::Commitment, Error>;
+
 	/// Get an (Optional) tx log entry by uuid
 	fn get_tx_log_entry(&self, uuid: &Uuid) -> Result<Option<TxLogEntry>, Error>;
 
@@ -84,7 +86,7 @@ where
 	fn tx_log_iter<'a>(&'a self) -> Box<Iterator<Item = TxLogEntry> + 'a>;
 
 	/// Create a new write batch to update or remove output data
-	fn batch<'a>(&'a mut self) -> Result<Box<WalletOutputBatch + 'a>, Error>;
+	fn batch<'a>(&'a mut self) -> Result<Box<WalletOutputBatch<K> + 'a>, Error>;
 
 	/// Next child ID when we want to create a new output
 	fn next_child<'a>(&mut self, root_key_id: Identifier) -> Result<u32, Error>;
@@ -101,7 +103,13 @@ where
 /// commit method can't take ownership.
 /// TODO: Should these be split into separate batch objects, for outputs,
 /// tx_log entries and meta/details?
-pub trait WalletOutputBatch {
+pub trait WalletOutputBatch<K>
+where
+	K: Keychain,
+{
+	/// Return the keychain being used
+	fn keychain(&mut self) -> &mut K;
+
 	/// Add or update data about an output to the backend
 	fn save(&mut self, out: OutputData) -> Result<(), Error>;
 
@@ -111,7 +119,7 @@ pub trait WalletOutputBatch {
 	/// Iterate over all output data stored by the backend
 	fn iter(&self) -> Box<Iterator<Item = OutputData>>;
 
-	/// Delete data about an output to the backend
+	/// Delete data about an output from the backend
 	fn delete(&mut self, id: &Identifier) -> Result<(), Error>;
 
 	/// save wallet details

wallet/src/lmdb_wallet.rs

@@ -25,9 +25,11 @@ use store::{self, option_to_not_found, to_key, u64_to_key};
 use libwallet::types::*;
 use libwallet::{internal, Error, ErrorKind};
 use types::{WalletConfig, WalletSeed};
+use util::secp::pedersen;
 
 pub const DB_DIR: &'static str = "wallet_data";
 
+const COMMITMENT_PREFIX: u8 = 'c' as u8;
 const OUTPUT_PREFIX: u8 = 'o' as u8;
 const DERIV_PREFIX: u8 = 'd' as u8;
 const CONFIRMED_HEIGHT_PREFIX: u8 = 'c' as u8;
@@ -119,6 +121,33 @@ where
 		option_to_not_found(self.db.get_ser(&key), &format!("Key Id: {}", id)).map_err(|e| e.into())
 	}
 
+	fn get_commitment(&mut self, id: &Identifier) -> Result<pedersen::Commitment, Error> {
+		let key = to_key(COMMITMENT_PREFIX, &mut id.to_bytes().to_vec());
+
+		let res: Result<pedersen::Commitment, Error> =
+			option_to_not_found(self.db.get_ser(&key), &format!("Key Id: {}", id))
+				.map_err(|e| e.into());
+
+		// "cache hit" and return the commitment
+		if let Ok(commit) = res {
+			Ok(commit)
+		} else {
+			let out = self.get(id)?;
+
+			// Save the output data back to the db
+			// which builds and saves the associated commitment.
+			{
+				let mut batch = self.batch()?;
+				batch.save(out)?;
+				batch.commit()?;
+			}
+
+			// Now retrieve the saved commitment and return it.
+			option_to_not_found(self.db.get_ser(&key), &format!("Key Id: {}", id))
+				.map_err(|e| e.into())
+		}
+	}
+
 	fn iter<'a>(&'a self) -> Box<Iterator<Item = OutputData> + 'a> {
 		Box::new(self.db.iter(&[OUTPUT_PREFIX]).unwrap())
 	}
@@ -132,10 +161,11 @@ where
 		Box::new(self.db.iter(&[TX_LOG_ENTRY_PREFIX]).unwrap())
 	}
 
-	fn batch<'a>(&'a mut self) -> Result<Box<WalletOutputBatch + 'a>, Error> {
+	fn batch<'a>(&'a mut self) -> Result<Box<WalletOutputBatch<K> + 'a>, Error> {
 		Ok(Box::new(Batch {
 			_store: self,
 			db: RefCell::new(Some(self.db.batch()?)),
+			keychain: self.keychain.clone(),
 		}))
 	}
 
@@ -184,17 +214,34 @@ where
 {
 	_store: &'a LMDBBackend<C, K>,
 	db: RefCell<Option<store::Batch<'a>>>,
+	/// Keychain
+	keychain: Option<K>,
 }
 
 #[allow(missing_docs)]
-impl<'a, C, K> WalletOutputBatch for Batch<'a, C, K>
+impl<'a, C, K> WalletOutputBatch<K> for Batch<'a, C, K>
 where
 	C: WalletClient,
 	K: Keychain,
 {
+	fn keychain(&mut self) -> &mut K {
+		self.keychain.as_mut().unwrap()
+	}
+
 	fn save(&mut self, out: OutputData) -> Result<(), Error> {
+		// Save the output data to the db.
+		{
 			let key = to_key(OUTPUT_PREFIX, &mut out.key_id.to_bytes().to_vec());
 			self.db.borrow().as_ref().unwrap().put_ser(&key, &out)?;
+		}
+
+		// Save the associated output commitment.
+		{
+			let key = to_key(COMMITMENT_PREFIX, &mut out.key_id.to_bytes().to_vec());
+			let commit = self.keychain().commit(out.value, &out.key_id)?;
+			self.db.borrow().as_ref().unwrap().put_ser(&key, &commit)?;
+		}
+
 		Ok(())
 	}
 
@@ -218,8 +265,18 @@ where
 	}
 
 	fn delete(&mut self, id: &Identifier) -> Result<(), Error> {
+		// Delete the output data.
+		{
 			let key = to_key(OUTPUT_PREFIX, &mut id.to_bytes().to_vec());
-		self.db.borrow().as_ref().unwrap().delete(&key)?;
+			let _ = self.db.borrow().as_ref().unwrap().delete(&key);
+		}
+
+		// Delete the associated output commitment.
+		{
+			let key = to_key(COMMITMENT_PREFIX, &mut id.to_bytes().to_vec());
+			let _ = self.db.borrow().as_ref().unwrap().delete(&key);
+		}
+
 		Ok(())
 	}
 

wallet/tests/common/mod.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+extern crate chrono;
 extern crate failure;
 extern crate grin_api as api;
 extern crate grin_chain as chain;
@@ -19,10 +20,9 @@ extern crate grin_core as core;
 extern crate grin_keychain as keychain;
 extern crate grin_wallet as wallet;
 extern crate serde_json;
-extern crate chrono;
 
-use std::sync::{Arc, Mutex};
 use chrono::Duration;
+use std::sync::{Arc, Mutex};
 
 use chain::Chain;
 use core::core::{OutputFeatures, OutputIdentifier, Transaction};

wallet/tests/transaction.rs

@@ -21,8 +21,8 @@ extern crate grin_wallet as wallet;
 extern crate rand;
 #[macro_use]
 extern crate slog;
-extern crate serde;
 extern crate chrono;
+extern crate serde;
 extern crate uuid;
 
 mod common;
@@ -315,7 +315,7 @@ fn tx_rollback(test_dir: &str, backend_type: common::BackendType) -> Result<(),
 		let mut unconfirmed_count = 0;
 		// get the tx entry, check outputs are as expected
 		let (_, outputs) = api.retrieve_outputs(true, false, Some(tx.unwrap().id))?;
-		for o in outputs.clone() {
+		for (o, _) in outputs.clone() {
 			if o.status == OutputStatus::Locked {
 				locked_count = locked_count + 1;
 			}
@@ -339,7 +339,7 @@ fn tx_rollback(test_dir: &str, backend_type: common::BackendType) -> Result<(),
 		assert!(tx.is_some());
 		// get the tx entry, check outputs are as expected
 		let (_, outputs) = api.retrieve_outputs(true, false, Some(tx.unwrap().id))?;
-		for o in outputs.clone() {
+		for (o, _) in outputs.clone() {
 			if o.status == OutputStatus::Unconfirmed {
 				unconfirmed_count = unconfirmed_count + 1;
 			}
@@ -362,7 +362,8 @@ fn tx_rollback(test_dir: &str, backend_type: common::BackendType) -> Result<(),
 		let res = api.cancel_tx(1);
 		assert!(res.is_err());
 		let (_, txs) = api.retrieve_txs(true, None)?;
-		let tx = txs.iter()
+		let tx = txs
+			.iter()
 			.find(|t| t.tx_slate_id == Some(slate.id))
 			.unwrap();
 		api.cancel_tx(tx.id)?;
@@ -383,7 +384,8 @@ fn tx_rollback(test_dir: &str, backend_type: common::BackendType) -> Result<(),
 	// Wallet 2 rolls back
 	wallet::controller::owner_single_use(wallet2.clone(), |api| {
 		let (_, txs) = api.retrieve_txs(true, None)?;
-		let tx = txs.iter()
+		let tx = txs
+			.iter()
 			.find(|t| t.tx_slate_id == Some(slate.id))
 			.unwrap();
 		api.cancel_tx(tx.id)?;