refactor: clean up error definitions

ampd/src/lib.rs

@@ -4,7 +4,7 @@ use std::pin::Pin;
 use cosmos_sdk_proto::cosmos::{
     auth::v1beta1::query_client::QueryClient, tx::v1beta1::service_client::ServiceClient,
 };
-use error_stack::{FutureExt, Report, Result, ResultExt};
+use error_stack::{FutureExt, Result, ResultExt};
 use tokio::signal::unix::{signal, SignalKind};
 use tokio::task::JoinSet;
 use tokio_stream::Stream;
@@ -52,20 +52,20 @@ pub async fn run(cfg: Config, state_path: PathBuf) -> Result<(), Error> {
         multisig,
     } = cfg;
 
-    let tm_client =
-        tendermint_rpc::HttpClient::new(tm_jsonrpc.to_string().as_str()).map_err(Error::new)?;
+    let tm_client = tendermint_rpc::HttpClient::new(tm_jsonrpc.to_string().as_str())
+        .change_context(Error::Connection)?;
     let service_client = ServiceClient::connect(tm_grpc.to_string())
         .await
-        .map_err(Error::new)?;
+        .change_context(Error::Connection)?;
     let query_client = QueryClient::connect(tm_grpc.to_string())
         .await
-        .map_err(Error::new)?;
+        .change_context(Error::Connection)?;
     let multisig_client = MultisigClient::connect(tofnd_config.party_uid, tofnd_config.url)
         .await
-        .map_err(Error::new)?;
+        .change_context(Error::Tofnd)?;
     let ecdsa_client = SharableEcdsaClient::new(multisig_client);
 
