From b9b666368ed4ec7a2a783a661180b9cfe2c532d3 Mon Sep 17 00:00:00 2001 From: Luca Joss <43531661+ljoss17@users.noreply.github.com> Date: Fri, 21 Jun 2024 12:17:17 +0200 Subject: [PATCH] Allow ranges for `excluded_sequences` config option (#4051) * Add new ExcludedSequences struct with custom deserialisation/serialisation * Use ExcludedSequences struct for 'excluded_sequences' configuration * Fix sequence_filter test * Add changelog entry * Improve example config for 'exclude_sequences' * Add default implementation to ExcludedSequences * Only allow '-' as separator for 'excluded_sequences' configuration --- .../4047-improve-excluded-sequences-config.md | 11 ++ config.toml | 12 +- crates/relayer-cli/src/chain_registry.rs | 3 +- crates/relayer/src/chain/cosmos.rs | 4 +- crates/relayer/src/chain/cosmos/config.rs | 7 +- .../relayer/src/chain/cosmos/config/error.rs | 38 ++++++ crates/relayer/src/config.rs | 36 +++++- crates/relayer/src/util.rs | 1 + crates/relayer/src/util/excluded_sequences.rs | 113 ++++++++++++++++++ .../config/fixtures/relayer_conf_example.toml | 8 +- ...nf_example_invalid_excluded_sequences.toml | 68 +++++++++++ .../src/tests/sequence_filter.rs | 7 +- tools/test-framework/src/types/single/node.rs | 3 +- 13 files changed, 291 insertions(+), 20 deletions(-) create mode 100644 .changelog/unreleased/improvements/ibc-relayer/4047-improve-excluded-sequences-config.md create mode 100644 crates/relayer/src/util/excluded_sequences.rs create mode 100644 crates/relayer/tests/config/fixtures/relayer_conf_example_invalid_excluded_sequences.toml diff --git a/.changelog/unreleased/improvements/ibc-relayer/4047-improve-excluded-sequences-config.md b/.changelog/unreleased/improvements/ibc-relayer/4047-improve-excluded-sequences-config.md new file mode 100644 index 0000000000..19de3111df --- /dev/null +++ b/.changelog/unreleased/improvements/ibc-relayer/4047-improve-excluded-sequences-config.md @@ -0,0 +1,11 @@ +- Improve the `excluded_sequences` configuration so that it now accepts + ranges of sequence values in addition to exact values. + Accepted format: + * Exact sequence, e.g. [1, 2, 3] + * "-" separator, e.g. ["1-3"] + + These can be combined making the following configurations equivalent: + * `excluded_sequences = { 'channel-0' = [1, "3-5", 7, "9-12"] }` + * `excluded_sequences = { 'channel-0' = [1, 3, 4, 5, 7, 9, 10, 11, 12] }` + + ([\#4047](https://github.com/informalsystems/hermes/issues/4047)) \ No newline at end of file diff --git a/config.toml b/config.toml index 897814008d..b7542579ca 100644 --- a/config.toml +++ b/config.toml @@ -443,15 +443,15 @@ memo_prefix = '' # Specify packet sequences which should not be cleared, per channel. # -# For each channel, specify a list of sequences which should not be cleared, eg. +# For each channel, specify a list of sequences which should not be cleared. Acceptable value +# include range of sequences with separator "-", eg. # -# excluded_sequences = [ -# ['channel-0', [1, 2, 3]], -# ['channel-1', [4, 5, 6]] -# ] +# [chains.excluded_sequences] +# channel-0 = [1, 2, "3-7", 9, 11], +# channel-1 = [4, 5, 6], # # Default: No filter -# excluded_sequences = [] +# excluded_sequences = {} [[chains]] id = 'ibc-1' diff --git a/crates/relayer-cli/src/chain_registry.rs b/crates/relayer-cli/src/chain_registry.rs index a0152b0af2..c52e6190db 100644 --- a/crates/relayer-cli/src/chain_registry.rs +++ b/crates/relayer-cli/src/chain_registry.rs @@ -24,6 +24,7 @@ use ibc_relayer::config::gas_multiplier::GasMultiplier; use ibc_relayer::config::types::{MaxMsgNum, MaxTxSize, Memo, TrustThreshold}; use ibc_relayer::config::{default, AddressType, ChainConfig, EventSourceMode, GasPrice}; use ibc_relayer::keyring::Store; +use ibc_relayer::util::excluded_sequences::ExcludedSequences; const MAX_HEALTHY_QUERY_RETRIES: u8 = 5; @@ -167,7 +168,7 @@ where extension_options: Vec::new(), compat_mode: None, clear_interval: None, - excluded_sequences: BTreeMap::new(), + excluded_sequences: ExcludedSequences::new(BTreeMap::new()), })) } diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index 6cd8916905..ae1291a625 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -2567,8 +2567,8 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> { let grpc_address = chain.grpc_addr.to_string(); let rpc_address = chain.config.rpc_addr.to_string(); - if !chain.config.excluded_sequences.is_empty() { - for (channel_id, seqs) in chain.config.excluded_sequences.iter() { + if !chain.config.excluded_sequences.map.is_empty() { + for (channel_id, seqs) in chain.config.excluded_sequences.map.iter() { if !seqs.is_empty() { warn!( "chain '{chain_id}' will not clear packets on channel '{channel_id}' with sequences: {}. \ diff --git a/crates/relayer/src/chain/cosmos/config.rs b/crates/relayer/src/chain/cosmos/config.rs index 5e85f2a11d..3993c653b9 100644 --- a/crates/relayer/src/chain/cosmos/config.rs +++ b/crates/relayer/src/chain/cosmos/config.rs @@ -1,14 +1,12 @@ use core::time::Duration; -use std::collections::BTreeMap; use std::path::PathBuf; use byte_unit::Byte; -use ibc_relayer_types::core::ics04_channel::packet::Sequence; use serde_derive::{Deserialize, Serialize}; use tendermint_rpc::Url; use ibc_relayer_types::core::ics23_commitment::specs::ProofSpecs; -use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId}; +use ibc_relayer_types::core::ics24_host::identifier::ChainId; use crate::chain::cosmos::config::error::Error as ConfigError; use crate::config::compat_mode::CompatMode; @@ -20,6 +18,7 @@ use crate::config::{ }; use crate::config::{default, RefreshRate}; use crate::keyring::Store; +use crate::util::excluded_sequences::ExcludedSequences; pub mod error; @@ -149,7 +148,7 @@ pub struct CosmosSdkConfig { pub compat_mode: Option, pub clear_interval: Option, #[serde(default)] - pub excluded_sequences: BTreeMap>, + pub excluded_sequences: ExcludedSequences, } impl CosmosSdkConfig { diff --git a/crates/relayer/src/chain/cosmos/config/error.rs b/crates/relayer/src/chain/cosmos/config/error.rs index 5980ad9ac4..545f845609 100644 --- a/crates/relayer/src/chain/cosmos/config/error.rs +++ b/crates/relayer/src/chain/cosmos/config/error.rs @@ -1,4 +1,6 @@ use flex_error::define_error; +use flex_error::TraceError; + use ibc_relayer_types::core::ics02_client::trust_threshold::TrustThreshold; use ibc_relayer_types::core::ics24_host::identifier::ChainId; @@ -28,5 +30,41 @@ define_error! { e.chain_id, e.gas_adjustment, e.gas_multiplier ) }, + + ExpectedExcludedSequencesArray + |_| { "expected excluded_sequences to be an array of values" }, + + InvalidExcludedSequencesSeparator + { separator: String } + |e| { + format!("excluded_sequences range `{}` is invalid, only '..', '..=' and '-' are valid separators", e.separator) + }, + + MissingStartExcludedSequence + { entry: String } + |e| { + format!("missing the excluded sequence value before the separator in the entry `{}`", e.entry) + }, + + MissingEndExcludedSequence + { entry: String } + |e| { + format!("missing the excluded sequence value after the separator in the entry `{}`", e.entry) + }, + + ParsingStartExcludedSequenceFailed + { entry: String } + [ TraceError ] + |e| { + format!("Error parsing starting sequence as integer in entry `{}`", e.entry) + }, + + + ParsingEndExcludedSequenceFailed + { entry: String } + [ TraceError ] + |e| { + format!("Error parsing ending sequence as integer in entry `{}`", e.entry) + }, } } diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 973a0d400a..066bd93a97 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -719,6 +719,7 @@ impl ChainConfig { match self { Self::CosmosSdk(config) => config .excluded_sequences + .map .get(channel_id) .map(|seqs| Cow::Borrowed(seqs.as_slice())) .unwrap_or_else(|| Cow::Owned(Vec::new())), @@ -834,7 +835,7 @@ impl From for Error { mod tests { use core::str::FromStr; - use super::{load, parse_gas_prices, store_writer}; + use super::{load, parse_gas_prices, store_writer, ChainConfig}; use crate::config::GasPrice; use test_log::test; @@ -925,6 +926,39 @@ mod tests { store_writer(&config, &mut buffer).unwrap(); } + #[test] + fn serialize_valid_excluded_sequences_config() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer_conf_example.toml" + ); + + let config = load(path).expect("could not parse config"); + + let excluded_sequences1 = match config.chains.first().unwrap() { + ChainConfig::CosmosSdk(chain_config) => chain_config.excluded_sequences.clone(), + }; + + let excluded_sequences2 = match config.chains.last().unwrap() { + ChainConfig::CosmosSdk(chain_config) => chain_config.excluded_sequences.clone(), + }; + + assert_eq!(excluded_sequences1, excluded_sequences2); + + let mut buffer = Vec::new(); + store_writer(&config, &mut buffer).unwrap(); + } + + #[test] + fn serialize_invalid_excluded_sequences_config() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer_conf_example_invalid_excluded_sequences.toml" + ); + + assert!(load(path).is_err()); + } + #[test] fn gas_price_from_str() { let gp_original = GasPrice::new(10.0, "atom".to_owned()); diff --git a/crates/relayer/src/util.rs b/crates/relayer/src/util.rs index e589e8c7c8..852764ca50 100644 --- a/crates/relayer/src/util.rs +++ b/crates/relayer/src/util.rs @@ -5,6 +5,7 @@ pub mod collate; pub mod compat_mode; pub mod debug_section; pub mod diff; +pub mod excluded_sequences; pub mod iter; pub mod lock; pub mod pretty; diff --git a/crates/relayer/src/util/excluded_sequences.rs b/crates/relayer/src/util/excluded_sequences.rs new file mode 100644 index 0000000000..800a4c8c15 --- /dev/null +++ b/crates/relayer/src/util/excluded_sequences.rs @@ -0,0 +1,113 @@ +use serde::de::{Error, MapAccess, Visitor}; +use serde::ser::SerializeMap; +use serde::Deserializer; +use serde::Serializer; +use serde_derive::Deserialize; +use serde_derive::Serialize; +use std::collections::BTreeMap; +use std::fmt; +use std::str::FromStr; + +use ibc_relayer_types::core::ics04_channel::packet::Sequence; +use ibc_relayer_types::core::ics24_host::identifier::ChannelId; + +use crate::chain::cosmos::config::error::Error as ConfigError; + +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] +pub struct ExcludedSequences { + #[serde( + deserialize_with = "deserialize_excluded_sequences", + serialize_with = "serialize_excluded_sequences", + flatten + )] + pub map: BTreeMap>, +} + +impl ExcludedSequences { + pub fn new(map: BTreeMap>) -> Self { + Self { map } + } +} + +fn serialize_excluded_sequences( + map: &BTreeMap>, + serializer: S, +) -> Result +where + S: Serializer, +{ + let mut seq = serializer.serialize_map(Some(map.len()))?; + for (k, v) in map { + seq.serialize_entry(k, v)?; + } + seq.end() +} + +fn deserialize_excluded_sequences<'de, D>( + deserializer: D, +) -> Result>, D::Error> +where + D: Deserializer<'de>, +{ + deserializer.deserialize_map(ExcludedSequencesVisitor) +} + +struct ExcludedSequencesVisitor; + +impl<'de> Visitor<'de> for ExcludedSequencesVisitor { + type Value = BTreeMap>; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("expected list of excluded sequences") + } + + fn visit_map(self, mut access: M) -> Result + where + M: MapAccess<'de>, + { + let mut map = BTreeMap::new(); + while let Some((key, value)) = access.next_entry::()? { + let channel_id = ChannelId::from_str(&key).map_err(|e| Error::custom(e.to_string()))?; + let sequences = + parse_sequence_range(&value).map_err(|e| Error::custom(e.to_string()))?; + map.insert(channel_id, sequences); + } + Ok(map) + } +} + +fn parse_sequence_range(value: &toml::Value) -> Result, ConfigError> { + let mut res = Vec::new(); + let sequences = value + .as_array() + .ok_or_else(ConfigError::expected_excluded_sequences_array)?; + for sequence in sequences.iter() { + if let Some(seq_str) = sequence.as_str() { + let (start, end) = get_start_and_end(seq_str)?; + for i in start..=end { + let seq = Sequence::from(i); + res.push(seq); + } + } else if let Some(seq) = sequence.as_integer() { + let seq = Sequence::from(seq as u64); + res.push(seq); + } + } + Ok(res) +} + +fn get_start_and_end(value: &str) -> Result<(u64, u64), ConfigError> { + let split: Vec<&str> = value.split('-').collect(); + let start: u64 = split + .first() + .ok_or_else(|| ConfigError::missing_start_excluded_sequence(value.to_string()))? + .parse() + .map_err(|e| ConfigError::parsing_start_excluded_sequence_failed(value.to_string(), e))?; + let end: u64 = split + .last() + .ok_or_else(|| ConfigError::missing_end_excluded_sequence(value.to_string()))? + .parse() + .map_err(|e| ConfigError::parsing_end_excluded_sequence_failed(value.to_string(), e))?; + + Ok((start, end)) +} diff --git a/crates/relayer/tests/config/fixtures/relayer_conf_example.toml b/crates/relayer/tests/config/fixtures/relayer_conf_example.toml index f864efecda..6033872b07 100644 --- a/crates/relayer/tests/config/fixtures/relayer_conf_example.toml +++ b/crates/relayer/tests/config/fixtures/relayer_conf_example.toml @@ -39,7 +39,10 @@ max_tx_size = 1048576 clock_drift = '5s' trusting_period = '14days' trust_threshold = { numerator = '1', denominator = '3' } -address_type = { derivation = 'cosmos' } + +[chains.excluded_sequences] +channel-0 = [1, "3-5", 7, "9-12", 14, "17-19"] +channel-1 = ["3-6"] [chains.packet_filter] policy = 'allow' @@ -62,4 +65,5 @@ gas_price = { price = 0.001, denom = 'stake' } clock_drift = '5s' trusting_period = '14days' trust_threshold = { numerator = '1', denominator = '3' } -address_type = { derivation = 'ethermint', proto_type = { pk_type = '/injective.crypto.v1beta1.ethsecp256k1.PubKey' } } \ No newline at end of file +address_type = { derivation = 'ethermint', proto_type = { pk_type = '/injective.crypto.v1beta1.ethsecp256k1.PubKey' } } +excluded_sequences = { 'channel-0' = [1, 3, 4, 5, 7, 9, 10, 11, 12, 14, 17, 18, 19], 'channel-1' = [3, 4, 5, 6] } \ No newline at end of file diff --git a/crates/relayer/tests/config/fixtures/relayer_conf_example_invalid_excluded_sequences.toml b/crates/relayer/tests/config/fixtures/relayer_conf_example_invalid_excluded_sequences.toml new file mode 100644 index 0000000000..1e72497055 --- /dev/null +++ b/crates/relayer/tests/config/fixtures/relayer_conf_example_invalid_excluded_sequences.toml @@ -0,0 +1,68 @@ +[global] +log_level = 'error' + +[mode] + +[mode.clients] +enabled = true +refresh = true +misbehaviour = true + +[mode.connections] +enabled = false + +[mode.channels] +enabled = false + +[mode.packets] +enabled = true +clear_interval = 100 +clear_on_start = true +tx_confirmation = true +ics20_max_memo_size = { enabled = true, size = "32KiB" } +ics20_max_receiver_size = { enabled = true, size = "2KiB" } + +[[chains]] +type = "CosmosSdk" +id = 'chain_A' +rpc_addr = 'http://127.0.0.1:26657' +grpc_addr = 'http://127.0.0.1:9090' +event_source = { mode = 'push', url = 'ws://localhost:26657/websocket', batch_delay = '500ms' } +rpc_timeout = '10s' +account_prefix = 'cosmos' +key_name = 'testkey' +store_prefix = 'ibc' +max_gas = 200000 +gas_price = { price = 0.001, denom = 'stake' } +max_msg_num = 4 +max_tx_size = 1048576 +clock_drift = '5s' +trusting_period = '14days' +trust_threshold = { numerator = '1', denominator = '3' } +excluded_sequences = [ + ['channel-0', [1, 2, 3]], + ['channel-1', [4, 5, 6]] +] + +[chains.packet_filter] +policy = 'allow' +list = [ + ['ica*', '*'], + ['transfer', 'channel-0'], +] + +[[chains]] +type = "CosmosSdk" +id = 'chain_B' +rpc_addr = 'http://127.0.0.1:26557' +grpc_addr = 'http://127.0.0.1:9090' +event_source = { mode = 'push', url = 'ws://localhost:26557/websocket', batch_delay = '500ms' } +rpc_timeout = '10s' +account_prefix = 'cosmos' +key_name = 'testkey' +store_prefix = 'ibc' +gas_price = { price = 0.001, denom = 'stake' } +clock_drift = '5s' +trusting_period = '14days' +trust_threshold = { numerator = '1', denominator = '3' } +address_type = { derivation = 'ethermint', proto_type = { pk_type = '/injective.crypto.v1beta1.ethsecp256k1.PubKey' } } \ No newline at end of file diff --git a/tools/integration-test/src/tests/sequence_filter.rs b/tools/integration-test/src/tests/sequence_filter.rs index 177773bf4a..a880f7720e 100644 --- a/tools/integration-test/src/tests/sequence_filter.rs +++ b/tools/integration-test/src/tests/sequence_filter.rs @@ -18,6 +18,7 @@ use std::collections::BTreeMap; use ibc_relayer::config::ChainConfig; +use ibc_relayer::util::excluded_sequences::ExcludedSequences; use ibc_test_framework::{ prelude::*, relayer::channel::{assert_eventually_channel_established, init_channel}, @@ -52,7 +53,7 @@ impl TestOverrides for FilterClearOnStartTest { let chain_a = &mut config.chains[0]; match chain_a { ChainConfig::CosmosSdk(chain_config) => { - chain_config.excluded_sequences = excluded_sequences; + chain_config.excluded_sequences = ExcludedSequences::new(excluded_sequences); } } config.mode.channels.enabled = true; @@ -89,7 +90,7 @@ impl TestOverrides for FilterClearIntervalTest { let chain_a = &mut config.chains[0]; match chain_a { ChainConfig::CosmosSdk(chain_config) => { - chain_config.excluded_sequences = excluded_sequences; + chain_config.excluded_sequences = ExcludedSequences::new(excluded_sequences); } } config.mode.channels.enabled = true; @@ -248,7 +249,7 @@ impl TestOverrides for StandardRelayingNoFilterTest { let chain_a = &mut config.chains[0]; match chain_a { ChainConfig::CosmosSdk(chain_config) => { - chain_config.excluded_sequences = excluded_sequences; + chain_config.excluded_sequences = ExcludedSequences::new(excluded_sequences); } } config.mode.packets.clear_on_start = true; diff --git a/tools/test-framework/src/types/single/node.rs b/tools/test-framework/src/types/single/node.rs index b6241c0ade..145b0a74f9 100644 --- a/tools/test-framework/src/types/single/node.rs +++ b/tools/test-framework/src/types/single/node.rs @@ -12,6 +12,7 @@ use ibc_relayer::config::compat_mode::CompatMode; use ibc_relayer::config::dynamic_gas::DynamicGasPrice; use ibc_relayer::config::gas_multiplier::GasMultiplier; use ibc_relayer::keyring::Store; +use ibc_relayer::util::excluded_sequences::ExcludedSequences; use ibc_relayer_types::core::ics24_host::identifier::ChainId; use std::collections::BTreeMap; use std::sync::{Arc, RwLock}; @@ -198,7 +199,7 @@ impl FullNode { sequential_batch_tx: false, compat_mode, clear_interval: None, - excluded_sequences: BTreeMap::new(), + excluded_sequences: ExcludedSequences::new(BTreeMap::new()), })) }