Skip to content

Commit

Permalink
imp(ibc-apps): migrate to DecodingError in TryFrom conversions (#…
Browse files Browse the repository at this point in the history
…1335)

* imp: migrate to DecodingError in TryFrom conversions, ibc-apps

* imp: apply on TryFrom<RawPacketData>

* fix: missing import

* fix: apply review comments

* imp: move away from From<Infallible>
  • Loading branch information
Farhad-Shabani authored Sep 13, 2024
1 parent 4b247a3 commit 6db56cc
Show file tree
Hide file tree
Showing 16 changed files with 144 additions and 163 deletions.
12 changes: 8 additions & 4 deletions ibc-apps/ics20-transfer/types/src/amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@ use core::ops::Deref;
use core::str::FromStr;

use derive_more::{Display, From, Into};
use ibc_core::host::types::error::DecodingError;
use ibc_core::primitives::prelude::*;
#[cfg(feature = "serde")]
use ibc_core::primitives::serializers;
use primitive_types::U256;

use super::error::TokenTransferError;

/// A type for representing token transfer amounts.
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
Expand Down Expand Up @@ -103,10 +102,15 @@ impl AsRef<U256> for Amount {
}

impl FromStr for Amount {
type Err = TokenTransferError;
type Err = DecodingError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let amount = U256::from_dec_str(s).map_err(TokenTransferError::InvalidAmount)?;
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}"
))
})?;

Ok(Self(amount))
}
}
Expand Down
24 changes: 13 additions & 11 deletions ibc-apps/ics20-transfer/types/src/coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
use core::fmt::{Display, Error as FmtError, Formatter};
use core::str::FromStr;

use ibc_core::host::types::error::DecodingError;
use ibc_core::primitives::prelude::*;
use ibc_proto::cosmos::base::v1beta1::Coin as ProtoCoin;

use super::amount::Amount;
use super::denom::{BaseDenom, PrefixedDenom};
use super::error::TokenTransferError;

