T3 Wallet Restore fix and refactor (#1202)

* clean up wallet restore and underlying functions

* rustfmt

* refactor restore

api/src/types.rs

@@ -162,22 +162,25 @@ pub struct Output {
 impl Output {
 	pub fn new(commit: &pedersen::Commitment) -> Output {
 		Output {
-			commit: PrintableCommitment(commit.clone()),
+			commit: PrintableCommitment {
+				commit: commit.clone(),
+			},
 		}
 	}
 }
 
 #[derive(Debug, Clone)]
-pub struct PrintableCommitment(pedersen::Commitment);
+pub struct PrintableCommitment {
+	pub commit: pedersen::Commitment,
+}
 
 impl PrintableCommitment {
 	pub fn commit(&self) -> pedersen::Commitment {
-		self.0.clone()
+		self.commit.clone()
 	}
 
 	pub fn to_vec(&self) -> Vec<u8> {
-		let commit = self.0;
-		commit.0.to_vec()
+		self.commit.0.to_vec()
 	}
 }
 
@@ -212,9 +215,9 @@ impl<'de> serde::de::Visitor<'de> for PrintableCommitmentVisitor {
 	where
 		E: serde::de::Error,
 	{
-		Ok(PrintableCommitment(pedersen::Commitment::from_vec(
-			util::from_hex(String::from(v)).unwrap(),
-		)))
+		Ok(PrintableCommitment {
+			commit: pedersen::Commitment::from_vec(util::from_hex(String::from(v)).unwrap()),
+		})
 	}
 }
 

src/bin/grin.rs

@@ -40,8 +40,8 @@ pub mod tui;
 
 use std::env::current_dir;
 use std::process::exit;
-use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::Arc;
 use std::thread;
 use std::time::Duration;
 
@@ -722,13 +722,18 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) {
 					Ok(())
 				}
 				("restore", Some(_)) => {
-					let _res = api.restore().unwrap_or_else(|e| {
-						panic!(
-							"Error getting restoring wallet: {:?} Config: {:?}",
-							e, wallet_config
-						)
-					});
-					Ok(())
+					let result = api.restore();
+					match result {
+						Ok(_) => {
+							info!(LOGGER, "Wallet restore complete",);
+							Ok(())
+						}
+						Err(e) => {
+							error!(LOGGER, "Wallet restore failed: {:?}", e);
+							error!(LOGGER, "Backtrace: {}", e.backtrace().unwrap());
+							Err(e)
+						}
+					}
 				}
 				_ => panic!("Unknown wallet command, use 'grin help wallet' for details"),
 			}

wallet/src/client.rs

@@ -53,11 +53,12 @@ pub fn create_coinbase(dest: &str, block_fees: &BlockFees) -> Result<CbData, Err
 /// Send the slate to a listening wallet instance
 pub fn send_tx_slate(dest: &str, slate: &Slate) -> Result<Slate, Error> {
 	if &dest[..4] != "http" {
-		error!(
-			LOGGER,
-			"dest formatted as {} but send -d expected stdout or http://IP:port", dest
+		let err_str = format!(
+			"dest formatted as {} but send -d expected stdout or http://IP:port",
+			dest
 		);
-		Err(ErrorKind::Node)?
+		error!(LOGGER, "{}", err_str,);
+		Err(ErrorKind::Uri)?
 	}
 	let url = format!("{}/v1/wallet/foreign/receive_tx", dest);
 	debug!(LOGGER, "Posting transaction slate to {}", url);
@@ -123,14 +124,14 @@ pub fn post_tx(dest: &str, tx: &TxWrapper, fluff: bool) -> Result<(), Error> {
 	} else {
 		url = format!("{}/v1/pool/push", dest);
 	}
-	let res = api::client::post(url.as_str(), tx).context(ErrorKind::Node)?;
-	Ok(res)
+	api::client::post(url.as_str(), tx)?;
+	Ok(())
 }
 
 /// Return the chain tip from a given node
 pub fn get_chain_height(addr: &str) -> Result<u64, Error> {
 	let url = format!("{}/v1/chain", addr);
-	let res = api::client::get::<api::Tip>(url.as_str()).context(ErrorKind::Node)?;
+	let res = api::client::get::<api::Tip>(url.as_str())?;
 	Ok(res.height)
 }
 
@@ -159,13 +160,54 @@ pub fn get_outputs_from_node(
 			Err(e) => {
 				// if we got anything other than 200 back from server, don't attempt to refresh
 				// the wallet data after
-				return Err(e).context(ErrorKind::Node)?;
+				return Err(e)?;
 			}
 		}
 	}
 	Ok(api_outputs)
 }
 
