Skip to content

Commit

Permalink
Backport "Add list of sequence numbers to clear pending CLI" to v.1.7…
Browse files Browse the repository at this point in the history 
….x (#3772)

* Remove unused code for encoding a `Packet` into a `Vec<abci::EventAttribute>` (#3769)

* Add list of sequence numbers to clear pending CLI (#3751)

* Add list of sequence numbers to clear pending CLI

* With sequences option only clear the packets from the specified chain and not counterparty

* Fix clippy, add sequence test

* Update guide template

* Allow specifying sequences to clear as a list of ranges (#3756)

* Allow specifying sequences to clear as a list of ranges

* Make the ranges inclusive

* Update guide templates

* Add packet-sequences to the packet tx CLIs.

* Align and wording

* Update guide templates

* Improve output of `query packet pending` command

* Add integration test for clearing packets by sequence number ranges

* Add changelog entry

---------

Co-authored-by: Romain Ruetschi <romain@informal.systems>

* Fix clippy warnings

---------

Co-authored-by: Anca Zamfir <ancazamfir@users.noreply.github.com>
  • Loading branch information
@romac @ancazamfir
romac and ancazamfir authored Jan 15, 2024
1 parent 77dc183 commit f3302db
Show file tree
Hide file tree
Showing 27 changed files with 589 additions and 248 deletions.
18 changes: 18 additions & 0 deletions .changelog/unreleased/features/ibc-relayer-cli/3672-clears-packet-seq.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
- Add a `--packet-sequences` flag to the `clear packets`, `tx packet-recv`, and `tx packet-ack` commands.
When this flag is specified, these commands will only clear the packets with the specified sequence numbers
on the given chain. If not provided, all pending packets will be cleared on both chains, as before.

This flag takes either a single sequence number or a range of sequences numbers.
Each element of the comma-separated list must be either a single sequence number or
a range of sequence numbers.

Examples:
- `10` will clear a single packet with sequence nymber `10`
- `1,2,3` will clear packets with sequence numbers `1, 2, 3`
- `1..5` will clear packets with sequence numbers `1, 2, 3, 4, 5`
- `..5` will clear packets with sequence numbers `1, 2, 3, 4, 5`
- `5..` will clear packets with sequence numbers greater than or equal to `5`
- `..5,10..20,25,30..` will clear packets with sequence numbers `1, 2, 3, 4, 5, 10, 11, ..., 20, 25, 30, 31, ...`
- `..5,10..20,25,30..` will clear packets with sequence numbers `1, 2, 3, 4, 5, 10, 11, ..., 20, 25, 30, 31, ...`

([\#3672](https://github.com/informalsystems/hermes/issues/3672))
4 changes: 2 additions & 2 deletions config.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ default_gas = 100000
# Specify the maximum amount of gas to be used as the gas limit for a transaction.
# If `default_gas` is unspecified, then `max_gas` will be used as `default_gas`.
# Default: 400 000
max_gas = 400000
max_gas = 4000000

# Specify the price per gas used of the fee to submit a transaction and
# the denomination of the fee.
Expand Down Expand Up @@ -401,7 +401,7 @@ account_prefix = 'cosmos'
key_name = 'testkey'
store_prefix = 'ibc'
default_gas = 100000
max_gas = 400000
max_gas = 4000000
gas_price = { price = 0.025, denom = 'stake' }
gas_multiplier = 1.1
max_msg_num = 30
Expand Down
123 changes: 110 additions & 13 deletions crates/relayer-cli/src/commands/clear.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::RangeInclusive;

use abscissa_core::clap::Parser;
use abscissa_core::config::Override;
use abscissa_core::{Command, FrameworkErrorKind, Runnable};
Expand All @@ -6,6 +8,8 @@ use ibc_relayer::chain::handle::{BaseChainHandle, ChainHandle};
use ibc_relayer::config::Config;
use ibc_relayer::link::error::LinkError;
use ibc_relayer::link::{Link, LinkParameters};
use ibc_relayer::util::seq_range::parse_seq_range;
use ibc_relayer_types::core::ics04_channel::packet::Sequence;
use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};
use ibc_relayer_types::events::IbcEvent;

Expand Down Expand Up @@ -52,17 +56,36 @@ pub struct ClearPacketsCmd {
)]
channel_id: ChannelId,

#[clap(
long = "packet-sequences",
help = "Sequences of packets to be cleared on the specified chain. \
Either a single sequence or a range of sequences can be specified. \
If not provided, all pending packets will be cleared on both chains. \
Each element of the comma-separated list must be either a single \
sequence or a range of sequences. \
Example: `1,10..20` will clear packets with sequences 1, 10, 11, ..., 20",
value_delimiter = ',',
value_parser = parse_seq_range
)]
packet_sequences: Vec<RangeInclusive<Sequence>>,

#[clap(
long = "key-name",
help = "use the given signing key for the specified chain (default: `key_name` config)"
help = "Use the given signing key for the specified chain (default: `key_name` config)"
)]
key_name: Option<String>,

