test: include PacketData serde test to keep proper JSON encoding

.changelog/unreleased/improvements/763-include-PacketData-JSON-encoding-test.md

@@ -0,0 +1,2 @@
+- Add test to ensure `PacketData`` keeps proper JSON encoding
+  ([#763](https://github.com/cosmos/ibc-rs/issues/763))

crates/ibc/src/applications/transfer/msgs/transfer.rs

@@ -127,71 +127,44 @@ pub mod test_util {
 
     use core::ops::Add;
     use core::time::Duration;
-    use primitive_types::U256;
 
     use super::MsgTransfer;
     use crate::applications::transfer::packet::PacketData;
-    use crate::applications::transfer::Coin;
     use crate::core::ics04_channel::packet::{Packet, Sequence};
     use crate::core::ics04_channel::timeout::TimeoutHeight;
-    use crate::signer::Signer;
-    use crate::{
-        applications::transfer::BaseCoin,
-        core::ics24_host::identifier::{ChannelId, PortId},
-        test_utils::get_dummy_bech32_account,
-    };
-
-    // Returns a dummy ICS20 `MsgTransfer`. If no `timeout_timestamp` is
-    // specified, a timestamp of 10 seconds in the future is used.
-    pub fn get_dummy_msg_transfer(
-        timeout_height: TimeoutHeight,
-        timeout_timestamp: Option<Timestamp>,
-    ) -> MsgTransfer {
-        let address: Signer = get_dummy_bech32_account().into();
-        MsgTransfer {
-            port_id_on_a: PortId::default(),
-            chan_id_on_a: ChannelId::default(),
-            packet_data: PacketData {
-                token: BaseCoin {
-                    denom: "uatom".parse().unwrap(),
-                    amount: U256::from(10).into(),
-                }
-                .into(),
-                sender: address.clone(),
-                receiver: address,
-                memo: "".to_owned().into(),
-            },
-            timeout_timestamp_on_b: timeout_timestamp
-                .unwrap_or_else(|| Timestamp::now().add(Duration::from_secs(10)).unwrap()),
-            timeout_height_on_b: timeout_height,
+    use crate::core::ics24_host::identifier::{ChannelId, PortId};
+
+    impl MsgTransfer {
+        // Returns a dummy ICS20 `MsgTransfer`. If no `timeout_timestamp` is
+        // specified, a timestamp of 10 seconds in the future is used.
+        pub fn new_dummy(
+            timeout_height: TimeoutHeight,
+            timeout_timestamp: Option<Timestamp>,
+        ) -> Self {
+            Self {
+                port_id_on_a: PortId::default(),
+                chan_id_on_a: ChannelId::default(),
+                packet_data: PacketData::new_dummy(),
+                timeout_timestamp_on_b: timeout_timestamp
+                    .unwrap_or_else(|| Timestamp::now().add(Duration::from_secs(10)).unwrap()),
+                timeout_height_on_b: timeout_height,
+            }
         }
-    }
 
-    pub fn get_dummy_transfer_packet(msg: MsgTransfer, sequence: Sequence) -> Packet {
-        let coin = Coin {
-            denom: msg.packet_data.token.denom.clone(),
-            amount: msg.packet_data.token.amount,
-        };
-
-        let data = {
-            let data = PacketData {
-                token: coin,
-                sender: msg.packet_data.sender.clone(),
-                receiver: msg.packet_data.receiver.clone(),
-                memo: msg.packet_data.memo.clone(),
-            };
-            serde_json::to_vec(&data).expect("PacketData's infallible Serialize impl failed")
-        };
-
-        Packet {
-            seq_on_a: sequence,
-            port_id_on_a: msg.port_id_on_a,
-            chan_id_on_a: msg.chan_id_on_a,
-            port_id_on_b: PortId::default(),
-            chan_id_on_b: ChannelId::default(),
-            data,
-            timeout_height_on_b: msg.timeout_height_on_b,
-            timeout_timestamp_on_b: msg.timeout_timestamp_on_b,
+        pub fn get_transfer_packet(&self, sequence: Sequence) -> Packet {
+            let data = serde_json::to_vec(&self.packet_data)
+                .expect("PacketData's infallible Serialize impl failed");
+
+            Packet {
+                seq_on_a: sequence,
+                port_id_on_a: self.port_id_on_a.clone(),
+                chan_id_on_a: self.chan_id_on_a.clone(),
+                port_id_on_b: PortId::default(),
+                chan_id_on_b: ChannelId::default(),
+                data,
+                timeout_height_on_b: self.timeout_height_on_b,
+                timeout_timestamp_on_b: self.timeout_timestamp_on_b,
+            }
         }
     }
 }

crates/ibc/src/applications/transfer/packet.rs

@@ -55,3 +55,57 @@ impl From<PacketData> for RawPacketData {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    use super::*;
+    use crate::applications::transfer::BaseCoin;
+    use crate::test_utils::get_dummy_bech32_account;
+    use primitive_types::U256;
+
+    impl PacketData {
+        pub fn new_dummy() -> Self {
+            let address: Signer = get_dummy_bech32_account().into();
+
+            Self {
+                token: BaseCoin {
+                    denom: "uatom".parse().unwrap(),
+                    amount: U256::from(10).into(),
+                }
+                .into(),
+                sender: address.clone(),
+                receiver: address,
+                memo: "".to_string().into(),
+            }
+        }
+
+        pub fn ser_json_assert_eq(&self, json: &str) {
+            let ser = serde_json::to_string(&self).unwrap();
+            assert_eq!(ser, json);
+        }
+
+        pub fn deser_json_assert_eq(&self, json: &str) {
+            let deser: Self = serde_json::from_str(json).unwrap();
+            assert_eq!(&deser, self);
+        }
+    }
+
+    pub fn dummy_json_packet_data() -> &'static str {
+        r#"{"denom":"uatom","amount":"10","sender":"cosmos1wxeyh7zgn4tctjzs0vtqpc6p5cxq5t2muzl7ng","receiver":"cosmos1wxeyh7zgn4tctjzs0vtqpc6p5cxq5t2muzl7ng","memo":""}"#
+    }
+
+    /// Ensures `PacketData` properly encodes to JSON by first converting to a
+    /// `RawPacketData` and then serializing that.
+    #[test]
+    fn test_packet_data_ser() {
+        PacketData::new_dummy().ser_json_assert_eq(dummy_json_packet_data());
+    }
+
+    /// Ensures `PacketData` properly decodes from JSON by first deserializing to a
+    /// `RawPacketData` and then converting from that.
+    #[test]
+    fn test_packet_data_deser() {
+        PacketData::new_dummy().deser_json_assert_eq(dummy_json_packet_data());
+    }
+}

crates/ibc/src/core/handler.rs

@@ -167,12 +167,8 @@ mod tests {
     use test_log::test;
 
     use crate::applications::transfer::error::TokenTransferError;
-    use crate::applications::transfer::msgs::transfer::test_util::get_dummy_transfer_packet;
     use crate::applications::transfer::send_transfer;
-    use crate::applications::transfer::{
-        msgs::transfer::test_util::get_dummy_msg_transfer, msgs::transfer::MsgTransfer,
-        packet::PacketData, PrefixedCoin, MODULE_ID_STR,
-    };
+    use crate::applications::transfer::{msgs::transfer::MsgTransfer, MODULE_ID_STR};
     use crate::core::dispatch;
     use crate::core::events::{IbcEvent, MessageEvent};
     use crate::core::ics02_client::msgs::{
@@ -321,10 +317,10 @@ mod tests {
             MsgChannelCloseConfirm::try_from(get_dummy_raw_msg_chan_close_confirm(client_height))
                 .unwrap();
 
-        let msg_transfer = get_dummy_msg_transfer(Height::new(0, 35).unwrap().into(), None);
-        let msg_transfer_two = get_dummy_msg_transfer(Height::new(0, 36).unwrap().into(), None);
-        let msg_transfer_no_timeout = get_dummy_msg_transfer(TimeoutHeight::no_timeout(), None);
-        let msg_transfer_no_timeout_or_timestamp = get_dummy_msg_transfer(
+        let msg_transfer = MsgTransfer::new_dummy(Height::new(0, 35).unwrap().into(), None);
+        let msg_transfer_two = MsgTransfer::new_dummy(Height::new(0, 36).unwrap().into(), None);
+        let msg_transfer_no_timeout = MsgTransfer::new_dummy(TimeoutHeight::no_timeout(), None);
+        let msg_transfer_no_timeout_or_timestamp = MsgTransfer::new_dummy(
             TimeoutHeight::no_timeout(),
             Some(Timestamp::from_nanoseconds(0).unwrap()),
         );
@@ -335,24 +331,14 @@ mod tests {
         msg_to_on_close.packet.timeout_height_on_b = msg_transfer_two.timeout_height_on_b;
         msg_to_on_close.packet.timeout_timestamp_on_b = msg_transfer_two.timeout_timestamp_on_b;
 
-        let denom = msg_transfer_two.packet_data.token.denom.clone();
-        let packet_data = {
-            let data = PacketData {
-                token: PrefixedCoin {
-                    denom,
-                    amount: msg_transfer_two.packet_data.token.amount,
-                },
-                sender: msg_transfer_two.packet_data.sender.clone(),
-                receiver: msg_transfer_two.packet_data.receiver.clone(),
-                memo: "".to_owned().into(),
-            };
-            serde_json::to_vec(&data).expect("PacketData's infallible Serialize impl failed")
-        };
+        let packet_data = serde_json::to_vec(&msg_transfer_two.packet_data)
+            .expect("PacketData's infallible Serialize impl failed");
+
         msg_to_on_close.packet.data = packet_data;
 
         let msg_recv_packet = MsgRecvPacket::try_from(get_dummy_raw_msg_recv_packet(35)).unwrap();
         let msg_ack_packet = MsgAcknowledgement::try_from(get_dummy_raw_msg_ack_with_packet(
-            get_dummy_transfer_packet(msg_transfer.clone(), 1u64.into()).into(),
+            msg_transfer.get_transfer_packet(1u64.into()).into(),
             35,
         ))
         .unwrap();