Skip to content

Commit 5bdd377

Browse files
seanchen1991Farhad-Shabani
seanchen1991
and
Farhad-Shabani
authored
Clean up generic String variants in error types (#1347)
* Clean up most instances of ClientError::Other * Rename CommitmentError variants * Clean up some DecodingError calls * Incorporate some PR feedback * Incorporate more PR feedback * A bunch of DecodingError associated type changes * Eliminate a bunch of redundant map_err calls * cargo fmt * Fix ics23 spec tests * Fix a typo * Clean up some error messages * Consolidate MismatchedTypeUrl and MismatchedEventKind variants * Miscellaneous clean up * cargo fmt * Remove unused CommitmentError variants * Clean up errors to match ADR description * More clean up * Add changelog entry * fix: remove now unused HeightError * Clean up TimestampError --------- Co-authored-by: Farhad Shabani <[email protected]>
1 parent 4aecaec commit 5bdd377

File tree

89 files changed

+788
-1037
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+788
-1037
lines changed

‎.changelog/unreleased/breaking-changes/1346-remove-generic-string-error-variants.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- [ibc-core] Remove `ClientError::Other` variant
2+
([\#1346](https://github.com/cosmos/ibc-rs/issues/1346))

‎ibc-apps/ics20-transfer/src/module.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,15 @@ pub fn on_chan_close_init_validate(
133133
_port_id: &PortId,
134134
_channel_id: &ChannelId,
135135
) -> Result<(), TokenTransferError> {
136-
Err(TokenTransferError::UnsupportedClosedChannel)
136+
Err(TokenTransferError::InvalidClosedChannel)
137137
}
138138

139139
pub fn on_chan_close_init_execute(
140140
_ctx: &mut impl TokenTransferExecutionContext,
141141
_port_id: &PortId,
142142
_channel_id: &ChannelId,
143143
) -> Result<ModuleExtras, TokenTransferError> {
144-
Err(TokenTransferError::UnsupportedClosedChannel)
144+
Err(TokenTransferError::InvalidClosedChannel)
145145
}
146146

147147
pub fn on_chan_close_confirm_validate(

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,7 @@ impl FromStr for Amount {
106106

107107
fn from_str(s: &str) -> Result<Self, Self::Err> {
108108
let amount = U256::from_dec_str(s).map_err(|e| {
109-
DecodingError::invalid_raw_data(format!(
110-
"invalid amount that could not be parsed as a U256: {e}"
111-
))
109+
DecodingError::invalid_raw_data(format!("amount could not be parsed as a U256: {e}"))
112110
})?;
113111

114112
Ok(Self(amount))

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ where
7777
.all(|x| x.is_alphanumeric() || VALID_DENOM_CHARACTERS.contains(x))
7878
})
7979
.ok_or(DecodingError::invalid_raw_data(format!(
80-
"invalid coin: {coin_str}"
80+
"coin str: {coin_str}"
8181
)))?;
8282

8383
Ok(Coin {

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,7 @@ impl FromStr for TracePath {
218218
remaining_parts
219219
.is_none()
220220
.then_some(trace_path)
221-
.ok_or(DecodingError::invalid_raw_data(format!(
222-
"invalid trace path: {s}"
223-
)))
221+
.ok_or(DecodingError::invalid_raw_data(format!("trace path: {s}")))
224222
}
225223
}
226224

‎ibc-apps/ics20-transfer/types/src/error.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ use ibc_core::primitives::prelude::*;
99

1010
#[derive(Display, Debug, derive_more::From)]
1111
pub enum TokenTransferError {
12-
/// host error: `{0}`
12+
/// host error: {0}
1313
Host(HostError),
14-
/// decoding error: `{0}`
14+
/// decoding error: {0}
1515
Decoding(DecodingError),
16-
/// channel error: `{0}`
16+
/// channel error: {0}
1717
Channel(ChannelError),
1818
/// missing destination channel `{channel_id}` on port `{port_id}`
1919
MissingDestinationChannel {
@@ -24,8 +24,8 @@ pub enum TokenTransferError {
2424
MismatchedChannelOrders { expected: Order, actual: Order },
2525
/// mismatched port IDs: expected `{expected}`, actual `{actual}`
2626
MismatchedPortIds { expected: PortId, actual: PortId },
27-
/// channel cannot be closed
28-
UnsupportedClosedChannel,
27+
/// invalid channel state: cannot be closed
28+
InvalidClosedChannel,
2929
/// failed to deserialize packet data
3030
FailedToDeserializePacketData,
3131
/// failed to deserialize acknowledgement

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl TryFrom<RawMsgTransfer> for MsgTransfer {
5555
// Packet timeout height and packet timeout timestamp cannot both be unset.
5656
if !timeout_height_on_b.is_set() && !timeout_timestamp_on_b.is_set() {
5757
return Err(DecodingError::missing_raw_data(
58-
"missing timeout height or timeout timestamp",
58+
"msg transfer timeout height or timeout timestamp",
5959
));
6060
}
6161

@@ -65,7 +65,7 @@ impl TryFrom<RawMsgTransfer> for MsgTransfer {
6565
packet_data: PacketData {
6666
token: raw_msg
6767
.token
68-
.ok_or(DecodingError::missing_raw_data("missing token"))?
68+
.ok_or(DecodingError::missing_raw_data("msg transfer token"))?
6969
.try_into()?,
7070
sender: raw_msg.sender.into(),
7171
receiver: raw_msg.receiver.into(),
@@ -100,7 +100,7 @@ impl TryFrom<Any> for MsgTransfer {
100100
fn try_from(raw: Any) -> Result<Self, Self::Error> {
101101
match raw.type_url.as_str() {
102102
TYPE_URL => Ok(MsgTransfer::decode_vec(&raw.value)?),
103-
_ => Err(DecodingError::MismatchedTypeUrls {
103+
_ => Err(DecodingError::MismatchedResourceName {
104104
expected: TYPE_URL.to_string(),
105105
actual: raw.type_url,
106106
})?,

‎ibc-apps/ics721-nft-transfer/src/module.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,15 +132,15 @@ pub fn on_chan_close_init_validate(
132132
_port_id: &PortId,
133133
_channel_id: &ChannelId,
134134
) -> Result<(), NftTransferError> {
135-
Err(NftTransferError::UnsupportedClosedChannel)
135+
Err(NftTransferError::InvalidClosedChannel)
136136
}
137137

138138
pub fn on_chan_close_init_execute(
139139
_ctx: &mut impl NftTransferExecutionContext,
140140
_port_id: &PortId,
141141
_channel_id: &ChannelId,
142142
) -> Result<ModuleExtras, NftTransferError> {
143-
Err(NftTransferError::UnsupportedClosedChannel)
143+
Err(NftTransferError::InvalidClosedChannel)
144144
}
145145

146146
pub fn on_chan_close_confirm_validate(

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,7 @@ impl FromStr for ClassUri {
247247
fn from_str(class_uri: &str) -> Result<Self, Self::Err> {
248248
match Uri::from_str(class_uri) {
249249
Ok(uri) => Ok(Self(uri)),
250-
Err(err) => Err(DecodingError::invalid_raw_data(format!(
251-
"invalid class URI: {err}"
252-
))),
250+
Err(err) => Err(DecodingError::invalid_raw_data(format!("class URI: {err}"))),
253251
}
254252
}
255253
}

‎ibc-apps/ics721-nft-transfer/types/src/error.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,19 @@ use ibc_core::primitives::prelude::*;
1010

1111
#[derive(Display, Debug, From)]
1212
pub enum NftTransferError {
13-
/// host error: `{0}`
13+
/// host error: {0}
1414
Host(HostError),
15-
/// channel error: `{0}`
15+
/// channel error: {0}
1616
Channel(ChannelError),
17-
/// decoding error: `{0}`
17+
/// decoding error: {0}
1818
Decoding(DecodingError),
1919
/// missing destination channel `{channel_id}` on port `{port_id}`
2020
MissingDestinationChannel {
2121
port_id: PortId,
2222
channel_id: ChannelId,
2323
},
24-
/// empty token ID
25-
EmptyTokenId,
24+
/// missing token ID
25+
MissingTokenId,
2626
/// mismatched number of token IDs: expected `{expected}`, actual `{actual}`
2727
MismatchedNumberOfTokenIds { expected: usize, actual: usize },
2828
/// mismatched channel orders: expected `{expected}`, actual `{actual}`
@@ -35,8 +35,8 @@ pub enum NftTransferError {
3535
FailedToDeserializeAck,
3636
/// failed to parse account ID
3737
FailedToParseAccount,
38-
/// channel cannot be closed
39-
UnsupportedClosedChannel,
38+
/// invalid channel state: cannot be closed
39+
InvalidClosedChannel,
4040
}
4141

4242
#[cfg(feature = "std")]

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use ibc_proto::google::protobuf::Any;
88
use ibc_proto::ibc::applications::nft_transfer::v1::MsgTransfer as RawMsgTransfer;
99
use ibc_proto::Protobuf;
1010

11-
use crate::error::NftTransferError;
1211
use crate::packet::PacketData;
1312

1413
pub(crate) const TYPE_URL: &str = "/ibc.applications.nft_transfer.v1.MsgTransfer";
@@ -115,12 +114,12 @@ impl From<MsgTransfer> for RawMsgTransfer {
115114
impl Protobuf<RawMsgTransfer> for MsgTransfer {}
116115

117116
impl TryFrom<Any> for MsgTransfer {
118-
type Error = NftTransferError;
117+
type Error = DecodingError;
119118

120119
fn try_from(raw: Any) -> Result<Self, Self::Error> {
121120
match raw.type_url.as_str() {
122-
TYPE_URL => Ok(MsgTransfer::decode_vec(&raw.value).map_err(DecodingError::Protobuf)?),
123-
_ => Err(DecodingError::MismatchedTypeUrls {
121+
TYPE_URL => Ok(MsgTransfer::decode_vec(&raw.value)?),
122+
_ => Err(DecodingError::MismatchedResourceName {
124123
expected: TYPE_URL.to_string(),
125124
actual: raw.type_url,
126125
})?,

‎ibc-apps/ics721-nft-transfer/types/src/packet.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ impl PacketData {
9292
/// Performs the basic validation of the packet data fields.
9393
pub fn validate_basic(&self) -> Result<(), NftTransferError> {
9494
if self.token_ids.0.is_empty() {
95-
return Err(NftTransferError::EmptyTokenId);
95+
return Err(NftTransferError::MissingTokenId);
9696
}
9797
let num = self.token_ids.0.len();
9898
let num_uri = self

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@
88
//! Rust). As such, this module also includes some trait implementations that
99
//! serve to pass through traits implemented on the wrapped `ClientState` type.
1010
11-
use ibc_client_tendermint_types::error::TendermintClientError;
1211
use ibc_client_tendermint_types::proto::v1::ClientState as RawTmClientState;
1312
use ibc_client_tendermint_types::ClientState as ClientStateType;
14-
use ibc_core_client::types::error::ClientError;
13+
use ibc_core_host::types::error::DecodingError;
1514
use ibc_primitives::prelude::*;
1615
use ibc_primitives::proto::{Any, Protobuf};
1716

@@ -44,7 +43,7 @@ impl ClientState {
4443
impl Protobuf<RawTmClientState> for ClientState {}
4544

4645
impl TryFrom<RawTmClientState> for ClientState {
47-
type Error = TendermintClientError;
46+
type Error = DecodingError;
4847

4948
fn try_from(raw: RawTmClientState) -> Result<Self, Self::Error> {
5049
Ok(Self(ClientStateType::try_from(raw)?))
@@ -60,7 +59,7 @@ impl From<ClientState> for RawTmClientState {
6059
impl Protobuf<Any> for ClientState {}
6160

6261
impl TryFrom<Any> for ClientState {
63-
type Error = ClientError;
62+
type Error = DecodingError;
6463

6564
fn try_from(raw: Any) -> Result<Self, Self::Error> {
6665
Ok(Self(ClientStateType::try_from(raw)?))

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,11 @@ pub fn verify_consensus_state(
159159
let tm_consensus_state = TmConsensusState::try_from(consensus_state)?;
160160

161161
if tm_consensus_state.root().is_empty() {
162-
Err(CommitmentError::EmptyCommitmentRoot)?;
162+
Err(CommitmentError::MissingCommitmentRoot)?;
163163
};
164164

165165
if consensus_state_status(&tm_consensus_state, host_timestamp, trusting_period)?.is_expired() {
166-
return Err(ClientError::UnexpectedStatus(Status::Expired));
166+
return Err(ClientError::InvalidStatus(Status::Expired));
167167
}
168168

169169
Ok(())
@@ -206,7 +206,7 @@ pub fn validate_proof_height(
206206
let latest_height = client_state.latest_height;
207207

208208
if latest_height < proof_height {
209-
return Err(ClientError::InvalidProofHeight {
209+
return Err(ClientError::InsufficientProofHeight {
210210
actual: latest_height,
211211
expected: proof_height,
212212
});
@@ -294,7 +294,7 @@ pub fn verify_membership<H: HostFunctionsProvider>(
294294
value: Vec<u8>,
295295
) -> Result<(), ClientError> {
296296
if prefix.is_empty() {
297-
Err(CommitmentError::EmptyCommitmentPrefix)?;
297+
Err(CommitmentError::MissingCommitmentPrefix)?;
298298
}
299299

300300
let merkle_path = MerklePath::new(vec![prefix.as_bytes().to_vec().into(), path]);

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use ibc_core_host::types::identifiers::ClientId;
88
use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath};
99
use ibc_primitives::prelude::*;
1010
use ibc_primitives::proto::Any;
11+
use ibc_primitives::TimestampError;
1112

1213
use super::ClientState;
1314

@@ -341,11 +342,7 @@ where
341342
let tm_consensus_state_timestamp = tm_consensus_state.timestamp();
342343
let tm_consensus_state_expiry = (tm_consensus_state_timestamp
343344
+ client_state.trusting_period)
344-
.map_err(|_| ClientError::Other {
345-
description: String::from(
346-
"Timestamp overflow error occurred while attempting to parse TmConsensusState",
347-
),
348-
})?;
345+
.map_err(|_| TimestampError::OverflowedTimestamp)?;
349346

350347
if tm_consensus_state_expiry > host_timestamp {
351348
break;

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use ibc_client_tendermint_types::{
44
};
55
use ibc_core_client::context::{Convertible, ExtClientValidationContext};
66
use ibc_core_client::types::error::ClientError;
7+
use ibc_core_host::types::error::IdentifierError;
78
use ibc_core_host::types::identifiers::{ChainId, ClientId};
89
use ibc_core_host::types::path::ClientConsensusStatePath;
910
use ibc_primitives::prelude::*;
@@ -116,12 +117,13 @@ where
116117
// main header verification, delegated to the tendermint-light-client crate.
117118
let untrusted_state = header.as_untrusted_block_state();
118119

119-
let tm_chain_id = &chain_id
120-
.as_str()
121-
.try_into()
122-
.map_err(|e| ClientError::Other {
123-
description: format!("failed to parse chain id: {e}"),
124-
})?;
120+
let tm_chain_id =
121+
&chain_id
122+
.as_str()
123+
.try_into()
124+
.map_err(|e| IdentifierError::FailedToParse {
125+
description: format!("chain ID `{chain_id}`: {e:?}"),
126+
})?;
125127

126128
let trusted_state =
127129
header.as_trusted_block_state(tm_chain_id, trusted_time, trusted_next_validator_hash)?;

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use ibc_client_tendermint_types::{ConsensusState as ConsensusStateType, Header a
33
use ibc_core_client::context::{Convertible, ExtClientValidationContext};
44
use ibc_core_client::types::error::ClientError;
55
use ibc_core_client::types::Height;
6+
use ibc_core_host::types::error::IdentifierError;
67
use ibc_core_host::types::identifiers::{ChainId, ClientId};
78
use ibc_core_host::types::path::ClientConsensusStatePath;
89
use ibc_primitives::prelude::*;
@@ -52,18 +53,17 @@ where
5253
)?;
5354

5455
TrustedBlockState {
55-
chain_id: &chain_id
56-
.as_str()
57-
.try_into()
58-
.map_err(|e| ClientError::Other {
59-
description: format!("failed to parse chain id: {}", e),
60-
})?,
56+
chain_id: &chain_id.as_str().try_into().map_err(|e| {
57+
IdentifierError::FailedToParse {
58+
description: format!("chain ID `{chain_id}`: {e:?}"),
59+
}
60+
})?,
6161
header_time: trusted_consensus_state.timestamp(),
6262
height: header
6363
.trusted_height
6464
.revision_height()
6565
.try_into()
66-
.map_err(|_| ClientError::FailedHeaderVerification {
66+
.map_err(|_| ClientError::FailedToVerifyHeader {
6767
description: TendermintClientError::InvalidHeaderHeight(
6868
header.trusted_height.revision_height(),
6969
)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,5 +278,5 @@ where
278278
&& subject_proof_specs == &substitute_proof_specs
279279
&& subject_upgrade_path == &substitute_upgrade_path)
280280
.then_some(())
281-
.ok_or(ClientError::MismatchedClientRecoveryStates)
281+
.ok_or(ClientError::FailedToVerifyClientRecoveryStates)
282282
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@
66
//! implementations that serve to pass through traits implemented on the wrapped
77
//! `ConsensusState` type.
88
9-
use ibc_client_tendermint_types::error::TendermintClientError;
109
use ibc_client_tendermint_types::proto::v1::ConsensusState as RawTmConsensusState;
1110
use ibc_client_tendermint_types::ConsensusState as ConsensusStateType;
1211
use ibc_core_client::context::consensus_state::ConsensusState as ConsensusStateTrait;
13-
use ibc_core_client::types::error::ClientError;
1412
use ibc_core_commitment_types::commitment::CommitmentRoot;
13+
use ibc_core_host::types::error::DecodingError;
1514
use ibc_primitives::prelude::*;
1615
use ibc_primitives::proto::{Any, Protobuf};
1716
use ibc_primitives::Timestamp;
@@ -52,7 +51,7 @@ impl From<ConsensusState> for ConsensusStateType {
5251
impl Protobuf<RawTmConsensusState> for ConsensusState {}
5352

5453
impl TryFrom<RawTmConsensusState> for ConsensusState {
55-
type Error = TendermintClientError;
54+
type Error = DecodingError;
5655

5756
fn try_from(raw: RawTmConsensusState) -> Result<Self, Self::Error> {
5857
Ok(Self(ConsensusStateType::try_from(raw)?))
@@ -68,7 +67,7 @@ impl From<ConsensusState> for RawTmConsensusState {
6867
impl Protobuf<Any> for ConsensusState {}
6968

7069
impl TryFrom<Any> for ConsensusState {
71-
type Error = ClientError;
70+
type Error = DecodingError;
7271

7372
fn try_from(raw: Any) -> Result<Self, Self::Error> {
7473
Ok(Self(ConsensusStateType::try_from(raw)?))

0 commit comments

Comments
 (0)