#[clap(
long = "counterparty-key-name",
help = "use the given signing key for the counterparty chain (default: `counterparty_key_name` config)"
help = "Use the given signing key for the counterparty chain (default: `counterparty_key_name` config)"
)]
counterparty_key_name: Option<String>,

#[clap(
long = "query-packets-chunk-size",
help = "Number of packets to fetch at once from the chain (default: `query_packets_chunk_size` config)"
)]
query_packets_chunk_size: Option<usize>,
}

impl Override<Config> for ClearPacketsCmd {
Expand Down Expand Up @@ -126,22 +149,28 @@ impl Runnable for ClearPacketsCmd {
Err(e) => Output::error(e).exit(),
};

// Schedule RecvPacket messages for pending packets in both directions.
// Schedule RecvPacket messages for pending packets in both directions or,
// if packet sequences are provided, only on the specified chain.
// This may produce pending acks which will be processed in the next phase.
run_and_collect_events("forward recv and timeout", &mut ev_list, || {
fwd_link.relay_recv_packet_and_timeout_messages()
});
run_and_collect_events("reverse recv and timeout", &mut ev_list, || {
rev_link.relay_recv_packet_and_timeout_messages()
fwd_link.relay_recv_packet_and_timeout_messages(self.packet_sequences.clone())
});
if self.packet_sequences.is_empty() {
run_and_collect_events("reverse recv and timeout", &mut ev_list, || {
rev_link.relay_recv_packet_and_timeout_messages(vec![])
});
}

// Schedule AckPacket messages in both directions.
run_and_collect_events("forward ack", &mut ev_list, || {
fwd_link.relay_ack_packet_messages()
});
// Schedule AckPacket messages in both directions or, if packet sequences are provided,
// only on the specified chain.
run_and_collect_events("reverse ack", &mut ev_list, || {
rev_link.relay_ack_packet_messages()
rev_link.relay_ack_packet_messages(self.packet_sequences.clone())
});
if self.packet_sequences.is_empty() {
run_and_collect_events("forward ack", &mut ev_list, || {
fwd_link.relay_ack_packet_messages(vec![])
});
}

Output::success(ev_list).exit()
}
Expand All @@ -164,6 +193,7 @@ mod tests {
use std::str::FromStr;

use abscissa_core::clap::Parser;
use ibc_relayer_types::core::ics04_channel::packet::Sequence;
use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};

