feat: return structured errors from contracts (axelarnetwork#527)

contracts/coordinator/src/contract.rs

@@ -2,7 +2,7 @@ mod execute;
 mod query;
 
 mod migrations;
-use axelar_wasm_std::permission_control;
+use axelar_wasm_std::{permission_control, FnExt};
 #[cfg(not(feature = "library"))]
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
@@ -67,8 +67,8 @@ pub fn execute(
         ExecuteMsg::SetActiveVerifiers { verifiers } => {
             execute::set_active_verifier_set(deps, info, verifiers)
         }
-    }
-    .map_err(axelar_wasm_std::ContractError::from)
+    }?
+    .then(Ok)
 }
 
 fn find_prover_address(
@@ -89,9 +89,9 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> Result<Binary, ContractErr
     match msg {
         QueryMsg::ReadyToUnbond { worker_address } => to_json_binary(
             &query::check_verifier_ready_to_unbond(deps, worker_address)?,
-        )
-        .map_err(|err| err.into()),
+        )?,
     }
+    .then(Ok)
 }
 
 #[cfg(test)]

contracts/multisig/src/contract.rs

@@ -1,12 +1,15 @@
-use axelar_wasm_std::{killswitch, permission_control};
+use std::collections::HashMap;
+
+use axelar_wasm_std::{killswitch, permission_control, FnExt};
 #[cfg(not(feature = "library"))]
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
-    to_json_binary, Addr, Binary, Deps, DepsMut, Env, HexBinary, MessageInfo, Response, StdResult,
-    Storage, Uint64,
+    to_json_binary, Addr, Binary, Deps, DepsMut, Env, HexBinary, MessageInfo, Response, StdError,
+    StdResult, Storage, Uint64,
 };
 use error_stack::{report, ResultExt};
 use itertools::Itertools;
+use router_api::ChainName;
 
 use crate::events::Event;
 use crate::msg::{ExecuteMsg, InstantiateMsg, MigrationMsg, QueryMsg};
@@ -121,31 +124,31 @@ pub fn execute(
             signed_sender_address,
         } => execute::register_pub_key(deps, info, public_key, signed_sender_address),
         ExecuteMsg::AuthorizeCallers { contracts } => {
-            let contracts = contracts
-                .into_iter()
-                .map(|(contract_address, chain_name)| {
-                    deps.api
-                        .addr_validate(&contract_address)
-                        .map(|addr| (addr, chain_name))
-                })
-                .try_collect()?;
+            let contracts = validate_contract_addresses(&deps, contracts)?;
             execute::authorize_callers(deps, contracts)
         }
         ExecuteMsg::UnauthorizeCallers { contracts } => {
-            let contracts = contracts
-                .into_iter()
-                .map(|(contract_address, chain_name)| {
-                    deps.api
-                        .addr_validate(&contract_address)
-                        .map(|addr| (addr, chain_name))
-                })
-                .try_collect()?;
+            let contracts = validate_contract_addresses(&deps, contracts)?;
             execute::unauthorize_callers(deps, contracts)
         }
         ExecuteMsg::DisableSigning => execute::disable_signing(deps),
         ExecuteMsg::EnableSigning => execute::enable_signing(deps),
-    }
-    .map_err(axelar_wasm_std::ContractError::from)
+    }?
+    .then(Ok)
+}
+
+fn validate_contract_addresses(
+    deps: &DepsMut,
+    contracts: HashMap<String, ChainName>,
+) -> Result<HashMap<Addr, ChainName>, StdError> {
+    contracts
+        .into_iter()
+        .map(|(contract_address, chain_name)| {
+            deps.api
+                .addr_validate(&contract_address)
+                .map(|addr| (addr, chain_name))
+        })
+        .try_collect()
 }
 
 fn can_start_signing_session(

contracts/multisig/src/contract/execute.rs

@@ -215,7 +215,7 @@ pub fn authorize_callers(
 
 pub fn unauthorize_callers(
     deps: DepsMut,
-    contracts: Vec<(Addr, ChainName)>,
+    contracts: HashMap<Addr, ChainName>,
 ) -> Result<Response, ContractError> {
     contracts.iter().for_each(|(contract_address, _)| {
         AUTHORIZED_CALLERS.remove(deps.storage, contract_address)

contracts/rewards/src/contract.rs

@@ -91,8 +91,7 @@ pub fn execute(
                 verifier_address,
                 pool_id,
                 env.block.height,
-            )
-            .map_err(axelar_wasm_std::ContractError::from)?;
+            )?;
 
             Ok(Response::new())
         }
@@ -122,8 +121,7 @@ pub fn execute(
             deps.api.addr_validate(pool_id.contract.as_str())?;
 
             let rewards =
-                execute::distribute_rewards(deps.storage, pool_id, env.block.height, epoch_count)
-                    .map_err(axelar_wasm_std::ContractError::from)?;
+                execute::distribute_rewards(deps.storage, pool_id, env.block.height, epoch_count)?;
 
             let msgs = rewards
                 .into_iter()

contracts/router/src/contract.rs

@@ -101,8 +101,8 @@ pub fn execute(
         }
         ExecuteMsg::DisableRouting => execute::disable_routing(deps.storage),
         ExecuteMsg::EnableRouting => execute::enable_routing(deps.storage),
-    }
-    .map_err(axelar_wasm_std::ContractError::from)
+    }?
+    .then(Ok)
 }
 
 fn find_gateway_address(

contracts/service-registry/src/contract.rs

@@ -1,3 +1,4 @@
+use axelar_wasm_std::FnExt;
 #[cfg(not(feature = "library"))]
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
@@ -130,8 +131,8 @@ pub fn execute(
         ExecuteMsg::ClaimStake { service_name } => {
             execute::claim_stake(deps, env, info, service_name)
         }
-    }
-    .map_err(axelar_wasm_std::ContractError::from)
+    }?
+    .then(Ok)
 }
 
 #[cfg_attr(not(feature = "library"), entry_point)]

