Add serde feature (#331)

* Add serde feature

* Fix clippy warnings

* Feature gate serde imports

* Fix for `ErasedSerialize`

* Fix unused imports

* Fix CI

* cargo fmt

* Add serde feature to no-std-check's ibc dep

* Document ErasedSerialize re-export

* Move TimeoutHeight serde into it's own module

* Move ClientState serde tests into it's own module

* Fix typo

* Apply suggestions from code review

Co-authored-by: Philippe Laferrière <[email protected]>
Signed-off-by: Shoaib Ahmed <[email protected]>

* Update issue templates and CONTRIBUTING.md (#345)

* Update issue templates and CONTRIBUTING.md

* Reflect review input

* few edits

Co-authored-by: Philippe Laferriere <[email protected]>

* make transfer application serde-dependent

Signed-off-by: Shoaib Ahmed <[email protected]>
Co-authored-by: Davirain <[email protected]>
Co-authored-by: Philippe Laferrière <[email protected]>
Co-authored-by: Farhad Shabani <[email protected]>

Author: 4 people

ci/no-std-check/Cargo.toml

@@ -5,7 +5,7 @@ edition = "2021"
 resolver = "2"
 
 [dependencies]
-ibc = { path = "../../crates/ibc", default-features = false, features = ["mocks-no-std"] }
+ibc = { path = "../../crates/ibc", default-features = false, features = ["serde", "mocks-no-std"] }
 ibc-proto = { version = "0.24.1", default-features = false, features = ["parity-scale-codec", "borsh"]}
 tendermint = { version = "0.28.0", default-features = false }
 tendermint-proto = { version = "0.28.0", default-features = false }

crates/ibc/Cargo.toml

@@ -39,24 +39,26 @@ std = [
 parity-scale-codec = ["dep:parity-scale-codec", "dep:scale-info"]
 borsh = ["dep:borsh"]
 
+# This feature is required for token transfer (ICS-20)
+serde = ["dep:serde", "dep:serde_derive", "serde_json", "erased-serde"]
+
 # This feature guards the unfinished implementation of ADR 5.
 val_exec_ctx = []
 
 # This feature grants access to development-time mocking libraries, such as `MockContext` or `MockHeader`.
 # Depends on the `testgen` suite for generating Tendermint light blocks.
 mocks = ["tendermint-testgen", "tendermint/clock", "cfg-if", "parking_lot"]
 mocks-no-std = ["cfg-if"]
-serde = []
 
 [dependencies]
 # Proto definitions for all IBC-related interfaces, e.g., connections or channels.
 ibc-proto = { version = "0.24.1", default-features = false, features = ["parity-scale-codec", "borsh"] }
 ics23 = { version = "0.9.0", default-features = false, features = ["host-functions"] }
 time = { version = ">=0.3.0, <0.3.18", default-features = false }
-serde_derive = { version = "1.0.104", default-features = false }
-serde = { version = "1.0", default-features = false }
-serde_json = { version = "1", default-features = false }
-erased-serde = { version = "0.3", default-features = false, features = ["alloc"] }
+serde_derive = { version = "1.0.104", default-features = false, optional = true }
+serde = { version = "1.0", default-features = false, optional = true }
+serde_json = { version = "1", default-features = false, optional = true }
+erased-serde = { version = "0.3", default-features = false, features = ["alloc"], optional = true }
 tracing = { version = "0.1.36", default-features = false }
 prost = { version = "0.11", default-features = false }
 bytes = { version = "1.2.1", default-features = false }

crates/ibc/src/applications/mod.rs

@@ -1,3 +1,4 @@
 //! Various packet encoding semantics which underpin the various types of transactions.
 
+#[cfg(feature = "serde")]
 pub mod transfer;

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

@@ -1,7 +1,5 @@
 use core::fmt::{Display, Error as FmtError, Formatter};
 
-use serde::{Deserialize, Serialize};
-
 use super::error::TokenTransferError;
 use crate::core::ics26_routing::context::Acknowledgement as AckTrait;
 use crate::prelude::*;
@@ -13,21 +11,23 @@ pub const ACK_ERR_STR: &str = "error handling packet on destination chain: see e
 /// A successful acknowledgement, equivalent to `base64::encode(0x01)`.
 pub const ACK_SUCCESS_B64: &str = "AQ==";
 
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum ConstAckSuccess {
-    #[serde(rename = "AQ==")]
+    #[cfg_attr(feature = "serde", serde(rename = "AQ=="))]
     Success,
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum Acknowledgement {
     /// Successful Acknowledgement
     /// e.g. `{"result":"AQ=="}`
-    #[serde(rename = "result")]
+    #[cfg_attr(feature = "serde", serde(rename = "result"))]
     Success(ConstAckSuccess),
     /// Error Acknowledgement
     /// e.g. `{"error":"cannot unmarshal ICS-20 transfer packet data"}`
-    #[serde(rename = "error")]
+    #[cfg_attr(feature = "serde", serde(rename = "error"))]
     Error(String),
 }
 

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

@@ -1,14 +1,12 @@
 use core::str::FromStr;
 use derive_more::{Display, From, Into};
-use serde::{Deserialize, Serialize};
 
 use super::error::TokenTransferError;
 use primitive_types::U256;
 
 /// A type for representing token transfer amounts.
-#[derive(
-    Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize, Display, From, Into,
-)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Display, From, Into)]
 pub struct Amount(U256);
 
 impl Amount {

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

@@ -2,12 +2,13 @@ use core::fmt::{Display, Error as FmtError, Formatter};
 use core::str::{from_utf8, FromStr};
 use ibc_proto::cosmos::base::v1beta1::Coin as ProtoCoin;
 use safe_regex::regex;
-use serde::{Deserialize, Serialize};
 
 use super::amount::Amount;
 use super::denom::{BaseDenom, PrefixedDenom};
 use super::error::TokenTransferError;
 use crate::prelude::*;
+
+#[cfg(feature = "serde")]
 use crate::serializers::serde_string;
 
 /// A `Coin` type with fully qualified `PrefixedDenom`.
@@ -19,12 +20,13 @@ pub type BaseCoin = Coin<BaseDenom>;
 pub type RawCoin = Coin<String>;
 
 /// Coin defines a token with a denomination and an amount.
-#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 pub struct Coin<D> {
     /// Denomination
     pub denom: D,
     /// Amount
-    #[serde(with = "serde_string")]
+    #[cfg_attr(feature = "serde", serde(with = "serde_string"))]
     pub amount: Amount,
 }
 

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

@@ -178,14 +178,14 @@ pub fn on_chan_open_init(
         });
     }
 
