From f9a3a2e5119fc64dcb773b470e47ca9b061f1cb5 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Fri, 12 Jan 2024 10:23:47 +0100 Subject: [PATCH] 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 --- .../ibc-relayer-cli/3672-clears-packet-seq.md | 18 +++ config.toml | 4 +- crates/relayer-cli/src/commands/clear.rs | 86 +++++++++-- .../src/commands/query/packet/pending.rs | 50 ++++++- crates/relayer-cli/src/commands/tx/packet.rs | 40 ++++- .../src/core/ics04_channel/packet.rs | 19 ++- crates/relayer/src/link/cli.rs | 31 +++- crates/relayer/src/util.rs | 1 + crates/relayer/src/util/collate.rs | 8 +- crates/relayer/src/util/seq_range.rs | 139 ++++++++++++++++++ .../templates/help_templates/clear/packets.md | 13 +- .../templates/help_templates/tx/packet-ack.md | 6 + .../help_templates/tx/packet-recv.md | 7 + .../src/tests/clear_packet.rs | 136 ++++++++++++++++- .../src/tests/ordered_channel_clear.rs | 7 +- .../src/tests/query_packet.rs | 4 +- 16 files changed, 529 insertions(+), 40 deletions(-) create mode 100644 .changelog/unreleased/features/ibc-relayer-cli/3672-clears-packet-seq.md create mode 100644 crates/relayer/src/util/seq_range.rs diff --git a/.changelog/unreleased/features/ibc-relayer-cli/3672-clears-packet-seq.md b/.changelog/unreleased/features/ibc-relayer-cli/3672-clears-packet-seq.md new file mode 100644 index 0000000000..c721d17bc7 --- /dev/null +++ b/.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)) diff --git a/config.toml b/config.toml index 2ceb166ccb..804025fea5 100644 --- a/config.toml +++ b/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 diff --git a/crates/relayer-cli/src/commands/clear.rs b/crates/relayer-cli/src/commands/clear.rs index 9e3b9c06a4..ef463808fd 100644 --- a/crates/relayer-cli/src/commands/clear.rs +++ b/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>, + #[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, #[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, #[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, } @@ -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,6 +260,37 @@ 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!( @@ -241,6 +298,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: 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), diff --git a/crates/relayer-cli/src/commands/query/packet/pending.rs b/crates/relayer-cli/src/commands/query/packet/pending.rs index 8abc121260..73a1781b5c 100644 --- a/crates/relayer-cli/src/commands/query/packet/pending.rs +++ b/crates/relayer-cli/src/commands/query/packet/pending.rs @@ -1,3 +1,5 @@ +use core::fmt; + use abscissa_core::clap::Parser; use abscissa_core::{Command, Runnable}; use serde::Serialize; @@ -5,7 +7,7 @@ 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,8 +21,15 @@ use super::util::CollatedPendingPackets; /// at both ends of a channel. #[derive(Debug, Serialize)] struct Summary

