Apply github suggestions

.changelog/unreleased/improvements/ibc-relayer/4101-add-pagination.md

@@ -1,3 +1,3 @@
-- Implemented pagination for `query_packet_commitments` and `query_packet_acknowledgements`
-  during the scanning phase to optimize the determination of whether to spawn a packet worker.
+- Paginate results of `query_packet_commitments` and `query_packet_acknowledgements`
+  queries to speed up the scanning phase.
   ([\#4101](https://github.com/informalsystems/hermes/issues/4101))

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

@@ -1,13 +1,13 @@
 use core::fmt;
 
 use abscissa_core::clap::Parser;
-use ibc_relayer::chain::requests::Paginate;
 use serde::Serialize;
 
 use ibc_relayer::chain::counterparty::{
     channel_on_destination, pending_packet_summary, PendingPackets,
 };
 use ibc_relayer::chain::handle::{BaseChainHandle, ChainHandle};
+use ibc_relayer::chain::requests::Paginate;
 use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId};
 
 use crate::cli_utils::spawn_chain_counterparty;

crates/relayer/src/chain/cosmos.rs

@@ -1944,7 +1944,8 @@ impl ChainEndpoint for CosmosSdkChain {
             let mut results = Vec::new();
             let mut page_key = Vec::new();
 
-            let mut limit = request.pagination.get_limit();
+            let pagination_information = request.pagination.get_values();
+            let mut current_results = 0;
 
             loop {
                 crate::time!(
@@ -1963,6 +1964,7 @@ impl ChainEndpoint for CosmosSdkChain {
                 }
 
                 let mut tonic_request = tonic::Request::new(raw_request);
+                // TODO: This should either be configurable or inferred from the pagination
                 tonic_request.set_timeout(Duration::from_secs(10));
 
                 let response = self.rt.block_on(async {
@@ -1981,6 +1983,7 @@ impl ChainEndpoint for CosmosSdkChain {
                             .map(|p| p.next_key.clone());
 
                         results.push(Ok(inner_response));
+                        current_results += pagination_information.0;
 
                         match next_key {
                             Some(next_key) if !next_key.is_empty() => {
@@ -1994,17 +1997,12 @@ impl ChainEndpoint for CosmosSdkChain {
                         break;
                     }
                 }
-                if limit == 0 {
+                if current_results >= pagination_information.1 {
                     break;
                 }
-                limit -= 1;
             }
 
-            let responses =
-                results.into_iter().collect::<Result<
-                    Vec<ibc_proto::ibc::core::channel::v1::QueryPacketCommitmentsResponse>,
-                    _,
-                >>()?;
+            let responses = results.into_iter().collect::<Result<Vec<_>, _>>()?;
 
             let mut commitment_sequences = Vec::new();
 
@@ -2183,6 +2181,7 @@ impl ChainEndpoint for CosmosSdkChain {
                 }
 
                 let mut tonic_request = tonic::Request::new(raw_request);
+                // TODO: This should either be configurable or inferred from the pagination
                 tonic_request.set_timeout(Duration::from_secs(10));
 
                 let response = self.rt.block_on(async {
@@ -2218,10 +2217,7 @@ impl ChainEndpoint for CosmosSdkChain {
                 }
             }
 
-            let responses = results.into_iter().collect::<Result<
-                Vec<ibc_proto::ibc::core::channel::v1::QueryPacketAcknowledgementsResponse>,
-                _,
-            >>()?;
+            let responses = results.into_iter().collect::<Result<Vec<_>, _>>()?;
 
             let mut acks_sequences = Vec::new();
 

crates/relayer/src/chain/counterparty.rs

@@ -595,7 +595,7 @@ pub fn unreceived_acknowledgements(
     pagination: Paginate,
 ) -> Result<Option<(Vec<Sequence>, Height)>, Error> {
     let (commitments_on_src, _) =
-        commitments_on_chain(chain, &path.port_id, &path.channel_id, pagination.clone())?;
+        commitments_on_chain(chain, &path.port_id, &path.channel_id, pagination)?;
 
     let acks_and_height_on_counterparty = packet_acknowledgements(
         counterparty_chain,

crates/relayer/src/chain/requests.rs

@@ -138,14 +138,14 @@ impl From<PageRequest> for RawPageRequest {
     }
 }
 
-#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
+#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
 pub enum Paginate {
     #[default]
     All,
 
     PerPage {
-        pagination: u64,
-        limit: u64,
+        per_page: u64,
+        total: u64,
     },
 }
 
@@ -154,26 +154,19 @@ impl Paginate {
         !matches!(self, Self::All)
     }
 
-    pub fn get_limit(&self) -> u64 {
-        if let Paginate::PerPage {
-            pagination: _,
-            limit,
-        } = self
-        {
-            return *limit;
+    pub fn get_values(&self) -> (u64, u64) {
+        match self {
+            Paginate::PerPage { total, per_page } => (*per_page, *total),
+            _ => (0, 0),
         }
-        0
     }
 }
 
 impl From<Paginate> for PageRequest {
     fn from(value: Paginate) -> Self {
         match value {
             Paginate::All => PageRequest::all(),
-            Paginate::PerPage {
-                pagination: _,
-                limit,
-            } => PageRequest::per_page(limit),
+            Paginate::PerPage { per_page, .. } => PageRequest::per_page(per_page),
         }
     }
 }

crates/relayer/src/supervisor/scan.rs

@@ -241,8 +241,8 @@ impl ChannelScan {
                     chain,
                     &ids,
                     Paginate::PerPage {
-                        pagination: 1,
-                        limit: 1,
+                        per_page: 1,
+                        total: 1,
                     },
                 )
                 .map(|(seq, _)| seq)
@@ -265,8 +265,8 @@ impl ChannelScan {
             chain,
             &ids,
             Paginate::PerPage {
-                pagination: 1,
-                limit: 1,
+                per_page: 1,
+                total: 1,
             },
         )
         .map(|sns| sns.map_or(vec![], |(sns, _)| sns))