imp(types): refactor Default implementations with concrete names

.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

@@ -1,10 +1,12 @@
 # ADR 005: Handlers validation and execution separation
 
 ## Status
+
 Accepted
 
 ## Changelog
-* 9/9/2022: initial proposal
+
+- 9/9/2022: initial proposal
 
 ## Context
 
@@ -15,9 +17,9 @@ Our current `deliver()` entrypoint can then be split into 2 entrypoints: `valida
 This ADR will only concern itself with the external-facing API.
 
 The following are out of scope for this ADR:
-+ Exposing an `async` API
-+ Enabling light clients to define and require the host to implement a set of traits
 
+- Exposing an `async` API
+- Enabling light clients to define and require the host to implement a set of traits
 
 ## Decision
 
@@ -26,7 +28,8 @@ Below is a diagram of how the main components of this ADR fit together.
 ![Main components](./assets/adr05.jpg)
 
 ### Validation vs Execution
-Each handler can be split into validation and execution. *Validation* is the set of statements which can make the transaction fail. It comprises all the "checks". Execution is the set of statements which mutate the state. In the IBC standard handlers, validation occurs before execution. Note that execution can fail in practice, if say a write operation fails.
+
+Each handler can be split into validation and execution. _Validation_ is the set of statements which can make the transaction fail. It comprises all the "checks". Execution is the set of statements which mutate the state. In the IBC standard handlers, validation occurs before execution. Note that execution can fail in practice, if say a write operation fails.
 
 As an example, consider the [`UpdateClient` handler](https://github.com/cosmos/ibc/blob/main/spec/core/ics-002-client-semantics/README.md#update).
 
@@ -54,6 +57,7 @@ function updateClient(
 ```
 
 ### `ValidationContext` and `ExecutionContext`
+
 Below, we define the `ValidationContext` and `ExecutionContext`.
 
 ```rust
@@ -78,7 +82,7 @@ trait ExecutionContext {
 }
 ```
 
-A useful way to understand how these traits work together is in seeing how they *could* be used to implement `deliver()`.
+A useful way to understand how these traits work together is in seeing how they _could_ be used to implement `deliver()`.
 
 ```rust
 fn deliver<V: ValidationContext, E: ExecutionContext>(val_ctx: &V, exec_ctx: &mut E, message: Any) -> Result<(), Error> {
@@ -87,6 +91,7 @@ fn deliver<V: ValidationContext, E: ExecutionContext>(val_ctx: &V, exec_ctx: &mu
     exec_ctx.execute(message)
 }
 ```
+
 Note however that we will not implement `deliver()` this way for efficiency reasons (see [discussion](https://github.com/informalsystems/ibc-rs/issues/2582#issuecomment-1229988512)).
 
 ### Host based API
@@ -209,18 +214,20 @@ See appendix C for an example of how we intend this to be used.
 ## Consequences
 
 ### Positive
-+ Architectures that run "validation" separately from "execution" will now be able to use the handlers
+
+- Architectures that run "validation" separately from "execution" will now be able to use the handlers
 
 ### Negative
-+ Still no async support
-+ Light clients still cannot specify new requirements on the host `Context`
+
+- Still no async support
+- Light clients still cannot specify new requirements on the host `Context`
 
 ### Neutral
 
 ## References
 
-* [Issue #2582: ADR for redesigning the modules' API](https://github.com/informalsystems/ibc-rs/issues/2582)
-* [ICS24 spec](https://github.com/cosmos/ibc/blob/1b73c158dcd3b08c6af3917618dce259e30bc21b/spec/core/ics-024-host-requirements/README.md)
+- [Issue #2582: ADR for redesigning the modules' API](https://github.com/informalsystems/ibc-rs/issues/2582)
+- [ICS24 spec](https://github.com/cosmos/ibc/blob/1b73c158dcd3b08c6af3917618dce259e30bc21b/spec/core/ics-024-host-requirements/README.md)
 
 ## Appendices
 
@@ -296,7 +303,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;
@@ -194,7 +193,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());
@@ -259,7 +258,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(),
     };
 
@@ -307,7 +306,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

@@ -112,8 +112,8 @@ mod tests {
             unbonding_period: Duration::new(128000, 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

@@ -440,8 +440,8 @@ mod tests {
             unbonding_period: Duration::new(128000, 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)