Trim redundant code (#314)

* Trim redundant code

* Move ics18_relayer under the mock module

* Add changelog entry

* Apply some other tweaks

* Remove references to relayer dir

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

.changelog/unreleased/improvements/154-trim-redundant-code.md

@@ -0,0 +1,2 @@
+- Remove redundant elements and move ics18_relayer under the mock module
+  ([#154](https://github.com/cosmos/ibc-rs/issues/154))

crates/ibc/Cargo.toml

@@ -95,6 +95,5 @@ default-features = false
 env_logger = "0.10.0"
 tracing-subscriber = { version = "0.3.14", features = ["fmt", "env-filter", "json"]}
 test-log = { version = "0.2.10", features = ["trace"] }
-sha2 = { version = "0.10.6" }
 tendermint-rpc = { version = "0.28", features = ["http-client", "websocket-client"] }
 tendermint-testgen = { version = "0.28" } # Needed for generating (synthetic) light blocks.

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

@@ -3,7 +3,7 @@ use derive_more::{Display, From, Into};
 use serde::{Deserialize, Serialize};
 
 use super::error::TokenTransferError;
-use crate::bigint::U256;
+use primitive_types::U256;
 
 /// A type for representing token transfer amounts.
 #[derive(

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

@@ -128,11 +128,11 @@ impl TryFrom<Any> for MsgTransfer {
 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::bigint::U256;
     use crate::core::ics04_channel::packet::{Packet, Sequence};
     use crate::core::ics04_channel::timeout::TimeoutHeight;
     use crate::signer::Signer;

crates/ibc/src/core/ics04_channel/handler/recv_packet.rs

@@ -180,7 +180,7 @@ mod tests {
     use crate::core::ics04_channel::Version;
     use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
     use crate::mock::context::MockContext;
-    use crate::relayer::ics18_relayer::context::RelayerContext;
+    use crate::mock::ics18_relayer::context::RelayerContext;
     use crate::test_utils::get_dummy_account_id;
     use crate::timestamp::Timestamp;
     use crate::timestamp::ZERO_DURATION;

crates/ibc/src/lib.rs

@@ -32,15 +32,10 @@
 //! `Applications` consists of various packet encoding and processing semantics which underpin the
 //! various types of transactions that users can perform on any IBC-compliant chain.
 //!
-//! `Relayer` contains utilities for testing the `ibc` crate against the [Hermes IBC relayer][relayer-repo]. It acts
-//! as scaffolding for gluing the `ibc` crate with Hermes for testing purposes.
-//!
 //! [core]: https://github.com/cosmos/ibc-rs/tree/main/crates/ibc/src/core
 //! [clients]: https://github.com/cosmos/ibc-rs/tree/main/crates/ibc/src/clients
 //! [applications]: https://github.com/cosmos/ibc-rs/tree/main/crates/ibc/src/applications
 //! [ics-standards]: https://github.com/cosmos/ibc#interchain-standards
-//! [relayer]: https://github.com/cosmos/ibc-rs/tree/main/crates/ibc/src/relayer
-//! [relayer-repo]: https://github.com/informalsystems/ibc-rs/tree/main/relayer
 
 extern crate alloc;
 
@@ -50,15 +45,12 @@ extern crate std;
 mod prelude;
 
 pub mod applications;
-pub mod bigint;
 pub mod clients;
 pub mod core;
 pub mod dynamic_typing;
 pub mod events;
 pub mod handler;
-pub mod macros;
 pub mod proofs;
-pub mod relayer;
 pub mod signer;
 pub mod timestamp;
 pub mod tx_msg;

crates/ibc/src/mock/context.rs

@@ -46,8 +46,8 @@ use crate::mock::client_state::{
 use crate::mock::consensus_state::MockConsensusState;
 use crate::mock::header::MockHeader;
 use crate::mock::host::{HostBlock, HostType};
-use crate::relayer::ics18_relayer::context::RelayerContext;
-use crate::relayer::ics18_relayer::error::RelayerError;
+use crate::mock::ics18_relayer::context::RelayerContext;
+use crate::mock::ics18_relayer::error::RelayerError;
 use crate::signer::Signer;
 use crate::timestamp::Timestamp;
 use crate::Height;

crates/ibc/src/relayer/ics18_relayer/utils.rs → crates/ibc/src/mock/ics18_relayer/context.rs

@@ -1,71 +1,102 @@
-use crate::core::ics02_client::header::Header;
-use crate::core::ics02_client::msgs::update_client::MsgUpdateClient;
-use crate::core::ics02_client::msgs::ClientMsg;
-use crate::core::ics24_host::identifier::ClientId;
-use crate::relayer::ics18_relayer::context::RelayerContext;
-use crate::relayer::ics18_relayer::error::RelayerError;
-
-/// Builds a `ClientMsg::UpdateClient` for a client with id `client_id` running on the `dest`
-/// context, assuming that the latest header on the source context is `src_header`.
-pub fn build_client_update_datagram<Ctx>(
-    dest: &Ctx,
-    client_id: &ClientId,
-    src_header: &dyn Header,
-) -> Result<ClientMsg, RelayerError>
-where
-    Ctx: RelayerContext,
-{
-    // Check if client for ibc0 on ibc1 has been updated to latest height:
-    // - query client state on destination chain
-    let dest_client_state = dest.query_client_full_state(client_id).ok_or_else(|| {
-        RelayerError::ClientStateNotFound {
-            client_id: client_id.clone(),
-        }
-    })?;
-
-    let dest_client_latest_height = dest_client_state.latest_height();
+use crate::prelude::*;
+use ibc_proto::google::protobuf::Any;
 
-    if src_header.height() == dest_client_latest_height {
-        return Err(RelayerError::ClientAlreadyUpToDate {
-            client_id: client_id.clone(),
-            source_height: src_header.height(),
-            destination_height: dest_client_latest_height,
-        });
-    };
+use crate::core::ics02_client::client_state::ClientState;
+use crate::core::ics02_client::header::Header;
+use crate::events::IbcEvent;
 
-    if dest_client_latest_height > src_header.height() {
-        return Err(RelayerError::ClientAtHigherHeight {
-            client_id: client_id.clone(),
-            source_height: src_header.height(),
-            destination_height: dest_client_latest_height,
-        });
-    };
-
-    // Client on destination chain can be updated.
-    Ok(ClientMsg::UpdateClient(MsgUpdateClient {
-        client_id: client_id.clone(),
-        header: src_header.clone_into(),
-        signer: dest.signer(),
-    }))
+use super::error::RelayerError;
+use crate::core::ics24_host::identifier::ClientId;
+use crate::signer::Signer;
+use crate::Height;
+
+/// Trait capturing all dependencies (i.e., the context) which algorithms in ICS18 require to
+/// relay packets between chains. This trait comprises the dependencies towards a single chain.
+/// Most of the functions in this represent wrappers over the ABCI interface.
+/// This trait mimics the `Chain` trait, but at a lower level of abstraction (no networking, header
+/// types, light client, RPC client, etc.)
+pub trait RelayerContext {
+    /// Returns the latest height of the chain.
+    fn query_latest_height(&self) -> Result<Height, RelayerError>;
+
+    /// Returns this client state for the given `client_id` on this chain.
+    /// Wrapper over the `/abci_query?path=..` endpoint.
+    fn query_client_full_state(&self, client_id: &ClientId) -> Option<Box<dyn ClientState>>;
+
+    /// Returns the most advanced header of this chain.
+    fn query_latest_header(&self) -> Option<Box<dyn Header>>;
+
+    /// Interface that the relayer uses to submit a datagram to this chain.
+    /// One can think of this as wrapping around the `/broadcast_tx_commit` ABCI endpoint.
+    fn send(&mut self, msgs: Vec<Any>) -> Result<Vec<IbcEvent>, RelayerError>;
+
+    /// Temporary solution. Similar to `CosmosSDKChain::key_and_signer()` but simpler.
+    fn signer(&self) -> Signer;
 }
 
 #[cfg(test)]
 mod tests {
     use crate::clients::ics07_tendermint::client_type as tm_client_type;
     use crate::core::ics02_client::header::{downcast_header, Header};
+    use crate::core::ics02_client::msgs::update_client::MsgUpdateClient;
+    use crate::core::ics02_client::msgs::ClientMsg;
     use crate::core::ics24_host::identifier::{ChainId, ClientId};
     use crate::core::ics26_routing::msgs::MsgEnvelope;
     use crate::mock::client_state::client_type as mock_client_type;
     use crate::mock::context::MockContext;
     use crate::mock::host::{HostBlock, HostType};
+    use crate::mock::ics18_relayer::context::RelayerContext;
+    use crate::mock::ics18_relayer::error::RelayerError;
     use crate::prelude::*;
-    use crate::relayer::ics18_relayer::context::RelayerContext;
-    use crate::relayer::ics18_relayer::utils::build_client_update_datagram;
     use crate::Height;
 
     use test_log::test;
     use tracing::debug;
 
+    /// Builds a `ClientMsg::UpdateClient` for a client with id `client_id` running on the `dest`
+    /// context, assuming that the latest header on the source context is `src_header`.
+    pub fn build_client_update_datagram<Ctx>(
+        dest: &Ctx,
+        client_id: &ClientId,
+        src_header: &dyn Header,
+    ) -> Result<ClientMsg, RelayerError>
+    where
+        Ctx: RelayerContext,
+    {
+        // Check if client for ibc0 on ibc1 has been updated to latest height:
+        // - query client state on destination chain
+        let dest_client_state = dest.query_client_full_state(client_id).ok_or_else(|| {
+            RelayerError::ClientStateNotFound {
+                client_id: client_id.clone(),
+            }
+        })?;
+
+        let dest_client_latest_height = dest_client_state.latest_height();
+
+        if src_header.height() == dest_client_latest_height {
+            return Err(RelayerError::ClientAlreadyUpToDate {
+                client_id: client_id.clone(),
+                source_height: src_header.height(),
+                destination_height: dest_client_latest_height,
+            });
+        };
+
+        if dest_client_latest_height > src_header.height() {
+            return Err(RelayerError::ClientAtHigherHeight {
+                client_id: client_id.clone(),
+                source_height: src_header.height(),
+                destination_height: dest_client_latest_height,
+            });
+        };
+
+        // Client on destination chain can be updated.
+        Ok(ClientMsg::UpdateClient(MsgUpdateClient {
+            client_id: client_id.clone(),
+            header: src_header.clone_into(),
+            signer: dest.signer(),
+        }))
+    }
+
     #[test]
     /// Serves to test both ICS 26 `dispatch` & `build_client_update_datagram` functions.
     /// Implements a "ping pong" of client update messages, so that two chains repeatedly
@@ -141,7 +172,7 @@ mod tests {
                 .latest_height();
             assert_eq!(client_height_b, ctx_a.query_latest_height().unwrap());
 
-            // Update client on chain B to latest height of B.
+            // Update client on chain A to latest height of B.
             // - create the client update message with the latest header from B
             // The test uses LightClientBlock that does not store the trusted height
             let b_latest_header = ctx_b.query_latest_header().unwrap();

crates/ibc/src/relayer/ics18_relayer/mod.rs → crates/ibc/src/mock/ics18_relayer/mod.rs

@@ -2,4 +2,3 @@
 
 pub mod context;
 pub mod error;
-pub mod utils;

crates/ibc/src/mock/mod.rs

@@ -5,4 +5,5 @@ pub mod consensus_state;
 pub mod context;
 pub mod header;
 pub mod host;
+pub mod ics18_relayer;
 pub mod misbehaviour;

crates/ibc/src/utils/mod.rs

@@ -1 +1,2 @@
+pub mod macros;
 pub mod pretty;

crates/ibc/src/utils/pretty.rs

@@ -1,29 +1,9 @@
-use core::fmt::{Debug, Display, Error as FmtError, Formatter};
-use core::time::Duration;
+use core::fmt::{Display, Error as FmtError, Formatter};
 use tendermint::block::signed_header::SignedHeader;
 use tendermint::validator::Set as ValidatorSet;
 
 use alloc::vec::Vec;
 
-pub struct PrettyDuration<'a>(pub &'a Duration);
-
-impl Display for PrettyDuration<'_> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> {
-        Debug::fmt(self.0, f)
-    }
-}
-
-pub struct PrettyOption<'a, T>(pub &'a Option<T>);
-
-impl<'a, T: Display> Display for PrettyOption<'a, T> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> {
-        match &self.0 {
-            Some(v) => write!(f, "{v}"),
-            None => write!(f, "None"),
-        }
-    }
-}
-
 pub struct PrettySignedHeader<'a>(pub &'a SignedHeader);
 
 impl Display for PrettySignedHeader<'_> {
@@ -86,56 +66,6 @@ mod tests {
     use crate::alloc::string::ToString;
     use std::{string::String, vec};
 
-    #[test]
-    fn test_pretty_duration_micros() {
-        let expected_output = "5µs";
-
-        let duration = Duration::from_micros(5);
-        let pretty_duration = PrettyDuration(&duration);
-
-        assert_eq!(pretty_duration.to_string(), expected_output);
-    }
-
-    #[test]
-    fn test_pretty_duration_millis() {
-        let expected_output = "5ms";
-
-        let duration = Duration::from_millis(5);
-        let pretty_duration = PrettyDuration(&duration);
-
-        assert_eq!(pretty_duration.to_string(), expected_output);
-    }
-
-    #[test]
-    fn test_pretty_duration_secs() {
-        let expected_output = "10s";
-
-        let duration = Duration::from_secs(10);
-        let pretty_duration = PrettyDuration(&duration);
-
-        assert_eq!(pretty_duration.to_string(), expected_output);
-    }
-
-    #[test]
-    fn test_pretty_option_some() {
-        let expected_output = "Option content";
-
-        let option_string = Some(String::from("Option content"));
-        let pretty_option = PrettyOption(&option_string);
-
-        assert_eq!(pretty_option.to_string(), expected_output);
-    }
-
-    #[test]
-    fn test_pretty_option_none() {
-        let expected_output = "None";
-
-        let option_string: Option<String> = None;
-        let pretty_option = PrettyOption(&option_string);
-
-        assert_eq!(pretty_option.to_string(), expected_output);
-    }
-
     #[test]
     fn test_pretty_vec_display() {
         let expected_output = "[ one, two, three ]";