contracts/voting-verifier/src/contract.rs

@@ -1,3 +1,4 @@
+use axelar_wasm_std::FnExt;
 #[cfg(not(feature = "library"))]
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
@@ -60,8 +61,8 @@ pub fn execute(
             execute::require_governance(&deps, info.sender)?;
             execute::update_voting_threshold(deps, new_voting_threshold)
         }
-    }
-    .map_err(axelar_wasm_std::ContractError::from)
+    }?
+    .then(Ok)
 }
 
 #[cfg_attr(not(feature = "library"), entry_point)]

packages/axelar-wasm-std-derive/src/lib.rs

@@ -9,14 +9,11 @@ pub fn into_contract_error_derive(input: TokenStream) -> TokenStream {
     let name = &ast.ident;
 
     let gen = quote! {
-        use axelar_wasm_std::ContractError as _ContractError;
-
-        impl From<#name> for _ContractError {
+        impl From<#name> for axelar_wasm_std::ContractError {
             fn from(error: #name) -> Self {
-                use report::LoggableError;
                 use error_stack::report;
 
-                LoggableError::from(&report!(error)).into()
+                report!(error).into()
             }
         }
     };

packages/axelar-wasm-std/src/error.rs

@@ -1,5 +1,7 @@
+use std::fmt::{Display, Formatter};
+
 use cosmwasm_std::StdError;
-use error_stack::{Context, Report};
+use error_stack::{report, Context, Report};
 use report::LoggableError;
 use thiserror::Error;
 
@@ -10,24 +12,55 @@ use crate::permission_control;
 /// but it won't show all necessary information (namely attachments) in the error message, and many places also return an [StdError].
 /// To this end, reports get converted into [LoggableError] and this [ContractError] type unifies [LoggableError] and [StdError],
 /// so we can return both to cosmwasm.
-#[derive(Error, Debug, PartialEq)]
-pub enum ContractError {
-    #[error(transparent)]
-    Std(#[from] StdError),
-    #[error(transparent)]
-    Structured(#[from] LoggableError),
-    #[error(transparent)]
-    Unauthorized(#[from] permission_control::Error),
-    #[error(transparent)]
-    WrongVersion(#[from] cw2::VersionError),
+#[derive(Error, Debug)]
+pub struct ContractError {
+    pub report: Report<Error>,
+}
+
+#[derive(Error, Debug)]
+pub enum Error {
+    #[error("")]
+    Report,
+}
+
+impl From<StdError> for ContractError {
+    fn from(err: StdError) -> Self {
+        ContractError {
+            report: report!(err).change_context(Error::Report),
+        }
+    }
+}
+
+impl From<cw2::VersionError> for ContractError {
+    fn from(err: cw2::VersionError) -> Self {
+        ContractError {
+            report: report!(err).change_context(Error::Report),
+        }
+    }
+}
+
+impl From<permission_control::Error> for ContractError {
+    fn from(err: permission_control::Error) -> Self {
+        ContractError {
+            report: report!(err).change_context(Error::Report),
+        }
+    }
 }
 
 impl<T> From<Report<T>> for ContractError
 where
     T: Context,
 {
     fn from(report: Report<T>) -> Self {
-        ContractError::Structured(LoggableError::from(&report))
+        ContractError {
+            report: report.change_context(Error::Report),
+        }
+    }
+}
+
+impl Display for ContractError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        LoggableError::from(&self.report).fmt(f)
     }
 }
 

packages/report/src/loggable.rs

@@ -81,7 +81,10 @@ impl<T> From<&Report<T>> for LoggableError {
             // because of the stack order of attachments we need to reverse them to get the historical order
             attachments.reverse();
             error.attachments = attachments;
-            errors.push(error)
+
+            if !error.msg.is_empty() || !error.attachments.is_empty() {
+                errors.push(error)
+            }
         }
 
         chain_causes(errors).expect("a report must have at least one error")