Collate consecutive heights and sequence numbers shown in logs (#2847)

* Remove unused `num-traits` dependency

* Implement `Add<u64>` and `Sub<u64>` for `Height`

* Implement `Add<u64>` and `Sub<u64>` for `Sequence`

* Collate consecutive sequence numbers and heights in logs

For example, whereas before we would only show the first 50 sequence numbers:

    sequence numbers of ack packets to send to the destination chain out of the ones with acknowledgments on the source chain (first 50 shown here) dst_chain=ibc-0 src_chain=ibc-1 total=184 sequences=[1259, 1260, 1261, 1262, 1263, 1264, 1265, 1266, 1267, 1268, 1269, 1270, 1271, 1272, 1273, 1274, 1275, 1276, 1277, 1278, 1279, 1280, 1281, 1282, 1283, 1284, 1285, 1286, 1287, 1288, 1289, 1290, 1291, 1292, 1293, 1294, 1295, 1296, 1297, 1298, 1299, 1300, 1301, 1302, 1303, 1304, 1305, 1306, 1307, 1308]

We now show all sequences numbers where consecutive ones are collated together:

    sequence numbers of ack packets to send to the destination chain out of the ones with acknowledgments on the source chain dst_chain=ibc-0 src_chain=ibc-1 total=184 sequences=1259..=1329, 1359..=1439, 1469..=1500

* Add changelog entry

* Rename `collate` extension method to `collated` to match `copied`, `cloned`, etc.

* Collate the output of the `query packet` commands

.changelog/unreleased/improvements/ibc-relayer-cli/2847-collate-seqs.md

@@ -0,0 +1,2 @@
+- Collate consecutive heights and sequence numbers shown in logs
+  ([#2847](https://github.com/informalsystems/ibc-rs/issues/2847))

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

@@ -8,6 +8,7 @@ mod commitments;
 mod pending;
 mod pending_acks;
 mod pending_sends;
+mod util;
 
 #[derive(Command, Debug, Parser, Runnable)]
 pub enum QueryPacketCmds {

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

@@ -1,23 +1,16 @@
 use abscissa_core::clap::Parser;
 use abscissa_core::{Command, Runnable};
-use ibc_relayer_types::core::ics04_channel::packet::Sequence;
-use serde::Serialize;
 
+use ibc_relayer::chain::counterparty::acknowledgements_on_chain;
 use ibc_relayer::chain::handle::BaseChainHandle;
 use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};
-use ibc_relayer_types::Height;
 
 use crate::cli_utils::spawn_chain_counterparty;
 use crate::conclude::Output;
 use crate::error::Error;
 use crate::prelude::*;
-use ibc_relayer::chain::counterparty::acknowledgements_on_chain;
 
-#[derive(Serialize, Debug)]
-struct PacketSeqs {
-    height: Height,
-    seqs: Vec<Sequence>,
-}
+use super::util::PacketSeqs;
 
 #[derive(Clone, Command, Debug, Parser, PartialEq, Eq)]
 pub struct QueryPacketAcknowledgementsCmd {
@@ -72,8 +65,11 @@ impl QueryPacketAcknowledgementsCmd {
 // cargo run --bin hermes -- query packet acknowledgements --chain ibc-0 --port transfer --connection ibconexfer --height 3
 impl Runnable for QueryPacketAcknowledgementsCmd {
     fn run(&self) {
+        use crate::conclude::json;
+
         match self.execute() {
-            Ok(ps) => Output::success(ps).exit(),
+            Ok(p) if json() => Output::success(p).exit(),
+            Ok(p) => Output::success(p.collated()).exit(),
             Err(e) => Output::error(format!("{}", e)).exit(),
         }
     }

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

@@ -1,22 +1,15 @@
 use abscissa_core::clap::Parser;
 use abscissa_core::{Command, Runnable};
-use ibc_relayer_types::core::ics04_channel::packet::Sequence;
-use serde::Serialize;
 
 use ibc_relayer::chain::counterparty::commitments_on_chain;
 use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};
-use ibc_relayer_types::Height;
 
 use crate::cli_utils::spawn_chain_runtime;
 use crate::conclude::Output;
 use crate::error::Error;
 use crate::prelude::*;
 
-#[derive(Serialize, Debug)]
-struct PacketSeqs {
-    height: Height,
-    seqs: Vec<Sequence>,
-}
+use super::util::PacketSeqs;
 
 #[derive(Clone, Command, Debug, Parser, PartialEq, Eq)]
 pub struct QueryPacketCommitmentsCmd {
@@ -67,8 +60,11 @@ impl QueryPacketCommitmentsCmd {
 // cargo run --bin hermes -- query packet commitments --chain ibc-0 --port transfer --channel ibconexfer --height 3
 impl Runnable for QueryPacketCommitmentsCmd {
     fn run(&self) {
+        use crate::conclude::json;
+
         match self.execute() {
-            Ok(p) => Output::success(p).exit(),
+            Ok(p) if json() => Output::success(p).exit(),
+            Ok(p) => Output::success(p.collated()).exit(),
             Err(e) => Output::error(format!("{}", e)).exit(),
         }
     }

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

@@ -13,14 +13,25 @@ use crate::conclude::Output;
 use crate::error::Error;
 use crate::prelude::*;
 
+use super::util::CollatedPendingPackets;
+
 /// A structure to display pending packet commitment sequence IDs
 /// at both ends of a channel.
 #[derive(Debug, Serialize)]
-struct Summary {
+struct Summary<P> {
     /// The packets sent on the source chain as identified by the command.
-    src: PendingPackets,
+    src: P,
     /// The packets sent on the counterparty chain.
-    dst: PendingPackets,
+    dst: P,
+}
+
+impl Summary<PendingPackets> {
+    fn collate(self) -> Summary<CollatedPendingPackets> {
+        Summary {
+            src: CollatedPendingPackets::new(self.src),
+            dst: CollatedPendingPackets::new(self.dst),
+        }
+    }
 }
 
 /// This command does the following:
@@ -61,7 +72,7 @@ pub struct QueryPendingPacketsCmd {
 }
 
 impl QueryPendingPacketsCmd {
-    fn execute(&self) -> Result<Summary, Error> {
+    fn execute(&self) -> Result<Summary<PendingPackets>, Error> {
         let config = app_config();
 
         let (chains, chan_conn_cli) = spawn_chain_counterparty::<BaseChainHandle>(
@@ -78,15 +89,18 @@ impl QueryPendingPacketsCmd {
 
         let src_summary = pending_packet_summary(&chains.src, &chains.dst, &chan_conn_cli.channel)
             .map_err(Error::supervisor)?;
+
         let counterparty_channel = channel_on_destination(
             &chan_conn_cli.channel,
             &chan_conn_cli.connection,
             &chains.dst,
         )
         .map_err(Error::supervisor)?
         .ok_or_else(|| Error::missing_counterparty_channel_id(chan_conn_cli.channel))?;
+
         let dst_summary = pending_packet_summary(&chains.dst, &chains.src, &counterparty_channel)
             .map_err(Error::supervisor)?;
+
         Ok(Summary {
             src: src_summary,
             dst: dst_summary,
@@ -96,9 +110,12 @@ impl QueryPendingPacketsCmd {
 
 impl Runnable for QueryPendingPacketsCmd {
     fn run(&self) {
+        use crate::conclude::json;
+
         match self.execute() {
-            Ok(pending) => Output::success(pending).exit(),
-            Err(e) => Output::error(format!("{}", e)).exit(),
+            Ok(summary) if json() => Output::success(summary).exit(),
+            Ok(summary) => Output::success(summary.collate()).exit(),
+            Err(e) => Output::error(e).exit(),
         }
     }
 }

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

@@ -4,6 +4,7 @@ use abscissa_core::{Command, Runnable};
 use ibc_relayer::chain::counterparty::unreceived_acknowledgements;
 use ibc_relayer::chain::handle::BaseChainHandle;
 use ibc_relayer::path::PathIdentifiers;
+use ibc_relayer::util::collate::CollatedIterExt;
 use ibc_relayer_types::core::ics04_channel::packet::Sequence;
 use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};
 
@@ -76,9 +77,12 @@ impl QueryPendingAcksCmd {
 
 impl Runnable for QueryPendingAcksCmd {
     fn run(&self) {
+        use crate::conclude::json;
+
         match self.execute() {
-            Ok(seqs) => Output::success(seqs).exit(),
-            Err(e) => Output::error(format!("{}", e)).exit(),
+            Ok(seqs) if json() => Output::success(seqs).exit(),
+            Ok(seqs) => Output::success(seqs.into_iter().collated().collect::<Vec<_>>()).exit(),
+            Err(e) => Output::error(e).exit(),
         }
     }
 }

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

@@ -4,6 +4,7 @@ use abscissa_core::{Command, Runnable};
 use ibc_relayer::chain::counterparty::unreceived_packets;
 use ibc_relayer::chain::handle::BaseChainHandle;
 use ibc_relayer::path::PathIdentifiers;
+use ibc_relayer::util::collate::CollatedIterExt;
 use ibc_relayer_types::core::ics04_channel::packet::Sequence;
 use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};
 
@@ -76,9 +77,12 @@ impl QueryPendingSendsCmd {
 
 impl Runnable for QueryPendingSendsCmd {
     fn run(&self) {
+        use crate::conclude::json;
+
         match self.execute() {
-            Ok(seqs) => Output::success(seqs).exit(),
-            Err(e) => Output::error(format!("{}", e)).exit(),
+            Ok(seqs) if json() => Output::success(seqs).exit(),
+            Ok(seqs) => Output::success(seqs.into_iter().collated().collect::<Vec<_>>()).exit(),
+            Err(e) => Output::error(e).exit(),
         }
     }
 }

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

@@ -0,0 +1,63 @@
+use core::fmt;
+
+use serde::Serialize;
+
+use ibc_relayer::util::collate::{Collated, CollatedIterExt};
+use ibc_relayer_types::core::ics04_channel::packet::Sequence;
+use ibc_relayer_types::Height;
+
+pub use ibc_relayer::chain::counterparty::PendingPackets;
+
+#[derive(Serialize)]
+pub struct CollatedPendingPackets {
+    pub unreceived_packets: Vec<Collated<Sequence>>,
+    pub unreceived_acks: Vec<Collated<Sequence>>,
+}
+
+impl fmt::Debug for CollatedPendingPackets {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("PendingPackets")
+            .field("unreceived_packets", &self.unreceived_packets)
+            .field("unreceived_acks", &self.unreceived_acks)
+            .finish()
+    }
+}
+
+impl CollatedPendingPackets {
+    pub fn new(pending: PendingPackets) -> Self {
+        Self {
+            unreceived_packets: pending.unreceived_packets.into_iter().collated().collect(),
+            unreceived_acks: pending.unreceived_acks.into_iter().collated().collect(),
+        }
+    }
+}
+
+#[derive(Serialize, Debug)]
+pub struct PacketSeqs {
+    pub height: Height,
+    pub seqs: Vec<Sequence>,
+}
+
+impl PacketSeqs {
+    pub fn collated(self) -> CollatedPacketSeqs {
+        CollatedPacketSeqs {
+            height: self.height,
+            seqs: self.seqs.into_iter().collated().collect(),
+        }
+    }
+}
+
+#[derive(Serialize)]
+pub struct CollatedPacketSeqs {
+    pub height: Height,
+    pub seqs: Vec<Collated<Sequence>>,
+}
+
+impl fmt::Debug for CollatedPacketSeqs {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("PacketSeqs")
+            .field("height", &self.height)
+            .field("seqs", &self.seqs)
+            .finish()
+    }
+}

crates/relayer-types/Cargo.toml

@@ -41,7 +41,6 @@ safe-regex = { version = "0.2.5", default-features = false }
 subtle-encoding = { version = "0.5", default-features = false }
 sha2 = { version = "0.10.6", default-features = false }
 flex-error = { version = "0.4.4", default-features = false }
-num-traits = { version = "0.2.15", default-features = false }
 derive_more = { version = "0.99.17", default-features = false, features = ["from", "into", "display"] }
 uint = { version = "0.9", default-features = false }
 itertools = { version = "0.10.3", default-features = false, features = ["use_alloc"] }

crates/relayer-types/src/clients/ics07_tendermint/client_state.rs

@@ -170,7 +170,7 @@ impl ClientState {
             return Err(Error::not_enough_time_elapsed(current_time, earliest_time));
         }
 
-        let earliest_height = processed_height.add(delay_period_blocks);
+        let earliest_height = processed_height + delay_period_blocks;
         if current_height < earliest_height {
             return Err(Error::not_enough_blocks_elapsed(
                 current_height,

crates/relayer-types/src/core/ics02_client/height.rs

@@ -41,30 +41,12 @@ impl Height {
         self.revision_height
     }
 
-    pub fn add(&self, delta: u64) -> Height {
-        Height {
-            revision_number: self.revision_number,
-            revision_height: self.revision_height + delta,
-        }
-    }
-
-    pub fn increment(&self) -> Height {
-        self.add(1)
-    }
-
-    pub fn sub(&self, delta: u64) -> Result<Height, Error> {
-        if self.revision_height <= delta {
-            return Err(Error::invalid_height_result());
-        }
-
-        Ok(Height {
-            revision_number: self.revision_number,
-            revision_height: self.revision_height - delta,
-        })
+    pub fn increment(self) -> Height {
+        self + 1
     }
 
-    pub fn decrement(&self) -> Result<Height, Error> {
-        self.sub(1)
+    pub fn decrement(self) -> Result<Height, Error> {
+        self - 1
     }
 }
 
@@ -90,6 +72,32 @@ impl Ord for Height {
     }
 }
 
+impl core::ops::Add<u64> for Height {
+    type Output = Self;
+
+    fn add(self, rhs: u64) -> Self::Output {
+        Self {
+            revision_number: self.revision_number,
+            revision_height: self.revision_height + rhs,
+        }
+    }
+}
+
+impl core::ops::Sub<u64> for Height {
+    type Output = Result<Self, Error>;
+
+    fn sub(self, delta: u64) -> Self::Output {
+        if self.revision_height <= delta {
+            return Err(Error::invalid_height_result());
+        }
+
+        Ok(Height {
+            revision_number: self.revision_number,
+            revision_height: self.revision_height - delta,
+        })
+    }
+}
+
 impl Protobuf<RawHeight> for Height {}
 
 impl TryFrom<RawHeight> for Height {

crates/relayer-types/src/core/ics04_channel/packet.rs

@@ -83,6 +83,22 @@ impl core::fmt::Display for Sequence {
     }
 }
 
+impl core::ops::Add<Self> for Sequence {
+    type Output = Self;
+
+    fn add(self, rhs: Self) -> Self {
+        Self(self.0 + rhs.0)
+    }
+}
+
+impl core::ops::Add<u64> for Sequence {
+    type Output = Self;
+
+    fn add(self, rhs: u64) -> Self {
+        Self(self.0 + rhs)
+    }
+}
+
 #[derive(Clone, Default, Hash, PartialEq, Eq, Deserialize, Serialize)]
 pub struct Packet {
     pub sequence: Sequence,