#[test]
Expand All @@ -173,8 +203,10 @@ mod tests {
chain_id: ChainId::from_string("chain_id"),
port_id: PortId::from_str("port_id").unwrap(),
channel_id: ChannelId::from_str("channel-07").unwrap(),
packet_sequences: vec![],
key_name: None,
counterparty_key_name: None,
query_packets_chunk_size: None,
},
ClearPacketsCmd::parse_from([
"test",
Expand All @@ -195,8 +227,10 @@ mod tests {
chain_id: ChainId::from_string("chain_id"),
port_id: PortId::from_str("port_id").unwrap(),
channel_id: ChannelId::from_str("channel-07").unwrap(),
packet_sequences: vec![],
key_name: None,
counterparty_key_name: None
counterparty_key_name: None,
query_packets_chunk_size: None,
},
ClearPacketsCmd::parse_from([
"test",
Expand All @@ -210,15 +244,48 @@ mod tests {
)
}

#[test]
fn test_clear_packets_sequences() {
assert_eq!(
ClearPacketsCmd {
chain_id: ChainId::from_string("chain_id"),
port_id: PortId::from_str("port_id").unwrap(),
channel_id: ChannelId::from_str("channel-07").unwrap(),
packet_sequences: vec![
Sequence::from(1)..=Sequence::from(1),
Sequence::from(10)..=Sequence::from(20)
],
key_name: Some("key_name".to_owned()),
counterparty_key_name: None,
query_packets_chunk_size: None
},
ClearPacketsCmd::parse_from([
"test",
"--chain",
"chain_id",
"--port",
"port_id",
"--channel",
"channel-07",
"--packet-sequences",
"1,10..20",
"--key-name",
"key_name"
])
)
}

#[test]
fn test_clear_packets_key_name() {
assert_eq!(
ClearPacketsCmd {
chain_id: ChainId::from_string("chain_id"),
port_id: PortId::from_str("port_id").unwrap(),
channel_id: ChannelId::from_str("channel-07").unwrap(),
packet_sequences: vec![],
key_name: Some("key_name".to_owned()),
counterparty_key_name: None,
query_packets_chunk_size: None,
},
ClearPacketsCmd::parse_from([
"test",
Expand All @@ -241,8 +308,10 @@ mod tests {
chain_id: ChainId::from_string("chain_id"),
port_id: PortId::from_str("port_id").unwrap(),
channel_id: ChannelId::from_str("channel-07").unwrap(),
packet_sequences: vec![],
key_name: None,
counterparty_key_name: Some("counterparty_key_name".to_owned()),
query_packets_chunk_size: None,
},
ClearPacketsCmd::parse_from([
"test",
Expand All @@ -258,6 +327,34 @@ mod tests {
)
}

#[test]
fn test_clear_packets_query_packets_chunk_size() {
assert_eq!(
ClearPacketsCmd {
chain_id: ChainId::from_string("chain_id"),
port_id: PortId::from_str("port_id").unwrap(),
channel_id: ChannelId::from_str("channel-07").unwrap(),
packet_sequences: vec![],
key_name: None,
counterparty_key_name: Some("counterparty_key_name".to_owned()),
query_packets_chunk_size: Some(100),
},
ClearPacketsCmd::parse_from([
"test",
"--chain",
"chain_id",
"--port",
"port_id",
"--channel",
"channel-07",
"--counterparty-key-name",
"counterparty_key_name",
"--query-packets-chunk-size",
"100"
])
)
}

#[test]
fn test_clear_packets_no_chan() {
assert!(ClearPacketsCmd::try_parse_from([
Expand Down
50 changes: 48 additions & 2 deletions crates/relayer-cli/src/commands/query/packet/pending.rs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use core::fmt;

use abscissa_core::clap::Parser;
use abscissa_core::{Command, Runnable};
use serde::Serialize;

use ibc_relayer::chain::counterparty::{
channel_on_destination, pending_packet_summary, PendingPackets,
};
use ibc_relayer::chain::handle::BaseChainHandle;
use ibc_relayer::chain::handle::{BaseChainHandle, ChainHandle};
use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};

use crate::cli_utils::spawn_chain_counterparty;
Expand All @@ -19,21 +21,63 @@ use super::util::CollatedPendingPackets;
/// at both ends of a channel.
#[derive(Debug, Serialize)]
struct Summary<P> {
/// Source chain
src_chain: ChainId,

/// Destination chain
dst_chain: ChainId,

/// The packets sent on the source chain as identified by the command.
src: P,

/// The packets sent on the counterparty chain.
dst: P,
}

impl Summary<PendingPackets> {
fn collate(self) -> Summary<CollatedPendingPackets> {
Summary {
src_chain: self.src_chain,
dst_chain: self.dst_chain,

src: CollatedPendingPackets::new(self.src),
dst: CollatedPendingPackets::new(self.dst),
}
}
}

impl fmt::Display for Summary<CollatedPendingPackets> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "Summary of pending packets:")?;

writeln!(f, "Packets pending on source chain ({}):", self.src_chain)?;
writeln!(f, " Unreceived packets:")?;
for seq in &self.src.unreceived_packets {
writeln!(f, " {}", seq)?;
}
writeln!(f, " Unreceived acks:")?;
for seq in &self.src.unreceived_acks {
writeln!(f, " {}", seq)?;
}

writeln!(
f,
"Packets pending on destination chain ({}):",
self.dst_chain
)?;
writeln!(f, " Unreceived packets:")?;
for seq in &self.dst.unreceived_packets {
writeln!(f, " {}", seq)?;
}
writeln!(f, " Unreceived acks:")?;
for seq in &self.dst.unreceived_acks {
writeln!(f, " {}", seq)?;
}

Ok(())
}
}

/// This command does the following:
///
/// 1. queries the chain to get its counterparty chain, channel and port identifiers (needed in 2)
Expand Down Expand Up @@ -102,6 +146,8 @@ impl QueryPendingPacketsCmd {
.map_err(Error::supervisor)?;

Ok(Summary {
src_chain: chains.src.id(),
dst_chain: chains.dst.id(),
src: src_summary,
dst: dst_summary,
})
Expand All @@ -114,7 +160,7 @@ impl Runnable for QueryPendingPacketsCmd {

match self.execute() {
Ok(summary) if json() => Output::success(summary).exit(),
Ok(summary) => Output::success(summary.collate()).exit(),
Ok(summary) => Output::success_msg(summary.collate().to_string()).exit(),
Err(e) => Output::error(e).exit(),
}
}
Expand Down
Loading

0 comments on commit f3302db

Please sign in to comment.