Skip to content

Commit 1cb4b64

Browse files
seanchen1991seanchen1991
authored
Clean up ibc-apps errors (#1318)
* Clean up ics20-transfer error * Clean up NftTransfer error * Incorporate PR feedback * Remove redundant TODO comment * Simplify NftTransferError variant * Add TODO reminder
1 parent dda4ea7 commit 1cb4b64

File tree

18 files changed

+171
-252
lines changed

18 files changed

+171
-252
lines changed

‎ibc-apps/ics20-transfer/src/handler/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub fn refund_packet_token_execute(
2020
.sender
2121
.clone()
2222
.try_into()
23-
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
23+
.map_err(|_| TokenTransferError::FailedToParseAccount)?;
2424

2525
if is_sender_chain_source(
2626
packet.port_id_on_a.clone(),
@@ -49,7 +49,7 @@ pub fn refund_packet_token_validate(
4949
.sender
5050
.clone()
5151
.try_into()
52-
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
52+
.map_err(|_| TokenTransferError::FailedToParseAccount)?;
5353

5454
if is_sender_chain_source(
5555
packet.port_id_on_a.clone(),

‎ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub fn process_recv_packet_execute<Ctx: TokenTransferExecutionContext>(
2626
let receiver_account = data.receiver.clone().try_into().map_err(|_| {
2727
(
2828
ModuleExtras::empty(),
29-
TokenTransferError::ParseAccountFailure,
29+
TokenTransferError::FailedToParseAccount,
3030
)
3131
})?;
3232

‎ibc-apps/ics20-transfer/src/handler/send_transfer.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ where
4545
let chan_id_on_b = chan_end_on_a
4646
.counterparty()
4747
.channel_id()
48-
.ok_or_else(|| TokenTransferError::DestinationChannelNotFound {
48+
.ok_or_else(|| TokenTransferError::MissingDestinationChannel {
4949
port_id: msg.port_id_on_a.clone(),
5050
channel_id: msg.chan_id_on_a.clone(),
5151
})?
@@ -61,7 +61,7 @@ where
6161
.sender
6262
.clone()
6363
.try_into()
64-
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
64+
.map_err(|_| TokenTransferError::FailedToParseAccount)?;
6565

6666
if is_sender_chain_source(
6767
msg.port_id_on_a.clone(),
@@ -117,7 +117,7 @@ where
117117
let chan_on_b = chan_end_on_a
118118
.counterparty()
119119
.channel_id()
120-
.ok_or_else(|| TokenTransferError::DestinationChannelNotFound {
120+
.ok_or_else(|| TokenTransferError::MissingDestinationChannel {
121121
port_id: msg.port_id_on_a.clone(),
122122
channel_id: msg.chan_id_on_a.clone(),
123123
})?
@@ -134,7 +134,7 @@ where
134134
.sender
135135
.clone()
136136
.try_into()
137-
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
137+
.map_err(|_| TokenTransferError::FailedToParseAccount)?;
138138

139139
if is_sender_chain_source(
140140
msg.port_id_on_a.clone(),

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,16 @@ pub fn on_chan_open_init_validate(
2727
version: &Version,
2828
) -> Result<(), TokenTransferError> {
2929
if order != Order::Unordered {
30-
return Err(TokenTransferError::ChannelNotUnordered {
31-
expect_order: Order::Unordered,
32-
got_order: order,
30+
return Err(TokenTransferError::MismatchedChannelOrders {
31+
expected: Order::Unordered,
32+
actual: order,
3333
});
3434
}
3535
let bound_port = ctx.get_port()?;
3636
if port_id != &bound_port {
37-
return Err(TokenTransferError::InvalidPort {
38-
port_id: port_id.clone(),
39-
exp_port_id: bound_port,
37+
return Err(TokenTransferError::MismatchedPortIds {
38+
actual: port_id.clone(),
39+
expected: bound_port,
4040
});
4141
}
4242

@@ -71,9 +71,9 @@ pub fn on_chan_open_try_validate(
7171
counterparty_version: &Version,
7272
) -> Result<(), TokenTransferError> {
7373
if order != Order::Unordered {
74-
return Err(TokenTransferError::ChannelNotUnordered {
75-
expect_order: Order::Unordered,
76-
got_order: order,
74+
return Err(TokenTransferError::MismatchedChannelOrders {
75+
expected: Order::Unordered,
76+
actual: order,
7777
});
7878
}
7979

@@ -139,15 +139,15 @@ pub fn on_chan_close_init_validate(
139139
_port_id: &PortId,
140140
_channel_id: &ChannelId,
141141
) -> Result<(), TokenTransferError> {
142-
Err(TokenTransferError::CantCloseChannel)
142+
Err(TokenTransferError::UnsupportedClosedChannel)
143143
}
144144

145145
pub fn on_chan_close_init_execute(
146146
_ctx: &mut impl TokenTransferExecutionContext,
147147
_port_id: &PortId,
148148
_channel_id: &ChannelId,
149149
) -> Result<ModuleExtras, TokenTransferError> {
150-
Err(TokenTransferError::CantCloseChannel)
150+
Err(TokenTransferError::UnsupportedClosedChannel)
151151
}
152152

153153
pub fn on_chan_close_confirm_validate(
@@ -172,7 +172,7 @@ pub fn on_recv_packet_execute(
172172
) -> (ModuleExtras, Acknowledgement) {
173173
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
174174
let ack =
175-
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into());
175+
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into());
176176
return (ModuleExtras::empty(), ack.into());
177177
};
178178

@@ -204,10 +204,10 @@ where
204204
Ctx: TokenTransferValidationContext,
205205
{
206206
let data = serde_json::from_slice::<PacketData>(&packet.data)
207-
.map_err(|_| TokenTransferError::PacketDataDeserialization)?;
207+
.map_err(|_| TokenTransferError::FailedToDeserializePacketData)?;
208208

209209
let acknowledgement = serde_json::from_slice::<AcknowledgementStatus>(acknowledgement.as_ref())
210-
.map_err(|_| TokenTransferError::AckDeserialization)?;
210+
.map_err(|_| TokenTransferError::FailedToDeserializeAck)?;
211211

212212
if !acknowledgement.is_successful() {
213213
refund_packet_token_validate(ctx, packet, &data)?;
@@ -225,7 +225,7 @@ pub fn on_acknowledgement_packet_execute(
225225
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
226226
return (
227227
ModuleExtras::empty(),
228-
Err(TokenTransferError::PacketDataDeserialization),
228+
Err(TokenTransferError::FailedToDeserializePacketData),
229229
);
230230
};
231231

@@ -234,7 +234,7 @@ pub fn on_acknowledgement_packet_execute(
234234
else {
235235
return (
236236
ModuleExtras::empty(),
237-
Err(TokenTransferError::AckDeserialization),
237+
Err(TokenTransferError::FailedToDeserializeAck),
238238
);
239239
};
240240

@@ -270,7 +270,7 @@ where
270270
Ctx: TokenTransferValidationContext,
271271
{
272272
let data = serde_json::from_slice::<PacketData>(&packet.data)
273-
.map_err(|_| TokenTransferError::PacketDataDeserialization)?;
273+
.map_err(|_| TokenTransferError::FailedToDeserializePacketData)?;
274274

275275
refund_packet_token_validate(ctx, packet, &data)?;
276276

@@ -285,7 +285,7 @@ pub fn on_timeout_packet_execute(
285285
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
286286
return (
287287
ModuleExtras::empty(),
288-
Err(TokenTransferError::PacketDataDeserialization),
288+
Err(TokenTransferError::FailedToDeserializePacketData),
289289
);
290290
};
291291

@@ -324,7 +324,7 @@ mod test {
324324
r#"{"result":"AQ=="}"#,
325325
);
326326
ser_json_assert_eq(
327-
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into()),
327+
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into()),
328328
r#"{"error":"failed to deserialize packet data"}"#,
329329
);
330330
}
@@ -342,7 +342,7 @@ mod test {
342342
#[test]
343343
fn test_ack_error_to_vec() {
344344
let ack_error: Vec<u8> =
345-
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into())
345+
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into())
346346
.into();
347347

348348
// Check that it's the same output as ibc-go
@@ -367,7 +367,7 @@ mod test {
367367
);
368368
de_json_assert_eq(
369369
r#"{"error":"failed to deserialize packet data"}"#,
370-
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into()),
370+
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into()),
371371
);
372372

373373
assert!(serde_json::from_str::<AcknowledgementStatus>(r#"{"success":"AQ=="}"#).is_err());

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,7 @@ where
7676
.chars()
7777
.all(|x| x.is_alphanumeric() || VALID_DENOM_CHARACTERS.contains(x))
7878
})
79-
.ok_or_else(|| TokenTransferError::InvalidCoin {
80-
coin: coin_str.to_string(),
81-
})?;
79+
.ok_or_else(|| TokenTransferError::InvalidCoin(coin_str.to_string()))?;
8280

8381
Ok(Coin {
8482
amount: amount.parse()?,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ impl FromStr for TracePath {
219219
remaining_parts
220220
.is_none()
221221
.then_some(trace_path)
222-
.ok_or_else(|| TokenTransferError::MalformedTrace(s.to_string()))
222+
.ok_or_else(|| TokenTransferError::InvalidTrace(s.to_string()))
223223
}
224224
}
225225

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

Lines changed: 29 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//! Defines the token transfer error type
22
use core::convert::Infallible;
3-
use core::str::Utf8Error;
43

54
use displaydoc::Display;
65
use ibc_core::channel::types::acknowledgement::StatusValue;
@@ -17,86 +16,48 @@ pub enum TokenTransferError {
1716
ContextError(ContextError),
1817
/// invalid identifier: `{0}`
1918
InvalidIdentifier(IdentifierError),
20-
/// insufficient funds: tried to send `{send_attempt}`, sender only has `{available_funds}`
21-
InsufficientFunds {
22-
send_attempt: String,
23-
available_funds: String,
24-
},
25-
/// destination channel not found in the counterparty of port_id `{port_id}` and channel_id `{channel_id}`
26-
DestinationChannelNotFound {
27-
port_id: PortId,
28-
channel_id: ChannelId,
29-
},
30-
/// base denomination is empty
31-
EmptyBaseDenom,
32-
/// invalid prot id n trace at position: `{pos}`, validation error: `{validation_error}`
33-
InvalidTracePortId {
34-
pos: u64,
35-
validation_error: IdentifierError,
36-
},
37-
/// invalid channel id in trace at position: `{pos}`, validation error: `{validation_error}`
38-
InvalidTraceChannelId {
39-
pos: u64,
40-
validation_error: IdentifierError,
41-
},
42-
/// malformed trace: `{0}`
43-
MalformedTrace(String),
44-
/// trace length must be even but got: `{len}`
45-
InvalidTraceLength { len: u64 },
19+
/// invalid trace: `{0}`
20+
InvalidTrace(String),
4621
/// invalid amount: `{0}`
4722
InvalidAmount(FromDecStrErr),
48-
/// invalid token
49-
InvalidToken,
50-
/// expected `{expect_order}` channel, got `{got_order}`
51-
ChannelNotUnordered {
52-
expect_order: Order,
53-
got_order: Order,
23+
/// invalid coin: `{0}`
24+
InvalidCoin(String),
25+
/// missing token
26+
MissingToken,
27+
/// missing destination channel `{channel_id}` on port `{port_id}`
28+
MissingDestinationChannel {
29+
port_id: PortId,
30+
channel_id: ChannelId,
5431
},
55-
/// channel cannot be closed
56-
CantCloseChannel,
32+
/// mismatched channel orders: expected `{expected}`, actual `{actual}`
33+
MismatchedChannelOrders { expected: Order, actual: Order },
34+
/// mismatched port IDs: expected `{expected}`, actual `{actual}`
35+
MismatchedPortIds { expected: PortId, actual: PortId },
5736
/// failed to deserialize packet data
58-
PacketDataDeserialization,
37+
FailedToDeserializePacketData,
5938
/// failed to deserialize acknowledgement
60-
AckDeserialization,
61-
/// receive is not enabled
62-
ReceiveDisabled { reason: String },
63-
/// send is not enabled
64-
SendDisabled { reason: String },
65-
/// failed to parse as AccountId
66-
ParseAccountFailure,
67-
/// invalid port: `{port_id}`, expected `{exp_port_id}`
68-
InvalidPort {
69-
port_id: PortId,
70-
exp_port_id: PortId,
71-
},
72-
/// decoding raw msg error: `{reason}`
73-
DecodeRawMsg { reason: String },
74-
/// unknown msg type: `{msg_type}`
75-
UnknownMsgType { msg_type: String },
76-
/// invalid coin string: `{coin}`
77-
InvalidCoin { coin: String },
78-
/// decoding raw bytes as UTF-8 string error: `{0}`
79-
Utf8Decode(Utf8Error),
80-
/// other error: `{0}`
81-
Other(String),
39+
FailedToDeserializeAck,
40+
// TODO(seanchen1991): Used in basecoin; this variant should be moved
41+
// to a host-relevant error
42+
/// failed to parse account ID
43+
FailedToParseAccount,
44+
/// failed to decode raw msg: `{description}`
45+
FailedToDecodeRawMsg { description: String },
46+
/// channel cannot be closed
47+
UnsupportedClosedChannel,
48+
/// empty base denomination
49+
EmptyBaseDenom,
50+
/// unknown msg type: `{0}`
51+
UnknownMsgType(String),
8252
}
8353

8454
#[cfg(feature = "std")]
8555
impl std::error::Error for TokenTransferError {
8656
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
8757
match &self {
8858
Self::ContextError(e) => Some(e),
89-
Self::InvalidIdentifier(e)
90-
| Self::InvalidTracePortId {
91-
validation_error: e,
92-
..
93-
}
94-
| Self::InvalidTraceChannelId {
95-
validation_error: e,
96-
..
97-
} => Some(e),
59+
Self::InvalidIdentifier(e) => Some(e),
9860
Self::InvalidAmount(e) => Some(e),
99-
Self::Utf8Decode(e) => Some(e),
10061
_ => None,
10162
}
10263
}

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ impl TryFrom<RawMsgTransfer> for MsgTransfer {
6969
packet_data: PacketData {
7070
token: raw_msg
7171
.token
72-
.ok_or(TokenTransferError::InvalidToken)?
72+
.ok_or(TokenTransferError::MissingToken)?
7373
.try_into()
74-
.map_err(|_| TokenTransferError::InvalidToken)?,
74+
.map_err(|_| TokenTransferError::MissingToken)?,
7575
sender: raw_msg.sender.into(),
7676
receiver: raw_msg.receiver.into(),
7777
memo: raw_msg.memo.into(),
@@ -104,14 +104,12 @@ impl TryFrom<Any> for MsgTransfer {
104104

105105
fn try_from(raw: Any) -> Result<Self, Self::Error> {
106106
match raw.type_url.as_str() {
107-
TYPE_URL => {
108-
MsgTransfer::decode_vec(&raw.value).map_err(|e| TokenTransferError::DecodeRawMsg {
109-
reason: e.to_string(),
110-
})
111-
}
112-
_ => Err(TokenTransferError::UnknownMsgType {
113-
msg_type: raw.type_url,
107+
TYPE_URL => MsgTransfer::decode_vec(&raw.value).map_err(|e| {
108+
TokenTransferError::FailedToDecodeRawMsg {
109+
description: e.to_string(),
110+
}
114111
}),
112+
_ => Err(TokenTransferError::UnknownMsgType(raw.type_url)),
115113
}
116114
}
117115
}

‎ibc-apps/ics721-nft-transfer/src/handler/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pub fn refund_packet_nft_execute(
2121
.sender
2222
.clone()
2323
.try_into()
24-
.map_err(|_| NftTransferError::ParseAccountFailure)?;
24+
.map_err(|_| NftTransferError::FailedToParseAccount)?;
2525

2626
if is_sender_chain_source(
2727
packet.port_id_on_a.clone(),
@@ -58,7 +58,7 @@ pub fn refund_packet_nft_validate(
5858
.sender
5959
.clone()
6060
.try_into()
61-
.map_err(|_| NftTransferError::ParseAccountFailure)?;
61+
.map_err(|_| NftTransferError::FailedToParseAccount)?;
6262

6363
if is_sender_chain_source(
6464
packet.port_id_on_a.clone(),

0 commit comments

Comments
 (0)