Fix for chain impersonation problem. Fix for incomplete channel create

CHANGELOG.md

@@ -8,12 +8,20 @@
   - Add `--hd-path` option to `keys restore` and `keys add` commands to specify
     derivation path when importing keys ([#1049])
 
+### BUG FIXES
+
+- [ibc-relayer-cli]
+  - Fix for chain impersonation bug. ([#1038])
+  - Fix for partially open handshake bug of `channel create` CLI. ([#1064])
+
 ### BREAKING CHANGES
 
 - [ibc-relayer-cli]
   - Removed `--coin-type` option from `keys restore` command. Use `--hd-path` instead. ([#1049])
 
-[#868]: https://github.com/informalsystems/ibc-rs/issues/1049
+[#1038]: https://github.com/informalsystems/ibc-rs/issues/1038
+[#1049]: https://github.com/informalsystems/ibc-rs/issues/1049
+[#1064]: https://github.com/informalsystems/ibc-rs/issues/1064
 
 ## v0.4.0
 *June 3rd, 2021*

modules/src/ics24_host/identifier.rs

@@ -391,3 +391,9 @@ pub struct PortChannelId {
     pub channel_id: ChannelId,
     pub port_id: PortId,
 }
+
+impl std::fmt::Display for PortChannelId {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        write!(f, "{}/{}", self.port_id, self.channel_id)
+    }
+}

relayer-cli/src/commands/tx/transfer.rs

@@ -1,18 +1,19 @@
-use std::sync::Arc;
-
 use abscissa_core::{config::Override, Command, FrameworkErrorKind, Options, Runnable};
 use anomaly::BoxError;
-use tokio::runtime::Runtime as TokioRuntime;
 
-use ibc::events::IbcEvent;
-use ibc::ics02_client::height::Height;
-use ibc::ics24_host::identifier::{ChainId, ChannelId, PortId};
+use ibc::{
+    events::IbcEvent,
+    ics02_client::client_state::ClientState,
+    ics02_client::height::Height,
+    ics24_host::identifier::{ChainId, ChannelId, PortChannelId, PortId},
+};
 use ibc_relayer::{
-    chain::{Chain, CosmosSdkChain},
+    channel::check_channel_counterparty,
     config::Config,
     transfer::{build_and_send_transfer_messages, TransferOptions},
 };
 
+use crate::cli_utils::ChainHandlePair;
 use crate::conclude::{exit_with_unrecoverable_error, Output};
 use crate::error::{Error, Kind};
 use crate::prelude::*;
@@ -132,48 +133,33 @@ impl Runnable for TxIcs20MsgTransferCmd {
 
         debug!("Message: {:?}", opts);
 
-        let rt = Arc::new(TokioRuntime::new().unwrap());
-
-        let src_chain_res =
-            CosmosSdkChain::bootstrap(opts.packet_src_chain_config.clone(), rt.clone())
-                .map_err(|e| Kind::Runtime.context(e));
-
-        let src_chain = match src_chain_res {
-            Ok(chain) => chain,
-            Err(e) => return Output::error(format!("{}", e)).exit(),
-        };
-
-        let dst_chain_res = CosmosSdkChain::bootstrap(opts.packet_dst_chain_config.clone(), rt)
-            .map_err(|e| Kind::Runtime.context(e));
-
-        let dst_chain = match dst_chain_res {
-            Ok(chain) => chain,
-            Err(e) => return Output::error(format!("{}", e)).exit(),
-        };
+        let chains = ChainHandlePair::spawn(&config, &self.src_chain_id, &self.dst_chain_id)
+            .unwrap_or_else(exit_with_unrecoverable_error);
 
         // Double check that channels and chain identifiers match.
         // To do this, fetch from the source chain the channel end, then the associated connection
         // end, and then the underlying client state; finally, check that this client is verifying
         // headers for the destination chain.
-        let channel_end = src_chain
+        let channel_end_src = chains
+            .src
             .query_channel(
                 &opts.packet_src_port_id,
                 &opts.packet_src_channel_id,
                 Height::zero(),
             )
             .unwrap_or_else(exit_with_unrecoverable_error);
-        if !channel_end.is_open() {
+        if !channel_end_src.is_open() {
             return Output::error(format!(
                 "the requested port/channel ('{}'/'{}') on chain id '{}' is in state '{}'; expected 'open' state",
                 opts.packet_src_port_id,
                 opts.packet_src_channel_id,
                 self.src_chain_id,
-                channel_end.state
+                channel_end_src.state
             ))
             .exit();
         }
 
-        let conn_id = match channel_end.connection_hops.first() {
+        let conn_id = match channel_end_src.connection_hops.first() {
             None => {
                 return Output::error(format!(
                     "could not retrieve the connection hop underlying port/channel '{}'/'{}' on chain '{}'",
@@ -184,13 +170,15 @@ impl Runnable for TxIcs20MsgTransferCmd {
             Some(cid) => cid,
         };
 
-        let conn_end = src_chain
+        let conn_end = chains
+            .src
             .query_connection(conn_id, Height::zero())
             .unwrap_or_else(exit_with_unrecoverable_error);
 
         debug!("connection hop underlying the channel: {:?}", conn_end);
 
-        let src_chain_client_state = src_chain
+        let src_chain_client_state = chains
+            .src
             .query_client_state(conn_end.client_id(), Height::zero())
             .unwrap_or_else(exit_with_unrecoverable_error);
 
@@ -199,18 +187,42 @@ impl Runnable for TxIcs20MsgTransferCmd {
             src_chain_client_state
         );
 
-        if src_chain_client_state.chain_id != self.dst_chain_id {
+        if src_chain_client_state.chain_id() != self.dst_chain_id {
             return Output::error(
                 format!("the requested port/channel ('{}'/'{}') provides a path from chain '{}' to \
                  chain '{}' (not to the destination chain '{}'). Bailing due to mismatching arguments.",
                         opts.packet_src_port_id, opts.packet_src_channel_id,
                         self.src_chain_id,
-                        src_chain_client_state.chain_id, self.dst_chain_id)).exit();
+                        src_chain_client_state.chain_id(), self.dst_chain_id)).exit();
         }
 
+        // Final verification:
+        // The socket (i.e., port/channel pair) representing the
+        // counterparty on the destination chain should match the
+        // socket on the source chain.
+        let channel_id_dest = channel_end_src
+            .remote
+            .channel_id
+            .ok_or(format!(
+                "the port/channel '{}'/'{}' on chain '{}' has no counterparty channel id",
+                opts.packet_src_port_id, opts.packet_src_channel_id, self.src_chain_id
+            ))
+            .unwrap_or_else(exit_with_unrecoverable_error);
+
+        let socket_dest = PortChannelId {
+            channel_id: channel_id_dest,
+            port_id: channel_end_src.remote.port_id,
+        };
+        let expected = PortChannelId {
+            channel_id: opts.packet_src_channel_id.clone(),
+            port_id: opts.packet_src_port_id.clone(),
+        };
+        check_channel_counterparty(chains.dst.clone(), &socket_dest, &expected)
+            .unwrap_or_else(exit_with_unrecoverable_error);
+
         // Checks pass, build and send the tx
         let res: Result<Vec<IbcEvent>, Error> =
-            build_and_send_transfer_messages(src_chain, dst_chain, opts)
+            build_and_send_transfer_messages(chains.src, chains.dst, opts)
                 .map_err(|e| Kind::Tx.context(e).into());
 
         match res {