From f8bb55a08677ba8cbcdbfe927ccf5e159202df05 Mon Sep 17 00:00:00 2001 From: AntiochP <30642645+antiochp@users.noreply.github.com> Date: Wed, 20 Dec 2017 17:16:40 -0500 Subject: [PATCH] fixup error handling in wallet restore (was failing when unable to recover amount) (#526) (#527) --- src/bin/grin.rs | 129 +++++++++++----------- wallet/src/restore.rs | 247 ++++++++++++++++++++++++++++-------------- wallet/src/types.rs | 1 + 3 files changed, 231 insertions(+), 146 deletions(-) diff --git a/src/bin/grin.rs b/src/bin/grin.rs index de6af4361..0b3483b01 100644 --- a/src/bin/grin.rs +++ b/src/bin/grin.rs @@ -69,14 +69,14 @@ fn start_from_config_file(mut global_config: GlobalConfig) { fn main() { // First, load a global config object, - // then modify that object with any switches - // found so that the switches override the - // global config file + // then modify that object with any switches + // found so that the switches override the + // global config file // This will return a global config object, - // which will either contain defaults for all // of the config structures or a - // configuration - // read from a config file + // which will either contain defaults for all // of the config structures or a + // configuration + // read from a config file let mut global_config = GlobalConfig::new(None).unwrap_or_else(|e| { panic!("Error parsing config file: {}", e); @@ -270,12 +270,14 @@ fn main() { } // client commands and options - ("client", Some(client_args)) => match client_args.subcommand() { - ("status", _) => { - println!("status info..."); + ("client", Some(client_args)) => { + match client_args.subcommand() { + ("status", _) => { + println!("status info..."); + } + _ => panic!("Unknown client command, use 'grin help client' for details"), } - _ => panic!("Unknown client command, use 'grin help client' for details"), - }, + } // client commands and options ("wallet", Some(wallet_args)) => { @@ -290,7 +292,7 @@ fn main() { start_from_config_file(global_config); } else { // won't attempt to just start with defaults, - // and will reject + // and will reject println!("Unknown command, and no configuration file was found."); println!("Use 'grin help' for a list of all commands."); } @@ -382,14 +384,14 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) { wallet_config.check_node_api_http_addr = sa.to_string().clone(); } - let mut key_derivations:u32=1000; + let mut key_derivations: u32 = 1000; if let Some(kd) = wallet_args.value_of("key_derivations") { - key_derivations=kd.parse().unwrap(); + key_derivations = kd.parse().unwrap(); } - let mut show_spent=false; + let mut show_spent = false; if wallet_args.is_present("show_spent") { - show_spent=true; + show_spent = true; } // Derive the keychain based on seed from seed file and specified passphrase. @@ -403,12 +405,12 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) { let wallet_seed = wallet::WalletSeed::from_file(&wallet_config).expect("Failed to read wallet seed file."); - let passphrase = wallet_args - .value_of("pass") - .expect("Failed to read passphrase."); - let keychain = wallet_seed - .derive_keychain(&passphrase) - .expect("Failed to derive keychain from seed file and passphrase."); + let passphrase = wallet_args.value_of("pass").expect( + "Failed to read passphrase.", + ); + let keychain = wallet_seed.derive_keychain(&passphrase).expect( + "Failed to derive keychain from seed file and passphrase.", + ); match wallet_args.subcommand() { ("listen", Some(listen_args)) => { @@ -416,38 +418,38 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) { wallet_config.api_listen_port = port.to_string(); } wallet::server::start_rest_apis(wallet_config, keychain); - }, + } ("receive", Some(receive_args)) => { - let input = receive_args - .value_of("input") - .expect("Input file required"); - let mut file = File::open(input) - .expect("Unable to open transaction file."); + let input = receive_args.value_of("input").expect("Input file required"); + let mut file = File::open(input).expect("Unable to open transaction file."); let mut contents = String::new(); - file.read_to_string(&mut contents) - .expect("Unable to read transaction file."); + file.read_to_string(&mut contents).expect( + "Unable to read transaction file.", + ); wallet::receive_json_tx_str(&wallet_config, &keychain, contents.as_str()).unwrap(); - }, + } ("send", Some(send_args)) => { - let amount = send_args - .value_of("amount") - .expect("Amount to send required"); - let amount = core::core::amount_from_hr_string(amount) - .expect("Could not parse amount as a number with optional decimal point."); - let minimum_confirmations: u64 = send_args - .value_of("minimum_confirmations") - .unwrap() - .parse() - .expect("Could not parse minimum_confirmations as a whole number."); - let selection_strategy = send_args - .value_of("selection_strategy") - .expect("Selection strategy required"); + let amount = send_args.value_of("amount").expect( + "Amount to send required", + ); + let amount = core::core::amount_from_hr_string(amount).expect( + "Could not parse amount as a number with optional decimal point.", + ); + let minimum_confirmations: u64 = + send_args + .value_of("minimum_confirmations") + .unwrap() + .parse() + .expect("Could not parse minimum_confirmations as a whole number."); + let selection_strategy = send_args.value_of("selection_strategy").expect( + "Selection strategy required", + ); let mut dest = "stdout"; if let Some(d) = send_args.value_of("dest") { dest = d; } let max_outputs = 500; - let result=wallet::issue_send_tx( + let result = wallet::issue_send_tx( &wallet_config, &keychain, amount, @@ -464,34 +466,33 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) { amount_to_hr_string(amount), dest, selection_strategy, - )}, + ) + } Err(wallet::Error::NotEnoughFunds(available)) => { error!( LOGGER, "Tx not sent: insufficient funds (max: {})", amount_to_hr_string(available), ); - }, + } Err(e) => { - error!( - LOGGER, - "Tx not sent: {:?}", - e - ); - }, + error!(LOGGER, "Tx not sent: {:?}", e); + } }; } ("burn", Some(send_args)) => { - let amount = send_args - .value_of("amount") - .expect("Amount to burn required"); - let amount = core::core::amount_from_hr_string(amount) - .expect("Could not parse amount as number with optional decimal point."); - let minimum_confirmations: u64 = send_args - .value_of("minimum_confirmations") - .unwrap() - .parse() - .expect("Could not parse minimum_confirmations as a whole number."); + let amount = send_args.value_of("amount").expect( + "Amount to burn required", + ); + let amount = core::core::amount_from_hr_string(amount).expect( + "Could not parse amount as number with optional decimal point.", + ); + let minimum_confirmations: u64 = + send_args + .value_of("minimum_confirmations") + .unwrap() + .parse() + .expect("Could not parse minimum_confirmations as a whole number."); let max_outputs = 500; wallet::issue_burn_tx( &wallet_config, @@ -508,7 +509,7 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) { wallet::show_outputs(&wallet_config, &keychain, show_spent); } ("restore", Some(_)) => { - let _=wallet::restore(&wallet_config, &keychain, key_derivations); + let _ = wallet::restore(&wallet_config, &keychain, key_derivations); } _ => panic!("Unknown wallet command, use 'grin help wallet' for details"), } diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index f985abde9..d750600a0 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -16,22 +16,16 @@ use keychain::{Keychain, Identifier}; use util::{LOGGER, from_hex}; use util::secp::pedersen; use api; -use core::core::{Output,SwitchCommitHash}; +use core::core::{Output, SwitchCommitHash}; use core::core::transaction::{COINBASE_OUTPUT, DEFAULT_OUTPUT, SWITCH_COMMIT_HASH_SIZE}; use types::{WalletConfig, WalletData, OutputData, OutputStatus, Error}; use byteorder::{BigEndian, ByteOrder}; -pub fn get_chain_height(config: &WalletConfig)-> - Result{ - let url = format!( - "{}/v1/chain", - config.check_node_api_http_addr - ); +pub fn get_chain_height(config: &WalletConfig) -> Result { + let url = format!("{}/v1/chain", config.check_node_api_http_addr); match api::client::get::(url.as_str()) { - Ok(tip) => { - Ok(tip.height) - }, + Ok(tip) => Ok(tip.height), Err(e) => { // if we got anything other than 200 back from server, bye error!(LOGGER, "Restore failed... unable to contact node: {}", e); @@ -40,10 +34,9 @@ pub fn get_chain_height(config: &WalletConfig)-> } } -fn output_with_range_proof(config:&WalletConfig, commit_id: &str) -> - Result{ - - let url = format!( +fn output_with_range_proof(config: &WalletConfig, commit_id: &str) -> Result { + let url = + format!( "{}/v1/chain/utxos/byids?id={}&include_rp&include_switch", config.check_node_api_http_addr, commit_id, @@ -51,43 +44,59 @@ fn output_with_range_proof(config:&WalletConfig, commit_id: &str) -> match api::client::get::>(url.as_str()) { Ok(outputs) => { - Ok(outputs[0].clone()) - }, + if let Some(output) = outputs.first() { + Ok(output.clone()) + } else { + Err(Error::Node(api::Error::NotFound)) + } + } Err(e) => { - // if we got anything other than 200 back from server, don't attempt to refresh the wallet + // if we got anything other than 200 back from server, don't attempt to refresh + // the wallet // data after Err(Error::Node(e)) } } } -fn retrieve_amount_and_coinbase_status(config:&WalletConfig, keychain: &Keychain, - key_id: Identifier, commit_id: &str) -> (u64, bool) { - let output = output_with_range_proof(config, commit_id).unwrap(); +fn retrieve_amount_and_coinbase_status( + config: &WalletConfig, + keychain: &Keychain, + key_id: Identifier, + commit_id: &str, +) -> Result<(u64, bool), Error> { + let output = output_with_range_proof(config, commit_id)?; let core_output = Output { - features : match output.output_type { + features: match output.output_type { api::OutputType::Coinbase => COINBASE_OUTPUT, api::OutputType::Transaction => DEFAULT_OUTPUT, }, - proof: output.proof.unwrap(), - switch_commit_hash: output.switch_commit_hash.unwrap(), + proof: output.proof.expect("output with proof"), + switch_commit_hash: output.switch_commit_hash.expect("output with switch_commit_hash"), commit: output.commit, }; - let amount=core_output.recover_value(keychain, &key_id).unwrap(); - let is_coinbase = match output.output_type { - api::OutputType::Coinbase => true, - api::OutputType::Transaction => false, - }; - (amount, is_coinbase) + if let Some(amount) = core_output.recover_value(keychain, &key_id) { + let is_coinbase = match output.output_type { + api::OutputType::Coinbase => true, + api::OutputType::Transaction => false, + }; + Ok((amount, is_coinbase)) + } else { + Err(Error::GenericError(format!("cannot recover value"))) + } } -pub fn utxos_batch_block(config: &WalletConfig, start_height: u64, end_height:u64)-> - Result, Error>{ +pub fn utxos_batch_block( + config: &WalletConfig, + start_height: u64, + end_height: u64, +) -> Result, Error> { // build the necessary query param - // ?height=x - let query_param= format!("start_height={}&end_height={}", start_height, end_height); + let query_param = format!("start_height={}&end_height={}", start_height, end_height); - let url = format!( + let url = + format!( "{}/v1/chain/utxos/atheight?{}", config.check_node_api_http_addr, query_param, @@ -103,85 +112,158 @@ pub fn utxos_batch_block(config: &WalletConfig, start_height: u64, end_height:u6 } } -fn find_utxos_with_key(config:&WalletConfig, keychain: &Keychain, - switch_commit_cache : &Vec<[u8;SWITCH_COMMIT_HASH_SIZE]>, - block_outputs:api::BlockOutputs, key_iterations: &mut usize, padding: &mut usize) - -> Vec<(pedersen::Commitment, Identifier, u32, u64, u64, bool) > { - //let key_id = keychain.clone().root_key_id(); - let mut wallet_outputs: Vec<(pedersen::Commitment, Identifier, u32, u64, u64, bool)> = Vec::new(); +fn find_utxos_with_key( + config: &WalletConfig, + keychain: &Keychain, + switch_commit_cache: &Vec<[u8; SWITCH_COMMIT_HASH_SIZE]>, + block_outputs: api::BlockOutputs, + key_iterations: &mut usize, + padding: &mut usize, +) -> Vec<(pedersen::Commitment, Identifier, u32, u64, u64, bool)> { + let mut wallet_outputs: Vec<(pedersen::Commitment, Identifier, u32, u64, u64, bool)> = + Vec::new(); + + info!( + LOGGER, + "Scanning block {}, {} outputs, over {} key derivations", + block_outputs.header.height, + block_outputs.outputs.len(), + *key_iterations, + ); - info!(LOGGER, "Scanning block {} over {} key derivation possibilities.", block_outputs.header.height, *key_iterations); for output in block_outputs.outputs { for i in 0..*key_iterations { - if switch_commit_cache[i as usize]==output.switch_commit_hash { - info!(LOGGER, "Output found: {:?}, key_index: {:?}", output.switch_commit_hash,i); - //add it to result set here + if switch_commit_cache[i as usize] == output.switch_commit_hash { + info!( + LOGGER, + "Output found: {:?}, key_index: {:?}", + output.switch_commit_hash, + i, + ); + + // add it to result set here let commit_id = from_hex(output.commit.clone()).unwrap(); let key_id = keychain.derive_key_id(i as u32).unwrap(); - let (amount, is_coinbase) = retrieve_amount_and_coinbase_status(config, - keychain, key_id.clone(), &output.commit); - info!(LOGGER, "Amount: {}", amount); - let commit = keychain.commit_with_key_index(BigEndian::read_u64(&commit_id), i as u32).unwrap(); - wallet_outputs.push((commit, key_id.clone(), i as u32, amount, output.height, is_coinbase)); - //probably don't have to look for indexes greater than this now - *key_iterations=i+*padding; - if *key_iterations > switch_commit_cache.len() { - *key_iterations = switch_commit_cache.len(); + let res = retrieve_amount_and_coinbase_status( + config, + keychain, + key_id.clone(), + &output.commit, + ); + + if let Ok((amount, is_coinbase)) = res { + info!(LOGGER, "Amount: {}", amount); + + let commit = keychain + .commit_with_key_index(BigEndian::read_u64(&commit_id), i as u32) + .expect("commit with key index"); + + wallet_outputs.push(( + commit, + key_id.clone(), + i as u32, + amount, + output.height, + is_coinbase, + )); + + // probably don't have to look for indexes greater than this now + *key_iterations = i + *padding; + if *key_iterations > switch_commit_cache.len() { + *key_iterations = switch_commit_cache.len(); + } + info!(LOGGER, "Setting max key index to: {}", *key_iterations); + break; + } else { + info!( + LOGGER, + "Unable to retrieve the amount (needs investigating)", + ); + } - info!(LOGGER, "Setting max key index to: {}", *key_iterations); - break; } } } + debug!( + LOGGER, + "Found {} wallet_outputs for block {}", + wallet_outputs.len(), + block_outputs.header.height, + ); + wallet_outputs } -pub fn restore(config: &WalletConfig, keychain: &Keychain, key_derivations:u32) -> - Result<(), Error>{ +pub fn restore( + config: &WalletConfig, + keychain: &Keychain, + key_derivations: u32, +) -> Result<(), Error> { // Don't proceed if wallet.dat has anything in it let is_empty = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { - wallet_data.outputs.len() == 0 + wallet_data.outputs.len() == 0 })?; if !is_empty { - error!(LOGGER, "Not restoring. Please back up and remove existing wallet.dat first."); - return Ok(()) + error!( + LOGGER, + "Not restoring. Please back up and remove existing wallet.dat first." + ); + return Ok(()); } -// Get height of chain from node (we'll check again when done) + // Get height of chain from node (we'll check again when done) let chain_height = get_chain_height(config)?; - info!(LOGGER, "Starting restore: Chain height is {}.", chain_height); + info!( + LOGGER, + "Starting restore: Chain height is {}.", + chain_height + ); - let mut switch_commit_cache : Vec<[u8;SWITCH_COMMIT_HASH_SIZE]> = vec![]; - info!(LOGGER, "Building key derivation cache to index {}.", key_derivations); + let mut switch_commit_cache: Vec<[u8; SWITCH_COMMIT_HASH_SIZE]> = vec![]; + info!( + LOGGER, + "Building key derivation cache ({}) ...", + key_derivations, + ); for i in 0..key_derivations { let switch_commit = keychain.switch_commit_from_index(i as u32).unwrap(); let switch_commit_hash = SwitchCommitHash::from_switch_commit(switch_commit); switch_commit_cache.push(switch_commit_hash.hash); } + debug!(LOGGER, "... done"); - let batch_size=100; - //this will start here, then lower as outputs are found, moving backwards on the chain - let mut key_iterations=key_derivations as usize; - //set to a percentage of the key_derivation value - let mut padding = (key_iterations as f64 *0.25) as usize; + let batch_size = 100; + // this will start here, then lower as outputs are found, moving backwards on + // the chain + let mut key_iterations = key_derivations as usize; + // set to a percentage of the key_derivation value + let mut padding = (key_iterations as f64 * 0.25) as usize; let mut h = chain_height; while { - let end_batch=h; + let end_batch = h; if h >= batch_size { - h-=batch_size; + h -= batch_size; } else { - h=0; + h = 0; } - let mut blocks = utxos_batch_block(config, h+1, end_batch)?; + let mut blocks = utxos_batch_block(config, h + 1, end_batch)?; blocks.reverse(); - for block in blocks { - let result_vec=find_utxos_with_key(config, keychain, &switch_commit_cache, - block, &mut key_iterations, &mut padding); - if result_vec.len() > 0 { - for output in result_vec.clone() { - let _ = WalletData::with_wallet(&config.data_file_dir, |wallet_data| { + + let _ = WalletData::with_wallet(&config.data_file_dir, |wallet_data| { + for block in blocks { + let result_vec = find_utxos_with_key( + config, + keychain, + &switch_commit_cache, + block, + &mut key_iterations, + &mut padding, + ); + if result_vec.len() > 0 { + for output in result_vec.clone() { let root_key_id = keychain.root_key_id(); - //Just plonk it in for now, and refresh actual values via wallet info command later + // Just plonk it in for now, and refresh actual values via wallet info + // command later wallet_data.add_output(OutputData { root_key_id: root_key_id.clone(), key_id: output.1.clone(), @@ -192,11 +274,12 @@ pub fn restore(config: &WalletConfig, keychain: &Keychain, key_derivations:u32) lock_height: 0, is_coinbase: output.5, }); - }); + }; } } - } + }); h > 0 - }{} + } + {} Ok(()) } diff --git a/wallet/src/types.rs b/wallet/src/types.rs index 5ea663957..67cabef8e 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -78,6 +78,7 @@ pub enum Error { Hyper(hyper::Error), /// Error originating from hyper uri parsing. Uri(hyper::error::UriError), + GenericError(String,) } impl error::Error for Error {