Clean up generic String variants in error types

ibc-apps/ics20-transfer/types/src/amount.rs

@@ -106,9 +106,7 @@ impl FromStr for Amount {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let amount = U256::from_dec_str(s).map_err(|e| {
-            DecodingError::invalid_raw_data(format!(
-                "invalid amount that could not be parsed as a U256: {e}"
-            ))
+            DecodingError::invalid_raw_data(format!("amount could not be parsed as a U256: {e}"))
         })?;
 
         Ok(Self(amount))

ibc-apps/ics20-transfer/types/src/coin.rs

@@ -77,7 +77,7 @@ where
                     .all(|x| x.is_alphanumeric() || VALID_DENOM_CHARACTERS.contains(x))
             })
             .ok_or(DecodingError::invalid_raw_data(format!(
-                "invalid coin: {coin_str}"
+                "coin str: {coin_str}"
             )))?;
 
         Ok(Coin {

ibc-apps/ics20-transfer/types/src/denom.rs

@@ -218,9 +218,7 @@ impl FromStr for TracePath {
         remaining_parts
             .is_none()
             .then_some(trace_path)
-            .ok_or(DecodingError::invalid_raw_data(format!(
-                "invalid trace path: {s}"
-            )))
+            .ok_or(DecodingError::invalid_raw_data(format!("trace path: {s}")))
     }
 }
 

ibc-apps/ics20-transfer/types/src/msgs/transfer.rs

@@ -55,7 +55,7 @@
         // Packet timeout height and packet timeout timestamp cannot both be unset.
         if !timeout_height_on_b.is_set() && !timeout_timestamp_on_b.is_set() {
             return Err(DecodingError::missing_raw_data(
-                "missing timeout height or timeout timestamp",
+                "msg transfer timeout height or timeout timestamp",
             ));
         }
 
@@ -65,7 +65,7 @@
             packet_data: PacketData {
                 token: raw_msg
                     .token
-                    .ok_or(DecodingError::missing_raw_data("missing token"))?
+                    .ok_or(DecodingError::missing_raw_data("msg transfer token"))?
                     .try_into()?,
                 sender: raw_msg.sender.into(),
                 receiver: raw_msg.receiver.into(),

ibc-apps/ics721-nft-transfer/types/src/class.rs

@@ -247,9 +247,7 @@
     fn from_str(class_uri: &str) -> Result<Self, Self::Err> {
         match Uri::from_str(class_uri) {
             Ok(uri) => Ok(Self(uri)),
-            Err(err) => Err(DecodingError::invalid_raw_data(format!(
-                "invalid class URI: {err}"
-            ))),
+            Err(err) => Err(DecodingError::invalid_raw_data(format!("class URI: {err}"))),
         }
     }
 }

ibc-apps/ics721-nft-transfer/types/src/msgs/transfer.rs

@@ -8,7 +8,6 @@
 use ibc_proto::ibc::applications::nft_transfer::v1::MsgTransfer as RawMsgTransfer;
 use ibc_proto::Protobuf;
 
-use crate::error::NftTransferError;
 use crate::packet::PacketData;
 
 pub(crate) const TYPE_URL: &str = "/ibc.applications.nft_transfer.v1.MsgTransfer";
@@ -115,11 +114,11 @@
 impl Protobuf<RawMsgTransfer> for MsgTransfer {}
 
 impl TryFrom<Any> for MsgTransfer {
-    type Error = NftTransferError;
+    type Error = DecodingError;
 
     fn try_from(raw: Any) -> Result<Self, Self::Error> {
         match raw.type_url.as_str() {
-            TYPE_URL => Ok(MsgTransfer::decode_vec(&raw.value).map_err(DecodingError::Protobuf)?),
+            TYPE_URL => Ok(MsgTransfer::decode_vec(&raw.value)?),
             _ => Err(DecodingError::MismatchedTypeUrls {
                 expected: TYPE_URL.to_string(),
                 actual: raw.type_url,

ibc-clients/ics07-tendermint/src/client_state.rs

@@ -8,10 +8,9 @@
 //! Rust). As such, this module also includes some trait implementations that
 //! serve to pass through traits implemented on the wrapped `ClientState` type.
 
-use ibc_client_tendermint_types::error::TendermintClientError;
 use ibc_client_tendermint_types::proto::v1::ClientState as RawTmClientState;
 use ibc_client_tendermint_types::ClientState as ClientStateType;
-use ibc_core_client::types::error::ClientError;
+use ibc_core_host::types::error::DecodingError;
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::{Any, Protobuf};
 
@@ -44,7 +43,7 @@ impl ClientState {
 impl Protobuf<RawTmClientState> for ClientState {}
 
 impl TryFrom<RawTmClientState> for ClientState {
-    type Error = TendermintClientError;
+    type Error = DecodingError;
 
     fn try_from(raw: RawTmClientState) -> Result<Self, Self::Error> {
         Ok(Self(ClientStateType::try_from(raw)?))
@@ -60,7 +59,7 @@ impl From<ClientState> for RawTmClientState {
 impl Protobuf<Any> for ClientState {}
 
 impl TryFrom<Any> for ClientState {
-    type Error = ClientError;
+    type Error = DecodingError;
 
     fn try_from(raw: Any) -> Result<Self, Self::Error> {
         Ok(Self(ClientStateType::try_from(raw)?))

ibc-clients/ics07-tendermint/src/client_state/common.rs

@@ -159,7 +159,7 @@
     let tm_consensus_state = TmConsensusState::try_from(consensus_state)?;
 
     if tm_consensus_state.root().is_empty() {
-        Err(CommitmentError::EmptyCommitmentRoot)?;
+        Err(CommitmentError::MissingCommitmentRoot)?;
     };
 
     if consensus_state_status(&tm_consensus_state, host_timestamp, trusting_period)?.is_expired() {
@@ -294,7 +294,7 @@
     value: Vec<u8>,
 ) -> Result<(), ClientError> {
     if prefix.is_empty() {
-        Err(CommitmentError::EmptyCommitmentPrefix)?;
+        Err(CommitmentError::MissingCommitmentPrefix)?;
     }
 
     let merkle_path = MerklePath::new(vec![prefix.as_bytes().to_vec().into(), path]);

ibc-clients/ics07-tendermint/src/client_state/execution.rs

@@ -8,6 +8,7 @@ use ibc_core_host::types::identifiers::ClientId;
 use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath};
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
+use ibc_primitives::TimestampError;
 
 use super::ClientState;
 
@@ -341,11 +342,7 @@ where
         let tm_consensus_state_timestamp = tm_consensus_state.timestamp();
         let tm_consensus_state_expiry = (tm_consensus_state_timestamp
             + client_state.trusting_period)
-            .map_err(|_| ClientError::Other {
-                description: String::from(
-                    "Timestamp overflow error occurred while attempting to parse TmConsensusState",
-                ),
-            })?;
+            .map_err(|_| TimestampError::TimestampOverflow)?;
 
         if tm_consensus_state_expiry > host_timestamp {
             break;

ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs

@@ -4,6 +4,7 @@
 };
 use ibc_core_client::context::{Convertible, ExtClientValidationContext};
 use ibc_core_client::types::error::ClientError;
+use ibc_core_host::types::error::IdentifierError;
 use ibc_core_host::types::identifiers::{ChainId, ClientId};
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
@@ -116,12 +117,13 @@
     // main header verification, delegated to the tendermint-light-client crate.
     let untrusted_state = header.as_untrusted_block_state();
 
-    let tm_chain_id = &chain_id
-        .as_str()
-        .try_into()
-        .map_err(|e| ClientError::Other {
-            description: format!("failed to parse chain id: {e}"),
-        })?;
+    let tm_chain_id =
+        &chain_id
+            .as_str()
+            .try_into()
+            .map_err(|e| IdentifierError::FailedToParse {
+                description: format!("chain ID `{chain_id}`: {e:?}"),
+            })?;
 
     let trusted_state =
         header.as_trusted_block_state(tm_chain_id, trusted_time, trusted_next_validator_hash)?;

ibc-clients/ics07-tendermint/src/client_state/update_client.rs

@@ -3,6 +3,7 @@
 use ibc_core_client::context::{Convertible, ExtClientValidationContext};
 use ibc_core_client::types::error::ClientError;
 use ibc_core_client::types::Height;
+use ibc_core_host::types::error::IdentifierError;
 use ibc_core_host::types::identifiers::{ChainId, ClientId};
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
@@ -52,12 +53,11 @@
             )?;
 
             TrustedBlockState {
-                chain_id: &chain_id
-                    .as_str()
-                    .try_into()
-                    .map_err(|e| ClientError::Other {
-                        description: format!("failed to parse chain id: {}", e),
-                    })?,
+                chain_id: &chain_id.as_str().try_into().map_err(|e| {
+                    IdentifierError::FailedToParse {
+                        description: format!("chain ID `{chain_id}`: {e:?}"),
+                    }
+                })?,
                 header_time: trusted_consensus_state.timestamp(),
                 height: header
                     .trusted_height

ibc-clients/ics07-tendermint/src/consensus_state.rs

@@ -6,12 +6,11 @@
 //! implementations that serve to pass through traits implemented on the wrapped
 //! `ConsensusState` type.
 
-use ibc_client_tendermint_types::error::TendermintClientError;
 use ibc_client_tendermint_types::proto::v1::ConsensusState as RawTmConsensusState;
 use ibc_client_tendermint_types::ConsensusState as ConsensusStateType;
 use ibc_core_client::context::consensus_state::ConsensusState as ConsensusStateTrait;
-use ibc_core_client::types::error::ClientError;
 use ibc_core_commitment_types::commitment::CommitmentRoot;
+use ibc_core_host::types::error::DecodingError;
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::{Any, Protobuf};
 use ibc_primitives::Timestamp;
@@ -52,7 +51,7 @@ impl From<ConsensusState> for ConsensusStateType {
 impl Protobuf<RawTmConsensusState> for ConsensusState {}
 
 impl TryFrom<RawTmConsensusState> for ConsensusState {
-    type Error = TendermintClientError;
+    type Error = DecodingError;
 
     fn try_from(raw: RawTmConsensusState) -> Result<Self, Self::Error> {
         Ok(Self(ConsensusStateType::try_from(raw)?))
@@ -68,7 +67,7 @@ impl From<ConsensusState> for RawTmConsensusState {
 impl Protobuf<Any> for ConsensusState {}
 
 impl TryFrom<Any> for ConsensusState {
-    type Error = ClientError;
+    type Error = DecodingError;
 
     fn try_from(raw: Any) -> Result<Self, Self::Error> {
         Ok(Self(ConsensusStateType::try_from(raw)?))

ibc-clients/ics07-tendermint/types/src/client_state.rs

@@ -4,7 +4,6 @@
 use core::str::FromStr;
 use core::time::Duration;
 
-use ibc_core_client_types::error::ClientError;
 use ibc_core_client_types::proto::v1::Height as RawHeight;
 use ibc_core_client_types::Height;
 use ibc_core_commitment_types::specs::ProofSpecs;
@@ -203,11 +202,7 @@
     /// Tendermint-specific light client verification.
     pub fn as_light_client_options(&self) -> Result<Options, TendermintClientError> {
         Ok(Options {
-            trust_threshold: self.trust_level.try_into().map_err(|e: ClientError| {
-                TendermintClientError::InvalidTrustThreshold {
-                    description: e.to_string(),
-                }
-            })?,
+            trust_threshold: self.trust_level.try_into()?,
             trusting_period: self.trusting_period,
             clock_drift: self.max_clock_drift,
         })
@@ -235,70 +230,64 @@
 impl Protobuf<RawTmClientState> for ClientState {}
 
 impl TryFrom<RawTmClientState> for ClientState {
-    type Error = TendermintClientError;
+    type Error = DecodingError;
 
     fn try_from(raw: RawTmClientState) -> Result<Self, Self::Error> {
         let chain_id = ChainId::from_str(raw.chain_id.as_str())?;
 
         let trust_level = {
-            let trust_level = raw.trust_level.ok_or(DecodingError::MissingRawData {
-                description: "trust level not set".to_string(),
-            })?;
-            trust_level
-                .try_into()
-                .map_err(|e| DecodingError::InvalidRawData {
-                    description: format!("failed to decoding trust threshold: {e}"),
-                })?
+            let trust_level = raw.trust_level.ok_or(DecodingError::missing_raw_data(
+                "tm client state trust level",
+            ))?;
+            trust_level.try_into()?
         };
 
         let trusting_period = raw
             .trusting_period
-            .ok_or(DecodingError::MissingRawData {
-                description: "trusting period not set".to_string(),
-            })?
+            .ok_or(DecodingError::missing_raw_data(
+                "tm client state trusting period",
+            ))?
             .try_into()
-            .map_err(|_| DecodingError::InvalidRawData {
-                description: "failed to decode trusting period".to_string(),
+            .map_err(|d| {
+                DecodingError::invalid_raw_data(format!("tm client state trusting period: {d:?}"))
             })?;
 
         let unbonding_period = raw
             .unbonding_period
-            .ok_or(DecodingError::MissingRawData {
-                description: "unbonding period not set".to_string(),
-            })?
+            .ok_or(DecodingError::missing_raw_data(
+                "tm client state unbonding period",
+            ))?
             .try_into()
-            .map_err(|_| DecodingError::InvalidRawData {
-                description: "failed to decode unbonding period".to_string(),
+            .map_err(|d| {
+                DecodingError::invalid_raw_data(format!("tm client state unbonding period: {d:?}"))
             })?;
 
         let max_clock_drift = raw
             .max_clock_drift
-            .ok_or(DecodingError::MissingRawData {
-                description: "max clock drift not set".to_string(),
-            })?
+            .ok_or(DecodingError::missing_raw_data(
+                "tm client state max clock drift",
+            ))?
             .try_into()
-            .map_err(|_| DecodingError::InvalidRawData {
-                description: "failed to decode max clock drift".to_string(),
+            .map_err(|d| {
+                DecodingError::invalid_raw_data(format!("tm client state max clock drift: {d:?}"))
             })?;
 
         let latest_height = raw
             .latest_height
-            .ok_or(DecodingError::MissingRawData {
-                description: "latest height not set".to_string(),
-            })?
-            .try_into()
-            .map_err(|e| DecodingError::InvalidRawData {
-                description: format!("failed to decode latest height: {e}"),
-            })?;
+            .ok_or(DecodingError::missing_raw_data(
+                "tm client state latest height",
+            ))?
+            .try_into()?;
+
+        let proof_specs = raw.proof_specs.try_into()?;
 
         // NOTE: In `RawClientState`, a `frozen_height` of `0` means "not
         // frozen". See:
         // https://github.com/cosmos/ibc-go/blob/8422d0c4c35ef970539466c5bdec1cd27369bab3/modules/light-clients/07-tendermint/types/client_state.go#L74
-        let frozen_height =
-            Height::try_from(raw.frozen_height.ok_or(DecodingError::MissingRawData {
-                description: "frozen height not set".to_string(),
-            })?)
-            .ok();
+        let frozen_height = Height::try_from(raw.frozen_height.ok_or(
+            DecodingError::missing_raw_data("tm client state frozen height"),
+        )?)
+        .ok();
 
         // We use set this deprecated field just so that we can properly convert
         // it back in its raw form
@@ -315,7 +304,7 @@
             unbonding_period,
             max_clock_drift,
             latest_height,
-            raw.proof_specs.try_into()?,
+            proof_specs,
             raw.upgrade_path,
             frozen_height,
             allow_update,
@@ -355,19 +344,16 @@
 impl Protobuf<Any> for ClientState {}
 
 impl TryFrom<Any> for ClientState {
-    type Error = ClientError;
+    type Error = DecodingError;
 
     fn try_from(raw: Any) -> Result<Self, Self::Error> {
         fn decode_client_state(value: &[u8]) -> Result<ClientState, DecodingError> {
-            let client_state =
-                Protobuf::<RawTmClientState>::decode(value).map_err(DecodingError::Protobuf)?;
+            let client_state = Protobuf::<RawTmClientState>::decode(value)?;
             Ok(client_state)
         }
 
         match raw.type_url.as_str() {
-            TENDERMINT_CLIENT_STATE_TYPE_URL => {
-                decode_client_state(&raw.value).map_err(ClientError::Decoding)
-            }
+            TENDERMINT_CLIENT_STATE_TYPE_URL => decode_client_state(&raw.value),
             _ => Err(DecodingError::MismatchedTypeUrls {
                 expected: TENDERMINT_CLIENT_STATE_TYPE_URL.to_string(),
                 actual: raw.type_url,