imp(types): refactor Default implementations with concrete names (#1099)

* use concrete channel order enum

* refactor(ics04): remove Version::default()

* refactor(ics04): remove TimeoutHeight::default()

* refactor: remove channelId::default()

* fix: Version::default() to Version::empty()

* remove derive default from Timestamp

* refactor default builder

* remove Sequence::default()

* remove Memo::default()

* refactor: add ChannelId::zero()

* add From<&str> implmentation

* refactor(ibccore): remove ClientId::default()

* refactor(ibccore): remove version::Default()

* refactor(ibccore): remove ConnectionEnd::default()

* refactor(ibc-apps): remove TracePath::default()

* refactor(ibc-core): remove ProofSpecs::default()

* fix(ibc-app): remove upgrade path default

* remove trust threshold default

* refactor(ibc-testkit): remove Default from client_state initialization

* refactor(ibc-testkit): remove Default() of ClientId, ConnectionId,...

* chore: add unclog

* refactor: add Connection::zero()

* use into() instead of ::from()

* refactor: add Version::compatibles()

* refactor(ics24-host): make 07-tendermint-0 a constant in tests

* chore: remove commented out impl Default

* add dummy ClientId

* refactor(ics24-host): remove From<&str> to avoid panic on host

* remove get_compatible_versions

* fix syntax error when import Version

* chore: run cargo fmt

* revert markdown format

* various refactor

* chore: update unclog

.changelog/unreleased/improvements/1074-refactor-default-implemetation.md

@@ -0,0 +1,2 @@
+- [types] Refactor `Default` implementations with concrete names
+  ([\#1074](https://github.com/cosmos/ibc-rs/issues/1074))

docs/architecture/adr-005-handlers-redesign.md

@@ -296,7 +296,7 @@ trait ValidationContext {
     /// Function required by ICS 03. Returns the list of all possible versions that the connection
     /// handshake protocol supports.
     fn get_compatible_versions(&self) -> Vec<Version> {
-        get_compatible_versions()
+        ConnectionVersion::compatibles()
     }
 
     /// Function required by ICS 03. Returns one version out of the supplied list of versions, which the

ibc-apps/ics20-transfer/types/src/denom.rs

@@ -109,7 +109,7 @@ impl Display for TracePrefix {
 )]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[derive(Clone, Debug, Default, Eq, PartialEq, PartialOrd, Ord, From)]
+#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, From)]
 pub struct TracePath(Vec<TracePrefix>);
 
 impl TracePath {
@@ -134,6 +134,11 @@ impl TracePath {
     pub fn is_empty(&self) -> bool {
         self.0.is_empty()
     }
+
+    /// Return empty trace path
+    pub fn empty() -> Self {
+        Self(vec![])
+    }
 }
 
 impl<'a> TryFrom<Vec<&'a str>> for TracePath {
@@ -294,7 +299,7 @@ impl FromStr for PrefixedDenom {
 
         let (base_denom, trace_path) = {
             if last_part == s {
-                (BaseDenom::from_str(s)?, TracePath::default())
+                (BaseDenom::from_str(s)?, TracePath::empty())
             } else {
                 let base_denom = BaseDenom::from_str(last_part)?;
                 let trace_path = TracePath::try_from(parts)?;
@@ -334,7 +339,7 @@ impl From<PrefixedDenom> for RawDenomTrace {
 impl From<BaseDenom> for PrefixedDenom {
     fn from(denom: BaseDenom) -> Self {
         Self {
-            trace_path: Default::default(),
+            trace_path: TracePath::empty(),
             base_denom: denom,
         }
     }

ibc-apps/ics20-transfer/types/src/memo.rs

@@ -45,6 +45,12 @@ impl From<String> for Memo {
     }
 }
 
+impl From<&str> for Memo {
+    fn from(memo: &str) -> Self {
+        Self(memo.to_owned())
+    }
+}
+
 impl FromStr for Memo {
     type Err = Infallible;
 

ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs

@@ -81,14 +81,14 @@ where
                 &msg.chan_id_on_a,
                 class_id,
                 token_id,
-                &packet_data.memo.clone().unwrap_or_default(),
+                &packet_data.memo.clone().unwrap_or("".into()),
             )?;
         } else {
             transfer_ctx.burn_nft_validate(
                 &sender,
                 class_id,
                 token_id,
-                &packet_data.memo.clone().unwrap_or_default(),
+                &packet_data.memo.clone().unwrap_or("".into()),
             )?;
         }
         let nft = transfer_ctx.get_nft(class_id, token_id)?;
@@ -184,14 +184,14 @@ where
                 &msg.chan_id_on_a,
                 class_id,
                 token_id,
-                &packet_data.memo.clone().unwrap_or_default(),
+                &packet_data.memo.clone().unwrap_or("".into()),
             )?;
         } else {
             transfer_ctx.burn_nft_execute(
                 &sender,
                 class_id,
                 token_id,
-                &packet_data.memo.clone().unwrap_or_default(),
+                &packet_data.memo.clone().unwrap_or("".into()),
             )?;
         }
         let nft = transfer_ctx.get_nft(class_id, token_id)?;
@@ -242,7 +242,7 @@ where
             receiver: packet_data.receiver,
             class: packet_data.class_id,
             tokens: packet_data.token_ids,
-            memo: packet_data.memo.unwrap_or_default(),
+            memo: packet_data.memo.unwrap_or("".into()),
         };
         send_packet_ctx_a.emit_ibc_event(ModuleEvent::from(transfer_event).into())?;
 

ibc-apps/ics721-nft-transfer/src/module.rs

@@ -1,5 +1,4 @@
 //! Provides IBC module callbacks implementation for the ICS-721 transfer.
-
 use ibc_core::channel::types::acknowledgement::{Acknowledgement, AcknowledgementStatus};
 use ibc_core::channel::types::channel::{Counterparty, Order};
 use ibc_core::channel::types::packet::Packet;
@@ -190,7 +189,7 @@ pub fn on_recv_packet_execute(
         receiver: data.receiver,
         class: data.class_id,
         tokens: data.token_ids,
-        memo: data.memo.unwrap_or_default(),
+        memo: data.memo.unwrap_or("".into()),
         success: ack.is_successful(),
     };
     extras.events.push(recv_event.into());
@@ -250,7 +249,7 @@ pub fn on_acknowledgement_packet_execute(
         receiver: data.receiver,
         class: data.class_id,
         tokens: data.token_ids,
-        memo: data.memo.unwrap_or_default(),
+        memo: data.memo.unwrap_or("".into()),
         acknowledgement: acknowledgement.clone(),
     };
 
@@ -295,7 +294,7 @@ pub fn on_timeout_packet_execute(
         refund_receiver: data.sender,
         refund_class: data.class_id,
         refund_tokens: data.token_ids,
-        memo: data.memo.unwrap_or_default(),
+        memo: data.memo.unwrap_or("".into()),
     };
 
     let extras = ModuleExtras {

ibc-apps/ics721-nft-transfer/types/src/class.rs

@@ -106,7 +106,7 @@ impl Display for TracePrefix {
 )]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[derive(Clone, Debug, Default, Eq, PartialEq, PartialOrd, Ord, From)]
+#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, From)]
 pub struct TracePath(Vec<TracePrefix>);
 
 impl TracePath {
@@ -131,6 +131,11 @@ impl TracePath {
     pub fn is_empty(&self) -> bool {
         self.0.is_empty()
     }
+
+    /// Return empty trace path
+    pub fn empty() -> Self {
+        Self(vec![])
+    }
 }
 
 impl<'a> TryFrom<Vec<&'a str>> for TracePath {
@@ -268,7 +273,7 @@ impl FromStr for PrefixedClassId {
 
         let (base_class_id, trace_path) = {
             if last_part == s {
-                (ClassId::from_str(s)?, TracePath::default())
+                (ClassId::from_str(s)?, TracePath::empty())
             } else {
                 let base_class_id = ClassId::from_str(last_part)?;
                 let trace_path = TracePath::try_from(parts)?;
@@ -308,7 +313,7 @@ impl From<PrefixedClassId> for RawClassTrace {
 impl From<ClassId> for PrefixedClassId {
     fn from(class_id: ClassId) -> Self {
         Self {
-            trace_path: Default::default(),
+            trace_path: TracePath::empty(),
             base_class_id: class_id,
         }
     }

ibc-apps/ics721-nft-transfer/types/src/data.rs

@@ -24,7 +24,7 @@ use crate::error::NftTransferError;
     derive(borsh::BorshSerialize, borsh::BorshDeserialize)
 )]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[derive(Clone, Debug, Default, PartialEq, Eq, derive_more::From)]
+#[derive(Clone, Debug, PartialEq, Eq, derive_more::From)]
 pub struct Data(String);
 
 #[cfg(feature = "serde")]
@@ -88,7 +88,7 @@ impl<'de> serde::Deserialize<'de> for Data {
 )]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Ics721Data(BTreeMap<String, DataValue>);
 
 #[cfg(feature = "serde")]
@@ -100,7 +100,7 @@ impl FromStr for Ics721Data {
     }
 }
 
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct DataValue {
     value: String,
     mime: Option<Mime>,

ibc-apps/ics721-nft-transfer/types/src/memo.rs

@@ -24,7 +24,7 @@ use ibc_core::primitives::prelude::*;
 )]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Memo(String);
 
 impl AsRef<str> for Memo {
@@ -45,6 +45,12 @@ impl From<String> for Memo {
     }
 }
 
+impl From<&str> for Memo {
+    fn from(memo: &str) -> Self {
+        Self(memo.to_owned())
+    }
+}
+
 impl FromStr for Memo {
     type Err = Infallible;
 

ibc-clients/ics07-tendermint/src/client_state.rs

@@ -111,8 +111,8 @@ mod tests {
             unbonding_period: Duration::new(128_000, 0),
             max_clock_drift: Duration::new(3, 0),
             latest_height: Height::new(1, 10).expect("Never fails"),
-            proof_specs: ProofSpecs::default(),
-            upgrade_path: Default::default(),
+            proof_specs: ProofSpecs::cosmos(),
+            upgrade_path: Vec::new(),
             allow_update: AllowUpdate {
                 after_expiry: false,
                 after_misbehaviour: false,

ibc-clients/ics07-tendermint/types/src/client_state.rs

@@ -438,8 +438,8 @@ mod tests {
             unbonding_period: Duration::new(128_000, 0),
             max_clock_drift: Duration::new(3, 0),
             latest_height: Height::new(0, 10).expect("Never fails"),
-            proof_specs: ProofSpecs::default(),
-            upgrade_path: Default::default(),
+            proof_specs: ProofSpecs::cosmos(),
+            upgrade_path: Vec::new(),
             allow_update: AllowUpdate {
                 after_expiry: false,
                 after_misbehaviour: false,

ibc-clients/ics07-tendermint/types/src/trust_threshold.rs

@@ -134,12 +134,6 @@ impl TryFrom<Fraction> for TrustThreshold {
     }
 }
 
-impl Default for TrustThreshold {
-    fn default() -> Self {
-        Self::ONE_THIRD
-    }
-}
-
 impl Display for TrustThreshold {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> {
         write!(f, "{}/{}", self.numerator, self.denominator)

ibc-core/ics03-connection/types/src/connection.rs

@@ -1,6 +1,7 @@
 //! Defines the types that define a connection
 
 use core::fmt::{Display, Error as FmtError, Formatter};
+use core::str::FromStr;
 use core::time::Duration;
 use core::u64;
 
@@ -200,18 +201,6 @@ mod sealed {
     }
 }
 
-impl Default for ConnectionEnd {
-    fn default() -> Self {
-        Self {
-            state: State::Uninitialized,
-            client_id: Default::default(),
-            counterparty: Default::default(),
-            versions: Vec::new(),
-            delay_period: ZERO_DURATION,
-        }
-    }
-}
-
 impl Protobuf<RawConnectionEnd> for ConnectionEnd {}
 
 impl TryFrom<RawConnectionEnd> for ConnectionEnd {
@@ -220,7 +209,17 @@ impl TryFrom<RawConnectionEnd> for ConnectionEnd {
         let state = value.state.try_into()?;
 
         if state == State::Uninitialized {
-            return Ok(ConnectionEnd::default());
+            return ConnectionEnd::new(
+                State::Uninitialized,
+                ClientId::from_str("07-tendermint-0").expect("should not fail"),
+                Counterparty::new(
+                    ClientId::from_str("07-tendermint-0").expect("should not fail"),
+                    None,
+                    CommitmentPrefix::empty(),
+                ),
+                Vec::new(),
+                ZERO_DURATION,
+            );
         }
 
         if value.client_id.is_empty() {
@@ -378,7 +377,7 @@ impl ConnectionEnd {
 )]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct Counterparty {
     pub client_id: ClientId,
     pub connection_id: Option<ConnectionId>,

ibc-core/ics03-connection/types/src/events.rs

@@ -29,7 +29,7 @@ pub const COUNTERPARTY_CLIENT_ID_ATTRIBUTE_KEY: &str = "counterparty_client_id";
     derive(borsh::BorshSerialize, borsh::BorshDeserialize)
 )]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 struct Attributes {
     pub connection_id: ConnectionId,
     pub client_id: ClientId,
@@ -323,7 +323,7 @@ mod tests {
 
         let client_type = ClientType::from_str("07-tendermint")
             .expect("never fails because it's a valid client type");
-        let conn_id_on_a = ConnectionId::default();
+        let conn_id_on_a = ConnectionId::zero();
         let client_id_on_a = client_type.build_client_id(0);
         let conn_id_on_b = ConnectionId::new(1);
         let client_id_on_b = client_type.build_client_id(1);