Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up ibc-apps errors #1318

Merged
merged 6 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ibc-apps/ics20-transfer/src/handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
.sender
.clone()
.try_into()
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
.map_err(|_| TokenTransferError::FailedToParseAccount)?;

Check warning on line 23 in ibc-apps/ics20-transfer/src/handler/mod.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/handler/mod.rs#L23

Added line #L23 was not covered by tests

if is_sender_chain_source(
packet.port_id_on_a.clone(),
Expand Down Expand Up @@ -49,7 +49,7 @@
.sender
.clone()
.try_into()
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
.map_err(|_| TokenTransferError::FailedToParseAccount)?;

Check warning on line 52 in ibc-apps/ics20-transfer/src/handler/mod.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/handler/mod.rs#L52

Added line #L52 was not covered by tests

if is_sender_chain_source(
packet.port_id_on_a.clone(),
Expand Down
2 changes: 1 addition & 1 deletion ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
let receiver_account = data.receiver.clone().try_into().map_err(|_| {
(
ModuleExtras::empty(),
TokenTransferError::ParseAccountFailure,
TokenTransferError::FailedToParseAccount,
)
})?;

Check warning on line 32 in ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs#L29-L32

Added lines #L29 - L32 were not covered by tests
let extras = if is_receiver_chain_source(
packet.port_id_on_a.clone(),
packet.chan_id_on_a.clone(),
Expand Down
8 changes: 4 additions & 4 deletions ibc-apps/ics20-transfer/src/handler/send_transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ where
let chan_id_on_b = chan_end_on_a
.counterparty()
.channel_id()
.ok_or_else(|| TokenTransferError::DestinationChannelNotFound {
.ok_or_else(|| TokenTransferError::MissingDestinationChannel {
port_id: msg.port_id_on_a.clone(),
channel_id: msg.chan_id_on_a.clone(),
})?
Expand All @@ -61,7 +61,7 @@ where
.sender
.clone()
.try_into()
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
.map_err(|_| TokenTransferError::FailedToParseAccount)?;

if is_sender_chain_source(
msg.port_id_on_a.clone(),
Expand Down Expand Up @@ -117,7 +117,7 @@ where
let chan_on_b = chan_end_on_a
.counterparty()
.channel_id()
.ok_or_else(|| TokenTransferError::DestinationChannelNotFound {
.ok_or_else(|| TokenTransferError::MissingDestinationChannel {
port_id: msg.port_id_on_a.clone(),
channel_id: msg.chan_id_on_a.clone(),
})?
Expand All @@ -134,7 +134,7 @@ where
.sender
.clone()
.try_into()
.map_err(|_| TokenTransferError::ParseAccountFailure)?;
.map_err(|_| TokenTransferError::FailedToParseAccount)?;

if is_sender_chain_source(
msg.port_id_on_a.clone(),
Expand Down
42 changes: 21 additions & 21 deletions ibc-apps/ics20-transfer/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@
version: &Version,
) -> Result<(), TokenTransferError> {
if order != Order::Unordered {
return Err(TokenTransferError::ChannelNotUnordered {
expect_order: Order::Unordered,
got_order: order,
return Err(TokenTransferError::MismatchedChannelOrders {
expected: Order::Unordered,
actual: order,
});
}
let bound_port = ctx.get_port()?;
if port_id != &bound_port {
return Err(TokenTransferError::InvalidPort {
port_id: port_id.clone(),
exp_port_id: bound_port,
return Err(TokenTransferError::MismatchedPortIds {
actual: port_id.clone(),
expected: bound_port,

Check warning on line 39 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L37-L39

Added lines #L37 - L39 were not covered by tests
});
}

Expand Down Expand Up @@ -71,9 +71,9 @@
counterparty_version: &Version,
) -> Result<(), TokenTransferError> {
if order != Order::Unordered {
return Err(TokenTransferError::ChannelNotUnordered {
expect_order: Order::Unordered,
got_order: order,
return Err(TokenTransferError::MismatchedChannelOrders {
expected: Order::Unordered,
actual: order,

Check warning on line 76 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L74-L76

Added lines #L74 - L76 were not covered by tests
});
}

Expand Down Expand Up @@ -139,15 +139,15 @@
_port_id: &PortId,
_channel_id: &ChannelId,
) -> Result<(), TokenTransferError> {
Err(TokenTransferError::CantCloseChannel)
Err(TokenTransferError::UnsupportedClosedChannel)

Check warning on line 142 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L142

Added line #L142 was not covered by tests
}

pub fn on_chan_close_init_execute(
_ctx: &mut impl TokenTransferExecutionContext,
_port_id: &PortId,
_channel_id: &ChannelId,
) -> Result<ModuleExtras, TokenTransferError> {
Err(TokenTransferError::CantCloseChannel)
Err(TokenTransferError::UnsupportedClosedChannel)

Check warning on line 150 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L150

Added line #L150 was not covered by tests
}

pub fn on_chan_close_confirm_validate(
Expand All @@ -172,7 +172,7 @@
) -> (ModuleExtras, Acknowledgement) {
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
let ack =
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into());
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into());

Check warning on line 175 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L175

Added line #L175 was not covered by tests
return (ModuleExtras::empty(), ack.into());
};

Expand Down Expand Up @@ -204,10 +204,10 @@
Ctx: TokenTransferValidationContext,
{
let data = serde_json::from_slice::<PacketData>(&packet.data)
.map_err(|_| TokenTransferError::PacketDataDeserialization)?;
.map_err(|_| TokenTransferError::FailedToDeserializePacketData)?;

Check warning on line 207 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L207

Added line #L207 was not covered by tests

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

Check warning on line 210 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L210

Added line #L210 was not covered by tests

if !acknowledgement.is_successful() {
refund_packet_token_validate(ctx, packet, &data)?;
Expand All @@ -225,7 +225,7 @@
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
return (
ModuleExtras::empty(),
Err(TokenTransferError::PacketDataDeserialization),
Err(TokenTransferError::FailedToDeserializePacketData),

Check warning on line 228 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L228

Added line #L228 was not covered by tests
);
};

Expand All @@ -234,7 +234,7 @@
else {
return (
ModuleExtras::empty(),
Err(TokenTransferError::AckDeserialization),
Err(TokenTransferError::FailedToDeserializeAck),

Check warning on line 237 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L237

Added line #L237 was not covered by tests
);
};

Expand Down Expand Up @@ -270,7 +270,7 @@
Ctx: TokenTransferValidationContext,
{
let data = serde_json::from_slice::<PacketData>(&packet.data)
.map_err(|_| TokenTransferError::PacketDataDeserialization)?;
.map_err(|_| TokenTransferError::FailedToDeserializePacketData)?;

Check warning on line 273 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L273

Added line #L273 was not covered by tests

refund_packet_token_validate(ctx, packet, &data)?;

Expand All @@ -285,7 +285,7 @@
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
return (
ModuleExtras::empty(),
Err(TokenTransferError::PacketDataDeserialization),
Err(TokenTransferError::FailedToDeserializePacketData),

Check warning on line 288 in ibc-apps/ics20-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/src/module.rs#L288

Added line #L288 was not covered by tests
);
};

Expand Down Expand Up @@ -324,7 +324,7 @@
r#"{"result":"AQ=="}"#,
);
ser_json_assert_eq(
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into()),
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into()),
r#"{"error":"failed to deserialize packet data"}"#,
);
}
Expand All @@ -342,7 +342,7 @@
#[test]
fn test_ack_error_to_vec() {
let ack_error: Vec<u8> =
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into())
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into())
.into();

// Check that it's the same output as ibc-go
Expand All @@ -367,7 +367,7 @@
);
de_json_assert_eq(
r#"{"error":"failed to deserialize packet data"}"#,
AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into()),
AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into()),
);

assert!(serde_json::from_str::<AcknowledgementStatus>(r#"{"success":"AQ=="}"#).is_err());
Expand Down
4 changes: 1 addition & 3 deletions ibc-apps/ics20-transfer/types/src/coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ where
.chars()
.all(|x| x.is_alphanumeric() || VALID_DENOM_CHARACTERS.contains(x))
})
.ok_or_else(|| TokenTransferError::InvalidCoin {
coin: coin_str.to_string(),
})?;
.ok_or_else(|| TokenTransferError::InvalidCoin(coin_str.to_string()))?;

Ok(Coin {
amount: amount.parse()?,
Expand Down
2 changes: 1 addition & 1 deletion ibc-apps/ics20-transfer/types/src/denom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl FromStr for TracePath {
remaining_parts
.is_none()
.then_some(trace_path)
.ok_or_else(|| TokenTransferError::MalformedTrace(s.to_string()))
.ok_or_else(|| TokenTransferError::InvalidTrace(s.to_string()))
}
}

Expand Down
95 changes: 27 additions & 68 deletions ibc-apps/ics20-transfer/types/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Defines the token transfer error type
use core::convert::Infallible;
use core::str::Utf8Error;

use displaydoc::Display;
use ibc_core::channel::types::acknowledgement::StatusValue;
Expand All @@ -17,86 +16,46 @@
ContextError(ContextError),
Farhad-Shabani marked this conversation as resolved.
Show resolved Hide resolved
/// invalid identifier: `{0}`
InvalidIdentifier(IdentifierError),
Farhad-Shabani marked this conversation as resolved.
Show resolved Hide resolved
/// insufficient funds: tried to send `{send_attempt}`, sender only has `{available_funds}`
InsufficientFunds {
send_attempt: String,
available_funds: String,
},
/// destination channel not found in the counterparty of port_id `{port_id}` and channel_id `{channel_id}`
DestinationChannelNotFound {
port_id: PortId,
channel_id: ChannelId,
},
/// base denomination is empty
EmptyBaseDenom,
/// invalid prot id n trace at position: `{pos}`, validation error: `{validation_error}`
InvalidTracePortId {
pos: u64,
validation_error: IdentifierError,
},
/// invalid channel id in trace at position: `{pos}`, validation error: `{validation_error}`
InvalidTraceChannelId {
pos: u64,
validation_error: IdentifierError,
},
/// malformed trace: `{0}`
MalformedTrace(String),
/// trace length must be even but got: `{len}`
InvalidTraceLength { len: u64 },
/// invalid trace: `{0}`
InvalidTrace(String),
/// invalid amount: `{0}`
InvalidAmount(FromDecStrErr),
/// invalid token
InvalidToken,
/// expected `{expect_order}` channel, got `{got_order}`
ChannelNotUnordered {
expect_order: Order,
got_order: Order,
/// invalid coin: `{0}`
InvalidCoin(String),
/// missing token
MissingToken,
/// missing destination channel `{channel_id}` on port `{port_id}`
MissingDestinationChannel {
port_id: PortId,
channel_id: ChannelId,
},
/// channel cannot be closed
CantCloseChannel,
/// mismatched channel orders: expected `{expected}`, actual `{actual}`
MismatchedChannelOrders { expected: Order, actual: Order },
/// mismatched port IDs: expected `{expected}`, actual `{actual}`
MismatchedPortIds { expected: PortId, actual: PortId },
/// failed to deserialize packet data
PacketDataDeserialization,
FailedToDeserializePacketData,
/// failed to deserialize acknowledgement
AckDeserialization,
/// receive is not enabled
ReceiveDisabled { reason: String },
/// send is not enabled
SendDisabled { reason: String },
/// failed to parse as AccountId
ParseAccountFailure,
/// invalid port: `{port_id}`, expected `{exp_port_id}`
InvalidPort {
port_id: PortId,
exp_port_id: PortId,
},
/// decoding raw msg error: `{reason}`
DecodeRawMsg { reason: String },
/// unknown msg type: `{msg_type}`
UnknownMsgType { msg_type: String },
/// invalid coin string: `{coin}`
InvalidCoin { coin: String },
/// decoding raw bytes as UTF-8 string error: `{0}`
Utf8Decode(Utf8Error),
/// other error: `{0}`
Other(String),
FailedToDeserializeAck,
/// failed to parse account ID
FailedToParseAccount,
/// failed to decode raw msg: `{description}`
FailedToDecodeRawMsg { description: String },
/// channel cannot be closed
UnsupportedClosedChannel,
/// empty base denomination
EmptyBaseDenom,
/// unknown msg type: `{0}`
UnknownMsgType(String),
}

#[cfg(feature = "std")]
impl std::error::Error for TokenTransferError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match &self {
Self::ContextError(e) => Some(e),
Self::InvalidIdentifier(e)
| Self::InvalidTracePortId {
validation_error: e,
..
}
| Self::InvalidTraceChannelId {
validation_error: e,
..
} => Some(e),
Self::InvalidIdentifier(e) => Some(e),

Check warning on line 57 in ibc-apps/ics20-transfer/types/src/error.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/types/src/error.rs#L57

Added line #L57 was not covered by tests
Self::InvalidAmount(e) => Some(e),
Self::Utf8Decode(e) => Some(e),
_ => None,
}
}
Expand Down
16 changes: 7 additions & 9 deletions ibc-apps/ics20-transfer/types/src/msgs/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@
packet_data: PacketData {
token: raw_msg
.token
.ok_or(TokenTransferError::InvalidToken)?
.ok_or(TokenTransferError::MissingToken)?

Check warning on line 72 in ibc-apps/ics20-transfer/types/src/msgs/transfer.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L72 was not covered by tests
.try_into()
.map_err(|_| TokenTransferError::InvalidToken)?,
.map_err(|_| TokenTransferError::MissingToken)?,

Check warning on line 74 in ibc-apps/ics20-transfer/types/src/msgs/transfer.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L74 was not covered by tests
sender: raw_msg.sender.into(),
receiver: raw_msg.receiver.into(),
memo: raw_msg.memo.into(),
Expand Down Expand Up @@ -104,14 +104,12 @@

fn try_from(raw: Any) -> Result<Self, Self::Error> {
match raw.type_url.as_str() {
TYPE_URL => {
MsgTransfer::decode_vec(&raw.value).map_err(|e| TokenTransferError::DecodeRawMsg {
reason: e.to_string(),
})
}
_ => Err(TokenTransferError::UnknownMsgType {
msg_type: raw.type_url,
TYPE_URL => MsgTransfer::decode_vec(&raw.value).map_err(|e| {
TokenTransferError::FailedToDecodeRawMsg {
description: e.to_string(),
}

Check warning on line 110 in ibc-apps/ics20-transfer/types/src/msgs/transfer.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics20-transfer/types/src/msgs/transfer.rs#L107-L110

Added lines #L107 - L110 were not covered by tests
}),
_ => Err(TokenTransferError::UnknownMsgType(raw.type_url)),

Check warning on line 112 in ibc-apps/ics20-transfer/types/src/msgs/transfer.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L112 was not covered by tests
}
}
}
4 changes: 2 additions & 2 deletions ibc-apps/ics721-nft-transfer/src/handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
.sender
.clone()
.try_into()
.map_err(|_| NftTransferError::ParseAccountFailure)?;
.map_err(|_| NftTransferError::FailedToParseAccount)?;

Check warning on line 24 in ibc-apps/ics721-nft-transfer/src/handler/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L24 was not covered by tests

if is_sender_chain_source(
packet.port_id_on_a.clone(),
Expand Down Expand Up @@ -58,7 +58,7 @@
.sender
.clone()
.try_into()
.map_err(|_| NftTransferError::ParseAccountFailure)?;
.map_err(|_| NftTransferError::FailedToParseAccount)?;

Check warning on line 61 in ibc-apps/ics721-nft-transfer/src/handler/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L61 was not covered by tests

if is_sender_chain_source(
packet.port_id_on_a.clone(),
Expand Down
Loading
Loading