{ + /// 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, } @@ -28,12 +37,47 @@ struct Summary

{ impl Summary { fn collate(self) -> Summary { 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 { + 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(), } } diff --git a/crates/relayer-cli/src/commands/tx/packet.rs b/crates/relayer-cli/src/commands/tx/packet.rs index 7313a46a3e..171ac93fad 100644 --- a/crates/relayer-cli/src/commands/tx/packet.rs +++ b/crates/relayer-cli/src/commands/tx/packet.rs @@ -1,9 +1,12 @@ use abscissa_core::clap::Parser; use abscissa_core::{Command, Runnable}; use ibc_relayer_types::core::ics02_client::height::Height; +use std::ops::RangeInclusive; use ibc_relayer::chain::handle::ChainHandle; 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; @@ -51,6 +54,19 @@ pub struct TxPacketRecvCmd { )] src_channel_id: ChannelId, + #[clap( + long = "packet-sequences", + help = "Sequences of packets to be cleared on `dst-chain`. \ + Either a single sequence or a range of sequences can be specified. \ + If not provided, all pending recv or timeout packets will be cleared. \ + 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>, + #[clap( long = "packet-data-query-height", help = "Exact height at which the packet data is queried via block_results RPC" @@ -82,6 +98,7 @@ impl Runnable for TxPacketRecvCmd { let res: Result, Error> = link .relay_recv_packet_and_timeout_messages_with_packet_data_query_height( + self.packet_sequences.clone(), packet_data_query_height, ) .map_err(Error::link); @@ -132,6 +149,19 @@ pub struct TxPacketAckCmd { )] src_channel_id: ChannelId, + #[clap( + long = "packet-sequences", + help = "Sequences of packets to be cleared on `dst-chain`. \ + Either a single sequence or a range of sequences can be specified. \ + If not provided, all pending ack packets will be cleared. \ + 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>, + #[clap( long = "packet-data-query-height", help = "Exact height at which the packet data is queried via block_results RPC" @@ -162,7 +192,10 @@ impl Runnable for TxPacketAckCmd { .map(|height| Height::new(link.a_to_b.src_chain().id().version(), height).unwrap()); let res: Result, Error> = link - .relay_ack_packet_messages_with_packet_data_query_height(packet_data_query_height) + .relay_ack_packet_messages_with_packet_data_query_height( + self.packet_sequences.clone(), + packet_data_query_height, + ) .map_err(Error::link); match res { @@ -189,6 +222,7 @@ mod tests { src_chain_id: ChainId::from_string("chain_sender"), src_port_id: PortId::from_str("port_sender").unwrap(), src_channel_id: ChannelId::from_str("channel_sender").unwrap(), + packet_sequences: vec![], packet_data_query_height: None }, TxPacketRecvCmd::parse_from([ @@ -213,6 +247,7 @@ mod tests { src_chain_id: ChainId::from_string("chain_sender"), src_port_id: PortId::from_str("port_sender").unwrap(), src_channel_id: ChannelId::from_str("channel_sender").unwrap(), + packet_sequences: vec![], packet_data_query_height: None }, TxPacketRecvCmd::parse_from([ @@ -236,6 +271,7 @@ mod tests { src_chain_id: ChainId::from_string("chain_sender"), src_port_id: PortId::from_str("port_sender").unwrap(), src_channel_id: ChannelId::from_str("channel_sender").unwrap(), + packet_sequences: vec![], packet_data_query_height: Some(5), }, TxPacketRecvCmd::parse_from([ @@ -318,6 +354,7 @@ mod tests { src_chain_id: ChainId::from_string("chain_sender"), src_port_id: PortId::from_str("port_sender").unwrap(), src_channel_id: ChannelId::from_str("channel_sender").unwrap(), + packet_sequences: vec![], packet_data_query_height: None }, TxPacketAckCmd::parse_from([ @@ -342,6 +379,7 @@ mod tests { src_chain_id: ChainId::from_string("chain_sender"), src_port_id: PortId::from_str("port_sender").unwrap(), src_channel_id: ChannelId::from_str("channel_sender").unwrap(), + packet_sequences: vec![], packet_data_query_height: None }, TxPacketAckCmd::parse_from([ diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index bd1630c1a3..316227c08f 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -44,9 +44,7 @@ impl core::fmt::Display for PacketMsgType { } /// The sequence number of a packet enforces ordering among packets from the same source. -#[derive( - Copy, Clone, Debug, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Deserialize, Serialize, -)] +#[derive(Copy, Clone, Default, PartialEq, Eq, Hash, PartialOrd, Ord, Deserialize, Serialize)] pub struct Sequence(u64); impl FromStr for Sequence { @@ -60,6 +58,9 @@ impl FromStr for Sequence { } impl Sequence { + pub const MIN: Self = Self(0); + pub const MAX: Self = Self(u64::MAX); + pub fn is_zero(&self) -> bool { self.0 == 0 } @@ -67,6 +68,10 @@ impl Sequence { pub fn increment(&self) -> Sequence { Sequence(self.0 + 1) } + + pub fn as_u64(&self) -> u64 { + self.0 + } } impl From for Sequence { @@ -81,9 +86,15 @@ impl From for u64 { } } +impl core::fmt::Debug for Sequence { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { + self.0.fmt(f) + } +} + impl core::fmt::Display for Sequence { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { - write!(f, "{}", self.0) + self.0.fmt(f) } } diff --git a/crates/relayer/src/link/cli.rs b/crates/relayer/src/link/cli.rs index 230c370e54..d5f896cc6e 100644 --- a/crates/relayer/src/link/cli.rs +++ b/crates/relayer/src/link/cli.rs @@ -1,4 +1,5 @@ use std::convert::TryInto; +use std::ops::RangeInclusive; use std::thread; use std::time::{Duration, Instant}; @@ -71,12 +72,16 @@ impl RelayPath { } impl Link { - pub fn relay_recv_packet_and_timeout_messages(&self) -> Result, LinkError> { - self.relay_recv_packet_and_timeout_messages_with_packet_data_query_height(None) + pub fn relay_recv_packet_and_timeout_messages( + &self, + sequences: Vec>, + ) -> Result, LinkError> { + self.relay_recv_packet_and_timeout_messages_with_packet_data_query_height(sequences, None) } /// Implements the `packet-recv` CLI pub fn relay_recv_packet_and_timeout_messages_with_packet_data_query_height( &self, + sequence_filter: Vec>, packet_data_query_height: Option, ) -> Result, LinkError> { let _span = error_span!( @@ -89,7 +94,7 @@ impl Link { .entered(); // Find the sequence numbers of unreceived packets - let (sequences, src_response_height) = unreceived_packets( + let (mut sequences, src_response_height) = unreceived_packets( self.a_to_b.dst_chain(), self.a_to_b.src_chain(), &self.a_to_b.path_id, @@ -100,6 +105,11 @@ impl Link { return Ok(vec![]); } + if !sequence_filter.is_empty() { + info!("filtering unreceived packets by given sequence ranges"); + sequences.retain(|seq| sequence_filter.iter().any(|range| range.contains(seq))); + } + info!( "{} unreceived packets found: {} ", sequences.len(), @@ -126,13 +136,17 @@ impl Link { ) } - pub fn relay_ack_packet_messages(&self) -> Result, LinkError> { - self.relay_ack_packet_messages_with_packet_data_query_height(None) + pub fn relay_ack_packet_messages( + &self, + sequences: Vec>, + ) -> Result, LinkError> { + self.relay_ack_packet_messages_with_packet_data_query_height(sequences, None) } /// Implements the `packet-ack` CLI pub fn relay_ack_packet_messages_with_packet_data_query_height( &self, + sequence_filter: Vec>, packet_data_query_height: Option, ) -> Result, LinkError> { let _span = error_span!( @@ -145,7 +159,7 @@ impl Link { .entered(); // Find the sequence numbers of unreceived acknowledgements - let Some((sequences, src_response_height)) = unreceived_acknowledgements( + let Some((mut sequences, src_response_height)) = unreceived_acknowledgements( self.a_to_b.dst_chain(), self.a_to_b.src_chain(), &self.a_to_b.path_id, @@ -159,6 +173,11 @@ impl Link { return Ok(vec![]); } + if !sequence_filter.is_empty() { + info!("filtering unreceived acknowledgements by given sequence ranges"); + sequences.retain(|seq| sequence_filter.iter().any(|range| range.contains(seq))); + } + info!( "{} unreceived acknowledgements found: {} ", sequences.len(), diff --git a/crates/relayer/src/util.rs b/crates/relayer/src/util.rs index e944c2023a..e589e8c7c8 100644 --- a/crates/relayer/src/util.rs +++ b/crates/relayer/src/util.rs @@ -11,5 +11,6 @@ pub mod pretty; pub mod profiling; pub mod queue; pub mod retry; +pub mod seq_range; pub mod stream; pub mod task; diff --git a/crates/relayer/src/util/collate.rs b/crates/relayer/src/util/collate.rs index 305374b425..092a579bd1 100644 --- a/crates/relayer/src/util/collate.rs +++ b/crates/relayer/src/util/collate.rs @@ -2,7 +2,7 @@ use std::{fmt, ops::Add}; use serde::{Deserialize, Serialize}; -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct Collated { pub start: T, pub end: T, @@ -14,6 +14,12 @@ impl Collated { } } +impl fmt::Debug for Collated { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}..={:?}", self.start, self.end) + } +} + impl fmt::Display for Collated { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}..={}", self.start, self.end) diff --git a/crates/relayer/src/util/seq_range.rs b/crates/relayer/src/util/seq_range.rs new file mode 100644 index 0000000000..d569d156ef --- /dev/null +++ b/crates/relayer/src/util/seq_range.rs @@ -0,0 +1,139 @@ +use std::ops::RangeInclusive; + +use ibc_relayer_types::core::ics04_channel::packet::Sequence; +use thiserror::Error; + +#[derive(Clone, Debug, PartialEq, Eq, Error)] +pub enum Error { + #[error("Invalid sequence number: {0}")] + InvalidSequenceNumber(String), + + #[error("Invalid range: {0}")] + InvalidRange(String), +} + +/// Parse a list of ranges over sequence numbers, separated by commas. +/// +/// - Each item in the list is either a single sequence number, or a range of sequence numbers. +/// - A range is specified as `start..end`, where `start` and `end` are sequence numbers. +/// - If `start` is omitted, the range starts at the minimum sequence number. +/// - If `end` is omitted, the range ends at the maximum sequence number. +/// - If both `start` and `end` are omitted, the range sastifies any sequence number. +/// +/// # Examples +/// - `1` Single sequence number `1` +/// - `1,2,3` Sequence numbers `1`, `2`, and `3` +/// - `..20` Sequence numbers less than or equal to `20` +/// - `10..` Sequence numbers greater than or equal to `10` +/// - `10..20` Sequence numbers `10`, `11`, `12`, ..., `20` +/// - `2,4..6,12,14..17,21,30..` Sequence numbers `2`, `4`, `5`, `6`, `12`, `14`, `15`, `16`, `17`, `21`, `30`, `31`, `32`, ... +/// - `30..,21,12,14..17,4..6,2` Same as previous +/// - `..` Any sequence number +pub fn parse_seq_ranges(s: &str) -> Result>, Error> { + s.split(',').map(parse_seq_range).collect() +} + +/// Parse a range of sequence numbers. +/// +/// - This can be a single sequence number, or a range of sequence numbers. +/// - A range is specified as `start..end`, where `start` and `end` are sequence numbers. +/// - If `start` is omitted, the range starts at the minimum sequence number. +/// - If `end` is omitted, the range ends at the maximum sequence number.` +/// - If both `start` and `end` are omitted, the range sastifies any sequence number. +/// +/// # Examples +/// - `1` Single sequence number `1` +/// - `1..2` Single sequence number `1` +/// - `..20` Sequence numbers strictly less than `20` +/// - `10..` Sequence numbers greater than or equal to `10` +/// - `10..20` Sequence numbers `10`, `11`, `12`, ..., `19` +/// - `..` Any sequence number +pub fn parse_seq_range(s: &str) -> Result, Error> { + if s.contains("..") { + parse_range(s) + } else { + parse_single(s) + } +} + +fn parse_int(s: &str) -> Result { + s.parse::() + .map_err(|_| Error::InvalidSequenceNumber(s.to_string())) +} + +fn parse_single(s: &str) -> Result, Error> { + parse_int(s).map(|num| num..=num) +} + +fn parse_range(s: &str) -> Result, Error> { + match s.split_once("..") { + // .. + Some(("", "")) => Ok(Sequence::MIN..=Sequence::MAX), + + // ..end + Some(("", end)) => { + let end = parse_int(end)?; + Ok(Sequence::MIN..=end) + } + + // start.. + Some((start, "")) => { + let start = parse_int(start)?; + Ok(start..=Sequence::MAX) + } + + // start..end + Some((start, end)) => { + let start = parse_int(start)?; + let end = parse_int(end)?; + Ok(start..=end) + } + + // not a range + None => Err(Error::InvalidRange(s.to_string())), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn r(range: RangeInclusive) -> RangeInclusive { + Sequence::from(*range.start())..=Sequence::from(*range.end()) + } + + #[test] + fn parse_seq_ranges_works() { + let tests = [ + ("1", vec![r(1..=1)]), + ("1,2", vec![r(1..=1), r(2..=2)]), + ("1,2,3", vec![r(1..=1), r(2..=2), r(3..=3)]), + ("1..3", vec![r(1..=3)]), + ("..3", vec![r(u64::MIN..=3)]), + ("3..", vec![r(3..=u64::MAX)]), + ("..", vec![r(u64::MIN..=u64::MAX)]), + ("1..3,4", vec![r(1..=3), r(4..=4)]), + ("1,2..4", vec![r(1..=1), r(2..=4)]), + ("1..3,4..6", vec![r(1..=3), r(4..=6)]), + ( + "..3,4..,..", + vec![r(u64::MIN..=3), r(4..=u64::MAX), r(u64::MIN..=u64::MAX)], + ), + ( + "1..,..6,7..7", + vec![r(1..=u64::MAX), r(u64::MIN..=6), r(7..=7)], + ), + ]; + + for (input, expected) in tests { + let actual = parse_seq_ranges(input).unwrap(); + assert_eq!(actual, expected); + } + + let fails = ["1-1", "1.1", "-1", "1..2..3", "1..-2", "-1.22"]; + + for fail in fails { + assert!(parse_seq_ranges(fail).is_err()); + } + } +} diff --git a/guide/src/templates/help_templates/clear/packets.md b/guide/src/templates/help_templates/clear/packets.md index 8228833170..35732330c6 100644 --- a/guide/src/templates/help_templates/clear/packets.md +++ b/guide/src/templates/help_templates/clear/packets.md @@ -7,17 +7,24 @@ USAGE: OPTIONS: --counterparty-key-name - use the given signing key for the counterparty chain (default: `counterparty_key_name` + Use the given signing key for the counterparty chain (default: `counterparty_key_name` config) -h, --help Print help information --key-name - use the given signing key for the specified chain (default: `key_name` config) + Use the given signing key for the specified chain (default: `key_name` config) + + --packet-sequences + 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 --query-packets-chunk-size - number of packets to fetch at once from the chain (default: `query_packets_chunk_size` + Number of packets to fetch at once from the chain (default: `query_packets_chunk_size` config) REQUIRED: diff --git a/guide/src/templates/help_templates/tx/packet-ack.md b/guide/src/templates/help_templates/tx/packet-ack.md index cb5ad49f54..ef90d47362 100644 --- a/guide/src/templates/help_templates/tx/packet-ack.md +++ b/guide/src/templates/help_templates/tx/packet-ack.md @@ -11,6 +11,12 @@ OPTIONS: --packet-data-query-height Exact height at which the packet data is queried via block_results RPC + --packet-sequences + Sequences of packets to be cleared on `dst-chain`. Either a single sequence or a range + of sequences can be specified. If not provided, all pending ack packets will be cleared. + 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 + REQUIRED: --dst-chain Identifier of the destination chain --src-chain Identifier of the source chain diff --git a/guide/src/templates/help_templates/tx/packet-recv.md b/guide/src/templates/help_templates/tx/packet-recv.md index 747a5a7ccc..1d44fe0482 100644 --- a/guide/src/templates/help_templates/tx/packet-recv.md +++ b/guide/src/templates/help_templates/tx/packet-recv.md @@ -11,6 +11,13 @@ OPTIONS: --packet-data-query-height Exact height at which the packet data is queried via block_results RPC + --packet-sequences + Sequences of packets to be cleared on `dst-chain`. Either a single sequence or a range + of sequences can be specified. If not provided, all pending recv or timeout packets will + be cleared. 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 + REQUIRED: --dst-chain Identifier of the destination chain --src-chain Identifier of the source chain diff --git a/tools/integration-test/src/tests/clear_packet.rs b/tools/integration-test/src/tests/clear_packet.rs index 5bff304825..4a02ba0d14 100644 --- a/tools/integration-test/src/tests/clear_packet.rs +++ b/tools/integration-test/src/tests/clear_packet.rs @@ -1,7 +1,9 @@ use std::thread; +use ibc_relayer::chain::counterparty::pending_packet_summary; use ibc_relayer::config::ChainConfig; use ibc_test_framework::prelude::*; +use ibc_test_framework::relayer::channel::query_identified_channel_end; use ibc_test_framework::util::random::random_u128_range; #[test] @@ -24,8 +26,16 @@ fn test_clear_packet_override() -> Result<(), Error> { run_binary_channel_test(&ClearPacketOverrideTest) } +#[test] +fn test_clear_packet_sequences() -> Result<(), Error> { + run_binary_channel_test(&ClearPacketSequencesTest) +} + pub struct ClearPacketTest; pub struct ClearPacketRecoveryTest; +pub struct ClearPacketNoScanTest; +pub struct ClearPacketOverrideTest; +pub struct ClearPacketSequencesTest; impl TestOverrides for ClearPacketTest { fn modify_relayer_config(&self, config: &mut Config) { @@ -187,8 +197,6 @@ impl BinaryChannelTest for ClearPacketRecoveryTest { } } -pub struct ClearPacketNoScanTest; - impl TestOverrides for ClearPacketNoScanTest { fn modify_relayer_config(&self, config: &mut Config) { // Disabling the client workers and clear_on_start should make the relayer not @@ -300,7 +308,6 @@ impl BinaryChannelTest for ClearPacketNoScanTest { }) } } -pub struct ClearPacketOverrideTest; impl TestOverrides for ClearPacketOverrideTest { fn modify_relayer_config(&self, config: &mut Config) { @@ -421,3 +428,126 @@ impl BinaryChannelTest for ClearPacketOverrideTest { }) } } + +impl TestOverrides for ClearPacketSequencesTest { + fn should_spawn_supervisor(&self) -> bool { + false + } +} + +use ibc_relayer::link::{Link, LinkParameters}; + +impl BinaryChannelTest for ClearPacketSequencesTest { + fn run( + &self, + _config: &TestConfig, + _relayer: RelayerDriver, + chains: ConnectedChains, + channel: ConnectedChannel, + ) -> Result<(), Error> { + const NUM_TRANSFERS: usize = 20; + + let denom_a = chains.node_a.denom(); + + let wallet_a = chains.node_a.wallets().user1().cloned(); + let wallet_b = chains.node_b.wallets().user1().cloned(); + + let amount = denom_a.with_amount(random_u128_range(1000, 5000)); + + info!("Performing {NUM_TRANSFERS} IBC transfer, which should *not* be relayed"); + + chains.node_a.chain_driver().ibc_transfer_token_multiple( + &channel.port_a.as_ref(), + &channel.channel_id_a.as_ref(), + &wallet_a.as_ref(), + &wallet_b.address(), + &amount.as_ref(), + NUM_TRANSFERS, + None, + )?; + + sleep(Duration::from_secs(5)); + + let channel_end_a = query_identified_channel_end( + chains.handle_a(), + channel.channel_id_a.as_ref(), + channel.port_a.as_ref(), + )?; + + let pending_packets_a = + pending_packet_summary(chains.handle_a(), chains.handle_b(), channel_end_a.value())?; + + info!("Pending packets: {:?}", pending_packets_a); + + assert_eq!(pending_packets_a.unreceived_packets.len(), NUM_TRANSFERS); + + let opts = LinkParameters { + src_port_id: channel.port_a.clone().into_value(), + src_channel_id: channel.channel_id_a.clone().into_value(), + }; + + // Clear all even packets + let to_clear = pending_packets_a + .unreceived_packets + .iter() + .filter(|seq| seq.as_u64() % 2 == 0) + .map(|&seq| seq..=seq) + .collect::>(); + + info!("Packets to clear: {:?}", to_clear); + + let link = Link::new_from_opts( + chains.handle_a().clone(), + chains.handle_b().clone(), + opts, + false, + false, + )?; + + info!("Clearing all even packets ({})", to_clear.len()); + + link.relay_recv_packet_and_timeout_messages(to_clear) + .unwrap(); + + sleep(Duration::from_secs(10)); + + let pending_packets = + pending_packet_summary(chains.handle_a(), chains.handle_b(), channel_end_a.value())?; + + info!("Pending packets: {pending_packets:?}"); + + assert_eq!(pending_packets.unreceived_packets.len(), NUM_TRANSFERS / 2); + assert_eq!(pending_packets.unreceived_acks.len(), NUM_TRANSFERS / 2); + + let to_clear = pending_packets + .unreceived_acks + .iter() + .map(|&seq| seq..=seq) + .collect::>(); + + info!("Packets to clear: {to_clear:?}"); + + info!("Clearing all unreceived ack packets ({})", to_clear.len()); + + let rev_link = link.reverse(false, false).unwrap(); + rev_link.relay_ack_packet_messages(to_clear).unwrap(); + + let pending_packets_a = + pending_packet_summary(chains.handle_a(), chains.handle_b(), channel_end_a.value())?; + + info!("Pending packets: {pending_packets_a:?}"); + + assert_eq!(pending_packets_a.unreceived_acks.len(), 0); + assert_eq!( + pending_packets_a.unreceived_packets.len(), + NUM_TRANSFERS / 2 + ); + + info!( + "Successfully cleared all even packets, remains {} odd packets", + pending_packets_a.unreceived_packets.len() + ); + + Ok(()) + } +} diff --git a/tools/integration-test/src/tests/ordered_channel_clear.rs b/tools/integration-test/src/tests/ordered_channel_clear.rs index 696a536a27..5885c98618 100644 --- a/tools/integration-test/src/tests/ordered_channel_clear.rs +++ b/tools/integration-test/src/tests/ordered_channel_clear.rs @@ -275,9 +275,10 @@ impl BinaryChannelTest for OrderedChannelClearEqualCLITest { )?; let events_returned: Vec = chain_a_link - .relay_recv_packet_and_timeout_messages_with_packet_data_query_height(Some( - clear_height, - )) + .relay_recv_packet_and_timeout_messages_with_packet_data_query_height( + vec![], + Some(clear_height), + ) .unwrap(); info!("recv packets sent, chain events: {:?}", events_returned); diff --git a/tools/integration-test/src/tests/query_packet.rs b/tools/integration-test/src/tests/query_packet.rs index 6c97c871d0..3f22600fed 100644 --- a/tools/integration-test/src/tests/query_packet.rs +++ b/tools/integration-test/src/tests/query_packet.rs @@ -81,7 +81,7 @@ impl BinaryChannelTest for QueryPacketPendingTest { assert!(summary.unreceived_acks.is_empty()); // Receive the packet on the destination chain - link.relay_recv_packet_and_timeout_messages()?; + link.relay_recv_packet_and_timeout_messages(vec![])?; let summary = pending_packet_summary(chains.handle_a(), chains.handle_b(), channel_end.value())?; @@ -91,7 +91,7 @@ impl BinaryChannelTest for QueryPacketPendingTest { // Acknowledge the packet on the source chain let link = link.reverse(false, false)?; - link.relay_ack_packet_messages()?; + link.relay_ack_packet_messages(vec![])?; let summary = pending_packet_summary(chains.handle_a(), chains.handle_b(), channel_end.value())?;