/// A `Coin` type with fully qualified `PrefixedDenom`.
pub type PrefixedCoin = Coin<PrefixedDenom>;
Expand Down Expand Up @@ -41,21 +41,21 @@ pub struct Coin<D> {

impl<D: FromStr> Coin<D>
where
D::Err: Into<TokenTransferError>,
D::Err: Display,
{
pub fn from_string_list(coin_str: &str) -> Result<Vec<Self>, TokenTransferError> {
pub fn from_string_list(coin_str: &str) -> Result<Vec<Self>, DecodingError> {
coin_str.split(',').map(FromStr::from_str).collect()
}
}

impl<D: FromStr> FromStr for Coin<D>
where
D::Err: Into<TokenTransferError>,
D::Err: Display,
{
type Err = TokenTransferError;
type Err = DecodingError;

#[allow(clippy::assign_op_pattern)]
fn from_str(coin_str: &str) -> Result<Self, TokenTransferError> {
fn from_str(coin_str: &str) -> Result<Self, DecodingError> {
// Denominations can be 3 ~ 128 characters long and support letters, followed by either
// a letter, a number or a separator ('/', ':', '.', '_' or '-').
// Loosely copy the regex from here:
Expand All @@ -76,20 +76,22 @@ where
.chars()
.all(|x| x.is_alphanumeric() || VALID_DENOM_CHARACTERS.contains(x))
})
.ok_or_else(|| TokenTransferError::InvalidCoin(coin_str.to_owned()))?;
.ok_or(DecodingError::invalid_raw_data(format!(
"invalid coin: {coin_str}"
)))?;

Ok(Coin {
amount: amount.parse()?,
denom: denom.parse().map_err(Into::into)?,
denom: denom.parse().map_err(DecodingError::invalid_raw_data)?,
})
}
}

impl<D: FromStr> TryFrom<ProtoCoin> for Coin<D>
where
D::Err: Into<TokenTransferError>,
D::Err: Into<DecodingError>,
{
type Error = TokenTransferError;
type Error = DecodingError;

fn try_from(proto: ProtoCoin) -> Result<Coin<D>, Self::Error> {
let denom = D::from_str(&proto.denom).map_err(Into::into)?;
Expand Down Expand Up @@ -176,7 +178,7 @@ mod tests {
fn test_parse_raw_coin_list(
#[case] coins_str: &str,
#[case] coins: &[(u64, &str)],
) -> Result<(), TokenTransferError> {
) -> Result<(), DecodingError> {
assert_eq!(
RawCoin::from_string_list(coins_str)?,
coins
Expand Down
25 changes: 13 additions & 12 deletions ibc-apps/ics20-transfer/types/src/denom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ use core::fmt::{Display, Error as FmtError, Formatter};
use core::str::FromStr;

use derive_more::{Display, From};
use ibc_core::host::types::error::DecodingError;
use ibc_core::host::types::identifiers::{ChannelId, PortId};
use ibc_core::primitives::prelude::*;
#[cfg(feature = "serde")]
use ibc_core::primitives::serializers;
use ibc_proto::ibc::applications::transfer::v1::DenomTrace as RawDenomTrace;

use super::error::TokenTransferError;

/// The "base" of a denomination.
///
/// For example, given the token `my_port-1/my_channel-1/my_port-2/my_channel-2/base_denom`,
Expand Down Expand Up @@ -40,11 +39,11 @@ impl BaseDenom {
}

impl FromStr for BaseDenom {
type Err = TokenTransferError;
type Err = DecodingError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.trim().is_empty() {
Err(TokenTransferError::EmptyBaseDenom)
Err(DecodingError::missing_raw_data("empty base denom"))
} else {
Ok(BaseDenom(s.to_owned()))
}
Expand Down Expand Up @@ -208,7 +207,7 @@ impl TracePath {
}

impl FromStr for TracePath {
type Err = TokenTransferError;
type Err = DecodingError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.is_empty() {
Expand All @@ -219,7 +218,9 @@ impl FromStr for TracePath {
remaining_parts
.is_none()
.then_some(trace_path)
.ok_or_else(|| TokenTransferError::InvalidTrace(s.to_owned()))
.ok_or(DecodingError::invalid_raw_data(format!(
"invalid trace path: {s}"
)))
}
}

Expand Down Expand Up @@ -323,7 +324,7 @@ pub fn is_receiver_chain_source(
}

impl FromStr for PrefixedDenom {
type Err = TokenTransferError;
type Err = DecodingError;

/// Initializes a [`PrefixedDenom`] from a string that adheres to the format
/// `{nth-port-id/channel-<index>}/{(n-1)th-port-id/channel-<index>}/.../{1st-port-id/channel-<index>}/<base_denom>`.
Expand Down Expand Up @@ -361,7 +362,7 @@ impl FromStr for PrefixedDenom {
}

impl TryFrom<RawDenomTrace> for PrefixedDenom {
type Error = TokenTransferError;
type Error = DecodingError;

fn try_from(value: RawDenomTrace) -> Result<Self, Self::Error> {
let base_denom = BaseDenom::from_str(&value.base_denom)?;
Expand Down Expand Up @@ -483,7 +484,7 @@ mod tests {
fn test_strange_but_accepted_prefixed_denom(
#[case] prefix: &str,
#[case] denom: &str,
) -> Result<(), TokenTransferError> {
) -> Result<(), DecodingError> {
let pd_s = if prefix.is_empty() {
denom.to_owned()
} else {
Expand All @@ -505,9 +506,9 @@ mod tests {
#[case("transfer/channel-1/transfer/channel-2/")]
#[case("transfer/channel-21/transfer/channel-23/ ")]
#[case("transfer/channel-0/")]
#[should_panic(expected = "EmptyBaseDenom")]
fn test_prefixed_empty_base_denom(#[case] pd_s: &str) {
PrefixedDenom::from_str(pd_s).expect("error");
PrefixedDenom::from_str(pd_s)
.expect_err("error: MissingRawData { description: \"empty base denom\" }");
}

#[rstest]
Expand Down Expand Up @@ -617,7 +618,7 @@ mod tests {
}

#[test]
fn test_trace_path() -> Result<(), TokenTransferError> {
fn test_trace_path() -> Result<(), DecodingError> {
assert!(TracePath::from_str("").is_ok(), "empty trace path");
assert!(
TracePath::from_str("transfer/uatom").is_err(),
Expand Down
28 changes: 1 addition & 27 deletions ibc-apps/ics20-transfer/types/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,18 @@
//! Defines the token transfer error type
use core::convert::Infallible;

use displaydoc::Display;
use ibc_core::channel::types::acknowledgement::StatusValue;
use ibc_core::channel::types::channel::Order;
use ibc_core::handler::types::error::ContextError;
use ibc_core::host::types::error::{DecodingError, IdentifierError};
use ibc_core::host::types::error::DecodingError;
use ibc_core::host::types::identifiers::{ChannelId, PortId};
use ibc_core::primitives::prelude::*;
use uint::FromDecStrErr;

#[derive(Display, Debug)]
pub enum TokenTransferError {
/// context error: `{0}`
ContextError(ContextError),
/// decoding error: `{0}`
Decoding(DecodingError),
/// invalid amount: `{0}`
InvalidAmount(FromDecStrErr),
/// invalid coin: `{0}`
InvalidCoin(String),
/// invalid trace: `{0}`
InvalidTrace(String),
/// missing token
MissingToken,
/// missing destination channel `{channel_id}` on port `{port_id}`
MissingDestinationChannel {
port_id: PortId,
Expand All @@ -35,8 +24,6 @@ pub enum TokenTransferError {
MismatchedPortIds { expected: PortId, actual: PortId },
/// channel cannot be closed
UnsupportedClosedChannel,
/// empty base denomination
EmptyBaseDenom,
/// failed to deserialize packet data
FailedToDeserializePacketData,
/// failed to deserialize acknowledgement
Expand All @@ -53,31 +40,18 @@ impl std::error::Error for TokenTransferError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match &self {
Self::ContextError(e) => Some(e),
Self::InvalidAmount(e) => Some(e),
Self::Decoding(e) => Some(e),
_ => None,
}
}
}

impl From<Infallible> for TokenTransferError {
fn from(e: Infallible) -> Self {
match e {}
}
}

impl From<ContextError> for TokenTransferError {
fn from(e: ContextError) -> Self {
Self::ContextError(e)
}
}

impl From<IdentifierError> for TokenTransferError {
fn from(e: IdentifierError) -> Self {
Self::Decoding(DecodingError::Identifier(e))
}
}

impl From<DecodingError> for TokenTransferError {
fn from(e: DecodingError) -> Self {
Self::Decoding(e)
Expand Down
23 changes: 9 additions & 14 deletions ibc-apps/ics20-transfer/types/src/msgs/transfer.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
//! Defines the token transfer message type

use ibc_core::channel::types::error::PacketError;
use ibc_core::channel::types::timeout::{TimeoutHeight, TimeoutTimestamp};
use ibc_core::handler::types::error::ContextError;
use ibc_core::host::types::error::DecodingError;
use ibc_core::host::types::identifiers::{ChannelId, PortId};
use ibc_core::primitives::prelude::*;
use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::applications::transfer::v1::MsgTransfer as RawMsgTransfer;
use ibc_proto::Protobuf;

use crate::error::TokenTransferError;
use crate::packet::PacketData;

pub(crate) const TYPE_URL: &str = "/ibc.applications.transfer.v1.MsgTransfer";
Expand Down Expand Up @@ -49,19 +46,18 @@ pub struct MsgTransfer {
}

impl TryFrom<RawMsgTransfer> for MsgTransfer {
type Error = TokenTransferError;
type Error = DecodingError;

fn try_from(raw_msg: RawMsgTransfer) -> Result<Self, Self::Error> {
let timeout_height_on_b: TimeoutHeight = raw_msg
.timeout_height
.try_into()
.map_err(ContextError::from)?;
let timeout_height_on_b: TimeoutHeight = raw_msg.timeout_height.try_into()?;

let timeout_timestamp_on_b: TimeoutTimestamp = raw_msg.timeout_timestamp.into();

// 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(ContextError::from(PacketError::MissingTimeout))?;
return Err(DecodingError::missing_raw_data(
"missing timeout height or timeout timestamp",
));
}

Ok(MsgTransfer {
Expand All @@ -70,9 +66,8 @@ impl TryFrom<RawMsgTransfer> for MsgTransfer {
packet_data: PacketData {
token: raw_msg
.token
.ok_or(TokenTransferError::MissingToken)?
.try_into()
.map_err(|_| TokenTransferError::MissingToken)?,
.ok_or(DecodingError::missing_raw_data("missing token"))?
.try_into()?,
sender: raw_msg.sender.into(),
receiver: raw_msg.receiver.into(),
memo: raw_msg.memo.into(),
Expand Down Expand Up @@ -101,11 +96,11 @@ impl From<MsgTransfer> for RawMsgTransfer {
impl Protobuf<RawMsgTransfer> for MsgTransfer {}

impl TryFrom<Any> for MsgTransfer {
type Error = TokenTransferError;
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,
Expand Down
4 changes: 2 additions & 2 deletions ibc-apps/ics20-transfer/types/src/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

use core::str::FromStr;

use ibc_core::host::types::error::DecodingError;
use ibc_core::primitives::prelude::*;
use ibc_core::primitives::Signer;
use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData;

use super::error::TokenTransferError;
use super::{Amount, Memo, PrefixedCoin, PrefixedDenom};

/// Defines the structure of token transfers' packet bytes
Expand All @@ -33,7 +33,7 @@ pub struct PacketData {
}

impl TryFrom<RawPacketData> for PacketData {
type Error = TokenTransferError;
type Error = DecodingError;

fn try_from(raw_pkt_data: RawPacketData) -> Result<Self, Self::Error> {
// This denom may be prefixed or unprefixed.
Expand Down
Loading

0 comments on commit 6db56cc

Please sign in to comment.