+pub fn get_outputs_by_pmmr_index(
+	addr: &str,
+	start_height: u64,
+	max_outputs: u64,
+) -> Result<
+	(
+		u64,
+		u64,
+		Vec<(pedersen::Commitment, pedersen::RangeProof, bool)>,
+	),
+	Error,
+> {
+	let query_param = format!("start_index={}&max={}", start_height, max_outputs);
+
+	let url = format!("{}/v1/txhashset/outputs?{}", addr, query_param,);
+
+	let mut api_outputs: Vec<(pedersen::Commitment, pedersen::RangeProof, bool)> = Vec::new();
+
+	match api::client::get::<api::OutputListing>(url.as_str()) {
+		Ok(o) => {
+			for out in o.outputs {
+				let is_coinbase = match out.output_type {
+					api::OutputType::Coinbase => true,
+					api::OutputType::Transaction => false,
+				};
+				api_outputs.push((out.commit, out.range_proof().unwrap(), is_coinbase));
+			}
+
+			Ok((o.highest_index, o.last_retrieved_index, api_outputs))
+		}
+		Err(e) => {
+			// if we got anything other than 200 back from server, bye
+			error!(
+				LOGGER,
+				"get_outputs_by_pmmr_index: unable to contact API {}. Error: {}", addr, e
+			);
+			Err(e)?
+		}
+	}
+}
+
 /// Get any missing block hashes from node
 pub fn get_missing_block_hashes_from_node(
 	addr: &str,
@@ -213,9 +255,28 @@ pub fn get_missing_block_hashes_from_node(
 			},
 			Err(e) => {
 				// if we got anything other than 200 back from server, bye
-				return Err(e).context(ErrorKind::Node)?;
+				return Err(e)?;
 			}
 		}
 	}
 	Ok((api_blocks, api_merkle_proofs))
 }
+
+/// Create a merkle proof at the given height for the given commit
+pub fn create_merkle_proof(addr: &str, commit: &str) -> Result<MerkleProofWrapper, Error> {
+	let url = format!("{}/v1/txhashset/merkleproof?id={}", addr, commit);
+
+	match api::client::get::<api::OutputPrintable>(url.as_str()) {
+		Ok(output) => Ok(MerkleProofWrapper(output.merkle_proof.unwrap())),
+		Err(e) => {
+			// if we got anything other than 200 back from server, bye
+			error!(
+				LOGGER,
+				"get_merkle_proof_for_pos: Restore failed... unable to create merkle proof for commit {}. Error: {}",
+				commit,
+				e
+			);
+			Err(e)?
+		}
+	}
+}

wallet/src/display.rs

@@ -13,8 +13,8 @@
 // limitations under the License.
 
 use core::core::{self, amount_to_hr_string};
-use libwallet::Error;
 use libwallet::types::{OutputData, WalletInfo};
+use libwallet::Error;
 use prettytable;
 use std::io::prelude::Write;
 use term;
