Return error for uninitialized connection end or channel end query re…

…sult (informalsystems#923) * Return query error if uninitialized conn/chan * Consistent output for query connection and channel comands * Apply suggestions from code review Co-authored-by: Romain Ruetschi <romain@informal.systems>
hu55a1n1 · May 11, 2021 · 6486cb1 · 6486cb1
1 parent 64c5868
commit 6486cb1
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,22 @@
 
 ## Unreleased
 
-> Nothing yet.
+> [TODO: high level summary]
+
+### FEATURES
+
+### IMPROVEMENTS
+- [ibc-relayer-cli]
+  - Improve UX when querying non-existing connections and channels ([#875], [#920])
+
+### BUG FIXES
+
+### BREAKING CHANGES
+
+
+[#875]: https://github.com/informalsystems/ibc-rs/issues/875
+[#920]: https://github.com/informalsystems/ibc-rs/issues/920
+
 
 ## v0.3.0
 *May 7h, 2021*

diff --git a/relayer-cli/src/commands/query/connection.rs b/relayer-cli/src/commands/query/connection.rs
@@ -3,7 +3,6 @@ use std::sync::Arc;
 use abscissa_core::{Command, Options, Runnable};
 use tokio::runtime::Runtime as TokioRuntime;
 
-use ibc::ics03_connection::connection::ConnectionEnd;
 use ibc::ics24_host::identifier::ChainId;
 use ibc::ics24_host::identifier::ConnectionId;
 use ibc_proto::ibc::core::channel::v1::QueryConnectionChannelsRequest;
@@ -47,11 +46,7 @@ impl Runnable for QueryConnectionEndCmd {
         let chain = CosmosSdkChain::bootstrap(chain_config.clone(), rt).unwrap();
 
         let height = ibc::Height::new(chain.id().version(), self.height.unwrap_or(0_u64));
-
-        let res: Result<ConnectionEnd, Error> = chain
-            .query_connection(&self.connection_id, height)
-            .map_err(|e| Kind::Query.context(e).into());
-
+        let res = chain.query_connection(&self.connection_id, height);
         match res {
             Ok(ce) => Output::success(ce).exit(),
             Err(e) => Output::error(format!("{}", e)).exit(),

diff --git a/relayer-cli/src/commands/tx/transfer.rs b/relayer-cli/src/commands/tx/transfer.rs
@@ -132,7 +132,7 @@ impl Runnable for TxIcs20MsgTransferCmd {
                 Height::zero(),
             )
             .unwrap_or_else(exit_with_unrecoverable_error);
-        // TODO: Support for multi-hop channels will impact this.
+
         let conn_id = match channel_end.connection_hops.first() {
             None => {
                 return Output::error(format!(

diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs
@@ -26,8 +26,10 @@ use ibc::ics02_client::client_consensus::{
 };
 use ibc::ics02_client::client_state::{AnyClientState, IdentifiedAnyClientState};
 use ibc::ics02_client::events as ClientEvents;
-use ibc::ics03_connection::connection::ConnectionEnd;
-use ibc::ics04_channel::channel::{ChannelEnd, IdentifiedChannelEnd, QueryPacketEventDataRequest};
+use ibc::ics03_connection::connection::{ConnectionEnd, State as ConnectionState};
+use ibc::ics04_channel::channel::{
+    ChannelEnd, IdentifiedChannelEnd, QueryPacketEventDataRequest, State as ChannelState,
+};
 use ibc::ics04_channel::events as ChannelEvents;
 use ibc::ics04_channel::packet::{PacketMsgType, Sequence};
 use ibc::ics07_tendermint::client_state::{AllowUpdate, ClientState};
@@ -742,8 +744,17 @@ impl Chain for CosmosSdkChain {
         height: ICSHeight,
     ) -> Result<ConnectionEnd, Error> {
         let res = self.query(Path::Connections(connection_id.clone()), height, false)?;
-        Ok(ConnectionEnd::decode_vec(&res.value)
-            .map_err(|e| Kind::Query(format!("connection {}", connection_id)).context(e))?)
+        let connection_end = ConnectionEnd::decode_vec(&res.value)
+            .map_err(|e| Kind::Query(format!("connection '{}'", connection_id)).context(e))?;
+
+        match connection_end.state() {
+            ConnectionState::Uninitialized => {
+                Err(Kind::Query(format!("connection '{}'", connection_id))
+                    .context("connection does not exist")
+                    .into())
+            }
+            _ => Ok(connection_end),
+        }
     }
 
     fn query_connection_channels(
@@ -819,8 +830,19 @@ impl Chain for CosmosSdkChain {
             height,
             false,
         )?;
-        Ok(ChannelEnd::decode_vec(&res.value)
-            .map_err(|e| Kind::Query("channel".into()).context(e))?)
+        let channel_end = ChannelEnd::decode_vec(&res.value).map_err(|e| {
+            Kind::Query(format!("port '{}' & channel '{}'", port_id, channel_id)).context(e)
+        })?;
+
+        match channel_end.state() {
+            ChannelState::Uninitialized => Err(Kind::Query(format!(
+                "port '{}' & channel '{}'",
+                port_id, channel_id
+            ))
+            .context("channel does not exist")
+            .into()),
+            _ => Ok(channel_end),
+        }
     }
 
     /// Queries the packet commitment hashes associated with a channel.

diff --git a/relayer/src/error.rs b/relayer/src/error.rs
@@ -143,7 +143,7 @@ pub enum Kind {
     MessageTransaction(String),
 
     /// Failed query
-    #[error("Query error occurred (failed to finish query for {0})")]
+    #[error("Query error occurred (failed to query for {0})")]
     Query(String),
 
     /// Keybase related error

diff --git a/relayer/src/link.rs b/relayer/src/link.rs
@@ -506,7 +506,7 @@ impl RelayPath {
                     )?,
                 IbcEvent::WriteAcknowledgement(ref write_ack_ev) => {
                     if self
-                        .dst_channel(dst_od.proofs_height)?
+                        .dst_channel(Height::zero())?
                         .state_matches(&ChannelState::Closed)
                     {
                         (None, None)