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

@@ -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<secp::Error> for Error {
 	fn from(e: secp::Error) -> Error {
 		Error::Secp(e)

grin/tests/framework/mod.rs

@@ -341,15 +341,17 @@ impl LocalServerContainer {
 					selection_strategy,
 				)
 			}
-			Err(wallet::Error::NotEnoughFunds(available)) => {
+            Err(e) => match e.kind() {
-				println!(
+			    wallet::ErrorKind::NotEnoughFunds(available) => {
-					"Tx not sent: insufficient funds (max: {})",
+			    	println!(
-					core::core::amount_to_hr_string(available),
+			    		"Tx not sent: insufficient funds (max: {})",
-				);
+			    		core::core::amount_to_hr_string(available),
-			}
+			    	);
-			Err(e) => {
+			    }
-				println!("Tx not sent to {}: {:?}", dest, e);
+			    _ => {
-			}
+			    	println!("Tx not sent to {}: {:?}", dest, e);
+			    }
+            }
 		};
 	}
 

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<extkey::Error> 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 {

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)) => {
+                Err(e) => match e.kind() {
-					error!(
+			    	wallet::ErrorKind::NotEnoughFunds(available) => {
-						LOGGER,
+			    		error!(
-						"Tx not sent: insufficient funds (max: {})",
+			    			LOGGER,
-						amount_to_hr_string(available),
+			    			"Tx not sent: insufficient funds (max: {})",
-					);
+			    			amount_to_hr_string(available),
-				}
+			    		);
-				Err(wallet::Error::FeeExceedsAmount {
+			    	}
-					sender_amount,
+			    	wallet::ErrorKind::FeeExceedsAmount {
-					recipient_fee,
+			    		sender_amount,
-				}) => {
+			    		recipient_fee,
-					error!(
+			    	} => {
-						LOGGER,
+			    		error!(
-						"Recipient rejected the transfer because transaction fee ({}) exceeded amount ({}).",
+			    			LOGGER,
-						amount_to_hr_string(recipient_fee),
+			    			"Recipient rejected the transfer because transaction fee ({}) exceeded amount ({}).",
-						amount_to_hr_string(sender_amount)
+			    			amount_to_hr_string(recipient_fee),
-					);
+			    			amount_to_hr_string(sender_amount)
-				}
+			    		);
-				Err(e) => {
+			    	}
-					error!(LOGGER, "Tx not sent: {:?}", e);
+			    	_ => {
-				}
+			    		error!(LOGGER, "Tx not sent: {:?}", e);
+			    	}
+                }
 			};
 		}
 		("burn", Some(send_args)) => {

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"

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();
+			let commit = keychain.commit_with_key_index(out.value, out.n_child).context(ErrorKind::Keychain)?;
-			wallet_outputs.insert(commit, out.key_id.clone());
+            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<api::Tip, Error> {
 	let url = format!("{}/v1/chain", config.check_node_api_http_addr);
-	api::client::get::<api::Tip>(url.as_str()).map_err(|e| Error::Node(e))
+	api::client::get::<api::Tip>(url.as_str()).context(ErrorKind::Node).map_err(|e| e.into())
 }

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, R>(f: F) -> Result<R, Error>
 where
 	F: FnMut() -> Result<R, Error>,
 {
-	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<PartialTx, E
 }
 
 fn single_send_partial_tx(url: &str, partial_tx: &PartialTx) -> Result<PartialTx, Error> {
-	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::<hyper::Uri>().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 =
+			let partial_tx: PartialTx = serde_json::from_slice(&body).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
-				serde_json::from_slice(&body).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
 			Ok(partial_tx)
 		})
 	});
-	let res = core.run(work)?;
+    let res = core.run(work).context(ErrorKind::Hyper)?;
-	Ok(res)
+    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<CbData, Error> {
-	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::<hyper::Uri>().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<CbData, E
 		})
 	});
 
-	let res = core.run(work)?;
+	let res = core.run(work).context(ErrorKind::GenericError("Could not run core"))?;
 	Ok(res)
 }

wallet/src/handlers.rs

@@ -24,6 +24,7 @@ use api;
 use keychain::Keychain;
 use types::*;
 use util;
