-
Notifications
You must be signed in to change notification settings - Fork 358
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
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
3dffc2a
Make self immutable on some methods of Link
mzabaluev 7fd297c
cli: Add `clear packets` command
mzabaluev a67ec51
Changelog for #1834
mzabaluev 5a80326
Improved help for `clear` and `clear packets`
mzabaluev 4bb294c
cli: clear packets in reverse direction as well
mzabaluev 2d05671
Merge branch 'master' into mikhail/clear-packets-cmd
mzabaluev c00696a
fix clippy lints about diverging exit method
mzabaluev 667faca
Interleave recv and ack in each direction
mzabaluev 248940d
Rename positional parameters in `clear packets`
mzabaluev 98b2a5c
No need for counterparty
ancazamfir df6c3d7
Clarify help on `clear packets` command
mzabaluev aef0e6a
Fix a syntax error
mzabaluev 8ab1a72
Improve logging in proven_packet
mzabaluev 0970302
Removed the extra debug logs
mzabaluev 0521081
Implement Link::reverse naively
mzabaluev 13869cf
Add tx confirmation flag to Link::reverse
mzabaluev 3f73fb9
Extended error types when port/channel is incorrect
adizere File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
3 changes: 3 additions & 0 deletions
3
.changelog/unreleased/improvements/ibc-relayer-cli/1834-clear-packets-cmd.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
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::chain::handle::ProdChainHandle; | ||
use ibc_relayer::link::error::LinkError; | ||
use ibc_relayer::link::{Link, LinkParameters}; | ||
|
||
use crate::application::app_config; | ||
use crate::cli_utils::spawn_chain_counterparty; | ||
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. 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 chain")] | ||
chain_id: ChainId, | ||
|
||
#[clap(required = true, help = "identifier of the port")] | ||
port_id: PortId, | ||
|
||
#[clap(required = true, help = "identifier of the channel")] | ||
channel_id: ChannelId, | ||
} | ||
|
||
impl Runnable for ClearPacketsCmd { | ||
fn run(&self) { | ||
let config = app_config(); | ||
|
||
let chains = match spawn_chain_counterparty::<ProdChainHandle>( | ||
&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![]; | ||
|
||
// 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(), | ||
}; | ||
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(), | ||
}; | ||
|
||
// 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, || { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(), | ||
}; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 reverseLink
?