@@ -32,6 +32,7 @@ pub fn outputs(cur_height: u64, validated: bool, outputs: Vec<OutputData>) -> Re
 
 	table.set_titles(row![
 		bMG->"Key Id",
+		bMG->"Child Key Index",
 		bMG->"Block Height",
 		bMG->"Locked Until",
 		bMG->"Status",
@@ -42,6 +43,7 @@ pub fn outputs(cur_height: u64, validated: bool, outputs: Vec<OutputData>) -> Re
 
 	for out in outputs {
 		let key_id = format!("{}", out.key_id);
+		let n_child = format!("{}", out.n_child);
 		let height = format!("{}", out.height);
 		let lock_height = format!("{}", out.lock_height);
 		let status = format!("{:?}", out.status);
@@ -50,6 +52,7 @@ pub fn outputs(cur_height: u64, validated: bool, outputs: Vec<OutputData>) -> Re
 		let value = format!("{}", core::amount_to_hr_string(out.value));
 		table.add_row(row![
 			bFC->key_id,
+			bFC->n_child,
 			bFB->height,
 			bFB->lock_height,
 			bFR->status,

wallet/src/error.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 //! Implementation specific error types
+use api;
 use keychain;
 use libtx;
 use libwallet;
@@ -64,7 +65,7 @@ pub enum ErrorKind {
 
 	/// Error when contacting a node through its API
 	#[fail(display = "Node API error")]
-	Node,
+	Node(api::ErrorKind),
 
 	/// Error originating from hyper.
 	#[fail(display = "Hyper error")]
@@ -136,6 +137,14 @@ impl From<Context<ErrorKind>> for Error {
 	}
 }
 
+impl From<api::Error> for Error {
+	fn from(error: api::Error) -> Error {
+		Error {
+			inner: Context::new(ErrorKind::Node(error.kind().clone())),
+		}
+	}
+}
+
 impl From<keychain::Error> for Error {
 	fn from(error: keychain::Error) -> Error {
 		Error {

wallet/src/file_wallet.rs

@@ -13,29 +13,30 @@
 // limitations under the License.
 
 use std::collections::HashMap;
-use std::collections::hash_map::Values;
 use std::fs::{self, File};
 use std::io::{Read, Write};
 use std::path::{Path, MAIN_SEPARATOR};
 
 use serde_json;
 use tokio_core::reactor;
-use tokio_retry::Retry;
 use tokio_retry::strategy::FibonacciBackoff;
+use tokio_retry::Retry;
 
 use failure::ResultExt;
 
 use keychain::{self, Identifier, Keychain};
-use util::LOGGER;
 use util::secp::pedersen;
+use util::LOGGER;
 
 use error::{Error, ErrorKind};
 
 use client;
 use libtx::slate::Slate;
 use libwallet;
-use libwallet::types::{BlockFees, BlockIdentifier, CbData, MerkleProofWrapper, OutputData,
-                       TxWrapper, WalletBackend, WalletClient, WalletDetails, WalletOutputBatch};
+use libwallet::types::{
+	BlockFees, BlockIdentifier, CbData, MerkleProofWrapper, OutputData, TxWrapper, WalletBackend,
+	WalletClient, WalletDetails, WalletOutputBatch,
+};
 use types::{WalletConfig, WalletSeed};
 
 const DETAIL_FILE: &'static str = "wallet.det";
@@ -105,7 +106,7 @@ impl<'a> Drop for FileBatch<'a> {
 		if let Err(e) = fs::remove_dir(&self.lock_file_path) {
 			error!(
 				LOGGER,
-				"Could not remove wallet lock file. Maybe insufficient rights? "
+				"Could not remove wallet lock file. Maybe insufficient rights? {:?} ", e
 			);
 		}
 		info!(LOGGER, "... released wallet lock");
@@ -286,8 +287,7 @@ where
 
 	/// Restore wallet contents
 	fn restore(&mut self) -> Result<(), libwallet::Error> {
-		libwallet::internal::restore::restore(self).context(libwallet::ErrorKind::Restore)?;
-		Ok(())
+		libwallet::internal::restore::restore(self)
 	}
 }
 
@@ -298,12 +298,8 @@ impl<K> WalletClient for FileWallet<K> {
 	}
 
 	/// Call the wallet API to create a coinbase transaction
-	fn create_coinbase(
-		&self,
-		dest: &str,
-		block_fees: &BlockFees,
-	) -> Result<CbData, libwallet::Error> {
-		let res = client::create_coinbase(dest, block_fees);
+	fn create_coinbase(&self, block_fees: &BlockFees) -> Result<CbData, libwallet::Error> {
+		let res = client::create_coinbase(self.node_url(), block_fees);
 		match res {
 			Ok(r) => Ok(r),
 			Err(e) => {
@@ -314,8 +310,8 @@ impl<K> WalletClient for FileWallet<K> {
 	}
 
 	/// Send a transaction slate to another listening wallet and return result
-	fn send_tx_slate(&self, dest: &str, slate: &Slate) -> Result<Slate, libwallet::Error> {
-		let res = client::send_tx_slate(dest, slate);
+	fn send_tx_slate(&self, slate: &Slate) -> Result<Slate, libwallet::Error> {
+		let res = client::send_tx_slate(self.node_url(), slate);
 		match res {
 			Ok(r) => Ok(r),
 			Err(e) => {
@@ -326,14 +322,14 @@ impl<K> WalletClient for FileWallet<K> {
 	}
 
 	/// Posts a transaction to a grin node
-	fn post_tx(&self, dest: &str, tx: &TxWrapper, fluff: bool) -> Result<(), libwallet::Error> {
-		let res = client::post_tx(dest, tx, fluff).context(libwallet::ErrorKind::Node)?;
+	fn post_tx(&self, tx: &TxWrapper, fluff: bool) -> Result<(), libwallet::Error> {
+		let res = client::post_tx(self.node_url(), tx, fluff).context(libwallet::ErrorKind::Node)?;
 		Ok(res)
 	}
 
 	/// retrieves the current tip from the specified grin node
-	fn get_chain_height(&self, addr: &str) -> Result<u64, libwallet::Error> {
-		let res = client::get_chain_height(addr).context(libwallet::ErrorKind::Node)?;
+	fn get_chain_height(&self) -> Result<u64, libwallet::Error> {
+		let res = client::get_chain_height(self.node_url()).context(libwallet::ErrorKind::Node)?;
 		Ok(res)
 	}
 
@@ -341,10 +337,27 @@ impl<K> WalletClient for FileWallet<K> {
 	/// need "by_height" and "by_id" variants
 	fn get_outputs_from_node(
 		&self,
-		addr: &str,
 		wallet_outputs: Vec<pedersen::Commitment>,
 	) -> Result<HashMap<pedersen::Commitment, String>, libwallet::Error> {
-		let res = client::get_outputs_from_node(addr, wallet_outputs)
+		let res = client::get_outputs_from_node(self.node_url(), wallet_outputs)
+			.context(libwallet::ErrorKind::Node)?;
+		Ok(res)
+	}
+
+	/// Outputs by PMMR index
+	fn get_outputs_by_pmmr_index(
+		&self,
+		start_height: u64,
+		max_outputs: u64,
+	) -> Result<
+		(
+			u64,
+			u64,
+			Vec<(pedersen::Commitment, pedersen::RangeProof, bool)>,
+		),
+		libwallet::Error,
+	> {
+		let res = client::get_outputs_by_pmmr_index(self.node_url(), start_height, max_outputs)
 			.context(libwallet::ErrorKind::Node)?;
 		Ok(res)
 	}
@@ -352,7 +365,6 @@ impl<K> WalletClient for FileWallet<K> {
 	/// Get any missing block hashes from node
 	fn get_missing_block_hashes_from_node(
 		&self,
-		addr: &str,
 		height: u64,
 		wallet_outputs: Vec<pedersen::Commitment>,
 	) -> Result<
@@ -362,18 +374,17 @@ impl<K> WalletClient for FileWallet<K> {
 		),
 		libwallet::Error,
 	> {
-		let res = client::get_missing_block_hashes_from_node(addr, height, wallet_outputs)
-			.context(libwallet::ErrorKind::Node)?;
+		let res =
+			client::get_missing_block_hashes_from_node(self.node_url(), height, wallet_outputs)
+				.context(libwallet::ErrorKind::Node)?;
 		Ok(res)
 	}
 
 	/// retrieve merkle proof for a commit from a node
-	fn get_merkle_proof_for_commit(
-		&self,
-		_addr: &str,
-		_commit: &str,
-	) -> Result<MerkleProofWrapper, libwallet::Error> {
-		Err(libwallet::ErrorKind::GenericError("Not Implemented"))?
+	fn create_merkle_proof(&self, commit: &str) -> Result<MerkleProofWrapper, libwallet::Error> {
+		let res = client::create_merkle_proof(self.node_url(), commit)
+			.context(libwallet::ErrorKind::Node)?;
+		Ok(res)
 	}
 }
 

wallet/src/libwallet/api.rs

@@ -22,10 +22,11 @@ use std::marker::PhantomData;
 use core::ser;
 use keychain::Keychain;
 use libtx::slate::Slate;
-use libwallet::Error;
 use libwallet::internal::{tx, updater};
-use libwallet::types::{BlockFees, CbData, OutputData, TxWrapper, WalletBackend, WalletClient,
-                       WalletInfo};
+use libwallet::types::{
+	BlockFees, CbData, OutputData, TxWrapper, WalletBackend, WalletClient, WalletInfo,
+};
+use libwallet::Error;
 use util::{self, LOGGER};
 
 /// Wrapper around internal API functions, containing a reference to
@@ -102,7 +103,7 @@ where
 			selection_strategy_is_use_all,
 		)?;
 
-		let mut slate = match self.wallet.send_tx_slate(dest, &slate) {
+		let mut slate = match self.wallet.send_tx_slate(&slate) {
 			Ok(s) => s,
 			Err(e) => {
 				error!(
@@ -117,8 +118,7 @@ where
 
 		// All good here, so let's post it
 		let tx_hex = util::to_hex(ser::ser_vec(&slate.tx).unwrap());
-		self.wallet
-			.post_tx(self.wallet.node_url(), &TxWrapper { tx_hex: tx_hex }, fluff)?;
+		self.wallet.post_tx(&TxWrapper { tx_hex: tx_hex }, fluff)?;
 
 		// All good here, lock our inputs
 		lock_fn(self.wallet)?;
@@ -134,8 +134,7 @@ where
 	) -> Result<(), Error> {
 		let tx_burn = tx::issue_burn_tx(self.wallet, amount, minimum_confirmations, max_outputs)?;
 		let tx_hex = util::to_hex(ser::ser_vec(&tx_burn).unwrap());
-		self.wallet
-			.post_tx(self.wallet.node_url(), &TxWrapper { tx_hex: tx_hex }, false)?;
+		self.wallet.post_tx(&TxWrapper { tx_hex: tx_hex }, false)?;
 		Ok(())
 	}
 
@@ -146,7 +145,7 @@ where
 
 	/// Retrieve current height from node
 	pub fn node_height(&mut self) -> Result<(u64, bool), Error> {
-		match self.wallet.get_chain_height(self.wallet.node_url()) {
+		match self.wallet.get_chain_height() {
 			Ok(height) => Ok((height, true)),
 			Err(_) => {
 				let outputs = self.retrieve_outputs(true, false)?;

wallet/src/libwallet/internal/restore.rs

@@ -13,182 +13,146 @@
 // limitations under the License.
 //! Functions to restore a wallet's outputs from just the master seed
 
-/// TODO: Remove api
-use api;
-use byteorder::{BigEndian, ByteOrder};
 use core::global;
-use error::{Error, ErrorKind};
-use failure::Fail;
 use keychain::{Identifier, Keychain};
 use libtx::proof;
 use libwallet::types::*;
-use util::secp::pedersen;
+use libwallet::Error;
+use util::secp::{key::SecretKey, pedersen};
 use util::{self, LOGGER};
 
-fn get_merkle_proof_for_commit(node_addr: &str, commit: &str) -> Result<MerkleProofWrapper, Error> {
-	let url = format!("{}/v1/txhashset/merkleproof?id={}", node_addr, commit);
-
-	match api::client::get::<api::OutputPrintable>(url.as_str()) {
-		Ok(output) => Ok(MerkleProofWrapper(output.merkle_proof.unwrap())),
-		Err(e) => {
-			// if we got anything other than 200 back from server, bye
-			error!(
-				LOGGER,
-				"get_merkle_proof_for_pos: Restore failed... unable to create merkle proof for commit {}. Error: {}",
-				commit,
-				e
-			);
-			Err(e.context(ErrorKind::Node).into())
-		}
-	}
-}
-fn coinbase_status(output: &api::OutputPrintable) -> bool {
-	match output.output_type {
-		api::OutputType::Coinbase => true,
-		api::OutputType::Transaction => false,
-	}
+/// Utility struct for return values from below
+struct OutputResult {
+	///
+	pub commit: pedersen::Commitment,
+	///
+	pub key_id: Option<Identifier>,
+	///
+	pub n_child: Option<u32>,
+	///
+	pub value: u64,
+	///
+	pub height: u64,
+	///
+	pub lock_height: u64,
+	///
+	pub is_coinbase: bool,
+	///
+	pub merkle_proof: Option<MerkleProofWrapper>,
+	///
+	pub blinding: SecretKey,
 }
 
-fn outputs_batch<T, K>(wallet: &T, start_height: u64, max: u64) -> Result<api::OutputListing, Error>
-where
-	T: WalletBackend<K> + WalletClient,
-	K: Keychain,
-{
-	let query_param = format!("start_index={}&max={}", start_height, max);
-
-	let url = format!("{}/v1/txhashset/outputs?{}", wallet.node_url(), query_param,);
-
-	match api::client::get::<api::OutputListing>(url.as_str()) {
-		Ok(o) => Ok(o),
-		Err(e) => {
-			// if we got anything other than 200 back from server, bye
-			error!(
-				LOGGER,
-				"outputs_batch: Restore failed... unable to contact API {}. Error: {}",
-				wallet.node_url(),
-				e
-			);
-			Err(e.context(ErrorKind::Node))?
-		}
-	}
-}
-
-// TODO - wrap the many return values in a struct
-fn find_outputs_with_key<T, K>(
+fn identify_utxo_outputs<T, K>(
 	wallet: &mut T,
-	outputs: Vec<api::OutputPrintable>
-) -> Vec<(
-	pedersen::Commitment,
-	Identifier,
-	u32,
-	u64,
-	u64,
-	u64,
-	bool,
-	Option<MerkleProofWrapper>,
-)>
+	outputs: Vec<(pedersen::Commitment, pedersen::RangeProof, bool)>,
+) -> Result<Vec<OutputResult>, Error>
 where
 	T: WalletBackend<K> + WalletClient,
 	K: Keychain,
 {
-	let mut wallet_outputs: Vec<(
-		pedersen::Commitment,
-		Identifier,
-		u32,
-		u64,
-		u64,
-		u64,
-		bool,
-		Option<MerkleProofWrapper>,
-	)> = Vec::new();
+	let mut wallet_outputs: Vec<OutputResult> = Vec::new();
 
-	let max_derivations = 1_000_000;
+	info!(
+		LOGGER,
+		"Scanning {} outputs in the current Grin utxo set",
+		outputs.len(),
+	);
+	let current_chain_height = wallet.get_chain_height()?;
 
-	info!(LOGGER, "Scanning {} outputs", outputs.len(),);
-	let current_chain_height = wallet.get_chain_height(wallet.node_url()).unwrap();
-
-	for output in outputs.iter().filter(|x| !x.spent) {
+	for output in outputs.iter() {
+		let (commit, proof, is_coinbase) = output;
 		// attempt to unwind message from the RP and get a value
 		// will fail if it's not ours
-		let info = proof::rewind(
-			wallet.keychain(),
-			output.commit,
-			None,
-			output.range_proof().unwrap(),
-		).unwrap();
+		let info = proof::rewind(wallet.keychain(), *commit, None, *proof)?;
 
 		if !info.success {
 			continue;
 		}
 
-		// we have a match, now check through our key iterations to find out which one it was
-		let mut found = false;
-		let mut start_index = 1;
+		info!(
+			LOGGER,
+			"Output found: {:?}, amount: {:?}", commit, info.value
+		);
 
-		for i in start_index..max_derivations {
-			let key_id = &wallet.keychain().derive_key_id(i as u32).unwrap();
-			let b = wallet.keychain().derived_key(key_id).unwrap();
-			if info.blinding != b {
+		let mut merkle_proof = None;
+		let commit_str = util::to_hex(commit.as_ref().to_vec());
+
+		if *is_coinbase {
+			merkle_proof = Some(wallet.create_merkle_proof(&commit_str)?);
+		}
+
+		let height = current_chain_height;
+		let lock_height = if *is_coinbase {
+			height + global::coinbase_maturity()
+		} else {
+			height
+		};
+
+		wallet_outputs.push(OutputResult {
+			commit: *commit,
+			key_id: None,
+			n_child: None,
+			value: info.value,
+			height: height,
+			lock_height: lock_height,
+			is_coinbase: *is_coinbase,
+			merkle_proof: merkle_proof,
+			blinding: info.blinding,
+		});
+	}
+	Ok(wallet_outputs)
+}
+
+/// Attempts to populate a list of outputs with their
+/// correct child indices based on the root key
+fn populate_child_indices<T, K>(
+	wallet: &mut T,
+	outputs: &mut Vec<OutputResult>,
+	max_derivations: u32,
+) -> Result<(), Error>
+where
+	T: WalletBackend<K> + WalletClient,
+	K: Keychain,
+{
+	info!(
+		LOGGER,
+		"Attempting to populate child indices and key identifiers for {} identified outputs",
+		outputs.len()
+	);
+
+	// keep track of child keys we've already found, and avoid some EC ops
+	let mut found_child_indices: Vec<u32> = vec![];
+	for output in outputs.iter_mut() {
+		let mut found = false;
+		for i in 1..max_derivations {
+			// seems to be a bug allowing multiple child keys at the moment
+			/*if found_child_indices.contains(&i){
+				continue;
+			}*/
+			let key_id = wallet.keychain().derive_key_id(i as u32)?;
+			let b = wallet.keychain().derived_key(&key_id)?;
+			if output.blinding != b {
 				continue;
 			}
 			found = true;
-			// we have a partial match, let's just confirm
+			found_child_indices.push(i);
 			info!(
 				LOGGER,
-				"Output found: {:?}, key_index: {:?}", output.commit, i,
+				"Key index {} found for output {:?}", i, output.commit
 			);
-			// add it to result set here
-			let commit_id = output.commit.0;
-			let is_coinbase = coinbase_status(output);
-
-			info!(LOGGER, "Amount: {}", info.value);
-
-			let commit = wallet
-				.keychain()
-				.commit_with_key_index(BigEndian::read_u64(&commit_id), i as u32)
-				.expect("commit with key index");
-
-			let mut merkle_proof = None;
-			let commit_str = util::to_hex(output.commit.as_ref().to_vec());
-
-			if is_coinbase {
-				merkle_proof =
-					Some(get_merkle_proof_for_commit(wallet.node_url(), &commit_str).unwrap());
-			}
-
-			let height = current_chain_height;
-			let lock_height = if is_coinbase {
-				height + global::coinbase_maturity()
-			} else {
-				height
-			};
-
-			wallet_outputs.push((
-				commit,
-				key_id.clone(),
-				i as u32,
-				info.value,
-				height,
-				lock_height,
-				is_coinbase,
-				merkle_proof,
-			));
-
+			output.key_id = Some(key_id);
+			output.n_child = Some(i);
 			break;
 		}
 		if !found {
 			warn!(
 				LOGGER,
-				"Very probable matching output found with amount: {} \
-				 but didn't match key child key up to {}",
-				info.value,
-				max_derivations,
+				"Unable to find child key index for: {:?}", output.commit,
 			);
 		}
 	}
-	debug!(LOGGER, "Found {} wallet_outputs", wallet_outputs.len(),);
-
-	wallet_outputs
+	Ok(())
 }
 
 /// Restore a wallet
@@ -197,6 +161,8 @@ where
 	T: WalletBackend<K> + WalletClient,
 	K: Keychain,
 {
+	let max_derivations = 1_000_000;
+
 	// Don't proceed if wallet.dat has anything in it
 	let is_empty = wallet.iter().next().is_none();
 	if !is_empty {
@@ -211,42 +177,59 @@ where
 
 	let batch_size = 1000;
 	let mut start_index = 1;
-	// this will start here, then lower as outputs are found, moving backwards on
-	// the chain
+	let mut result_vec: Vec<OutputResult> = vec![];
 	loop {
-		let output_listing = outputs_batch(wallet, start_index, batch_size)?;
+		let (highest_index, last_retrieved_index, outputs) =
+			wallet.get_outputs_by_pmmr_index(start_index, batch_size)?;
 		info!(
 			LOGGER,
 			"Retrieved {} outputs, up to index {}. (Highest index: {})",
-			output_listing.outputs.len(),
-			output_listing.last_retrieved_index,
-			output_listing.highest_index
+			outputs.len(),
+			highest_index,
+			last_retrieved_index,
 		);
 
-		let root_key_id = wallet.keychain().root_key_id();
-		let result_vec =
-			find_outputs_with_key(wallet, output_listing.outputs.clone());
-		let mut batch = wallet.batch()?;
-		for output in result_vec {
-			let _ = batch.save(OutputData {
-				root_key_id: root_key_id.clone(),
-				key_id: output.1.clone(),
-				n_child: output.2,
-				value: output.3,
-				status: OutputStatus::Unconfirmed,
-				height: output.4,
-				lock_height: output.5,
-				is_coinbase: output.6,
-				block: None,
-				merkle_proof: output.7,
-			});
-		}
-		batch.commit()?;
+		result_vec.append(&mut identify_utxo_outputs(wallet, outputs.clone())?);
 
-		if output_listing.highest_index == output_listing.last_retrieved_index {
+		if highest_index == last_retrieved_index {
 			break;
 		}
-		start_index = output_listing.last_retrieved_index + 1;
+		start_index = last_retrieved_index + 1;
 	}
+
+	info!(
+		LOGGER,
+		"Identified {} wallet_outputs as belonging to this wallet",
+		result_vec.len(),
+	);
+
+	populate_child_indices(wallet, &mut result_vec, max_derivations)?;
+
+	// Now save what we have
+	let root_key_id = wallet.keychain().root_key_id();
+	let mut batch = wallet.batch()?;
+	for output in result_vec {
+		if output.key_id.is_some() && output.n_child.is_some() {
+			let _ = batch.save(OutputData {
+				root_key_id: root_key_id.clone(),
+				key_id: output.key_id.unwrap(),
+				n_child: output.n_child.unwrap(),
+				value: output.value,
+				status: OutputStatus::Unconfirmed,
+				height: output.height,
+				lock_height: output.lock_height,
+				is_coinbase: output.is_coinbase,
+				block: None,
+				merkle_proof: output.merkle_proof,
+			});
+		} else {
+			warn!(
+				LOGGER,
+				"Commit {:?} identified but unable to recover key. Output has not been restored.",
+				output.commit
+			);
+		}
+	}
+	batch.commit()?;
 	Ok(())
 }

wallet/src/libwallet/internal/tx.rs

@@ -72,7 +72,7 @@ where
 	K: Keychain,
 {
 	// Get lock height
-	let current_height = wallet.get_chain_height(wallet.node_url())?;
+	let current_height = wallet.get_chain_height()?;
 	// ensure outputs we're selecting are up to date
 	updater::refresh_outputs(wallet)?;
 
@@ -144,7 +144,7 @@ where
 	// &Identifier::zero());
 	let keychain = wallet.keychain().clone();
 
-	let current_height = wallet.get_chain_height(wallet.node_url())?;
+	let current_height = wallet.get_chain_height()?;
 
 	let _ = updater::refresh_outputs(wallet);
 

wallet/src/libwallet/internal/updater.rs

@@ -16,8 +16,8 @@
 //! the wallet storage and update them.
 
 use failure::ResultExt;
-use std::collections::HashMap;
 use std::collections::hash_map::Entry;
+use std::collections::HashMap;
 
 use core::consensus::reward;
 use core::core::{Output, TxKernel};
@@ -27,8 +27,9 @@ use libtx::reward;
 use libwallet;
 use libwallet::error::{Error, ErrorKind};
 use libwallet::internal::keys;
-use libwallet::types::{BlockFees, CbData, OutputData, OutputStatus, WalletBackend, WalletClient,
-                       WalletInfo};
+use libwallet::types::{
+	BlockFees, CbData, OutputData, OutputStatus, WalletBackend, WalletClient, WalletInfo,
+};
 use util::secp::pedersen;
 use util::{self, LOGGER};
 
@@ -63,7 +64,7 @@ where
 	T: WalletBackend<K> + WalletClient,
 	K: Keychain,
 {
-	let height = wallet.get_chain_height(wallet.node_url())?;
+	let height = wallet.get_chain_height()?;
 	refresh_output_state(wallet, height)?;
 	refresh_missing_block_hashes(wallet, height)?;
 	Ok(())
@@ -94,7 +95,7 @@ where
 	);
 
 	let (api_blocks, api_merkle_proofs) =
-		wallet.get_missing_block_hashes_from_node(wallet.node_url(), height, wallet_output_keys)?;
+		wallet.get_missing_block_hashes_from_node(height, wallet_output_keys)?;
 
 	// now for each commit, find the output in the wallet and
 	// the corresponding api output (if it exists)
@@ -151,7 +152,8 @@ where
 	let mut wallet_outputs: HashMap<pedersen::Commitment, Identifier> = HashMap::new();
 	let keychain = wallet.keychain().clone();
 	let unspents = wallet.iter().filter(|x| {
-		x.root_key_id == keychain.root_key_id() && x.block.is_none()
+		x.root_key_id == keychain.root_key_id()
+			&& x.block.is_none()
 			&& x.status == OutputStatus::Unspent
 	});
 	for out in unspents {
@@ -208,7 +210,7 @@ where
 
 	let wallet_output_keys = wallet_outputs.keys().map(|commit| commit.clone()).collect();
 
-	let api_outputs = wallet.get_outputs_from_node(wallet.node_url(), wallet_output_keys)?;
+	let api_outputs = wallet.get_outputs_from_node(wallet_output_keys)?;
 	apply_api_outputs(wallet, &wallet_outputs, &api_outputs, height)?;
 	clean_old_unconfirmed(wallet, height)?;
 	Ok(())

wallet/src/libwallet/types.rs

@@ -108,30 +108,46 @@ pub trait WalletClient {
 	fn node_url(&self) -> &str;
 
 	/// Call the wallet API to create a coinbase transaction
-	fn create_coinbase(&self, dest: &str, block_fees: &BlockFees) -> Result<CbData, Error>;
+	fn create_coinbase(&self, block_fees: &BlockFees) -> Result<CbData, Error>;
 
 	/// Send a transaction slate to another listening wallet and return result
 	/// TODO: Probably need a slate wrapper type
-	fn send_tx_slate(&self, dest: &str, slate: &Slate) -> Result<Slate, Error>;
+	fn send_tx_slate(&self, slate: &Slate) -> Result<Slate, Error>;
 
 	/// Posts a transaction to a grin node
-	fn post_tx(&self, dest: &str, tx: &TxWrapper, fluff: bool) -> Result<(), Error>;
+	fn post_tx(&self, tx: &TxWrapper, fluff: bool) -> Result<(), Error>;
 
 	/// retrieves the current tip from the specified grin node
-	fn get_chain_height(&self, addr: &str) -> Result<u64, Error>;
+	fn get_chain_height(&self) -> Result<u64, Error>;
 
 	/// retrieve a list of outputs from the specified grin node
 	/// need "by_height" and "by_id" variants
 	fn get_outputs_from_node(
 		&self,
-		addr: &str,
 		wallet_outputs: Vec<pedersen::Commitment>,
 	) -> Result<HashMap<pedersen::Commitment, String>, Error>;
 
+	/// Get a list of outputs from the node by traversing the UTXO
+	/// set in PMMR index order.
+	/// Returns
+	/// (last available output index, last insertion index retrieved,
+	/// outputs(commit, proof, is_coinbase))
+	fn get_outputs_by_pmmr_index(
+		&self,
+		start_height: u64,
+		max_outputs: u64,
+	) -> Result<
+		(
+			u64,
+			u64,
+			Vec<(pedersen::Commitment, pedersen::RangeProof, bool)>,
+		),
+		Error,
+	>;
+
 	/// Get any missing block hashes from node
 	fn get_missing_block_hashes_from_node(
 		&self,
-		addr: &str,
 		height: u64,
 		wallet_outputs: Vec<pedersen::Commitment>,
 	) -> Result<
@@ -142,12 +158,8 @@ pub trait WalletClient {
 		Error,
 	>;
 
-	/// retrieve merkle proof for a commit from a node
-	fn get_merkle_proof_for_commit(
-		&self,
-		addr: &str,
-		commit: &str,
-	) -> Result<MerkleProofWrapper, Error>;
+	/// create merkle proof for a commit from a node at the current height
+	fn create_merkle_proof(&self, commit: &str) -> Result<MerkleProofWrapper, Error>;
 }
 
 /// Information about an output that's being tracked by the wallet. Must be

wallet/src/lmdb_wallet.rs

@@ -13,8 +13,8 @@
 // limitations under the License.
 
 use std::cell::RefCell;
-use std::collections::HashMap;
 use std::collections::hash_map::Values;
+use std::collections::HashMap;
 use std::ops::Deref;
 use std::sync::Arc;
 use std::{fs, path};
@@ -190,28 +190,28 @@ impl<K> WalletClient for LMDBBackend<K> {
 	}
 
 	/// Call the wallet API to create a coinbase transaction
-	fn create_coinbase(&self, dest: &str, block_fees: &BlockFees) -> Result<CbData, Error> {
-		let res = client::create_coinbase(dest, block_fees)
+	fn create_coinbase(&self, block_fees: &BlockFees) -> Result<CbData, Error> {
+		let res = client::create_coinbase(self.node_url(), block_fees)
 			.context(ErrorKind::WalletComms(format!("Creating Coinbase")))?;
 		Ok(res)
 	}
 
 	/// Send a transaction slate to another listening wallet and return result
-	fn send_tx_slate(&self, dest: &str, slate: &Slate) -> Result<Slate, Error> {
-		let res = client::send_tx_slate(dest, slate)
+	fn send_tx_slate(&self, slate: &Slate) -> Result<Slate, Error> {
+		let res = client::send_tx_slate(self.node_url(), slate)
 			.context(ErrorKind::WalletComms(format!("Sending transaction")))?;
 		Ok(res)
 	}
 
 	/// Posts a tranaction to a grin node
-	fn post_tx(&self, dest: &str, tx: &TxWrapper, fluff: bool) -> Result<(), Error> {
-		let res = client::post_tx(dest, tx, fluff).context(ErrorKind::Node)?;
+	fn post_tx(&self, tx: &TxWrapper, fluff: bool) -> Result<(), Error> {
+		let res = client::post_tx(self.node_url(), tx, fluff).context(ErrorKind::Node)?;
 		Ok(res)
 	}
 
 	/// retrieves the current tip from the specified grin node
-	fn get_chain_height(&self, addr: &str) -> Result<u64, Error> {
-		let res = client::get_chain_height(addr).context(ErrorKind::Node)?;
+	fn get_chain_height(&self) -> Result<u64, Error> {
+		let res = client::get_chain_height(self.node_url()).context(ErrorKind::Node)?;
 		Ok(res)
 	}
 
@@ -219,17 +219,34 @@ impl<K> WalletClient for LMDBBackend<K> {
 	/// need "by_height" and "by_id" variants
 	fn get_outputs_from_node(
 		&self,
-		addr: &str,
 		wallet_outputs: Vec<pedersen::Commitment>,
 	) -> Result<HashMap<pedersen::Commitment, String>, Error> {
-		let res = client::get_outputs_from_node(addr, wallet_outputs).context(ErrorKind::Node)?;
+		let res = client::get_outputs_from_node(self.node_url(), wallet_outputs)
+			.context(ErrorKind::Node)?;
+		Ok(res)
+	}
+
+	/// Outputs by PMMR index
+	fn get_outputs_by_pmmr_index(
+		&self,
+		start_height: u64,
+		max_outputs: u64,
+	) -> Result<
+		(
+			u64,
+			u64,
+			Vec<(pedersen::Commitment, pedersen::RangeProof, bool)>,
+		),
+		Error,
+	> {
+		let res = client::get_outputs_by_pmmr_index(self.node_url(), start_height, max_outputs)
+			.context(ErrorKind::Node)?;
 		Ok(res)
 	}
 
 	/// Get any missing block hashes from node
 	fn get_missing_block_hashes_from_node(
 		&self,
-		addr: &str,
 		height: u64,
 		wallet_outputs: Vec<pedersen::Commitment>,
 	) -> Result<
@@ -239,17 +256,15 @@ impl<K> WalletClient for LMDBBackend<K> {
 		),
 		Error,
 	> {
-		let res = client::get_missing_block_hashes_from_node(addr, height, wallet_outputs)
-			.context(ErrorKind::Node)?;
+		let res =
+			client::get_missing_block_hashes_from_node(self.node_url(), height, wallet_outputs)
+				.context(ErrorKind::Node)?;
 		Ok(res)
 	}
 
 	/// retrieve merkle proof for a commit from a node
-	fn get_merkle_proof_for_commit(
-		&self,
-		addr: &str,
-		commit: &str,
-	) -> Result<MerkleProofWrapper, Error> {
-		Err(ErrorKind::GenericError("Not Implemented"))?
+	fn create_merkle_proof(&self, commit: &str) -> Result<MerkleProofWrapper, Error> {
+		let res = client::create_merkle_proof(self.node_url(), commit).context(ErrorKind::Node)?;
+		Ok(res)
 	}
 }