-    if !version.is_empty() && version != &Version::ics20() {
+    if !version.is_empty() && version != &Version::new(VERSION.to_string()) {
         return Err(TokenTransferError::InvalidVersion {
-            expect_version: Version::ics20(),
+            expect_version: Version::new(VERSION.to_string()),
             got_version: version.clone(),
         });
     }
 
-    Ok((ModuleExtras::empty(), Version::ics20()))
+    Ok((ModuleExtras::empty(), Version::new(VERSION.to_string())))
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -204,14 +204,14 @@ pub fn on_chan_open_try(
             got_order: order,
         });
     }
-    if counterparty_version != &Version::ics20() {
+    if counterparty_version != &Version::new(VERSION.to_string()) {
         return Err(TokenTransferError::InvalidCounterpartyVersion {
-            expect_version: Version::ics20(),
+            expect_version: Version::new(VERSION.to_string()),
             got_version: counterparty_version.clone(),
         });
     }
 
-    Ok((ModuleExtras::empty(), Version::ics20()))
+    Ok((ModuleExtras::empty(), Version::new(VERSION.to_string())))
 }
 
 pub fn on_chan_open_ack(
@@ -220,9 +220,9 @@ pub fn on_chan_open_ack(
     _channel_id: &ChannelId,
     counterparty_version: &Version,
 ) -> Result<ModuleExtras, TokenTransferError> {
-    if counterparty_version != &Version::ics20() {
+    if counterparty_version != &Version::new(VERSION.to_string()) {
         return Err(TokenTransferError::InvalidCounterpartyVersion {
-            expect_version: Version::ics20(),
+            expect_version: Version::new(VERSION.to_string()),
             got_version: counterparty_version.clone(),
         });
     }
@@ -335,6 +335,7 @@ pub fn on_timeout_packet(
 
 #[cfg(test)]
 pub(crate) mod test {
+    use super::*;
     use subtle_encoding::bech32;
 
     use crate::applications::transfer::context::{cosmos_adr028_escrow_address, on_chan_open_try};
@@ -347,7 +348,6 @@ pub(crate) mod test {
     use crate::core::ics04_channel::Version;
     use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
     use crate::handler::HandlerOutputBuilder;
-    use crate::prelude::*;
     use crate::test_utils::{get_dummy_transfer_module, DummyTransferModule};
 
     use super::on_chan_open_init;
@@ -436,15 +436,15 @@ pub(crate) mod test {
         )
         .unwrap();
 
-        assert_eq!(out_version, Version::ics20());
+        assert_eq!(out_version, Version::new(VERSION.to_string()));
     }
 
     /// If the relayer passed in the only supported version (ics20), then return ics20
     #[test]
     fn test_on_chan_open_init_ics20_version() {
         let (mut ctx, order, connection_hops, port_id, channel_id, counterparty) = get_defaults();
 
-        let in_version = Version::ics20();
+        let in_version = Version::new(VERSION.to_string());
         let (_, out_version) = on_chan_open_init(
             &mut ctx,
             order,
@@ -456,7 +456,7 @@ pub(crate) mod test {
         )
         .unwrap();
 
-        assert_eq!(out_version, Version::ics20());
+        assert_eq!(out_version, Version::new(VERSION.to_string()));
     }
 
     /// If the relayer passed in an unsupported version, then fail
@@ -483,7 +483,7 @@ pub(crate) mod test {
     fn test_on_chan_open_try_counterparty_correct_version() {
         let (mut ctx, order, connection_hops, port_id, channel_id, counterparty) = get_defaults();
 
-        let counterparty_version = Version::ics20();
+        let counterparty_version = Version::new(VERSION.to_string());
 
         let (_, out_version) = on_chan_open_try(
             &mut ctx,
@@ -496,7 +496,7 @@ pub(crate) mod test {
         )
         .unwrap();
 
-        assert_eq!(out_version, Version::ics20());
+        assert_eq!(out_version, Version::new(VERSION.to_string()));
     }
 
     /// If the counterparty doesn't support ics20, then fail

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

@@ -3,16 +3,18 @@ use core::str::FromStr;
 
 use derive_more::{Display, From};
 use ibc_proto::ibc::applications::transfer::v1::DenomTrace as RawDenomTrace;
-use serde::{Deserialize, Serialize};
 
 use super::error::TokenTransferError;
 use crate::core::ics24_host::identifier::{ChannelId, PortId};
 use crate::prelude::*;
+
+#[cfg(feature = "serde")]
 use crate::serializers::serde_string;
 
 /// Base denomination type
-#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize, Display)]
-#[serde(transparent)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "serde", serde(transparent))]
+#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Display)]
 pub struct BaseDenom(String);
 
 impl BaseDenom {
@@ -147,10 +149,11 @@ impl Display for TracePath {
 }
 
 /// A type that contains the base denomination for ICS20 and the source tracing information path.
-#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 pub struct PrefixedDenom {
     /// A series of `{port-id}/{channel-id}`s for tracing the source of the token.
-    #[serde(with = "serde_string")]
+    #[cfg_attr(feature = "serde", serde(with = "serde_string"))]
     pub trace_path: TracePath,
     /// Base denomination of the relayed fungible token.
     pub base_denom: BaseDenom,

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

@@ -3,14 +3,17 @@ use core::convert::TryFrom;
 use core::str::FromStr;
 
 use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData;
-use serde::{Deserialize, Serialize};
 
 use super::error::TokenTransferError;
 use super::{Amount, PrefixedCoin, PrefixedDenom};
 use crate::signer::Signer;
 
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(try_from = "RawPacketData", into = "RawPacketData")]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(
+    feature = "serde",
+    serde(try_from = "RawPacketData", into = "RawPacketData")
+)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct PacketData {
     pub token: PrefixedCoin,
     pub sender: Signer,