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

Implement clear packets command #1834

Merged
merged 17 commits into from
Feb 14, 2022
Merged
Show file tree
Hide file tree
Changes from 8 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
Original file line number Diff line number Diff line change
@@ -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))
12 changes: 9 additions & 3 deletions relayer-cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -63,6 +65,10 @@ pub enum CliCmd {
#[clap(subcommand)]
Upgrade(UpgradeCmds),

/// Clear objects, such as outstanding packets on a channel.
#[clap(subcommand)]
Clear(ClearCmds),

/// Start the relayer in multi-chain mode.
///
/// Relays packets and open handshake messages between all chains in the config.
Expand Down
98 changes: 98 additions & 0 deletions relayer-cli/src/commands/clear.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
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;
use crate::cli_utils::ChainHandlePair;
use crate::conclude::Output;
use crate::error::Error;

/// `clear` subcommands
#[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.
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,
}
ancazamfir marked this conversation as resolved.
Show resolved Hide resolved

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 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.src_port_id.clone(),
src_channel_id: self.src_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 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) {
Ok(link) => link,
Err(e) => Output::error(format!("{}", e)).exit(),
};
Copy link
Contributor Author

@mzabaluev mzabaluev Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be the case for a Link::reverse method, which would do this and maybe skip on some redundant checks and queries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a nice addition, would you mind doing that as part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romac Can you provide some clue on which of the checks done inside Link::new_from_opts can be bypassed, and which initializations reused, when creating the reverse Link?


// 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, || {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

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()
}
}

fn run_and_collect_events<F>(ev_list: &mut Vec<IbcEvent>, f: F)
where
F: FnOnce() -> Result<Vec<IbcEvent>, LinkError>,
{
match f() {
Ok(mut ev) => ev_list.append(&mut ev),
Err(e) => Output::error(Error::link(e)).exit(),
};
}
4 changes: 2 additions & 2 deletions relayer-cli/src/commands/tx/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => Output::error(format!("{}", e)).exit(),
};
Expand Down Expand Up @@ -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) => Output::error(format!("{}", e)).exit(),
};
Expand Down
4 changes: 2 additions & 2 deletions relayer/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Link<ChainA, ChainB> {
}

/// Implements the `packet-recv` CLI
pub fn build_and_send_recv_packet_messages(&mut self) -> Result<Vec<IbcEvent>, LinkError> {
adizere marked this conversation as resolved.
Show resolved Hide resolved
pub fn build_and_send_recv_packet_messages(&self) -> Result<Vec<IbcEvent>, LinkError> {
let _span = error_span!(
"PacketRecvCmd",
src_chain = %self.a_to_b.src_chain().id(),
Expand All @@ -189,7 +189,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Link<ChainA, ChainB> {
}

/// Implements the `packet-ack` CLI
pub fn build_and_send_ack_packet_messages(&mut self) -> Result<Vec<IbcEvent>, LinkError> {
pub fn build_and_send_ack_packet_messages(&self) -> Result<Vec<IbcEvent>, LinkError> {
let _span = error_span!(
"PacketAckCmd",
src_chain = %self.a_to_b.src_chain().id(),
Expand Down