From 9e11afe8a2fab8e518a320d3a2bc9a1391faf7c7 Mon Sep 17 00:00:00 2001 From: Alexey Miroshkin Date: Wed, 28 Feb 2018 18:56:09 +0100 Subject: [PATCH] Error handling with failure (using Error and ErrorKind) (#713) * Initial version * store failure parameters inside ErrorKind variants * continue failure transformation * 4 errors left * still two errors * return old code back * finally compiling * Fix compilation and test errors after merge --- core/src/core/transaction.rs | 17 +++ grin/tests/framework/mod.rs | 20 +-- keychain/src/keychain.rs | 17 +++ src/bin/grin.rs | 44 +++--- wallet/Cargo.toml | 2 + wallet/src/checker.rs | 17 ++- wallet/src/client.rs | 27 ++-- wallet/src/handlers.rs | 11 +- wallet/src/info.rs | 7 +- wallet/src/lib.rs | 5 +- wallet/src/outputs.rs | 1 + wallet/src/receiver.rs | 47 ++++--- wallet/src/restore.rs | 26 ++-- wallet/src/sender.rs | 43 +++--- wallet/src/types.rs | 262 ++++++++++++++++------------------- 15 files changed, 289 insertions(+), 257 deletions(-) diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index d7a3d09fd..836d44080 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -19,6 +19,7 @@ use util::{static_secp_instance, kernel_sig_msg}; use util::secp::pedersen::{Commitment, RangeProof, ProofMessage}; use std::cmp::{min, max}; use std::cmp::Ordering; +use std::{error, fmt}; use consensus; use consensus::VerifySortOrder; @@ -95,6 +96,22 @@ pub enum Error { RangeProof, } +impl error::Error for Error { + fn description(&self) -> &str { + match *self { + _ => "some kind of keychain error", + } + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + _ => write!(f, "some kind of keychain error"), + } + } +} + impl From for Error { fn from(e: secp::Error) -> Error { Error::Secp(e) diff --git a/grin/tests/framework/mod.rs b/grin/tests/framework/mod.rs index 3755d8ae9..9f2665ead 100644 --- a/grin/tests/framework/mod.rs +++ b/grin/tests/framework/mod.rs @@ -341,15 +341,17 @@ impl LocalServerContainer { selection_strategy, ) } - Err(wallet::Error::NotEnoughFunds(available)) => { - println!( - "Tx not sent: insufficient funds (max: {})", - core::core::amount_to_hr_string(available), - ); - } - Err(e) => { - println!("Tx not sent to {}: {:?}", dest, e); - } + Err(e) => match e.kind() { + wallet::ErrorKind::NotEnoughFunds(available) => { + println!( + "Tx not sent: insufficient funds (max: {})", + core::core::amount_to_hr_string(available), + ); + } + _ => { + println!("Tx not sent to {}: {:?}", dest, e); + } + } }; } diff --git a/keychain/src/keychain.rs b/keychain/src/keychain.rs index ae485d837..7e63b6daf 100644 --- a/keychain/src/keychain.rs +++ b/keychain/src/keychain.rs @@ -15,6 +15,7 @@ use rand::{thread_rng, Rng}; use std::collections::HashMap; use std::sync::{Arc, RwLock}; +use std::{error, fmt}; use util::secp; use util::secp::{Message, Secp256k1, Signature}; @@ -49,6 +50,22 @@ impl From for Error { } } +impl error::Error for Error { + fn description(&self) -> &str { + match *self { + _ => "some kind of keychain error", + } + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + _ => write!(f, "some kind of keychain error"), + } + } +} + /// Holds internal information about an aggsig operation #[derive(Clone, Debug)] pub struct AggSigTxContext { diff --git a/src/bin/grin.rs b/src/bin/grin.rs index df6ea2245..d2f854d78 100644 --- a/src/bin/grin.rs +++ b/src/bin/grin.rs @@ -515,27 +515,29 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) { dest, selection_strategy, ), - Err(wallet::Error::NotEnoughFunds(available)) => { - error!( - LOGGER, - "Tx not sent: insufficient funds (max: {})", - amount_to_hr_string(available), - ); - } - Err(wallet::Error::FeeExceedsAmount { - sender_amount, - recipient_fee, - }) => { - error!( - LOGGER, - "Recipient rejected the transfer because transaction fee ({}) exceeded amount ({}).", - amount_to_hr_string(recipient_fee), - amount_to_hr_string(sender_amount) - ); - } - Err(e) => { - error!(LOGGER, "Tx not sent: {:?}", e); - } + Err(e) => match e.kind() { + wallet::ErrorKind::NotEnoughFunds(available) => { + error!( + LOGGER, + "Tx not sent: insufficient funds (max: {})", + amount_to_hr_string(available), + ); + } + wallet::ErrorKind::FeeExceedsAmount { + sender_amount, + recipient_fee, + } => { + error!( + LOGGER, + "Recipient rejected the transfer because transaction fee ({}) exceeded amount ({}).", + amount_to_hr_string(recipient_fee), + amount_to_hr_string(sender_amount) + ); + } + _ => { + error!(LOGGER, "Tx not sent: {:?}", e); + } + } }; } ("burn", Some(send_args)) => { diff --git a/wallet/Cargo.toml b/wallet/Cargo.toml index 33c886e84..65961718f 100644 --- a/wallet/Cargo.toml +++ b/wallet/Cargo.toml @@ -17,6 +17,8 @@ serde = "~1.0.8" serde_derive = "~1.0.8" serde_json = "~1.0.8" bodyparser = "~0.7.0" +failure = "0.1.1" +failure_derive = "0.1.1" futures = "^0.1.15" iron = "~0.5.1" hyper = "~0.11.4" diff --git a/wallet/src/checker.rs b/wallet/src/checker.rs index 0f4539723..4e03b0f5f 100644 --- a/wallet/src/checker.rs +++ b/wallet/src/checker.rs @@ -17,6 +17,7 @@ use std::collections::hash_map::Entry; use std::collections::HashMap; +use failure::{ResultExt}; use api; use core::core::hash::Hash; @@ -68,9 +69,10 @@ fn refresh_missing_block_hashes(config: &WalletConfig, keychain: &Keychain) -> R x.status == OutputStatus::Unspent }) { - let commit = keychain.commit_with_key_index(out.value, out.n_child).unwrap(); - wallet_outputs.insert(commit, out.key_id.clone()); + let commit = keychain.commit_with_key_index(out.value, out.n_child).context(ErrorKind::Keychain)?; + wallet_outputs.insert(commit, out.key_id.clone()); } + Ok(()) }); // nothing to do so return (otherwise we hit the api with a monster query...) @@ -122,7 +124,7 @@ fn refresh_missing_block_hashes(config: &WalletConfig, keychain: &Keychain) -> R Err(e) => { // if we got anything other than 200 back from server, bye error!(LOGGER, "Refresh failed... unable to contact node: {}", e); - return Err(Error::Node(e)); + return Err(e).context(ErrorKind::Node)?; } } @@ -161,9 +163,10 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() x.status != OutputStatus::Spent }) { - let commit = keychain.commit_with_key_index(out.value, out.n_child).unwrap(); + let commit = keychain.commit_with_key_index(out.value, out.n_child).context(ErrorKind::Keychain)?; wallet_outputs.insert(commit, out.key_id.clone()); - } + }; + Ok(()) }); // build the necessary query params - @@ -193,7 +196,7 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() Err(e) => { // if we got anything other than 200 back from server, don't attempt to refresh // the wallet data after - return Err(Error::Node(e)); + return Err(e).context(ErrorKind::Node)?; } }; @@ -214,5 +217,5 @@ fn refresh_output_state(config: &WalletConfig, keychain: &Keychain) -> Result<() pub fn get_tip_from_node(config: &WalletConfig) -> Result { let url = format!("{}/v1/chain", config.check_node_api_http_addr); - api::client::get::(url.as_str()).map_err(|e| Error::Node(e)) + api::client::get::(url.as_str()).context(ErrorKind::Node).map_err(|e| e.into()) } diff --git a/wallet/src/client.rs b/wallet/src/client.rs index 7401663b5..a67fa9667 100644 --- a/wallet/src/client.rs +++ b/wallet/src/client.rs @@ -12,10 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{io, time}; +use std::time; use std::ops::FnMut; use futures::{Future, Stream}; +use failure::ResultExt; use hyper; use hyper::{Method, Request}; use hyper::header::ContentType; @@ -26,6 +27,7 @@ use serde_json; use types::*; use util::LOGGER; +use std::io; /// Call the wallet API to create a coinbase output for the given block_fees. /// Will retry based on default "retry forever with backoff" behavior. @@ -54,7 +56,7 @@ fn retry_backoff_forever(f: F) -> Result where F: FnMut() -> Result, { - let mut core = reactor::Core::new()?; + let mut core = reactor::Core::new().context(ErrorKind::GenericError("Could not create reactor"))?; let retry_strategy = FibonacciBackoff::from_millis(100).max_delay(time::Duration::from_secs(10)); let retry_future = Retry::spawn(core.handle(), retry_strategy, f); @@ -67,33 +69,32 @@ pub fn send_partial_tx(url: &str, partial_tx: &PartialTx) -> Result Result { - let mut core = reactor::Core::new()?; + let mut core = reactor::Core::new().context(ErrorKind::Hyper)?; let client = hyper::Client::new(&core.handle()); - let mut req = Request::new(Method::Post, url.parse()?); + let mut req = Request::new(Method::Post, url.parse::().context(ErrorKind::Hyper)?); req.headers_mut().set(ContentType::json()); - let json = serde_json::to_string(&partial_tx)?; + let json = serde_json::to_string(&partial_tx).context(ErrorKind::Hyper)?; req.set_body(json); let work = client.request(req).and_then(|res| { res.body().concat2().and_then(move |body| { - let partial_tx: PartialTx = - serde_json::from_slice(&body).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; + let partial_tx: PartialTx = serde_json::from_slice(&body).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; Ok(partial_tx) }) }); - let res = core.run(work)?; - Ok(res) + let res = core.run(work).context(ErrorKind::Hyper)?; + Ok(res) } /// Makes a single request to the wallet API to create a new coinbase output. fn single_create_coinbase(url: &str, block_fees: &BlockFees) -> Result { - let mut core = reactor::Core::new()?; + let mut core = reactor::Core::new().context(ErrorKind::GenericError("Could not create reactor"))?; let client = hyper::Client::new(&core.handle()); - let mut req = Request::new(Method::Post, url.parse()?); + let mut req = Request::new(Method::Post, url.parse::().context(ErrorKind::Uri)?); req.headers_mut().set(ContentType::json()); - let json = serde_json::to_string(&block_fees)?; + let json = serde_json::to_string(&block_fees).context(ErrorKind::Format)?; req.set_body(json); let work = client.request(req).and_then(|res| { @@ -104,6 +105,6 @@ fn single_create_coinbase(url: &str, block_fees: &BlockFees) -> Result ser::ser_vec(&key_id).map_err(|e| { api::Error::Internal(format!("Error serializing kernel: {:?}", e)) - })?, + }).context(ErrorKind::Node)?, None => vec![], }; @@ -69,7 +70,7 @@ impl Handler for CoinbaseHandler { if let Ok(Some(block_fees)) = struct_body { let coinbase = self.build_coinbase(&block_fees) - .map_err(|e| IronError::new(e, status::BadRequest))?; + .map_err(|e| IronError::new(Fail::compat(e), status::BadRequest))?; if let Ok(json) = serde_json::to_string(&coinbase) { Ok(Response::with((status::Ok, json))) } else { diff --git a/wallet/src/info.rs b/wallet/src/info.rs index 05e966dac..36a095a88 100644 --- a/wallet/src/info.rs +++ b/wallet/src/info.rs @@ -44,8 +44,7 @@ pub fn show_info(config: &WalletConfig, keychain: &Keychain) { } } -pub fn retrieve_info(config: &WalletConfig, keychain: &Keychain) - -> WalletInfo { +pub fn retrieve_info(config: &WalletConfig, keychain: &Keychain) -> WalletInfo { let result = checker::refresh_outputs(&config, &keychain); let ret_val = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { @@ -83,7 +82,7 @@ pub fn retrieve_info(config: &WalletConfig, keychain: &Keychain) if let Err(_) = result { data_confirmed = false; } - WalletInfo { + Ok(WalletInfo { current_height : current_height, total: unspent_total+unconfirmed_total, amount_awaiting_confirmation: unconfirmed_total, @@ -92,7 +91,7 @@ pub fn retrieve_info(config: &WalletConfig, keychain: &Keychain) amount_locked: locked_total, data_confirmed: data_confirmed, data_confirmed_from: String::from(from), - } + }) }); ret_val.unwrap() } diff --git a/wallet/src/lib.rs b/wallet/src/lib.rs index d280c4e38..77087f7a2 100644 --- a/wallet/src/lib.rs +++ b/wallet/src/lib.rs @@ -29,6 +29,9 @@ extern crate prettytable; extern crate term; extern crate bodyparser; +extern crate failure; +#[macro_use] +extern crate failure_derive; extern crate futures; extern crate hyper; extern crate iron; @@ -57,5 +60,5 @@ pub use outputs::show_outputs; pub use info::{show_info, retrieve_info}; pub use receiver::{WalletReceiver}; pub use sender::{issue_burn_tx, issue_send_tx}; -pub use types::{BlockFees, CbData, Error, WalletConfig, WalletReceiveRequest, WalletInfo, WalletSeed}; +pub use types::{BlockFees, CbData, Error, ErrorKind, WalletConfig, WalletReceiveRequest, WalletInfo, WalletSeed}; pub use restore::restore; diff --git a/wallet/src/outputs.rs b/wallet/src/outputs.rs index 070cf3629..d6d104e44 100644 --- a/wallet/src/outputs.rs +++ b/wallet/src/outputs.rs @@ -90,6 +90,7 @@ pub fn show_outputs(config: &WalletConfig, keychain: &Keychain, show_spent:bool) table.set_format(*prettytable::format::consts::FORMAT_NO_COLSEP); table.printstd(); println!(); + Ok(()) }); if let Err(_) = result { diff --git a/wallet/src/receiver.rs b/wallet/src/receiver.rs index 0185842a3..1df34e212 100644 --- a/wallet/src/receiver.rs +++ b/wallet/src/receiver.rs @@ -30,6 +30,7 @@ use core::{global, ser}; use keychain::{Identifier, Keychain, BlindingFactor}; use types::*; use util::{LOGGER, to_hex, secp}; +use failure::{ResultExt}; /// Dummy wrapper for the hex-encoded serialized transaction. #[derive(Serialize, Deserialize)] @@ -61,10 +62,10 @@ fn handle_sender_initiation( // we could just overwrite the fee here (but we won't) due to the ecdsa sig let fee = tx_fee(tx.inputs.len(), tx.outputs.len() + 1, None); if fee != tx.fee() { - return Err(Error::FeeDispute { + return Err(ErrorKind::FeeDispute { sender_fee: tx.fee(), recipient_fee: fee, - }); + })?; } if fee > amount { @@ -74,10 +75,10 @@ fn handle_sender_initiation( amount_to_hr_string(fee), amount_to_hr_string(amount) ); - return Err(Error::FeeExceedsAmount { + return Err(ErrorKind::FeeExceedsAmount { sender_amount: amount, recipient_fee: fee, - }); + })?; } let out_amount = amount - fee; @@ -109,13 +110,13 @@ fn handle_sender_initiation( build::output(out_amount, key_id.clone()), ], keychain, - )?; + ).context(ErrorKind::Keychain)?; warn!(LOGGER, "Creating new aggsig context"); // Create a new aggsig context // this will create a new blinding sum and nonce, and store them - let blind = blind_sum.secret_key(&keychain.secp())?; - keychain.aggsig_create_context(&partial_tx.id, blind)?; + let blind = blind_sum.secret_key(&keychain.secp()).context(ErrorKind::Keychain)?; + keychain.aggsig_create_context(&partial_tx.id, blind).context(ErrorKind::Keychain)?; keychain.aggsig_add_output(&partial_tx.id, &key_id); let sig_part = keychain.aggsig_calculate_partial_sig( @@ -161,7 +162,7 @@ fn handle_sender_confirmation( if !res { error!(LOGGER, "Partial Sig from sender invalid."); - return Err(Error::Signature(String::from("Partial Sig from sender invalid."))); + return Err(ErrorKind::Signature("Partial Sig from sender invalid."))?; } // Just calculate our sig part again instead of storing @@ -196,7 +197,7 @@ fn handle_sender_confirmation( if !res { error!(LOGGER, "Final aggregated signature invalid."); - return Err(Error::Signature(String::from("Final aggregated signature invalid."))); + return Err(ErrorKind::Signature("Final aggregated signature invalid."))?; } let final_tx = build_final_transaction( @@ -213,7 +214,7 @@ fn handle_sender_confirmation( let url = format!("{}/v1/pool/push", config.check_node_api_http_addr.as_str()); api::client::post(url.as_str(), &TxWrapper { tx_hex: tx_hex }) - .map_err(|e| Error::Node(e))?; + .context(ErrorKind::Node)?; // Return what we've actually posted // TODO - why build_partial_tx here? Just a naming issue? @@ -344,7 +345,7 @@ pub fn receive_coinbase( &key_id, block_fees.fees, block_fees.height, - )?; + ).context(ErrorKind::Keychain)?; Ok((out, kern, block_fees)) } @@ -365,10 +366,10 @@ fn build_final_transaction( // we could just overwrite the fee here (but we won't) due to the ecdsa sig let fee = tx_fee(tx.inputs.len(), tx.outputs.len() + 1, None); if fee != tx.fee() { - return Err(Error::FeeDispute { + return Err(ErrorKind::FeeDispute { sender_fee: tx.fee(), recipient_fee: fee, - }); + })?; } if fee > amount { @@ -378,11 +379,11 @@ fn build_final_transaction( amount_to_hr_string(fee), amount_to_hr_string(amount) ); - return Err(Error::FeeExceedsAmount { - sender_amount: amount, - recipient_fee: fee, - }); - } + return Err(ErrorKind::FeeExceedsAmount { + sender_amount: amount, + recipient_fee: fee, + })?; + } let out_amount = amount - fee; @@ -418,16 +419,16 @@ fn build_final_transaction( build::with_offset(kernel_offset), ], keychain, - )?; + ).context(ErrorKind::Keychain)?; // build the final excess based on final tx and offset let final_excess = { // sum the input/output commitments on the final tx - let tx_excess = final_tx.sum_commitments()?; + let tx_excess = final_tx.sum_commitments().context(ErrorKind::Transaction)?; // subtract the kernel_excess (built from kernel_offset) let offset_excess = keychain.secp().commit(0, kernel_offset.secret_key(&keychain.secp()).unwrap()).unwrap(); - keychain.secp().commit_sum(vec![tx_excess], vec![offset_excess])? + keychain.secp().commit_sum(vec![tx_excess], vec![offset_excess]).context(ErrorKind::Transaction)? }; // update the tx kernel to reflect the offset excess and sig @@ -436,10 +437,10 @@ fn build_final_transaction( final_tx.kernels[0].excess_sig = excess_sig.clone(); // confirm the kernel verifies successfully before proceeding - final_tx.kernels[0].verify()?; + final_tx.kernels[0].verify().context(ErrorKind::Transaction)?; // confirm the overall transaction is valid (including the updated kernel) - final_tx.validate()?; + let _ = final_tx.validate().context(ErrorKind::Transaction)?; debug!( LOGGER, diff --git a/wallet/src/restore.rs b/wallet/src/restore.rs index c4b721fba..913ec5813 100644 --- a/wallet/src/restore.rs +++ b/wallet/src/restore.rs @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. - +use failure::{ResultExt, Fail}; use keychain::{Keychain, Identifier}; use util::{LOGGER, to_hex}; use util::secp::pedersen; @@ -19,7 +19,7 @@ use api; use core::global; use core::core::{Output, SwitchCommitHash}; use core::core::transaction::OutputFeatures; -use types::{BlockIdentifier, WalletConfig, WalletData, OutputData, OutputStatus, Error}; +use types::{BlockIdentifier, WalletConfig, WalletData, OutputData, OutputStatus, Error, ErrorKind}; use byteorder::{BigEndian, ByteOrder}; @@ -36,7 +36,7 @@ pub fn get_chain_height(config: &WalletConfig) -> Result { config.check_node_api_http_addr, e ); - Err(Error::Node(e)) + Err(e.context(ErrorKind::Node).into()) } } } @@ -61,17 +61,17 @@ fn output_with_range_proof( if let Some(output) = block_output.outputs.first() { Ok(output.clone()) } else { - Err(Error::Node(api::Error::NotFound)) + Err(ErrorKind::Node)? } } else { - Err(Error::Node(api::Error::NotFound)) + Err(ErrorKind::Node)? } } Err(e) => { // if we got anything other than 200 back from server, don't attempt to refresh // the wallet // data after - Err(Error::Node(e)) + Err(e.context(ErrorKind::Node))? } } } @@ -90,9 +90,9 @@ fn retrieve_amount_and_coinbase_status( api::OutputType::Coinbase => OutputFeatures::COINBASE_OUTPUT, api::OutputType::Transaction => OutputFeatures::DEFAULT_OUTPUT, }, - proof: output.range_proof()?, - switch_commit_hash: output.switch_commit_hash()?, - commit: output.commit()?, + proof: output.range_proof().context(ErrorKind::GenericError("range proof error"))?, + switch_commit_hash: output.switch_commit_hash().context(ErrorKind::GenericError("switch commit hash error"))?, + commit: output.commit().context(ErrorKind::GenericError("commit error"))?, }; if let Some(amount) = core_output.recover_value(keychain, &key_id) { @@ -102,7 +102,7 @@ fn retrieve_amount_and_coinbase_status( }; Ok((amount, is_coinbase)) } else { - Err(Error::GenericError(format!("cannot recover value"))) + Err(ErrorKind::GenericError("cannot recover value"))? } } @@ -130,7 +130,7 @@ pub fn utxos_batch_block( config.check_node_api_http_addr, e ); - Err(Error::Node(e)) + Err(e.context(ErrorKind::Node))? } } } @@ -244,8 +244,8 @@ pub fn restore( ) -> 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 - })?; + Ok(wallet_data.outputs.len() == 0) + }).context(ErrorKind::WalletData("could not read wallet"))?; if !is_empty { error!( LOGGER, diff --git a/wallet/src/sender.rs b/wallet/src/sender.rs index 7149a975b..0487e56b1 100644 --- a/wallet/src/sender.rs +++ b/wallet/src/sender.rs @@ -26,6 +26,7 @@ use types::*; use util::LOGGER; use util::secp::key::SecretKey; use util; +use failure::ResultExt; /// Issue a new transaction to the provided sender by spending some of our /// wallet @@ -80,8 +81,8 @@ pub fn issue_send_tx( // // Create a new aggsig context let tx_id = Uuid::new_v4(); - let skey = blind_offset.secret_key(&keychain.secp())?; - keychain.aggsig_create_context(&tx_id, skey)?; + let skey = blind_offset.secret_key(&keychain.secp()).context(ErrorKind::Keychain)?; + keychain.aggsig_create_context(&tx_id, skey).context(ErrorKind::Keychain)?; let partial_tx = build_partial_tx(&tx_id, keychain, amount_with_fee, kernel_offset, None, tx); @@ -116,8 +117,8 @@ pub fn issue_send_tx( debug!(LOGGER, "Posting partial transaction to {}", url); let res = client::send_partial_tx(&url, &partial_tx); if let Err(e) = res { - match e { - Error::FeeExceedsAmount {sender_amount, recipient_fee} => + match e.kind() { + ErrorKind::FeeExceedsAmount {sender_amount, recipient_fee} => error!( LOGGER, "Recipient rejected the transfer because transaction fee ({}) exceeded amount ({}).", @@ -147,7 +148,7 @@ pub fn issue_send_tx( ); if !res { error!(LOGGER, "Partial Sig from recipient invalid."); - return Err(Error::Signature(String::from("Partial Sig from recipient invalid."))); + return Err(ErrorKind::Signature("Partial Sig from recipient invalid."))?; } let sig_part = keychain.aggsig_calculate_partial_sig(&tx_id, &recp_pub_nonce, tx.fee(), tx.lock_height()).unwrap(); @@ -160,8 +161,8 @@ pub fn issue_send_tx( // And send again let res = client::send_partial_tx(&url, &partial_tx); if let Err(e) = res { - match e { - Error::FeeExceedsAmount {sender_amount, recipient_fee} => + match e.kind() { + ErrorKind::FeeExceedsAmount {sender_amount, recipient_fee} => error!( LOGGER, "Recipient rejected the transfer because transaction fee ({}) exceeded amount ({}).", @@ -196,26 +197,26 @@ fn build_send_tx( // select some spendable coins from the wallet let mut coins = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { - wallet_data.select_coins( + Ok(wallet_data.select_coins( key_id.clone(), amount, current_height, minimum_confirmations, max_outputs, selection_strategy_is_use_all, - ) + )) })?; // Get the maximum number of outputs in the wallet let max_outputs = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { - wallet_data.select_coins( + Ok(wallet_data.select_coins( key_id.clone(), amount, current_height, minimum_confirmations, max_outputs, true, - ) + )) })?.len(); // sender is responsible for setting the fee on the partial tx @@ -230,19 +231,19 @@ fn build_send_tx( while total <= amount_with_fee { // End the loop if we have selected all the outputs and still not enough funds if coins.len() == max_outputs { - return Err(Error::NotEnoughFunds(total as u64)); + return Err(ErrorKind::NotEnoughFunds(total as u64))?; } // select some spendable coins from the wallet coins = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { - wallet_data.select_coins( + Ok(wallet_data.select_coins( key_id.clone(), amount_with_fee, current_height, minimum_confirmations, max_outputs, selection_strategy_is_use_all, - ) + )) })?; fee = tx_fee(coins.len(), 2, None); total = coins.iter().map(|c| c.value).sum(); @@ -256,7 +257,7 @@ fn build_send_tx( // on tx being sent (based on current chain height via api). parts.push(build::with_lock_height(lock_height)); - let (tx, blind) = build::partial_transaction(parts, &keychain)?; + let (tx, blind) = build::partial_transaction(parts, &keychain).context(ErrorKind::Keychain)?; Ok((tx, blind, coins, change_key, amount_with_fee)) } @@ -279,14 +280,14 @@ pub fn issue_burn_tx( // select some spendable coins from the wallet let coins = WalletData::read_wallet(&config.data_file_dir, |wallet_data| { - wallet_data.select_coins( + Ok(wallet_data.select_coins( key_id.clone(), amount, current_height, minimum_confirmations, max_outputs, false, - ) + )) })?; debug!(LOGGER, "selected some coins - {}", coins.len()); @@ -298,13 +299,13 @@ pub fn issue_burn_tx( parts.push(build::output(amount - fee, Identifier::zero())); // finalize the burn transaction and send - let tx_burn = build::transaction(parts, &keychain)?; - tx_burn.validate()?; + let tx_burn = build::transaction(parts, &keychain).context(ErrorKind::Keychain)?; + tx_burn.validate().context(ErrorKind::Transaction)?; let tx_hex = util::to_hex(ser::ser_vec(&tx_burn).unwrap()); let url = format!("{}/v1/pool/push", config.check_node_api_http_addr.as_str()); let _: () = - api::client::post(url.as_str(), &TxWrapper { tx_hex: tx_hex }).map_err(|e| Error::Node(e))?; + api::client::post(url.as_str(), &TxWrapper { tx_hex: tx_hex }).context(ErrorKind::Node)?; Ok(()) } @@ -328,7 +329,7 @@ fn inputs_and_change( // build inputs using the appropriate derived key_ids for coin in coins { - let key_id = keychain.derive_key_id(coin.n_child)?; + let key_id = keychain.derive_key_id(coin.n_child).context(ErrorKind::Keychain)?; if coin.is_coinbase { parts.push(build::coinbase_input(coin.value, coin.block.hash(), key_id)); } else { diff --git a/wallet/src/types.rs b/wallet/src/types.rs index c37bb0e70..b30f4f961 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -14,27 +14,27 @@ use blake2; use rand::{thread_rng, Rng}; -use std::{error, fmt, num}; +use std::{fmt}; +use std::fmt::Display; use uuid::Uuid; use std::convert::From; use std::fs::{self, File, OpenOptions}; -use std::io::{self, Read, Write}; +use std::io::{Read, Write}; use std::path::Path; use std::path::MAIN_SEPARATOR; use std::collections::HashMap; use std::cmp::min; -use hyper; use serde; use serde_json; use tokio_core::reactor; use tokio_retry::Retry; use tokio_retry::strategy::FibonacciBackoff; +use failure::{Backtrace, Context, Fail, ResultExt}; -use api; use core::consensus; -use core::core::{transaction, Transaction}; +use core::core::Transaction; use core::core::hash::Hash; use core::ser; use keychain; @@ -65,112 +65,106 @@ pub fn tx_fee(input_len: usize, output_len: usize, base_fee: Option) -> u64 (tx_weight as u64) * use_base_fee } -/// Wallet errors, mostly wrappers around underlying crypto or I/O errors. #[derive(Debug)] -pub enum Error { - NotEnoughFunds(u64), - FeeDispute { sender_fee: u64, recipient_fee: u64 }, - FeeExceedsAmount { sender_amount: u64, recipient_fee: u64 }, - Keychain(keychain::Error), - Transaction(transaction::Error), - Secp(secp::Error), - WalletData(String), - /// An error in the format of the JSON structures exchanged by the wallet - Format(String), - /// An IO Error - IOError(io::Error), - /// Error when contacting a node through its API - Node(api::Error), - /// Error originating from hyper. - Hyper(hyper::Error), - /// Error originating from hyper uri parsing. - Uri(hyper::error::UriError), - /// Error with signatures during exchange - Signature(String), +pub struct Error { + inner: Context, +} + +/// Wallet errors, mostly wrappers around underlying crypto or I/O errors. +#[derive(Copy, Clone, Eq, PartialEq, Debug, Fail)] +pub enum ErrorKind { + #[fail(display = "Not enough funds")] + NotEnoughFunds(u64), + + #[fail(display = "Fee dispute: sender fee {}, recipient fee {}", sender_fee, recipient_fee)] + FeeDispute{sender_fee: u64, recipient_fee: u64}, + + #[fail(display = "Fee exceeds amount: sender amount {}, recipient fee {}", sender_amount, recipient_fee)] + FeeExceedsAmount{sender_amount: u64,recipient_fee: u64}, + + #[fail(display = "Keychain error")] + Keychain, + + #[fail(display = "Transaction error")] + Transaction, + + #[fail(display = "Secp error")] + Secp, + + #[fail(display = "Wallet data error: {}", _0)] + WalletData(&'static str), + + /// An error in the format of the JSON structures exchanged by the wallet + #[fail(display = "JSON format error")] + Format, + + + #[fail(display = "I/O error")] + IO, + + /// Error when contacting a node through its API + #[fail(display = "Node API error")] + Node, + + /// Error originating from hyper. + #[fail(display = "Hyper error")] + Hyper, + + /// Error originating from hyper uri parsing. + #[fail(display = "Uri parsing error")] + Uri, + + #[fail(display = "Signature error")] + Signature(&'static str), + /// Attempt to use duplicate transaction id in separate transactions + #[fail(display = "Duplicate transaction ID error")] DuplicateTransactionId, + /// Wallet seed already exists + #[fail(display = "Wallet seed exists error")] WalletSeedExists, - /// Other - GenericError(String,) + + + #[fail(display = "Generic error: {}", _0)] + GenericError(&'static str), } -impl error::Error for Error { - fn description(&self) -> &str { - match *self { - _ => "some kind of wallet error", - } - } + +impl Fail for Error { + fn cause(&self) -> Option<&Fail> { + self.inner.cause() + } + + fn backtrace(&self) -> Option<&Backtrace> { + self.inner.backtrace() + } } -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - _ => write!(f, "some kind of wallet error"), - } - } +impl Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Display::fmt(&self.inner, f) + } } -impl From for Error { - fn from(e: keychain::Error) -> Error { - Error::Keychain(e) - } +impl Error { + pub fn kind(&self) -> ErrorKind { + *self.inner.get_context() + } } -impl From for Error { - fn from(e: secp::Error) -> Error { - Error::Secp(e) - } +impl From for Error { + fn from(kind: ErrorKind) -> Error { + Error { + inner: Context::new(kind), + } + } } -impl From for Error { - fn from(e: transaction::Error) -> Error { - Error::Transaction(e) - } -} - -impl From for Error { - fn from(e: serde_json::Error) -> Error { - Error::Format(e.to_string()) - } -} - -// TODO - rethink this, would be nice not to have to worry about -// low level hex conversion errors like this -impl From for Error { - fn from(_: num::ParseIntError) -> Error { - Error::Format("Invalid hex".to_string()) - } -} - -impl From for Error { - fn from(e: ser::Error) -> Error { - Error::Format(e.to_string()) - } -} - -impl From for Error { - fn from(e: api::Error) -> Error { - Error::Node(e) - } -} - -impl From for Error { - fn from(e: io::Error) -> Error { - Error::IOError(e) - } -} - -impl From for Error { - fn from(e: hyper::Error) -> Error { - Error::Hyper(e) - } -} - -impl From for Error { - fn from(e: hyper::error::UriError) -> Error { - Error::Uri(e) - } +impl From> for Error { + fn from(inner: Context) -> Error { + Error { inner: inner } + } } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -241,7 +235,7 @@ impl BlockIdentifier { } pub fn from_str(hex: &str) -> Result { - let hash = Hash::from_hex(hex)?; + let hash = Hash::from_hex(hex).context(ErrorKind::GenericError("Invalid hex"))?; Ok(BlockIdentifier(hash)) } @@ -367,7 +361,7 @@ impl WalletSeed { } fn from_hex(hex: &str) -> Result { - let bytes = util::from_hex(hex.to_string())?; + let bytes = util::from_hex(hex.to_string()).context(ErrorKind::GenericError("Invalid hex"))?; Ok(WalletSeed::from_bytes(&bytes)) } @@ -377,7 +371,7 @@ impl WalletSeed { pub fn derive_keychain(&self, password: &str) -> Result { let seed = blake2::blake2b::blake2b(64, &password.as_bytes(), &self.0); - let result = keychain::Keychain::from_seed(seed.as_bytes())?; + let result = keychain::Keychain::from_seed(seed.as_bytes()).context(ErrorKind::Keychain)?; Ok(result) } @@ -388,7 +382,7 @@ impl WalletSeed { pub fn init_file(wallet_config: &WalletConfig) -> Result { // create directory if it doesn't exist - fs::create_dir_all(&wallet_config.data_file_dir)?; + fs::create_dir_all(&wallet_config.data_file_dir).context(ErrorKind::IO)?; let seed_file_path = &format!( "{}{}{}", @@ -400,18 +394,18 @@ impl WalletSeed { debug!(LOGGER, "Generating wallet seed file at: {}", seed_file_path,); if Path::new(seed_file_path).exists() { - Err(Error::WalletSeedExists) + Err(ErrorKind::WalletSeedExists)? } else { let seed = WalletSeed::init_new(); - let mut file = File::create(seed_file_path)?; - file.write_all(&seed.to_hex().as_bytes())?; + let mut file = File::create(seed_file_path).context(ErrorKind::IO)?; + file.write_all(&seed.to_hex().as_bytes()).context(ErrorKind::IO)?; Ok(seed) } } pub fn from_file(wallet_config: &WalletConfig) -> Result { // create directory if it doesn't exist - fs::create_dir_all(&wallet_config.data_file_dir)?; + fs::create_dir_all(&wallet_config.data_file_dir).context(ErrorKind::IO)?; let seed_file_path = &format!( "{}{}{}", @@ -423,9 +417,9 @@ impl WalletSeed { debug!(LOGGER, "Using wallet seed file at: {}", seed_file_path,); if Path::new(seed_file_path).exists() { - let mut file = File::open(seed_file_path)?; + let mut file = File::open(seed_file_path).context(ErrorKind::IO)?; let mut buffer = String::new(); - file.read_to_string(&mut buffer)?; + file.read_to_string(&mut buffer).context(ErrorKind::IO)?; let wallet_seed = WalletSeed::from_hex(&buffer)?; Ok(wallet_seed) } else { @@ -453,13 +447,12 @@ impl WalletData { /// lock). pub fn read_wallet(data_file_dir: &str, f: F) -> Result where - F: FnOnce(&WalletData) -> T, + F: FnOnce(&WalletData) -> Result, { // open the wallet readonly and do what needs to be done with it let data_file_path = &format!("{}{}{}", data_file_dir, MAIN_SEPARATOR, DAT_FILE); let wdat = WalletData::read_or_create(data_file_path)?; - let res = f(&wdat); - Ok(res) + f(&wdat) } /// Allows the reading and writing of the wallet data within a file lock. @@ -498,12 +491,12 @@ impl WalletData { match retry_result { Ok(_) => {} - Err(_) => { + Err(e) => { error!( LOGGER, "Failed to acquire wallet lock file (multiple retries)", ); - return Err(Error::WalletData(format!("Failed to acquire lock file"))); + return Err(e.context(ErrorKind::WalletData("Failed to acquire lock file")).into()); } } @@ -513,11 +506,7 @@ impl WalletData { wdat.write(data_file_path)?; // delete the lock file - fs::remove_file(lock_file_path).map_err(|_| { - Error::WalletData(format!( - "Could not remove wallet lock file. Maybe insufficient rights?" - )) - })?; + fs::remove_file(lock_file_path).context(ErrorKind::WalletData("Could not remove wallet lock file. Maybe insufficient rights?"))?; info!(LOGGER, "... released wallet lock"); @@ -538,12 +527,10 @@ impl WalletData { /// Read output_data vec from disk. fn read_outputs(data_file_path: &str) -> Result, Error> { - let data_file = File::open(data_file_path).map_err(|e| { - Error::WalletData(format!("Could not open {}: {}", data_file_path, e)) - })?; - serde_json::from_reader(data_file).map_err(|e| { - Error::WalletData(format!("Error reading {}: {}", data_file_path, e)) - }) + let data_file = File::open(data_file_path).context(ErrorKind::WalletData(&"Could not open wallet file"))?; + serde_json::from_reader(data_file).map_err(|e| { e.context(ErrorKind::WalletData(&"Error reading wallet file ")).into()}) + + } /// Populate wallet_data with output_data from disk. @@ -561,16 +548,13 @@ impl WalletData { /// Write the wallet data to disk. fn write(&self, data_file_path: &str) -> Result<(), Error> { let mut data_file = File::create(data_file_path).map_err(|e| { - Error::WalletData(format!("Could not create {}: {}", data_file_path, e)) - })?; + e.context(ErrorKind::WalletData(&"Could not create "))})?; let mut outputs = self.outputs.values().collect::>(); outputs.sort(); let res_json = serde_json::to_vec_pretty(&outputs).map_err(|e| { - Error::WalletData(format!("Error serializing wallet data: {}", e)) + e.context(ErrorKind::WalletData("Error serializing wallet data")) })?; - data_file.write_all(res_json.as_slice()).map_err(|e| { - Error::WalletData(format!("Error writing {}: {}", data_file_path, e)) - }) + data_file.write_all(res_json.as_slice()).context(ErrorKind::WalletData(&"Error writing wallet file")).map_err(|e| e.into()) } /// Append a new output data to the wallet data. @@ -765,24 +749,22 @@ pub fn read_partial_tx( keychain: &keychain::Keychain, partial_tx: &PartialTx, ) -> Result<(u64, PublicKey, PublicKey, BlindingFactor, Option, Transaction), Error> { - let blind_bin = util::from_hex(partial_tx.public_blind_excess.clone())?; - let blinding = PublicKey::from_slice(keychain.secp(), &blind_bin[..])?; + let blind_bin = util::from_hex(partial_tx.public_blind_excess.clone()).context(ErrorKind::GenericError("Could not decode HEX"))?; + let blinding = PublicKey::from_slice(keychain.secp(), &blind_bin[..]).context(ErrorKind::GenericError("Could not construct public key"))?; - let nonce_bin = util::from_hex(partial_tx.public_nonce.clone())?; - let nonce = PublicKey::from_slice(keychain.secp(), &nonce_bin[..])?; + let nonce_bin = util::from_hex(partial_tx.public_nonce.clone()).context(ErrorKind::GenericError("Could not decode HEX"))?; + let nonce = PublicKey::from_slice(keychain.secp(), &nonce_bin[..]).context(ErrorKind::GenericError("Could not construct public key"))?; - let kernel_offset = BlindingFactor::from_hex(&partial_tx.kernel_offset.clone())?; + let kernel_offset = BlindingFactor::from_hex(&partial_tx.kernel_offset.clone()).context(ErrorKind::GenericError("Could not decode HEX"))?; - let sig_bin = util::from_hex(partial_tx.part_sig.clone())?; + let sig_bin = util::from_hex(partial_tx.part_sig.clone()).context(ErrorKind::GenericError("Could not decode HEX"))?; let sig = match sig_bin.len() { 1 => None, - _ => Some(Signature::from_der(keychain.secp(), &sig_bin[..])?), + _ => Some(Signature::from_der(keychain.secp(), &sig_bin[..]).context(ErrorKind::GenericError("Could not create signature"))?), }; - let tx_bin = util::from_hex(partial_tx.tx.clone())?; - let tx = ser::deserialize(&mut &tx_bin[..]).map_err(|_| { - Error::Format("Could not deserialize transaction, invalid format.".to_string()) - })?; - Ok((partial_tx.amount, blinding, nonce, kernel_offset, sig, tx)) + let tx_bin = util::from_hex(partial_tx.tx.clone()).context(ErrorKind::GenericError("Could not decode HEX"))?; + let tx = ser::deserialize(&mut &tx_bin[..]).context(ErrorKind::GenericError("Could not deserialize transaction, invalid format."))?; + Ok((partial_tx.amount, blinding, nonce, kernel_offset, sig, tx)) } /// Amount in request to build a coinbase output.