Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return error for uninitialized connection end or channel end query result #923

Merged
merged 3 commits into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
8 changes: 4 additions & 4 deletions modules/src/ics03_connection/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ impl Protobuf<RawConnectionEnd> for ConnectionEnd {}
impl TryFrom<RawConnectionEnd> for ConnectionEnd {
type Error = anomaly::Error<Kind>;
fn try_from(value: RawConnectionEnd) -> Result<Self, Self::Error> {
if value.client_id.is_empty() {
return Err(Kind::EmptyProtoConnectionEnd.into());
}

let state = value.state.try_into()?;
if state == State::Uninitialized {
return Ok(ConnectionEnd::default());
}
if value.client_id.is_empty() {
return Err(Kind::EmptyProtoConnectionEnd.into());
}

Ok(Self::new(
state,
value
Expand Down
2 changes: 1 addition & 1 deletion modules/src/ics04_channel/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,14 @@ impl TryFrom<RawChannel> for ChannelEnd {
type Error = anomaly::Error<Kind>;

fn try_from(value: RawChannel) -> Result<Self, Self::Error> {
// Parse the ordering type. Propagate the error, if any, to our caller.
let chan_state: State = State::from_i32(value.state)?;

if chan_state == State::Uninitialized {
return Ok(ChannelEnd::default());
}

let chan_ordering = Order::from_i32(value.ordering)?;

// Assemble the 'remote' attribute of the Channel, which represents the Counterparty.
let remote = value
.counterparty
Expand Down
7 changes: 1 addition & 6 deletions relayer-cli/src/commands/query/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion relayer-cli/src/commands/tx/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
34 changes: 28 additions & 6 deletions relayer/src/chain/cosmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
romac marked this conversation as resolved.
Show resolved Hide resolved
})?;

match channel_end.state() {
ChannelState::Uninitialized => Err(Kind::Query(format!(
"port '{}' channel '{}'",
romac marked this conversation as resolved.
Show resolved Hide resolved
port_id, channel_id
))
.context("channel does not exist")
.into()),
_ => Ok(channel_end),
}
}

/// Queries the packet commitment hashes associated with a channel.
Expand Down
2 changes: 1 addition & 1 deletion relayer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion relayer/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down