+use failure::{Fail, ResultExt};
 
 
 pub struct CoinbaseHandler {
@@ -36,20 +37,20 @@ impl CoinbaseHandler {
 		let (out, kern, block_fees) = receive_coinbase(&self.config, &self.keychain, block_fees)
 			.map_err(|e| {
 				api::Error::Internal(format!("Error building coinbase: {:?}", e))
-			})?;
+			}).context(ErrorKind::Node)?;
 
 		let out_bin = ser::ser_vec(&out).map_err(|e| {
 			api::Error::Internal(format!("Error serializing output: {:?}", e))
-		})?;
+		}).context(ErrorKind::Node)?;
 
 		let kern_bin = ser::ser_vec(&kern).map_err(|e| {
 			api::Error::Internal(format!("Error serializing kernel: {:?}", e))
-		})?;
+		}).context(ErrorKind::Node)?;
 
 		let key_id_bin = match block_fees.key_id {
 			Some(key_id) => 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 {

wallet/src/info.rs

@@ -44,8 +44,7 @@ pub fn show_info(config: &WalletConfig, keychain: &Keychain) {
 	}
 }
 
-pub fn retrieve_info(config: &WalletConfig, keychain: &Keychain)
+pub fn retrieve_info(config: &WalletConfig, keychain: &Keychain) -> WalletInfo {
-	-> 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()
 }

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;

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 {

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())?;
+	let blind = blind_sum.secret_key(&keychain.secp()).context(ErrorKind::Keychain)?;
-	keychain.aggsig_create_context(&partial_tx.id, blind)?;
+	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 {
+   		return Err(ErrorKind::FeeExceedsAmount {
-			sender_amount: amount,
+            		sender_amount: amount,
-			recipient_fee: fee,
+            		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,

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<u64, Error> {
 				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()?,
+		proof: output.range_proof().context(ErrorKind::GenericError("range proof error"))?,
-		switch_commit_hash: output.switch_commit_hash()?,
+		switch_commit_hash: output.switch_commit_hash().context(ErrorKind::GenericError("switch commit hash error"))?,
-		commit: output.commit()?,
+		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,

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())?;
+	let skey = blind_offset.secret_key(&keychain.secp()).context(ErrorKind::Keychain)?;
-	keychain.aggsig_create_context(&tx_id, skey)?;
+	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 {
+		match e.kind() {
-			Error::FeeExceedsAmount {sender_amount, recipient_fee} =>
+			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 {
+		match e.kind() {
-			Error::FeeExceedsAmount {sender_amount, recipient_fee} =>
+			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)?;
+	let tx_burn = build::transaction(parts, &keychain).context(ErrorKind::Keychain)?;
-	tx_burn.validate()?;
+	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 {

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>) -> u64
 	(tx_weight as u64) * use_base_fee
 }
 
-/// Wallet errors, mostly wrappers around underlying crypto or I/O errors.
 #[derive(Debug)]
-pub enum Error {
+pub struct Error {
-	NotEnoughFunds(u64),
+    inner: Context<ErrorKind>,
-	FeeDispute { sender_fee: u64, recipient_fee: u64 },
+}
-	FeeExceedsAmount { sender_amount: u64, recipient_fee: u64 },
+
-	Keychain(keychain::Error),
+/// Wallet errors, mostly wrappers around underlying crypto or I/O errors.
-	Transaction(transaction::Error),
+#[derive(Copy, Clone, Eq, PartialEq, Debug, Fail)]
-	Secp(secp::Error),
+pub enum ErrorKind {
-	WalletData(String),
+    #[fail(display = "Not enough funds")]
-	/// An error in the format of the JSON structures exchanged by the wallet
+    NotEnoughFunds(u64),
-	Format(String),
+
-	/// An IO Error
+    #[fail(display = "Fee dispute: sender fee {}, recipient fee {}", sender_fee, recipient_fee)]
-	IOError(io::Error),
+    FeeDispute{sender_fee: u64, recipient_fee: u64},
-	/// Error when contacting a node through its API
+
-	Node(api::Error),
+    #[fail(display = "Fee exceeds amount: sender amount {}, recipient fee {}", sender_amount, recipient_fee)]
-	/// Error originating from hyper.
+    FeeExceedsAmount{sender_amount: u64,recipient_fee: u64},
-	Hyper(hyper::Error),
+
-	/// Error originating from hyper uri parsing.
+    #[fail(display = "Keychain error")]
-	Uri(hyper::error::UriError),
+    Keychain,
-	/// Error with signatures during exchange
+
-	Signature(String),
+    #[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 {
+impl Fail for Error {
-		match *self {
+    fn cause(&self) -> Option<&Fail> {
-			_ => "some kind of wallet error",
+        self.inner.cause()
-		}
+    }
-	}
+
+    fn backtrace(&self) -> Option<&Backtrace> {
+        self.inner.backtrace()
+    }
 }
 
-impl fmt::Display for Error {
+impl Display for Error {
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		match *self {
+        Display::fmt(&self.inner, f)
-			_ => write!(f, "some kind of wallet error"),
+    }
-		}
-	}
 }
 
-impl From<keychain::Error> for Error {
+impl Error {
-	fn from(e: keychain::Error) -> Error {
+    pub fn kind(&self) -> ErrorKind {
-		Error::Keychain(e)
+        *self.inner.get_context()
-	}
+    }
 }
 
-impl From<secp::Error> for Error {
+impl From<ErrorKind> for Error {
-	fn from(e: secp::Error) -> Error {
+    fn from(kind: ErrorKind) -> Error {
-		Error::Secp(e)
+        Error {
-	}
+            inner: Context::new(kind),
+        }
+    }
 }
 
-impl From<transaction::Error> for Error {
+impl From<Context<ErrorKind>> for Error {
-	fn from(e: transaction::Error) -> Error {
+    fn from(inner: Context<ErrorKind>) -> Error {
-		Error::Transaction(e)
+        Error { inner: inner }
-	}
+    }
-}
-
-impl From<serde_json::Error> 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<num::ParseIntError> for Error {
-	fn from(_: num::ParseIntError) -> Error {
-		Error::Format("Invalid hex".to_string())
-	}
-}
-
-impl From<ser::Error> for Error {
-	fn from(e: ser::Error) -> Error {
-		Error::Format(e.to_string())
-	}
-}
-
-impl From<api::Error> for Error {
-	fn from(e: api::Error) -> Error {
-		Error::Node(e)
-	}
-}
-
-impl From<io::Error> for Error {
-	fn from(e: io::Error) -> Error {
-		Error::IOError(e)
-	}
-}
-
-impl From<hyper::Error> for Error {
-	fn from(e: hyper::Error) -> Error {
-		Error::Hyper(e)
-	}
-}
-
-impl From<hyper::error::UriError> for Error {
-	fn from(e: hyper::error::UriError) -> Error {
-		Error::Uri(e)
-	}
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -241,7 +235,7 @@ impl BlockIdentifier {
 	}
 
 	pub fn from_str(hex: &str) -> Result<BlockIdentifier, Error> {
-		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<WalletSeed, Error> {
-		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<keychain::Keychain, Error> {
 		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<WalletSeed, Error> {
 		// 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)?;
+			let mut file = File::create(seed_file_path).context(ErrorKind::IO)?;
-			file.write_all(&seed.to_hex().as_bytes())?;
+			file.write_all(&seed.to_hex().as_bytes()).context(ErrorKind::IO)?;
 			Ok(seed)
 		}
 	}
 
 	pub fn from_file(wallet_config: &WalletConfig) -> Result<WalletSeed, Error> {
 		// 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<T, F>(data_file_dir: &str, f: F) -> Result<T, Error>
 	where
-		F: FnOnce(&WalletData) -> T,
+		F: FnOnce(&WalletData) -> Result<T, Error>,
 	{
 		// 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);
+		f(&wdat)
-		Ok(res)
 	}
 
 	/// 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(|_| {
+		fs::remove_file(lock_file_path).context(ErrorKind::WalletData("Could not remove wallet lock file. Maybe insufficient rights?"))?;
-			Error::WalletData(format!(
-				"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<Vec<OutputData>, Error> {
-		let data_file = File::open(data_file_path).map_err(|e| {
+		let data_file = File::open(data_file_path).context(ErrorKind::WalletData(&"Could not open wallet file"))?;
-			Error::WalletData(format!("Could not open {}: {}", data_file_path, e))
+		serde_json::from_reader(data_file).map_err(|e| { e.context(ErrorKind::WalletData(&"Error reading wallet file ")).into()})
-		})?;
+            
-		serde_json::from_reader(data_file).map_err(|e| {
+            
-			Error::WalletData(format!("Error reading {}: {}", data_file_path, e))
-		})
 	}
 
 	/// 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::<Vec<_>>();
 		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| {
+		data_file.write_all(res_json.as_slice()).context(ErrorKind::WalletData(&"Error writing wallet file")).map_err(|e| e.into())
-			Error::WalletData(format!("Error writing {}: {}", data_file_path, e))
-		})
 	}
 
 	/// 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<Signature>, Transaction), Error> {
-	let blind_bin = util::from_hex(partial_tx.public_blind_excess.clone())?;
+	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[..])?;
+	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_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[..])?;
+	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_bin = util::from_hex(partial_tx.tx.clone()).context(ErrorKind::GenericError("Could not decode HEX"))?;
-	let tx = ser::deserialize(&mut &tx_bin[..]).map_err(|_| {
+	let tx = ser::deserialize(&mut &tx_bin[..]).context(ErrorKind::GenericError("Could not deserialize transaction, invalid format."))?;
-		Error::Format("Could not deserialize transaction, invalid format.".to_string())
+    Ok((partial_tx.amount, blinding, nonce, kernel_offset, sig, tx))
-	})?;
-	Ok((partial_tx.amount, blinding, nonce, kernel_offset, sig, tx))
 }
 
 /// Amount in request to build a coinbase output.