From 3dffc2aaea628301046d00b0198e7515633212ce Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Tue, 1 Feb 2022 18:27:32 +0200 Subject: [PATCH 01/16] Make self immutable on some methods of Link Mutability does not seem to be required, and this guarantees we can reuse the same Link instance for receiving and acking packets in turn. --- relayer-cli/src/commands/tx/packet.rs | 4 ++-- relayer/src/link.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/relayer-cli/src/commands/tx/packet.rs b/relayer-cli/src/commands/tx/packet.rs index 39afa63af2..5ab31ecd3f 100644 --- a/relayer-cli/src/commands/tx/packet.rs +++ b/relayer-cli/src/commands/tx/packet.rs @@ -38,7 +38,7 @@ impl Runnable for TxRawPacketRecvCmd { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), }; - let mut link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { + let link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { Ok(link) => link, Err(e) => return Output::error(format!("{}", e)).exit(), }; @@ -82,7 +82,7 @@ impl Runnable for TxRawPacketAckCmd { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), }; - let mut link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { + let link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { Ok(link) => link, Err(e) => return Output::error(format!("{}", e)).exit(), }; diff --git a/relayer/src/link.rs b/relayer/src/link.rs index 5f0c394648..74984a32e7 100644 --- a/relayer/src/link.rs +++ b/relayer/src/link.rs @@ -163,7 +163,7 @@ impl Link { } /// Implements the `packet-recv` CLI - pub fn build_and_send_recv_packet_messages(&mut self) -> Result, LinkError> { + pub fn build_and_send_recv_packet_messages(&self) -> Result, LinkError> { let _span = error_span!( "PacketRecvCmd", src_chain = %self.a_to_b.src_chain().id(), @@ -189,7 +189,7 @@ impl Link { } /// Implements the `packet-ack` CLI - pub fn build_and_send_ack_packet_messages(&mut self) -> Result, LinkError> { + pub fn build_and_send_ack_packet_messages(&self) -> Result, LinkError> { let _span = error_span!( "PacketAckCmd", src_chain = %self.a_to_b.src_chain().id(), From 7fd297cfbcc811e42cd80f523a7c37e663bfff6a Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Tue, 1 Feb 2022 18:47:14 +0200 Subject: [PATCH 02/16] cli: Add `clear packets` command --- relayer-cli/src/commands.rs | 12 ++++-- relayer-cli/src/commands/clear.rs | 70 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 relayer-cli/src/commands/clear.rs diff --git a/relayer-cli/src/commands.rs b/relayer-cli/src/commands.rs index 48564c240f..c5c02cbcde 100644 --- a/relayer-cli/src/commands.rs +++ b/relayer-cli/src/commands.rs @@ -5,6 +5,7 @@ //! See the `impl Configurable` below for how to specify the path to the //! application's configuration file. +mod clear; mod completions; mod config; mod create; @@ -20,9 +21,10 @@ mod upgrade; mod version; use self::{ - completions::CompletionsCmd, config::ConfigCmd, create::CreateCmds, health::HealthCheckCmd, - keys::KeysCmd, listen::ListenCmd, misbehaviour::MisbehaviourCmd, query::QueryCmd, - start::StartCmd, tx::TxCmd, update::UpdateCmds, upgrade::UpgradeCmds, version::VersionCmd, + clear::ClearCmds, completions::CompletionsCmd, config::ConfigCmd, create::CreateCmds, + health::HealthCheckCmd, keys::KeysCmd, listen::ListenCmd, misbehaviour::MisbehaviourCmd, + query::QueryCmd, start::StartCmd, tx::TxCmd, update::UpdateCmds, upgrade::UpgradeCmds, + version::VersionCmd, }; use core::time::Duration; @@ -63,6 +65,10 @@ pub enum CliCmd { #[clap(subcommand)] Upgrade(UpgradeCmds), + /// Clear entities (e.g. packets) on chains + #[clap(subcommand)] + Clear(ClearCmds), + /// Start the relayer in multi-chain mode. /// /// Relays packets and open handshake messages between all chains in the config. diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs new file mode 100644 index 0000000000..fc863173ed --- /dev/null +++ b/relayer-cli/src/commands/clear.rs @@ -0,0 +1,70 @@ +use abscissa_core::clap::Parser; +use abscissa_core::{Command, Runnable}; + +use ibc::core::ics24_host::identifier::{ChainId, ChannelId, PortId}; +use ibc_relayer::link::{Link, LinkParameters}; + +use crate::application::app_config; +use crate::cli_utils::ChainHandlePair; +use crate::conclude::Output; +use crate::error::Error; + +/// `clear` subcommands +#[derive(Command, Debug, Parser, Runnable)] +pub enum ClearCmds { + /// Clears all packets on the identified path + Packets(ClearPacketsCmd), +} + +#[derive(Debug, Parser)] +pub struct ClearPacketsCmd { + #[clap(required = true, help = "identifier of the destination chain")] + dst_chain_id: ChainId, + + #[clap(required = true, help = "identifier of the source chain")] + src_chain_id: ChainId, + + #[clap(required = true, help = "identifier of the source port")] + src_port_id: PortId, + + #[clap(required = true, help = "identifier of the source channel")] + src_channel_id: ChannelId, +} + +impl Runnable for ClearPacketsCmd { + fn run(&self) { + let config = app_config(); + + let chains = match ChainHandlePair::spawn(&config, &self.src_chain_id, &self.dst_chain_id) { + Ok(chains) => chains, + Err(e) => return Output::error(format!("{}", e)).exit(), + }; + + let opts = LinkParameters { + src_port_id: self.src_port_id.clone(), + src_channel_id: self.src_channel_id.clone(), + }; + let link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { + Ok(link) => link, + Err(e) => return Output::error(format!("{}", e)).exit(), + }; + + let mut ev = match link + .build_and_send_recv_packet_messages() + .map_err(Error::link) + { + Ok(ev) => ev, + Err(e) => return Output::error(format!("{}", e)).exit(), + }; + + match link + .build_and_send_ack_packet_messages() + .map_err(Error::link) + { + Ok(mut ev1) => ev.append(&mut ev1), + Err(e) => return Output::error(format!("{}", e)).exit(), + }; + + Output::success(ev).exit() + } +} From a67ec51525b312f2c557924589babc21f04a3535 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Tue, 1 Feb 2022 22:00:40 +0200 Subject: [PATCH 03/16] Changelog for #1834 --- .../improvements/ibc-relayer-cli/1834-clear-packets-cmd.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/improvements/ibc-relayer-cli/1834-clear-packets-cmd.md diff --git a/.changelog/unreleased/improvements/ibc-relayer-cli/1834-clear-packets-cmd.md b/.changelog/unreleased/improvements/ibc-relayer-cli/1834-clear-packets-cmd.md new file mode 100644 index 0000000000..d9bb22fb4d --- /dev/null +++ b/.changelog/unreleased/improvements/ibc-relayer-cli/1834-clear-packets-cmd.md @@ -0,0 +1,3 @@ +- Added `clear packets` command, combining the effects of + `tx raw packet-recv` and `tx raw packet-ack`. + ([#1834](https://github.com/informalsystems/ibc-rs/pull/1834)) From 5a80326b24749b12cf77bfb1486364ffc25955c3 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 2 Feb 2022 22:52:12 +0200 Subject: [PATCH 04/16] Improved help for `clear` and `clear packets` --- relayer-cli/src/commands.rs | 2 +- relayer-cli/src/commands/clear.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/relayer-cli/src/commands.rs b/relayer-cli/src/commands.rs index c5c02cbcde..c2da281dcc 100644 --- a/relayer-cli/src/commands.rs +++ b/relayer-cli/src/commands.rs @@ -65,7 +65,7 @@ pub enum CliCmd { #[clap(subcommand)] Upgrade(UpgradeCmds), - /// Clear entities (e.g. packets) on chains + /// Clear objects, such as outstanding packets on a channel. #[clap(subcommand)] Clear(ClearCmds), diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index fc863173ed..f1fe63ffae 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -12,7 +12,8 @@ use crate::error::Error; /// `clear` subcommands #[derive(Command, Debug, Parser, Runnable)] pub enum ClearCmds { - /// Clears all packets on the identified path + /// Clear outstanding packets (i.e., packet-recv and packet-ack) + /// on a given channel in both directions. Packets(ClearPacketsCmd), } From 4bb294c54cc7d9cc9bbaccb2aaf312d19ac31077 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 4 Feb 2022 11:14:10 +0200 Subject: [PATCH 05/16] cli: clear packets in reverse direction as well --- relayer-cli/src/commands/clear.rs | 47 ++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index f1fe63ffae..4a89c5841c 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -2,6 +2,8 @@ use abscissa_core::clap::Parser; use abscissa_core::{Command, Runnable}; use ibc::core::ics24_host::identifier::{ChainId, ChannelId, PortId}; +use ibc::events::IbcEvent; +use ibc_relayer::link::error::LinkError; use ibc_relayer::link::{Link, LinkParameters}; use crate::application::app_config; @@ -41,31 +43,48 @@ impl Runnable for ClearPacketsCmd { Err(e) => return Output::error(format!("{}", e)).exit(), }; + let mut ev_list = vec![]; + + // Clear packets in the src -> dst direction + let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), }; - let link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { + let link = match Link::new_from_opts(chains.src.clone(), chains.dst.clone(), opts, false) { Ok(link) => link, Err(e) => return Output::error(format!("{}", e)).exit(), }; - let mut ev = match link - .build_and_send_recv_packet_messages() - .map_err(Error::link) - { - Ok(ev) => ev, - Err(e) => return Output::error(format!("{}", e)).exit(), - }; + run_and_collect_events(&mut ev_list, || link.build_and_send_recv_packet_messages()); + run_and_collect_events(&mut ev_list, || link.build_and_send_ack_packet_messages()); + + // Clear packets in the dst -> src direction. + // Some of the checks in the Link constructor may be redundant; + // going slowly, but reliably. - match link - .build_and_send_ack_packet_messages() - .map_err(Error::link) - { - Ok(mut ev1) => ev.append(&mut ev1), + let opts = LinkParameters { + src_port_id: link.a_to_b.dst_port_id().clone(), + src_channel_id: link.a_to_b.dst_channel_id().clone(), + }; + let link = match Link::new_from_opts(chains.dst, chains.src, opts, false) { + Ok(link) => link, Err(e) => return Output::error(format!("{}", e)).exit(), }; - Output::success(ev).exit() + run_and_collect_events(&mut ev_list, || link.build_and_send_recv_packet_messages()); + run_and_collect_events(&mut ev_list, || link.build_and_send_ack_packet_messages()); + + Output::success(ev_list).exit() } } + +fn run_and_collect_events(ev_list: &mut Vec, f: F) +where + F: FnOnce() -> Result, LinkError>, +{ + match f() { + Ok(mut ev) => ev_list.append(&mut ev), + Err(e) => return Output::error(Error::link(e)).exit(), + }; +} From c00696a92c27011b5a65cf397b5fccb96ce86d83 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 4 Feb 2022 15:16:36 +0200 Subject: [PATCH 06/16] fix clippy lints about diverging exit method --- relayer-cli/src/commands/clear.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index 4a89c5841c..63d902129e 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -40,7 +40,7 @@ impl Runnable for ClearPacketsCmd { let chains = match ChainHandlePair::spawn(&config, &self.src_chain_id, &self.dst_chain_id) { Ok(chains) => chains, - Err(e) => return Output::error(format!("{}", e)).exit(), + Err(e) => Output::error(format!("{}", e)).exit(), }; let mut ev_list = vec![]; @@ -53,7 +53,7 @@ impl Runnable for ClearPacketsCmd { }; let link = match Link::new_from_opts(chains.src.clone(), chains.dst.clone(), opts, false) { Ok(link) => link, - Err(e) => return Output::error(format!("{}", e)).exit(), + Err(e) => Output::error(format!("{}", e)).exit(), }; run_and_collect_events(&mut ev_list, || link.build_and_send_recv_packet_messages()); @@ -69,7 +69,7 @@ impl Runnable for ClearPacketsCmd { }; let link = match Link::new_from_opts(chains.dst, chains.src, opts, false) { Ok(link) => link, - Err(e) => return Output::error(format!("{}", e)).exit(), + Err(e) => Output::error(format!("{}", e)).exit(), }; run_and_collect_events(&mut ev_list, || link.build_and_send_recv_packet_messages()); @@ -85,6 +85,6 @@ where { match f() { Ok(mut ev) => ev_list.append(&mut ev), - Err(e) => return Output::error(Error::link(e)).exit(), + Err(e) => Output::error(Error::link(e)).exit(), }; } From 667faca11a783e4c3436a949eaf6bd559f126f97 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 4 Feb 2022 15:34:40 +0200 Subject: [PATCH 07/16] Interleave recv and ack in each direction When processing pending packets for the `clear packets` command, schedule RecvPacket in both directions before proceeding with AckPacket, also in both directions. This ensures we process also the acks created by the packets just received. --- relayer-cli/src/commands/clear.rs | 46 ++++++++++++++++++------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index 63d902129e..2c905dd90c 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -45,35 +45,43 @@ impl Runnable for ClearPacketsCmd { let mut ev_list = vec![]; - // Clear packets in the src -> dst direction - + // Construct links in both directions. + // Some of the checks in the two Link constructor calls may be redundant; + // going slowly, but reliably. let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), }; - let link = match Link::new_from_opts(chains.src.clone(), chains.dst.clone(), opts, false) { - Ok(link) => link, - Err(e) => Output::error(format!("{}", e)).exit(), - }; - - run_and_collect_events(&mut ev_list, || link.build_and_send_recv_packet_messages()); - run_and_collect_events(&mut ev_list, || link.build_and_send_ack_packet_messages()); - - // Clear packets in the dst -> src direction. - // Some of the checks in the Link constructor may be redundant; - // going slowly, but reliably. - + let fwd_link = + match Link::new_from_opts(chains.src.clone(), chains.dst.clone(), opts, false) { + Ok(link) => link, + Err(e) => Output::error(format!("{}", e)).exit(), + }; let opts = LinkParameters { - src_port_id: link.a_to_b.dst_port_id().clone(), - src_channel_id: link.a_to_b.dst_channel_id().clone(), + src_port_id: fwd_link.a_to_b.dst_port_id().clone(), + src_channel_id: fwd_link.a_to_b.dst_channel_id().clone(), }; - let link = match Link::new_from_opts(chains.dst, chains.src, opts, false) { + let rev_link = match Link::new_from_opts(chains.dst, chains.src, opts, false) { Ok(link) => link, Err(e) => Output::error(format!("{}", e)).exit(), }; - run_and_collect_events(&mut ev_list, || link.build_and_send_recv_packet_messages()); - run_and_collect_events(&mut ev_list, || link.build_and_send_ack_packet_messages()); + // Schedule RecvPacket messages for pending packets in both directions. + // This may produce pending acks which will be processed in the next phase. + run_and_collect_events(&mut ev_list, || { + fwd_link.build_and_send_recv_packet_messages() + }); + run_and_collect_events(&mut ev_list, || { + rev_link.build_and_send_recv_packet_messages() + }); + + // Schedule AckPacket messages in both directions. + run_and_collect_events(&mut ev_list, || { + fwd_link.build_and_send_ack_packet_messages() + }); + run_and_collect_events(&mut ev_list, || { + rev_link.build_and_send_ack_packet_messages() + }); Output::success(ev_list).exit() } From 248940dece0c7412ec221e557cc3232deff9c30c Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Tue, 8 Feb 2022 12:14:56 +0200 Subject: [PATCH 08/16] Rename positional parameters in `clear packets` --- relayer-cli/src/commands/clear.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index 2c905dd90c..03dc0e7cd0 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -22,26 +22,27 @@ pub enum ClearCmds { #[derive(Debug, Parser)] pub struct ClearPacketsCmd { #[clap(required = true, help = "identifier of the destination chain")] - dst_chain_id: ChainId, + counterparty_chain_id: ChainId, #[clap(required = true, help = "identifier of the source chain")] - src_chain_id: ChainId, + chain_id: ChainId, #[clap(required = true, help = "identifier of the source port")] - src_port_id: PortId, + port_id: PortId, #[clap(required = true, help = "identifier of the source channel")] - src_channel_id: ChannelId, + channel_id: ChannelId, } impl Runnable for ClearPacketsCmd { fn run(&self) { let config = app_config(); - let chains = match ChainHandlePair::spawn(&config, &self.src_chain_id, &self.dst_chain_id) { - Ok(chains) => chains, - Err(e) => Output::error(format!("{}", e)).exit(), - }; + let chains = + match ChainHandlePair::spawn(&config, &self.chain_id, &self.counterparty_chain_id) { + Ok(chains) => chains, + Err(e) => Output::error(format!("{}", e)).exit(), + }; let mut ev_list = vec![]; @@ -49,8 +50,8 @@ impl Runnable for ClearPacketsCmd { // Some of the checks in the two Link constructor calls may be redundant; // going slowly, but reliably. let opts = LinkParameters { - src_port_id: self.src_port_id.clone(), - src_channel_id: self.src_channel_id.clone(), + src_port_id: self.port_id.clone(), + src_channel_id: self.channel_id.clone(), }; let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst.clone(), opts, false) { From 98b2a5cfdb924ce6d26f7097566eb6c50a2a6865 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Wed, 9 Feb 2022 14:44:19 +0100 Subject: [PATCH 09/16] No need for counterparty --- relayer-cli/src/commands/clear.rs | 21 ++++++++++++--------- relayer/src/chain/cosmos.rs | 30 +++++++++++++++++++++++------- relayer/src/link/relay_path.rs | 1 + 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index 03dc0e7cd0..0424b98648 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -3,11 +3,12 @@ use abscissa_core::{Command, Runnable}; use ibc::core::ics24_host::identifier::{ChainId, ChannelId, PortId}; use ibc::events::IbcEvent; +use ibc_relayer::chain::handle::ProdChainHandle; use ibc_relayer::link::error::LinkError; use ibc_relayer::link::{Link, LinkParameters}; use crate::application::app_config; -use crate::cli_utils::ChainHandlePair; +use crate::cli_utils::spawn_chain_counterparty; use crate::conclude::Output; use crate::error::Error; @@ -21,9 +22,6 @@ pub enum ClearCmds { #[derive(Debug, Parser)] pub struct ClearPacketsCmd { - #[clap(required = true, help = "identifier of the destination chain")] - counterparty_chain_id: ChainId, - #[clap(required = true, help = "identifier of the source chain")] chain_id: ChainId, @@ -38,11 +36,16 @@ impl Runnable for ClearPacketsCmd { fn run(&self) { let config = app_config(); - let chains = - match ChainHandlePair::spawn(&config, &self.chain_id, &self.counterparty_chain_id) { - Ok(chains) => chains, - Err(e) => Output::error(format!("{}", e)).exit(), - }; + + let chains = match spawn_chain_counterparty::( + &config, + &self.chain_id, + &self.port_id, + &self.channel_id, + ) { + Ok((chains, _)) => chains, + Err(e) => Output::error(format!("{}", e)).exit(), + }; let mut ev_list = vec![]; diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 82851f857c..6f61b2bfbe 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -2026,13 +2026,29 @@ impl ChainEndpoint for CosmosSdkChain { sequence, } .into(), - PacketMsgType::TimeoutUnordered => ReceiptsPath { - port_id, - channel_id, - sequence, - } - .into(), - PacketMsgType::TimeoutOrdered => SeqRecvsPath(port_id, channel_id).into(), + PacketMsgType::TimeoutUnordered => { + debug!( + "XXXX getting UNORDERED timout proof with SeqRecvsPath {}, {}, {}", + port_id, + channel_id, + sequence + ); + ReceiptsPath { + port_id, + channel_id, + sequence, + } + .into() + }, + PacketMsgType::TimeoutOrdered => { + debug!( + "XXXX getting ORDRED timout proof with SeqRecvsPath {}, {}", + port_id, + channel_id, + ); + + SeqRecvsPath(port_id, channel_id).into() + }, PacketMsgType::TimeoutOnClose => ReceiptsPath { port_id, channel_id, diff --git a/relayer/src/link/relay_path.rs b/relayer/src/link/relay_path.rs index cb6df819dd..9ead903c44 100644 --- a/relayer/src/link/relay_path.rs +++ b/relayer/src/link/relay_path.rs @@ -1121,6 +1121,7 @@ impl RelayPath { ) -> Result, LinkError> { let dst_channel_id = self.dst_channel_id(); + debug!("build timeout for channel") let (packet_type, next_sequence_received) = if self.ordered_channel() { let next_seq = self .dst_chain() From df6c3d76f3a10d4159301a6449c8415455fe447b Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 9 Feb 2022 15:57:53 +0200 Subject: [PATCH 10/16] Clarify help on `clear packets` command There is no longer a "source" chain, but we need to clarify that both directions of the channel are cleared even though only one end is used for identification. --- relayer-cli/src/commands/clear.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index 0424b98648..dd52aa6599 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -16,19 +16,20 @@ use crate::error::Error; #[derive(Command, Debug, Parser, Runnable)] pub enum ClearCmds { /// Clear outstanding packets (i.e., packet-recv and packet-ack) - /// on a given channel in both directions. + /// on a given channel in both directions. The channel is identified + /// by the chain, port, and channel IDs at one of its ends. Packets(ClearPacketsCmd), } #[derive(Debug, Parser)] pub struct ClearPacketsCmd { - #[clap(required = true, help = "identifier of the source chain")] + #[clap(required = true, help = "identifier of the chain")] chain_id: ChainId, - #[clap(required = true, help = "identifier of the source port")] + #[clap(required = true, help = "identifier of the port")] port_id: PortId, - #[clap(required = true, help = "identifier of the source channel")] + #[clap(required = true, help = "identifier of the channel")] channel_id: ChannelId, } @@ -36,7 +37,6 @@ impl Runnable for ClearPacketsCmd { fn run(&self) { let config = app_config(); - let chains = match spawn_chain_counterparty::( &config, &self.chain_id, From aef0e6a92736239d50048e35c73c5eca8c9b7814 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 9 Feb 2022 16:01:41 +0200 Subject: [PATCH 11/16] Fix a syntax error --- relayer/src/link/relay_path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relayer/src/link/relay_path.rs b/relayer/src/link/relay_path.rs index 9ead903c44..fef915e062 100644 --- a/relayer/src/link/relay_path.rs +++ b/relayer/src/link/relay_path.rs @@ -1121,7 +1121,7 @@ impl RelayPath { ) -> Result, LinkError> { let dst_channel_id = self.dst_channel_id(); - debug!("build timeout for channel") + debug!("build timeout for channel"); let (packet_type, next_sequence_received) = if self.ordered_channel() { let next_seq = self .dst_chain() From 8ab1a72a11e371114c64f24eca48f6ad8711e6cc Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 9 Feb 2022 16:24:34 +0200 Subject: [PATCH 12/16] Improve logging in proven_packet Use a span and KV pairs to provide the context in ChainEndpoint::build_packet_proofs, so the log event macro invocations are short and to the point. --- relayer/src/chain.rs | 7 +++++++ relayer/src/chain/cosmos.rs | 20 +++++--------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/relayer/src/chain.rs b/relayer/src/chain.rs index 495f08899e..e9f9be3c61 100644 --- a/relayer/src/chain.rs +++ b/relayer/src/chain.rs @@ -3,6 +3,7 @@ use core::convert::TryFrom; use tendermint::block::Height; use tokio::runtime::Runtime as TokioRuntime; +use tracing::debug_span; pub use cosmos::CosmosSdkChain; @@ -435,6 +436,12 @@ pub trait ChainEndpoint: Sized { sequence: Sequence, height: ICSHeight, ) -> Result<(Vec, Proofs), Error> { + debug_span!( + "packet_proofs", + port = %port_id, + channel = %channel_id, + sequence = %sequence, + ); let channel_proof = if packet_type == PacketMsgType::TimeoutOnClose { Some( CommitmentProofBytes::try_from( diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 6f61b2bfbe..11011b35eb 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -2027,28 +2027,18 @@ impl ChainEndpoint for CosmosSdkChain { } .into(), PacketMsgType::TimeoutUnordered => { - debug!( - "XXXX getting UNORDERED timout proof with SeqRecvsPath {}, {}, {}", - port_id, - channel_id, - sequence - ); + debug!("got unordered timeout proof"); ReceiptsPath { port_id, channel_id, sequence, } - .into() - }, + .into() + } PacketMsgType::TimeoutOrdered => { - debug!( - "XXXX getting ORDRED timout proof with SeqRecvsPath {}, {}", - port_id, - channel_id, - ); - + debug!("got ordered timeout proof"); SeqRecvsPath(port_id, channel_id).into() - }, + } PacketMsgType::TimeoutOnClose => ReceiptsPath { port_id, channel_id, From 0970302defa9b641c5ab9f6c8659ce1bc6ebd6e4 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 9 Feb 2022 18:01:36 +0200 Subject: [PATCH 13/16] Removed the extra debug logs --- relayer/src/chain.rs | 7 ------- relayer/src/chain/cosmos.rs | 18 ++++++------------ 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/relayer/src/chain.rs b/relayer/src/chain.rs index e9f9be3c61..495f08899e 100644 --- a/relayer/src/chain.rs +++ b/relayer/src/chain.rs @@ -3,7 +3,6 @@ use core::convert::TryFrom; use tendermint::block::Height; use tokio::runtime::Runtime as TokioRuntime; -use tracing::debug_span; pub use cosmos::CosmosSdkChain; @@ -436,12 +435,6 @@ pub trait ChainEndpoint: Sized { sequence: Sequence, height: ICSHeight, ) -> Result<(Vec, Proofs), Error> { - debug_span!( - "packet_proofs", - port = %port_id, - channel = %channel_id, - sequence = %sequence, - ); let channel_proof = if packet_type == PacketMsgType::TimeoutOnClose { Some( CommitmentProofBytes::try_from( diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 11011b35eb..82851f857c 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -2026,19 +2026,13 @@ impl ChainEndpoint for CosmosSdkChain { sequence, } .into(), - PacketMsgType::TimeoutUnordered => { - debug!("got unordered timeout proof"); - ReceiptsPath { - port_id, - channel_id, - sequence, - } - .into() - } - PacketMsgType::TimeoutOrdered => { - debug!("got ordered timeout proof"); - SeqRecvsPath(port_id, channel_id).into() + PacketMsgType::TimeoutUnordered => ReceiptsPath { + port_id, + channel_id, + sequence, } + .into(), + PacketMsgType::TimeoutOrdered => SeqRecvsPath(port_id, channel_id).into(), PacketMsgType::TimeoutOnClose => ReceiptsPath { port_id, channel_id, From 0521081cea3c9c267a8de2c6dfb644dd7a0ba690 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 11 Feb 2022 18:58:24 +0200 Subject: [PATCH 14/16] Implement Link::reverse naively This is the slowest implementation, not optimizing on any shared state or eliminating redundant checks. --- relayer-cli/src/commands/clear.rs | 8 +------- relayer/src/link.rs | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index dd52aa6599..95066b3bf3 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -50,8 +50,6 @@ impl Runnable for ClearPacketsCmd { let mut ev_list = vec![]; // Construct links in both directions. - // Some of the checks in the two Link constructor calls may be redundant; - // going slowly, but reliably. let opts = LinkParameters { src_port_id: self.port_id.clone(), src_channel_id: self.channel_id.clone(), @@ -61,11 +59,7 @@ impl Runnable for ClearPacketsCmd { Ok(link) => link, Err(e) => Output::error(format!("{}", e)).exit(), }; - let opts = LinkParameters { - src_port_id: fwd_link.a_to_b.dst_port_id().clone(), - src_channel_id: fwd_link.a_to_b.dst_channel_id().clone(), - }; - let rev_link = match Link::new_from_opts(chains.dst, chains.src, opts, false) { + let rev_link = match fwd_link.reverse() { Ok(link) => link, Err(e) => Output::error(format!("{}", e)).exit(), }; diff --git a/relayer/src/link.rs b/relayer/src/link.rs index 74984a32e7..6eef7d02a4 100644 --- a/relayer/src/link.rs +++ b/relayer/src/link.rs @@ -162,6 +162,21 @@ impl Link { Link::new(channel, with_tx_confirmation) } + /// Constructs a link around the channel that is reverse to the channel + /// in this link. + pub fn reverse(&self) -> Result, LinkError> { + let opts = LinkParameters { + src_port_id: self.a_to_b.dst_port_id().clone(), + src_channel_id: self.a_to_b.dst_channel_id().clone(), + }; + let chain_b = self.a_to_b.dst_chain().clone(); + let chain_a = self.a_to_b.src_chain().clone(); + + // Some of the checks and initializations may be redundant; + // going slowly, but reliably. + Link::new_from_opts(chain_b, chain_a, opts, false) + } + /// Implements the `packet-recv` CLI pub fn build_and_send_recv_packet_messages(&self) -> Result, LinkError> { let _span = error_span!( From 13869cfbc229b60615e6c5088c4d6d743fb2b62a Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 11 Feb 2022 22:52:21 +0200 Subject: [PATCH 15/16] Add tx confirmation flag to Link::reverse This is an option on Link::new_from_opts, so it seems logical to also make it optional on the same level API. --- relayer-cli/src/commands/clear.rs | 2 +- relayer/src/link.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index 95066b3bf3..bcb13ddd55 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -59,7 +59,7 @@ impl Runnable for ClearPacketsCmd { Ok(link) => link, Err(e) => Output::error(format!("{}", e)).exit(), }; - let rev_link = match fwd_link.reverse() { + let rev_link = match fwd_link.reverse(false) { Ok(link) => link, Err(e) => Output::error(format!("{}", e)).exit(), }; diff --git a/relayer/src/link.rs b/relayer/src/link.rs index 6eef7d02a4..2957e5e410 100644 --- a/relayer/src/link.rs +++ b/relayer/src/link.rs @@ -164,7 +164,7 @@ impl Link { /// Constructs a link around the channel that is reverse to the channel /// in this link. - pub fn reverse(&self) -> Result, LinkError> { + pub fn reverse(&self, with_tx_confirmation: bool) -> Result, LinkError> { let opts = LinkParameters { src_port_id: self.a_to_b.dst_port_id().clone(), src_channel_id: self.a_to_b.dst_channel_id().clone(), @@ -174,7 +174,7 @@ impl Link { // Some of the checks and initializations may be redundant; // going slowly, but reliably. - Link::new_from_opts(chain_b, chain_a, opts, false) + Link::new_from_opts(chain_b, chain_a, opts, with_tx_confirmation) } /// Implements the `packet-recv` CLI From 3f73fb9b82bc72998b1faf79d76901fb0a188b1d Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Mon, 14 Feb 2022 15:37:25 +0100 Subject: [PATCH 16/16] Extended error types when port/channel is incorrect --- relayer-cli/src/commands/clear.rs | 9 ++++----- relayer/src/link.rs | 26 ++++++++++++++++++++++---- relayer/src/link/error.rs | 7 ++++--- relayer/src/supervisor/error.rs | 4 ++-- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index bcb13ddd55..624b5ff119 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -54,11 +54,10 @@ impl Runnable for ClearPacketsCmd { src_port_id: self.port_id.clone(), src_channel_id: self.channel_id.clone(), }; - let fwd_link = - match Link::new_from_opts(chains.src.clone(), chains.dst.clone(), opts, false) { - Ok(link) => link, - Err(e) => Output::error(format!("{}", e)).exit(), - }; + let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false) { + Ok(link) => link, + Err(e) => Output::error(format!("{}", e)).exit(), + }; let rev_link = match fwd_link.reverse(false) { Ok(link) => link, Err(e) => Output::error(format!("{}", e)).exit(), diff --git a/relayer/src/link.rs b/relayer/src/link.rs index 2957e5e410..2c38aab6ed 100644 --- a/relayer/src/link.rs +++ b/relayer/src/link.rs @@ -55,7 +55,12 @@ impl Link { .src_chain() .query_channel(self.a_to_b.src_port_id(), a_channel_id, Height::default()) .map_err(|e| { - LinkError::channel_not_found(a_channel_id.clone(), self.a_to_b.src_chain().id(), e) + LinkError::channel_not_found( + self.a_to_b.src_port_id().clone(), + a_channel_id.clone(), + self.a_to_b.src_chain().id(), + e, + ) })?; let b_channel_id = self.a_to_b.dst_channel_id(); @@ -65,7 +70,12 @@ impl Link { .dst_chain() .query_channel(self.a_to_b.dst_port_id(), b_channel_id, Height::default()) .map_err(|e| { - LinkError::channel_not_found(b_channel_id.clone(), self.a_to_b.dst_chain().id(), e) + LinkError::channel_not_found( + self.a_to_b.dst_port_id().clone(), + b_channel_id.clone(), + self.a_to_b.dst_chain().id(), + e, + ) })?; if a_channel.state_matches(&ChannelState::Closed) @@ -85,9 +95,17 @@ impl Link { ) -> Result, LinkError> { // Check that the packet's channel on source chain is Open let a_channel_id = &opts.src_channel_id; + let a_port_id = &opts.src_port_id; let a_channel = a_chain - .query_channel(&opts.src_port_id, a_channel_id, Height::default()) - .map_err(|e| LinkError::channel_not_found(a_channel_id.clone(), a_chain.id(), e))?; + .query_channel(a_port_id, a_channel_id, Height::default()) + .map_err(|e| { + LinkError::channel_not_found( + a_port_id.clone(), + a_channel_id.clone(), + a_chain.id(), + e, + ) + })?; if !a_channel.state_matches(&ChannelState::Open) && !a_channel.state_matches(&ChannelState::Closed) diff --git a/relayer/src/link/error.rs b/relayer/src/link/error.rs index 83dd120a52..0bdd67fbdf 100644 --- a/relayer/src/link/error.rs +++ b/relayer/src/link/error.rs @@ -1,6 +1,6 @@ use flex_error::define_error; use ibc::core::ics02_client::error::Error as Ics02Error; -use ibc::core::ics24_host::identifier::{ChainId, ChannelId}; +use ibc::core::ics24_host::identifier::{ChainId, ChannelId, PortId}; use ibc::events::IbcEvent; use ibc::Height; @@ -45,13 +45,14 @@ define_error! { ChannelNotFound { + port_id: PortId, channel_id: ChannelId, chain_id: ChainId, } [ Error ] |e| { - format!("channel {} does not exist on chain {}", - e.channel_id, e.chain_id) + format!("channel {}/{} does not exist on chain {}", + e.port_id, e.channel_id, e.chain_id) }, Connection diff --git a/relayer/src/supervisor/error.rs b/relayer/src/supervisor/error.rs index a1310979d4..0da9b6d48f 100644 --- a/relayer/src/supervisor/error.rs +++ b/relayer/src/supervisor/error.rs @@ -16,8 +16,8 @@ define_error! { chain_id: ChainId, } |e| { - format_args!("channel {0} on chain {1} is not open", - e.channel_id, e.chain_id) + format_args!("channel {0}/{1} on chain {2} is not open", + e.port_id, e.channel_id, e.chain_id) }, ChannelConnectionUninitialized