diff --git a/ibc-apps/ics20-transfer/src/handler/mod.rs b/ibc-apps/ics20-transfer/src/handler/mod.rs index 39dcd373df..6d3cde510b 100644 --- a/ibc-apps/ics20-transfer/src/handler/mod.rs +++ b/ibc-apps/ics20-transfer/src/handler/mod.rs @@ -20,7 +20,7 @@ pub fn refund_packet_token_execute( .sender .clone() .try_into() - .map_err(|_| TokenTransferError::ParseAccountFailure)?; + .map_err(|_| TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( packet.port_id_on_a.clone(), @@ -49,7 +49,7 @@ pub fn refund_packet_token_validate( .sender .clone() .try_into() - .map_err(|_| TokenTransferError::ParseAccountFailure)?; + .map_err(|_| TokenTransferError::FailedToParseAccount)?; if is_sender_chain_source( packet.port_id_on_a.clone(), diff --git a/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs b/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs index 193b331969..b0fcbd8775 100644 --- a/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs +++ b/ibc-apps/ics20-transfer/src/handler/on_recv_packet.rs @@ -26,7 +26,7 @@ pub fn process_recv_packet_execute( let receiver_account = data.receiver.clone().try_into().map_err(|_| { ( ModuleExtras::empty(), - TokenTransferError::ParseAccountFailure, + TokenTransferError::FailedToParseAccount, ) })?; diff --git a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs index 439e5604c6..39d40ea46e 100644 --- a/ibc-apps/ics20-transfer/src/handler/send_transfer.rs +++ b/ibc-apps/ics20-transfer/src/handler/send_transfer.rs @@ -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(), })? @@ -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(), @@ -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(), })? @@ -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(), diff --git a/ibc-apps/ics20-transfer/src/module.rs b/ibc-apps/ics20-transfer/src/module.rs index 392ca5939c..66370f10c0 100644 --- a/ibc-apps/ics20-transfer/src/module.rs +++ b/ibc-apps/ics20-transfer/src/module.rs @@ -27,16 +27,16 @@ pub fn on_chan_open_init_validate( 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, }); } @@ -71,9 +71,9 @@ pub fn on_chan_open_try_validate( 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, }); } @@ -139,7 +139,7 @@ pub fn on_chan_close_init_validate( _port_id: &PortId, _channel_id: &ChannelId, ) -> Result<(), TokenTransferError> { - Err(TokenTransferError::CantCloseChannel) + Err(TokenTransferError::UnsupportedClosedChannel) } pub fn on_chan_close_init_execute( @@ -147,7 +147,7 @@ pub fn on_chan_close_init_execute( _port_id: &PortId, _channel_id: &ChannelId, ) -> Result { - Err(TokenTransferError::CantCloseChannel) + Err(TokenTransferError::UnsupportedClosedChannel) } pub fn on_chan_close_confirm_validate( @@ -172,7 +172,7 @@ pub fn on_recv_packet_execute( ) -> (ModuleExtras, Acknowledgement) { let Ok(data) = serde_json::from_slice::(&packet.data) else { let ack = - AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into()); + AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into()); return (ModuleExtras::empty(), ack.into()); }; @@ -204,10 +204,10 @@ where Ctx: TokenTransferValidationContext, { let data = serde_json::from_slice::(&packet.data) - .map_err(|_| TokenTransferError::PacketDataDeserialization)?; + .map_err(|_| TokenTransferError::FailedToDeserializePacketData)?; let acknowledgement = serde_json::from_slice::(acknowledgement.as_ref()) - .map_err(|_| TokenTransferError::AckDeserialization)?; + .map_err(|_| TokenTransferError::FailedToDeserializeAck)?; if !acknowledgement.is_successful() { refund_packet_token_validate(ctx, packet, &data)?; @@ -225,7 +225,7 @@ pub fn on_acknowledgement_packet_execute( let Ok(data) = serde_json::from_slice::(&packet.data) else { return ( ModuleExtras::empty(), - Err(TokenTransferError::PacketDataDeserialization), + Err(TokenTransferError::FailedToDeserializePacketData), ); }; @@ -234,7 +234,7 @@ pub fn on_acknowledgement_packet_execute( else { return ( ModuleExtras::empty(), - Err(TokenTransferError::AckDeserialization), + Err(TokenTransferError::FailedToDeserializeAck), ); }; @@ -270,7 +270,7 @@ where Ctx: TokenTransferValidationContext, { let data = serde_json::from_slice::(&packet.data) - .map_err(|_| TokenTransferError::PacketDataDeserialization)?; + .map_err(|_| TokenTransferError::FailedToDeserializePacketData)?; refund_packet_token_validate(ctx, packet, &data)?; @@ -285,7 +285,7 @@ pub fn on_timeout_packet_execute( let Ok(data) = serde_json::from_slice::(&packet.data) else { return ( ModuleExtras::empty(), - Err(TokenTransferError::PacketDataDeserialization), + Err(TokenTransferError::FailedToDeserializePacketData), ); }; @@ -324,7 +324,7 @@ mod test { r#"{"result":"AQ=="}"#, ); ser_json_assert_eq( - AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into()), + AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into()), r#"{"error":"failed to deserialize packet data"}"#, ); } @@ -342,7 +342,7 @@ mod test { #[test] fn test_ack_error_to_vec() { let ack_error: Vec = - AcknowledgementStatus::error(TokenTransferError::PacketDataDeserialization.into()) + AcknowledgementStatus::error(TokenTransferError::FailedToDeserializePacketData.into()) .into(); // Check that it's the same output as ibc-go @@ -367,7 +367,7 @@ mod test { ); 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::(r#"{"success":"AQ=="}"#).is_err()); diff --git a/ibc-apps/ics20-transfer/types/src/coin.rs b/ibc-apps/ics20-transfer/types/src/coin.rs index 65a1472183..9470bbdbc5 100644 --- a/ibc-apps/ics20-transfer/types/src/coin.rs +++ b/ibc-apps/ics20-transfer/types/src/coin.rs @@ -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()?, diff --git a/ibc-apps/ics20-transfer/types/src/denom.rs b/ibc-apps/ics20-transfer/types/src/denom.rs index 4bd30f60ca..cc814280f5 100644 --- a/ibc-apps/ics20-transfer/types/src/denom.rs +++ b/ibc-apps/ics20-transfer/types/src/denom.rs @@ -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())) } } diff --git a/ibc-apps/ics20-transfer/types/src/error.rs b/ibc-apps/ics20-transfer/types/src/error.rs index a394eb77d1..7924e2609c 100644 --- a/ibc-apps/ics20-transfer/types/src/error.rs +++ b/ibc-apps/ics20-transfer/types/src/error.rs @@ -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; @@ -17,68 +16,39 @@ pub enum TokenTransferError { ContextError(ContextError), /// invalid identifier: `{0}` InvalidIdentifier(IdentifierError), - /// 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, + // TODO(seanchen1991): Used in basecoin; this variant should be moved + // to a host-relevant error + /// 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")] @@ -86,17 +56,8 @@ 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), Self::InvalidAmount(e) => Some(e), - Self::Utf8Decode(e) => Some(e), _ => None, } } diff --git a/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs b/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs index cc311a2caf..048ea956cb 100644 --- a/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs +++ b/ibc-apps/ics20-transfer/types/src/msgs/transfer.rs @@ -69,9 +69,9 @@ impl TryFrom for MsgTransfer { packet_data: PacketData { token: raw_msg .token - .ok_or(TokenTransferError::InvalidToken)? + .ok_or(TokenTransferError::MissingToken)? .try_into() - .map_err(|_| TokenTransferError::InvalidToken)?, + .map_err(|_| TokenTransferError::MissingToken)?, sender: raw_msg.sender.into(), receiver: raw_msg.receiver.into(), memo: raw_msg.memo.into(), @@ -104,14 +104,12 @@ impl TryFrom for MsgTransfer { fn try_from(raw: Any) -> Result { 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(), + } }), + _ => Err(TokenTransferError::UnknownMsgType(raw.type_url)), } } } diff --git a/ibc-apps/ics721-nft-transfer/src/handler/mod.rs b/ibc-apps/ics721-nft-transfer/src/handler/mod.rs index 2adbb47f6d..8f8d0775c2 100644 --- a/ibc-apps/ics721-nft-transfer/src/handler/mod.rs +++ b/ibc-apps/ics721-nft-transfer/src/handler/mod.rs @@ -21,7 +21,7 @@ pub fn refund_packet_nft_execute( .sender .clone() .try_into() - .map_err(|_| NftTransferError::ParseAccountFailure)?; + .map_err(|_| NftTransferError::FailedToParseAccount)?; if is_sender_chain_source( packet.port_id_on_a.clone(), @@ -58,7 +58,7 @@ pub fn refund_packet_nft_validate( .sender .clone() .try_into() - .map_err(|_| NftTransferError::ParseAccountFailure)?; + .map_err(|_| NftTransferError::FailedToParseAccount)?; if is_sender_chain_source( packet.port_id_on_a.clone(), diff --git a/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs b/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs index 8782bf7a2f..59e6f3cb83 100644 --- a/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs +++ b/ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs @@ -25,11 +25,12 @@ where .can_receive_nft() .map_err(|err| (ModuleExtras::empty(), err))?; - let receiver_account = data - .receiver - .clone() - .try_into() - .map_err(|_| (ModuleExtras::empty(), NftTransferError::ParseAccountFailure))?; + let receiver_account = data.receiver.clone().try_into().map_err(|_| { + ( + ModuleExtras::empty(), + NftTransferError::FailedToParseAccount, + ) + })?; let extras = if is_receiver_chain_source( packet.port_id_on_a.clone(), diff --git a/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs b/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs index 4cf1a759cd..8a33fef566 100644 --- a/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs +++ b/ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs @@ -47,7 +47,7 @@ where let chan_id_on_b = chan_end_on_a .counterparty() .channel_id() - .ok_or_else(|| NftTransferError::DestinationChannelNotFound { + .ok_or_else(|| NftTransferError::MissingDestinationChannel { port_id: msg.port_id_on_a.clone(), channel_id: msg.chan_id_on_a.clone(), })? @@ -61,7 +61,7 @@ where .sender .clone() .try_into() - .map_err(|_| NftTransferError::ParseAccountFailure)?; + .map_err(|_| NftTransferError::FailedToParseAccount)?; let mut packet_data = msg.packet_data; let class_id = &packet_data.class_id; @@ -149,7 +149,7 @@ where let chan_on_b = chan_end_on_a .counterparty() .channel_id() - .ok_or_else(|| NftTransferError::DestinationChannelNotFound { + .ok_or_else(|| NftTransferError::MissingDestinationChannel { port_id: msg.port_id_on_a.clone(), channel_id: msg.chan_id_on_a.clone(), })? @@ -164,7 +164,7 @@ where .sender .clone() .try_into() - .map_err(|_| NftTransferError::ParseAccountFailure)?; + .map_err(|_| NftTransferError::FailedToParseAccount)?; let mut packet_data = msg.packet_data; let class_id = &packet_data.class_id; diff --git a/ibc-apps/ics721-nft-transfer/src/module.rs b/ibc-apps/ics721-nft-transfer/src/module.rs index 074e7c8692..5560b686f0 100644 --- a/ibc-apps/ics721-nft-transfer/src/module.rs +++ b/ibc-apps/ics721-nft-transfer/src/module.rs @@ -28,16 +28,16 @@ pub fn on_chan_open_init_validate( version: &Version, ) -> Result<(), NftTransferError> { if order != Order::Unordered { - return Err(NftTransferError::ChannelNotUnordered { - expect_order: Order::Unordered, - got_order: order, + return Err(NftTransferError::MismatchedChannelOrders { + expected: Order::Unordered, + actual: order, }); } let bound_port = ctx.get_port()?; if port_id != &bound_port { - return Err(NftTransferError::InvalidPort { - port_id: port_id.clone(), - exp_port_id: bound_port, + return Err(NftTransferError::MismatchedPortIds { + actual: port_id.clone(), + expected: bound_port, }); } @@ -72,9 +72,9 @@ pub fn on_chan_open_try_validate( counterparty_version: &Version, ) -> Result<(), NftTransferError> { if order != Order::Unordered { - return Err(NftTransferError::ChannelNotUnordered { - expect_order: Order::Unordered, - got_order: order, + return Err(NftTransferError::MismatchedChannelOrders { + expected: Order::Unordered, + actual: order, }); } @@ -140,7 +140,7 @@ pub fn on_chan_close_init_validate( _port_id: &PortId, _channel_id: &ChannelId, ) -> Result<(), NftTransferError> { - Err(NftTransferError::CantCloseChannel) + Err(NftTransferError::UnsupportedClosedChannel) } pub fn on_chan_close_init_execute( @@ -148,7 +148,7 @@ pub fn on_chan_close_init_execute( _port_id: &PortId, _channel_id: &ChannelId, ) -> Result { - Err(NftTransferError::CantCloseChannel) + Err(NftTransferError::UnsupportedClosedChannel) } pub fn on_chan_close_confirm_validate( @@ -172,7 +172,8 @@ pub fn on_recv_packet_execute( packet: &Packet, ) -> (ModuleExtras, Acknowledgement) { let Ok(data) = serde_json::from_slice::(&packet.data) else { - let ack = AcknowledgementStatus::error(NftTransferError::PacketDataDeserialization.into()); + let ack = + AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into()); return (ModuleExtras::empty(), ack.into()); }; @@ -204,10 +205,10 @@ pub fn on_acknowledgement_packet_validate( _relayer: &Signer, ) -> Result<(), NftTransferError> { let data = serde_json::from_slice::(&packet.data) - .map_err(|_| NftTransferError::PacketDataDeserialization)?; + .map_err(|_| NftTransferError::FailedToDeserializePacketData)?; let acknowledgement = serde_json::from_slice::(acknowledgement.as_ref()) - .map_err(|_| NftTransferError::AckDeserialization)?; + .map_err(|_| NftTransferError::FailedToDeserializeAck)?; if !acknowledgement.is_successful() { refund_packet_nft_validate(ctx, packet, &data)?; @@ -225,7 +226,7 @@ pub fn on_acknowledgement_packet_execute( let Ok(data) = serde_json::from_slice::(&packet.data) else { return ( ModuleExtras::empty(), - Err(NftTransferError::PacketDataDeserialization), + Err(NftTransferError::FailedToDeserializePacketData), ); }; @@ -234,7 +235,7 @@ pub fn on_acknowledgement_packet_execute( else { return ( ModuleExtras::empty(), - Err(NftTransferError::AckDeserialization), + Err(NftTransferError::FailedToDeserializeAck), ); }; @@ -267,7 +268,7 @@ pub fn on_timeout_packet_validate( _relayer: &Signer, ) -> Result<(), NftTransferError> { let data = serde_json::from_slice::(&packet.data) - .map_err(|_| NftTransferError::PacketDataDeserialization)?; + .map_err(|_| NftTransferError::FailedToDeserializePacketData)?; refund_packet_nft_validate(ctx, packet, &data)?; @@ -282,7 +283,7 @@ pub fn on_timeout_packet_execute( let Ok(data) = serde_json::from_slice::(&packet.data) else { return ( ModuleExtras::empty(), - Err(NftTransferError::PacketDataDeserialization), + Err(NftTransferError::FailedToDeserializePacketData), ); }; @@ -321,7 +322,7 @@ mod test { r#"{"result":"AQ=="}"#, ); ser_json_assert_eq( - AcknowledgementStatus::error(NftTransferError::PacketDataDeserialization.into()), + AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into()), r#"{"error":"failed to deserialize packet data"}"#, ); } @@ -339,7 +340,8 @@ mod test { #[test] fn test_ack_error_to_vec() { let ack_error: Vec = - AcknowledgementStatus::error(NftTransferError::PacketDataDeserialization.into()).into(); + AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into()) + .into(); // Check that it's the same output as ibc-go // Note: this also implicitly checks that the ack bytes are non-empty, @@ -363,7 +365,7 @@ mod test { ); de_json_assert_eq( r#"{"error":"failed to deserialize packet data"}"#, - AcknowledgementStatus::error(NftTransferError::PacketDataDeserialization.into()), + AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into()), ); assert!(serde_json::from_str::(r#"{"success":"AQ=="}"#).is_err()); diff --git a/ibc-apps/ics721-nft-transfer/types/src/class.rs b/ibc-apps/ics721-nft-transfer/types/src/class.rs index 831883331c..8a7c941e6d 100644 --- a/ibc-apps/ics721-nft-transfer/types/src/class.rs +++ b/ibc-apps/ics721-nft-transfer/types/src/class.rs @@ -142,9 +142,9 @@ impl TryFrom for PrefixedClassId { fn try_from(value: RawClassTrace) -> Result { let base_class_id = ClassId::from_str(&value.base_class_id)?; - // FIXME: separate `TracePath` error. let trace_path = TracePath::from_str(&value.path) - .map_err(|err| NftTransferError::Other(err.to_string()))?; + .map_err(|_| NftTransferError::InvalidTrace(value.path))?; + Ok(Self { trace_path, base_class_id, @@ -248,10 +248,7 @@ impl FromStr for ClassUri { fn from_str(class_uri: &str) -> Result { match Uri::from_str(class_uri) { Ok(uri) => Ok(Self(uri)), - Err(err) => Err(NftTransferError::InvalidUri { - uri: class_uri.to_string(), - validation_error: err, - }), + Err(err) => Err(NftTransferError::InvalidUri(err)), } } } diff --git a/ibc-apps/ics721-nft-transfer/types/src/data.rs b/ibc-apps/ics721-nft-transfer/types/src/data.rs index a9aecc5099..8150545bb4 100644 --- a/ibc-apps/ics721-nft-transfer/types/src/data.rs +++ b/ibc-apps/ics721-nft-transfer/types/src/data.rs @@ -97,7 +97,9 @@ impl FromStr for Ics721Data { type Err = NftTransferError; fn from_str(s: &str) -> Result { - serde_json::from_str(s).map_err(|_| NftTransferError::InvalidIcs721Data) + serde_json::from_str(s).map_err(|e| NftTransferError::InvalidJsonData { + description: e.to_string(), + }) } } diff --git a/ibc-apps/ics721-nft-transfer/types/src/error.rs b/ibc-apps/ics721-nft-transfer/types/src/error.rs index 94c230adde..626eb9f6d6 100644 --- a/ibc-apps/ics721-nft-transfer/types/src/error.rs +++ b/ibc-apps/ics721-nft-transfer/types/src/error.rs @@ -1,6 +1,5 @@ //! Defines the Non-Fungible Token Transfer (ICS-721) error types. use core::convert::Infallible; -use core::str::Utf8Error; use displaydoc::Display; use ibc_core::channel::types::acknowledgement::StatusValue; @@ -16,80 +15,39 @@ pub enum NftTransferError { ContextError(ContextError), /// invalid identifier: `{0}` InvalidIdentifier(IdentifierError), - /// invalid URI: `{uri}`, validation error: `{validation_error}`` - InvalidUri { - uri: String, - validation_error: http::uri::InvalidUri, - }, - /// destination channel not found in the counterparty of port_id `{port_id}` and channel_id `{channel_id}` - DestinationChannelNotFound { + /// invalid URI: `{0}` + InvalidUri(http::uri::InvalidUri), + /// invalid json data: `{description}` + InvalidJsonData { description: String }, + /// invalid trace `{0}` + InvalidTrace(String), + /// missing destination channel `{channel_id}` on port `{port_id}` + MissingDestinationChannel { port_id: PortId, channel_id: ChannelId, }, - /// base class ID is empty + /// empty base class ID EmptyBaseClassId, - /// 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, - }, - /// trace length must be even but got: `{len}` - InvalidTraceLength { len: u64 }, - /// no token ID - NoTokenId, - /// invalid token ID - InvalidTokenId, - /// duplicated token IDs - DuplicatedTokenIds, - /// The length of token IDs mismatched that of token URIs or token data - TokenMismatched, - /// invalid json data - InvalidJsonData, - /// the data is not in the JSON format specified by ICS-721 - InvalidIcs721Data, - /// expected `{expect_order}` channel, got `{got_order}` - ChannelNotUnordered { - expect_order: Order, - got_order: Order, - }, - /// channel cannot be closed - CantCloseChannel, - /// `{sender}` doesn't own the NFT - InvalidOwner { sender: String }, - /// owner is not found - OwnerNotFound, - /// nft is not found - NftNotFound, - /// nft class is not found - NftClassNotFound, + /// empty token ID + EmptyTokenId, + /// mismatched number of token IDs: expected `{expected}`, actual `{actual}` + MismatchedNumberOfTokenIds { expected: usize, actual: usize }, + /// 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 }, - /// 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, + /// unknown msg type: `{0}` + UnknownMsgType(String), } #[cfg(feature = "std")] @@ -97,19 +55,8 @@ impl std::error::Error for NftTransferError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match &self { Self::ContextError(e) => Some(e), - Self::InvalidUri { - validation_error: e, - .. - } => Some(e), - Self::InvalidIdentifier(e) - | Self::InvalidTracePortId { - validation_error: e, - .. - } - | Self::InvalidTraceChannelId { - validation_error: e, - .. - } => Some(e), + Self::InvalidUri(e) => Some(e), + Self::InvalidIdentifier(e) => Some(e), _ => None, } } diff --git a/ibc-apps/ics721-nft-transfer/types/src/msgs/transfer.rs b/ibc-apps/ics721-nft-transfer/types/src/msgs/transfer.rs index b35a5c1cc2..499d7d4484 100644 --- a/ibc-apps/ics721-nft-transfer/types/src/msgs/transfer.rs +++ b/ibc-apps/ics721-nft-transfer/types/src/msgs/transfer.rs @@ -122,14 +122,12 @@ impl TryFrom for MsgTransfer { fn try_from(raw: Any) -> Result { match raw.type_url.as_str() { - TYPE_URL => { - MsgTransfer::decode_vec(&raw.value).map_err(|e| NftTransferError::DecodeRawMsg { - reason: e.to_string(), - }) - } - _ => Err(NftTransferError::UnknownMsgType { - msg_type: raw.type_url, + TYPE_URL => MsgTransfer::decode_vec(&raw.value).map_err(|e| { + NftTransferError::FailedToDecodeRawMsg { + description: e.to_string(), + } }), + _ => Err(NftTransferError::UnknownMsgType(raw.type_url)), } } } diff --git a/ibc-apps/ics721-nft-transfer/types/src/packet.rs b/ibc-apps/ics721-nft-transfer/types/src/packet.rs index c2f9d2d2f9..027e5fcb10 100644 --- a/ibc-apps/ics721-nft-transfer/types/src/packet.rs +++ b/ibc-apps/ics721-nft-transfer/types/src/packet.rs @@ -91,7 +91,7 @@ impl PacketData { /// Performs the basic validation of the packet data fields. pub fn validate_basic(&self) -> Result<(), NftTransferError> { if self.token_ids.0.is_empty() { - return Err(NftTransferError::NoTokenId); + return Err(NftTransferError::EmptyTokenId); } let num = self.token_ids.0.len(); let num_uri = self @@ -105,7 +105,10 @@ impl PacketData { .map(|t| t.len()) .unwrap_or_default(); if (num_uri != 0 && num_uri != num) || (num_data != 0 && num_data != num) { - return Err(NftTransferError::TokenMismatched); + return Err(NftTransferError::MismatchedNumberOfTokenIds { + actual: num, + expected: num_uri, + }); } Ok(()) } @@ -125,9 +128,13 @@ impl TryFrom for PacketData { } else { let decoded = BASE64_STANDARD .decode(raw_pkt_data.class_data) - .map_err(|_| NftTransferError::InvalidJsonData)?; + .map_err(|e| NftTransferError::InvalidJsonData { + description: e.to_string(), + })?; let data_str = - String::from_utf8(decoded).map_err(|_| NftTransferError::InvalidJsonData)?; + String::from_utf8(decoded).map_err(|e| NftTransferError::InvalidJsonData { + description: e.to_string(), + })?; Some(data_str.parse()?) }; @@ -138,11 +145,15 @@ impl TryFrom for PacketData { .token_data .iter() .map(|data| { - let decoded = BASE64_STANDARD - .decode(data) - .map_err(|_| NftTransferError::InvalidJsonData)?; + let decoded = BASE64_STANDARD.decode(data).map_err(|e| { + NftTransferError::InvalidJsonData { + description: e.to_string(), + } + })?; let data_str = - String::from_utf8(decoded).map_err(|_| NftTransferError::InvalidJsonData)?; + String::from_utf8(decoded).map_err(|e| NftTransferError::InvalidJsonData { + description: e.to_string(), + })?; data_str.parse() }) .collect(); diff --git a/ibc-apps/ics721-nft-transfer/types/src/token.rs b/ibc-apps/ics721-nft-transfer/types/src/token.rs index 1571eac487..fa11e93a1c 100644 --- a/ibc-apps/ics721-nft-transfer/types/src/token.rs +++ b/ibc-apps/ics721-nft-transfer/types/src/token.rs @@ -45,7 +45,7 @@ impl FromStr for TokenId { fn from_str(token_id: &str) -> Result { if token_id.trim().is_empty() { - Err(NftTransferError::InvalidTokenId) + Err(NftTransferError::EmptyTokenId) } else { Ok(Self(token_id.to_string())) } @@ -94,15 +94,22 @@ impl TryFrom> for TokenIds { fn try_from(token_ids: Vec) -> Result { if token_ids.is_empty() { - return Err(NftTransferError::NoTokenId); + return Err(NftTransferError::EmptyTokenId); } + let ids: Result, _> = token_ids.iter().map(|t| t.parse()).collect(); let mut ids = ids?; + ids.sort(); ids.dedup(); + if ids.len() != token_ids.len() { - return Err(NftTransferError::DuplicatedTokenIds); + return Err(NftTransferError::MismatchedNumberOfTokenIds { + expected: token_ids.len(), + actual: ids.len(), + }); } + Ok(Self(ids)) } } @@ -175,10 +182,7 @@ impl FromStr for TokenUri { fn from_str(token_uri: &str) -> Result { match Uri::from_str(token_uri) { Ok(uri) => Ok(Self(uri)), - Err(err) => Err(NftTransferError::InvalidUri { - uri: token_uri.to_string(), - validation_error: err, - }), + Err(err) => Err(NftTransferError::InvalidUri(err)), } } }