-    let mut state_updater = StateUpdater::new(state_path).map_err(Error::new)?;
+    let mut state_updater = StateUpdater::new(state_path).change_context(Error::StateUpdater)?;
     let pub_key = match state_updater.state().pub_key {
         Some(pub_key) => pub_key,
         None => {
@@ -83,7 +83,9 @@ pub async fn run(cfg: Config, state_path: PathBuf) -> Result<(), Error> {
         .account_id(PREFIX)
         .expect("failed to convert to account identifier")
         .into();
-    let account = account(query_client, &worker).await.map_err(Error::new)?;
+    let account = account(query_client, &worker)
+        .await
+        .change_context(Error::Broadcaster)?;
 
     let broadcaster = broadcaster::BroadcastClientBuilder::default()
         .client(service_client)
@@ -176,7 +178,8 @@ where
                 worker.clone(),
                 config.voting_verifier,
                 config.name,
-                evm::json_rpc::Client::new_http(&config.rpc_url).map_err(Error::new)?,
+                evm::json_rpc::Client::new_http(&config.rpc_url)
+                    .change_context(Error::Connection)?,
                 self.broadcaster.client(),
             );
 
@@ -258,7 +261,7 @@ where
         let res = match (set.join_next().await, token.is_cancelled()) {
             (Some(result), false) => {
                 token.cancel();
-                result.map_err(Error::new).map_err(Report::from)?
+                result.change_context(Error::Threading)?
             }
             (Some(_), true) => Ok(()),
             (None, _) => panic!("all tasks exited unexpectedly"),

ampd/src/report.rs

@@ -1,10 +1,11 @@
+use std::backtrace::Backtrace;
 use std::collections::VecDeque;
 use std::iter::FromIterator;
 use std::panic::Location;
-use std::{backtrace::Backtrace, fmt::Display};
 
 use error_stack::{AttachmentKind, Context, Frame, FrameKind, Report};
 use thiserror::Error;
+use tracing::error;
 use valuable::Valuable;
 
 #[derive(Error, Debug)]
@@ -19,17 +20,10 @@ pub enum Error {
     StateUpdater,
     #[error("tofnd failed")]
     Tofnd,
-    #[error("{0}")]
-    Error(String),
-}
-
-impl Error {
-    pub fn new<T>(msg: T) -> Error
-    where
-        T: Display,
-    {
-        Error::Error(msg.to_string())
-    }
+    #[error("connection failed")]
+    Connection,
+    #[error("thread execution failed")]
+    Threading,
 }
 
 #[derive(Valuable, PartialEq, Debug, Default)]

ampd/src/tofnd/error.rs

@@ -8,7 +8,7 @@ pub enum Error {
     KeygenFailed,
     #[error("sign failed")]
     SignFailed,
-    #[error("{0}")]
+    #[error(transparent)]
     FromHex(#[from] hex::FromHexError),
     #[error("parsing failed")]
     ParsingFailed,

ampd/tests/report.rs

@@ -1,16 +1,23 @@
 use error_stack::Report;
+use thiserror::Error;
 
-use ampd::report::{Error, LoggableError};
+use ampd::report::LoggableError;
+
+#[derive(Error, Debug)]
+enum Error {
+    #[error("{0}")]
+    FromString(String),
+}
 
 // Do not move this test or the location field checks break
 #[test]
 fn correct_error_log() {
-    let report = Report::new(Error::new("error1".to_string()))
+    let report = Report::new(Error::FromString("error1".to_string()))
         .attach_printable("foo1")
-        .change_context(Error::new("error2".to_string()))
+        .change_context(Error::FromString("error2".to_string()))
         .attach_printable("test1")
         .attach_printable("test2")
-        .change_context(Error::new("error3".to_string()))
+        .change_context(Error::FromString("error3".to_string()))
         .attach(5);
 
     let mut err = LoggableError::from(&report);
@@ -25,15 +32,15 @@ fn correct_error_log() {
     let expected_err = LoggableError {
         msg: "error3".to_string(),
         attachments: vec!["opaque attachment".to_string()],
-        location: "ampd/tests/report.rs:13:10".to_string(),
+        location: "ampd/tests/report.rs:20:10".to_string(),
         cause: Some(Box::new(LoggableError {
             msg: "error2".to_string(),
             attachments: vec!["test1".to_string(), "test2".to_string()],
-            location: "ampd/tests/report.rs:10:10".to_string(),
+            location: "ampd/tests/report.rs:17:10".to_string(),
             cause: Some(Box::new(LoggableError {
                 msg: "error1".to_string(),
                 attachments: vec!["foo1".to_string()],
-                location: "ampd/tests/report.rs:8:18".to_string(),
+                location: "ampd/tests/report.rs:15:18".to_string(),
                 cause: None,
                 backtrace: None,
             })),

contracts/aggregate-verifier/src/error.rs

@@ -3,10 +3,10 @@ use thiserror::Error;
 
 #[derive(Error, Debug, PartialEq)]
 pub enum ContractError {
-    #[error("{0}")]
+    #[error(transparent)]
     Std(#[from] StdError),
 
-    #[error("{0}")]
+    #[error(transparent)]
     RouterError(#[from] connection_router::ContractError),
 
     #[error("received invalid verifier reply: {0}")]

contracts/batching/src/error.rs

@@ -3,6 +3,6 @@ use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum ContractError {
-    #[error("{0}")]
+    #[error(transparent)]
     Std(#[from] StdError),
 }

contracts/connection-router/src/error.rs

@@ -5,29 +5,29 @@ use crate::types::ChainName;
 
 #[derive(Error, Debug, PartialEq)]
 pub enum ContractError {
-    #[error("{0}")]
+    #[error(transparent)]
     Std(#[from] StdError),
 
     #[error("caller is not authorized")]
-    Unauthorized {},
+    Unauthorized,
 
     #[error("chain already exists")]
-    ChainAlreadyExists {},
+    ChainAlreadyExists,
 
     #[error("chain name is invalid")]
-    InvalidChainName {},
+    InvalidChainName,
 
     #[error("message ID is invalid")]
-    InvalidMessageID {},
+    InvalidMessageID,
 
     #[error("chain is not found")]
-    ChainNotFound {},
+    ChainNotFound,
 
     #[error("gateway is not registered")]
-    GatewayNotRegistered {},
+    GatewayNotRegistered,
 
     #[error("gateway is already registered")]
-    GatewayAlreadyRegistered {},
+    GatewayAlreadyRegistered,
 
     #[error("chain is frozen")]
     ChainFrozen { chain: ChainName },
@@ -36,7 +36,7 @@ pub enum ContractError {
     InvalidAddress(String),
 
     #[error("source chain does not match registered gateway")]
-    WrongSourceChain {},
+    WrongSourceChain,
 }
 
 impl From<ContractError> for StdError {

contracts/connection-router/tests/test.rs

@@ -143,7 +143,7 @@ fn route_non_existing_chain() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::ChainNotFound {}, res.downcast().unwrap());
+    assert_eq!(ContractError::ChainNotFound , res.downcast().unwrap());
 }
 
 #[test]
@@ -188,7 +188,7 @@ fn message_id() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::InvalidMessageID {}, res.downcast().unwrap());
+    assert_eq!(ContractError::InvalidMessageID , res.downcast().unwrap());
 
     // don't prepend source chain
     let bad_id = msg.id.split(&msg.source_chain).collect::<Vec<&str>>()[0];
@@ -204,7 +204,7 @@ fn message_id() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::InvalidMessageID {}, res.downcast().unwrap());
+    assert_eq!(ContractError::InvalidMessageID , res.downcast().unwrap());
 
     let res = config
         .app
@@ -218,7 +218,7 @@ fn message_id() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::InvalidMessageID {}, res.downcast().unwrap());
+    assert_eq!(ContractError::InvalidMessageID , res.downcast().unwrap());
 
     let res = config
         .app
@@ -232,7 +232,7 @@ fn message_id() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::InvalidMessageID {}, res.downcast().unwrap());
+    assert_eq!(ContractError::InvalidMessageID , res.downcast().unwrap());
 }
 
 #[test]
@@ -301,7 +301,7 @@ fn wrong_source_chain() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::WrongSourceChain {}, res.downcast().unwrap());
+    assert_eq!(ContractError::WrongSourceChain , res.downcast().unwrap());
 }
 
 #[test]
@@ -379,7 +379,7 @@ fn authorization() {
         )
         .unwrap_err();
 
-    assert_eq!(ContractError::Unauthorized {}, res.downcast().unwrap());
+    assert_eq!(ContractError::Unauthorized , res.downcast().unwrap());
 
     let res = config.app.execute_contract(
         config.admin_address.clone(),
@@ -405,7 +405,7 @@ fn authorization() {
         )
         .unwrap_err();
 
-    assert_eq!(ContractError::Unauthorized {}, res.downcast().unwrap());
+    assert_eq!(ContractError::Unauthorized , res.downcast().unwrap());
 
     let res = config.app.execute_contract(
         config.admin_address.clone(),
@@ -431,7 +431,7 @@ fn authorization() {
         )
         .unwrap_err();
 
-    assert_eq!(ContractError::Unauthorized {}, res.downcast().unwrap());
+    assert_eq!(ContractError::Unauthorized , res.downcast().unwrap());
 
     let res = config
         .app
@@ -446,7 +446,7 @@ fn authorization() {
         )
         .unwrap_err();
 
-    assert_eq!(ContractError::Unauthorized {}, res.downcast().unwrap());
+    assert_eq!(ContractError::Unauthorized, res.downcast().unwrap());
 
     let res = config.app.execute_contract(
         config.admin_address.clone(),
@@ -536,7 +536,7 @@ fn upgrade_gateway_incoming() {
         )
         .unwrap_err();
     assert_eq!(
-        ContractError::GatewayNotRegistered {},
+        ContractError::GatewayNotRegistered ,
         res.downcast().unwrap()
     );
 
@@ -569,7 +569,7 @@ fn register_chain_test() {
         )
         .unwrap_err();
     assert_eq!(
-        ContractError::GatewayNotRegistered {},
+        ContractError::GatewayNotRegistered ,
         res.downcast().unwrap()
     );
 
@@ -583,7 +583,7 @@ fn register_chain_test() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::ChainNotFound {}, res.downcast().unwrap());
+    assert_eq!(ContractError::ChainNotFound , res.downcast().unwrap());
 
     register_chain(&mut config, &polygon);
     let res = config.app.execute_contract(
@@ -614,7 +614,7 @@ fn chain_already_registered() {
         )
         .unwrap_err();
     assert_eq!(
-        ContractError::ChainAlreadyExists {},
+        ContractError::ChainAlreadyExists ,
         res.downcast().unwrap()
     );
 
@@ -632,7 +632,7 @@ fn chain_already_registered() {
         )
         .unwrap_err();
     assert_eq!(
-        ContractError::ChainAlreadyExists {},
+        ContractError::ChainAlreadyExists ,
         res.downcast().unwrap()
     );
 }
@@ -652,7 +652,7 @@ fn invalid_chain_name() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::InvalidChainName {}, res.downcast().unwrap());
+    assert_eq!(ContractError::InvalidChainName , res.downcast().unwrap());
 
     let res = config
         .app
@@ -666,7 +666,7 @@ fn invalid_chain_name() {
             &[],
         )
         .unwrap_err();
-    assert_eq!(ContractError::InvalidChainName {}, res.downcast().unwrap());
+    assert_eq!(ContractError::InvalidChainName , res.downcast().unwrap());
 }
 
 #[test]
@@ -688,7 +688,7 @@ fn gateway_already_registered() {
         )
         .unwrap_err();
     assert_eq!(
-        ContractError::GatewayAlreadyRegistered {},
+        ContractError::GatewayAlreadyRegistered ,
         res.downcast().unwrap()
     );
 
@@ -706,7 +706,7 @@ fn gateway_already_registered() {
         )
         .unwrap_err();
     assert_eq!(
-        ContractError::GatewayAlreadyRegistered {},
+        ContractError::GatewayAlreadyRegistered ,
         res.downcast().unwrap()
     );
 }