fix(ucs01): refactor and cleanup

unionlabs · Sep 7, 2023 · 3a35514 · 3a35514
1 parent a20d9e6
commit 3a35514
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 67 deletions.
diff --git a/cosmwasm/ucs01-relay-api/src/protocol.rs b/cosmwasm/ucs01-relay-api/src/protocol.rs
@@ -37,7 +37,12 @@ pub struct TransferInput {
     pub tokens: Vec<TransferToken>,
 }
 
-// We follow the following module implementation, events and attributes are almost 1:1 with the traditional go implementation.
+// We follow the following module implementation, events and attributes are
+// almost 1:1 with the traditional go implementation. As we generalized the base
+// implementation for multi-tokens transfer, the events are not containing a
+// single ('denom', 'value') and ('amount', 'value') attributes but rather a set
+// of ('denom:x', 'amount_value') attributes for each denom `x` that is
+// transferred. i.e. [('denom:muno', '10'), ('denom:port/channel/weth', '150'), ..]
 // https://github.com/cosmos/ibc-go/blob/7be17857b10457c67cbf66a49e13a9751eb10e8e/modules/apps/transfer/ibc_module.go
 pub trait TransferProtocol {
     /// Must be unique per Protocol

diff --git a/cosmwasm/ucs01-relay-api/src/types.rs b/cosmwasm/ucs01-relay-api/src/types.rs
@@ -27,13 +27,10 @@ pub struct TransferToken {
 }
 
 impl TransferToken {
-    /*
-      If a denom originated from a remote network, it will be in the form:
-        `factory/{contract_address}/{port_id}/{channel_id}/denom`
-
-      In order for the remote module to consider this denom as local, we must
-      strip the `factory/{contract_address}/` prefix.
-    */
+    // If a denom originated from a remote network, it will be in the form:
+    //   `factory/{contract_address}/{port_id}/{channel_id}/denom`
+    // In order for the remote module to consider this denom as local, we must
+    // strip the `factory/{contract_address}/` prefix before sending the tokens.
     pub fn normalize_for_ibc_transfer(
         self,
         contract_address: &str,
@@ -186,24 +183,6 @@ pub struct Ics20Packet {
     pub memo: String,
 }
 
-impl Ics20Packet {
-    pub fn new(
-        sender: String,
-        receiver: String,
-        denom: String,
-        amount: Uint256,
-        memo: String,
-    ) -> Self {
-        Self {
-            sender,
-            receiver,
-            denom,
-            amount,
-            memo,
-        }
-    }
-}
-
 impl TryFrom<Ics20Packet> for Binary {
     type Error = EncodingError;
     fn try_from(value: Ics20Packet) -> Result<Binary, Self::Error> {
@@ -265,10 +244,8 @@ impl TransferPacket for Ucs01TransferPacket {
     }
 }
 
-pub type Memo = String;
-
 impl TransferPacket for Ics20Packet {
-    type Extension = Memo;
+    type Extension = String;
 
     fn tokens(&self) -> Vec<TransferToken> {
         vec![TransferToken {
@@ -376,7 +353,7 @@ impl TryFrom<TransferPacketCommon<NoExtension>> for Ucs01TransferPacket {
     }
 }
 
-impl TryFrom<TransferPacketCommon<Memo>> for Ics20Packet {
+impl TryFrom<TransferPacketCommon<String>> for Ics20Packet {
     type Error = EncodingError;
 
     fn try_from(
@@ -385,13 +362,19 @@ impl TryFrom<TransferPacketCommon<Memo>> for Ics20Packet {
             receiver,
             tokens,
             extension,
-        }: TransferPacketCommon<Memo>,
+        }: TransferPacketCommon<String>,
     ) -> Result<Self, Self::Error> {
         let (denom, amount) = match &tokens[..] {
             [TransferToken { denom, amount }] => Ok((denom.clone(), amount.clone())),
             _ => Err(EncodingError::Ics20OnlyOneCoin),
         }?;
-        Ok(Self::new(sender, receiver, denom, amount.into(), extension))
+        Ok(Self {
+            sender,
+            receiver,
+            denom,
+            amount: amount.into(),
+            memo: extension,
+        })
     }
 }
 
@@ -411,7 +394,7 @@ impl<'a> From<(&'a str, &IbcEndpoint)> for DenomOrigin<'a> {
     fn from((denom, remote_endpoint): (&'a str, &IbcEndpoint)) -> Self {
         // https://github.com/cosmos/ibc/blob/main/spec/app/ics-020-fungible-token-transfer/README.md#data-structures
         // SPEC: {ics20Port}/{ics20Channel}/{denom}
-        // The denom is local IIF we can strip all prefixes
+        // The denom is local IFF we can strip all prefixes
         match denom
             .strip_prefix(&remote_endpoint.port_id)
             .and_then(|denom| denom.strip_prefix("/"))
@@ -453,7 +436,7 @@ mod tests {
         };
         assert_eq!(
             packet,
-            <Binary>::try_from(packet.clone())
+            Binary::try_from(packet.clone())
                 .unwrap()
                 .try_into()
                 .unwrap()
@@ -462,10 +445,14 @@ mod tests {
 
     #[test]
     fn ucs01_ack_encode_decode_iso() {
-        let ack = Ucs01Ack::Success;
-        assert_eq!(ack, <Binary>::try_from(ack).unwrap().try_into().unwrap());
-        let ack = Ucs01Ack::Failure;
-        assert_eq!(ack, <Binary>::try_from(ack).unwrap().try_into().unwrap());
+        assert_eq!(
+            Ok(Ucs01Ack::Success),
+            Binary::try_from(Ucs01Ack::Success).unwrap().try_into()
+        );
+        assert_eq!(
+            Ok(Ucs01Ack::Failure),
+            Binary::try_from(Ucs01Ack::Failure).unwrap().try_into()
+        );
     }
 
     #[test]
@@ -479,7 +466,7 @@ mod tests {
         };
         assert_eq!(
             packet,
-            <Binary>::try_from(packet.clone())
+            Binary::try_from(packet.clone())
                 .unwrap()
                 .try_into()
                 .unwrap()
@@ -488,15 +475,17 @@ mod tests {
 
     #[test]
     fn ics20_ack_encode_decode_iso() {
-        let ack = Ics20Ack::Result(b"blabla".into());
         assert_eq!(
-            ack,
-            <Binary>::try_from(ack.clone()).unwrap().try_into().unwrap()
+            Ok(Ics20Ack::Result(b"blabla".into())),
+            Binary::try_from(Ics20Ack::Result(b"blabla".into()))
+                .unwrap()
+                .try_into()
         );
-        let ack = Ics20Ack::Error("ok".into());
         assert_eq!(
-            ack,
-            <Binary>::try_from(ack.clone()).unwrap().try_into().unwrap()
+            Ok(Ics20Ack::Error("ok".into())),
+            Binary::try_from(Ics20Ack::Error("ok".into()))
+                .unwrap()
+                .try_into()
         );
     }
 

diff --git a/cosmwasm/ucs01-relay/src/contract.rs b/cosmwasm/ucs01-relay/src/contract.rs
@@ -90,8 +90,8 @@ pub fn execute_transfer(
         tokens,
     };
 
-    match channel_info.protocol_version {
-        ref v if v == Ics20Protocol::VERSION => Ics20Protocol {
+    match channel_info.protocol_version.as_str() {
+        Ics20Protocol::VERSION => Ics20Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -100,7 +100,7 @@ pub fn execute_transfer(
             },
         }
         .send(input, msg.memo),
-        ref v if v == Ucs01Protocol::VERSION => Ucs01Protocol {
+        Ucs01Protocol::VERSION => Ucs01Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -111,7 +111,7 @@ pub fn execute_transfer(
         .send(input, NoExtension),
         v => Err(ContractError::UnknownProtocol {
             channel_id: msg.channel,
-            protocol_version: v,
+            protocol_version: v.into(),
         }),
     }
 }
@@ -123,8 +123,9 @@ pub fn execute_receive_phase1(
     msg: ReceivePhase1Msg,
 ) -> Result<Response<TokenFactoryMsg>, ContractError> {
     let channel_info = CHANNEL_INFO.load(deps.storage, &msg.channel)?;
-    match channel_info.protocol_version {
-        ref v if v == Ics20Protocol::VERSION => Ics20Protocol {
+
+    match channel_info.protocol_version.as_str() {
+        Ics20Protocol::VERSION => Ics20Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -133,7 +134,7 @@ pub fn execute_receive_phase1(
             },
         }
         .receive_phase1(msg.raw_packet),
-        ref v if v == Ucs01Protocol::VERSION => Ucs01Protocol {
+        Ucs01Protocol::VERSION => Ucs01Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -144,7 +145,7 @@ pub fn execute_receive_phase1(
         .receive_phase1(msg.raw_packet),
         v => Err(ContractError::UnknownProtocol {
             channel_id: msg.channel,
-            protocol_version: v,
+            protocol_version: v.into(),
         }),
     }
 }

diff --git a/cosmwasm/ucs01-relay/src/ibc.rs b/cosmwasm/ucs01-relay/src/ibc.rs
@@ -126,8 +126,8 @@ pub fn ibc_packet_receive(
         funds: Default::default(),
     };
 
-    match channel_info.protocol_version {
-        ref v if v == Ics20Protocol::VERSION => Ok(Ics20Protocol {
+    match channel_info.protocol_version.as_str() {
+        Ics20Protocol::VERSION => Ok(Ics20Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -136,7 +136,7 @@ pub fn ibc_packet_receive(
             },
         }
         .receive_phase0(msg.packet.data)),
-        ref v if v == Ucs01Protocol::VERSION => Ok(Ucs01Protocol {
+        Ucs01Protocol::VERSION => Ok(Ucs01Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -147,7 +147,7 @@ pub fn ibc_packet_receive(
         .receive_phase0(msg.packet.data)),
         v => Err(ContractError::UnknownProtocol {
             channel_id: msg.packet.dest.channel_id,
-            protocol_version: v,
+            protocol_version: v.into(),
         }),
     }
 }
@@ -166,8 +166,8 @@ pub fn ibc_packet_ack(
         funds: Default::default(),
     };
 
-    match channel_info.protocol_version {
-        ref v if v == Ics20Protocol::VERSION => Ics20Protocol {
+    match channel_info.protocol_version.as_str() {
+        Ics20Protocol::VERSION => Ics20Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -176,7 +176,7 @@ pub fn ibc_packet_ack(
             },
         }
         .send_ack(msg.acknowledgement.data, msg.original_packet.data),
-        ref v if v == Ucs01Protocol::VERSION => Ucs01Protocol {
+        Ucs01Protocol::VERSION => Ucs01Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -187,7 +187,7 @@ pub fn ibc_packet_ack(
         .send_ack(msg.acknowledgement.data, msg.original_packet.data),
         v => Err(ContractError::UnknownProtocol {
             channel_id: msg.original_packet.dest.channel_id,
-            protocol_version: v,
+            protocol_version: v.into(),
         }),
     }
 }
@@ -206,8 +206,8 @@ pub fn ibc_packet_timeout(
         funds: Default::default(),
     };
 
-    match channel_info.protocol_version {
-        ref v if v == Ics20Protocol::VERSION => Ics20Protocol {
+    match channel_info.protocol_version.as_str() {
+        Ics20Protocol::VERSION => Ics20Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -216,7 +216,7 @@ pub fn ibc_packet_timeout(
             },
         }
         .send_timeout(msg.packet.data),
-        ref v if v == Ucs01Protocol::VERSION => Ucs01Protocol {
+        Ucs01Protocol::VERSION => Ucs01Protocol {
             common: ProtocolCommon {
                 deps,
                 env,
@@ -227,7 +227,7 @@ pub fn ibc_packet_timeout(
         .send_timeout(msg.packet.data),
         v => Err(ContractError::UnknownProtocol {
             channel_id: msg.packet.dest.channel_id,
-            protocol_version: v,
+            protocol_version: v.into(),
         }),
     }
 }
diff --git a/cosmwasm/ucs01-relay/src/protocol.rs b/cosmwasm/ucs01-relay/src/protocol.rs
@@ -19,8 +19,8 @@ use crate::{
 
 pub fn protocol_ordering(version: &str) -> Option<IbcOrder> {
     match version {
-        v if v == Ics20Protocol::VERSION => Some(Ics20Protocol::ORDERING),
-        v if v == Ucs01Protocol::VERSION => Some(Ucs01Protocol::ORDERING),
+        Ics20Protocol::VERSION => Some(Ics20Protocol::ORDERING),
+        Ucs01Protocol::VERSION => Some(Ucs01Protocol::ORDERING),
         _ => None,
     }
 }