From 6486cb16d39a5e3d53fdca17ec7171babb4e48f7 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Tue, 11 May 2021 16:45:49 +0200 Subject: [PATCH] Return error for uninitialized connection end or channel end query result (#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 --- CHANGELOG.md | 17 +++++++++- relayer-cli/src/commands/query/connection.rs | 7 +--- relayer-cli/src/commands/tx/transfer.rs | 2 +- relayer/src/chain/cosmos.rs | 34 ++++++++++++++++---- relayer/src/error.rs | 2 +- relayer/src/link.rs | 2 +- 6 files changed, 48 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe99b6bba2..f6b553fbb3 100644 --- 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 index 8956d7a59b..cf427e337a 100644 --- 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 = 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 index 2e6c8f3308..884e40ff04 100644 --- 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 index d8c8731e56..6c25ad32da 100644 --- 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 { 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 index c82292621d..a79caeffba 100644 --- 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 index fd7e802315..1a8a99b625 100644 --- 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)