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

Closes: #3672

* 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>

.changelog/unreleased/features/ibc-relayer-cli/3672-clears-packet-seq.md

@@ -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))

config.toml

@@ -261,7 +261,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. 
@@ -418,7 +418,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

crates/relayer-cli/src/commands/clear.rs

@@ -1,3 +1,5 @@
+use std::ops::RangeInclusive;
+
 use abscissa_core::clap::Parser;
 use abscissa_core::config::Override;
 use abscissa_core::{Command, FrameworkErrorKind, Runnable};
@@ -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;
 
@@ -52,21 +56,34 @@ 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)"
+        help = "Number of packets to fetch at once from the chain (default: `query_packets_chunk_size` config)"
     )]
     query_packets_chunk_size: Option<usize>,
 }
@@ -148,22 +165,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()
     }
@@ -186,6 +209,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]
@@ -195,6 +219,7 @@ 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
@@ -218,6 +243,7 @@ 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
@@ -234,13 +260,45 @@ 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
@@ -266,6 +324,7 @@ 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
@@ -291,6 +350,7 @@ 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: Some(100),

crates/relayer-cli/src/commands/query/packet/pending.rs

@@ -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;
@@ -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)
@@ -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,
         })
@@ -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(),
         }
     }