Skip to content

Commit

Permalink
Add per chain clear_interval configuration (#3693)
Browse files Browse the repository at this point in the history
* Add per chain clear_interval configuration

* Add changelog entry

* Add guide entry for packet clearing

* Reword config comment

Signed-off-by: Romain Ruetschi <[email protected]>

---------

Signed-off-by: Romain Ruetschi <[email protected]>
Co-authored-by: Romain Ruetschi <[email protected]>
  • Loading branch information
ljoss17 and romac authored Nov 9, 2023
1 parent 1bb24fe commit a09b556
Show file tree
Hide file tree
Showing 11 changed files with 211 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add a configuration which allows to override the `clear_interval` for specific
chains ([\#3691](https://github.com/informalsystems/hermes/issues/3691))
4 changes: 4 additions & 0 deletions config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,10 @@ memo_prefix = ''
# Possible values: [`0.34`, `0.37`]
# compat_mode = '0.34'

# Specify the a clear interval for the chain.
# This will override the global clear interval for this chain only, allowing different intervals for each chain.
# clear_interval = 50

[[chains]]
id = 'ibc-1'
type = "CosmosSdk"
Expand Down
1 change: 1 addition & 0 deletions crates/relayer-cli/src/chain_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ where
sequential_batch_tx: false,
extension_options: Vec::new(),
compat_mode: None,
clear_interval: None,
}))
}

Expand Down
1 change: 1 addition & 0 deletions crates/relayer/src/chain/cosmos/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ pub struct CosmosSdkConfig {
#[serde(default = "Vec::new", skip_serializing_if = "Vec::is_empty")]
pub extension_options: Vec<ExtensionOption>,
pub compat_mode: Option<CompatMode>,
pub clear_interval: Option<u64>,
}

impl CosmosSdkConfig {
Expand Down
6 changes: 6 additions & 0 deletions crates/relayer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,12 @@ impl ChainConfig {
};
Ok(keys)
}

pub fn clear_interval(&self) -> Option<u64> {
match self {
Self::CosmosSdk(config) => config.clear_interval,
}
}
}

/// Attempt to load and parse the TOML config file as a `Config`.
Expand Down
31 changes: 20 additions & 11 deletions crates/relayer/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,27 +130,36 @@ pub fn spawn_worker_tasks<ChainA: ChainHandle, ChainB: ChainHandle>(

let (cmd_tx, cmd_rx) = crossbeam_channel::unbounded();
let link = Arc::new(Mutex::new(link));
let resubmit = Resubmit::from_clear_interval(packets_config.clear_interval);

let src_chain_config = config
.chains
.iter()
.find(|chain| *chain.id() == chains.a.id());

let fee_filter = match src_chain_config {
Some(chain_config) => chain_config
.packet_filter()
.min_fees
.iter()
.find(|(channel, _)| channel.matches(&path.src_channel_id))
.map(|(_, filter)| filter)
.cloned(),
let (fee_filter, clear_interval) = match src_chain_config {
Some(chain_config) => {
let chain_clear_interval = chain_config
.clear_interval()
.unwrap_or(packets_config.clear_interval);

let fee_filter = chain_config
.packet_filter()
.min_fees
.iter()
.find(|(channel, _)| channel.matches(&path.src_channel_id))
.map(|(_, filter)| filter)
.cloned();

(fee_filter, chain_clear_interval)
}
None => {
error!("configuration for chain {} not found", chains.a.id());
None
(None, packets_config.clear_interval)
}
};

let resubmit = Resubmit::from_clear_interval(clear_interval);

// Only spawn the incentivized worker if a fee filter is specified in the configuration
let packet_task = match fee_filter {
Some(filter) => packet::spawn_incentivized_packet_cmd_worker(
Expand All @@ -163,7 +172,7 @@ pub fn spawn_worker_tasks<ChainA: ChainHandle, ChainB: ChainHandle>(
cmd_rx,
link.clone(),
should_clear_on_start,
packets_config.clear_interval,
clear_interval,
path.clone(),
),
};
Expand Down
1 change: 1 addition & 0 deletions guide/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- [Configure Hermes](./documentation/configuration/configure-hermes.md)
- [Description of the parameters](./documentation/configuration/description.md)
- [Filter incentivized packets](./documentation/configuration/filter-incentivized.md)
- [Packet clearing](./documentation/configuration/packet-clearing.md)
- [Performance tuning](./documentation/configuration/performance.md)
- [CometBFT Compatibility modes](./documentation/configuration/comet-compat-mode.md)

Expand Down
3 changes: 3 additions & 0 deletions guide/src/documentation/configuration/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ This section includes everything you need to know to configure Hermes.
* **[Filter incentivized packets](./filter-incentivized.md)**
* Examples on how to configure Hermes in order to filter incentivized packets

* **[Packet clearing](./packet-clearing.md)**
* Description on packet clearing configurations

- **[Performance Tuning](./performance.md)**
* Learn about configurations allowing more refined performance tuning.

Expand Down
45 changes: 45 additions & 0 deletions guide/src/documentation/configuration/packet-clearing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Packet clearing

Hermes can be configured in order to clear packets which haven't been relayed. This can happen if there wasn't a relayer instance running when the packet event was submitted or if there was an issue relaying the packet.

There are three different configurations to determine when Hermes will clear packets.

## Global configurations

Two of these configurations are global to all chains and are in the `[mode.packet]` section.

### 1. `clear_on_start`

```toml
[mode.packet]
...
clear_on_start = true
```

This configuration is used to specify if Hermes should query and relay pending packets when starting the instance. If set this will only trigger once per running instance.

>__NOTE__: If this configuration is enabled Hermes will need to scan for channels as the pending packets will require the channel worker, refer to the [Slow start section](./performance.md#3-slow-start) for more information.
### 2. `clear_interval`

```toml
[mode.packet]
...
clear_interval = 100
```

This configuration defines how often Hermes will verify if there are pending packets and relay them. The value is the number of blocks observed, so the time between each clearing might very from chain to chain.

## Chain specific configuration

The third configuration is specific for each chain.

### 3. `clear_interval`

```toml
[[chains]]
...
clear_interval = 50
```

An additional `clear_interval` can be specified for each chain, this value is also in number of blocks. This configuration will override the clear interval value for the specific chain and can be used if chains need to have different clear values. This configuration is optional, if it is not set the global value will be used.
127 changes: 127 additions & 0 deletions tools/integration-test/src/tests/clear_packet.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::thread;

use ibc_relayer::config::ChainConfig;
use ibc_test_framework::prelude::*;
use ibc_test_framework::util::random::random_u128_range;

Expand All @@ -18,6 +19,11 @@ fn test_clear_packet_no_scan() -> Result<(), Error> {
run_binary_channel_test(&ClearPacketNoScanTest)
}

#[test]
fn test_clear_packet_override() -> Result<(), Error> {
run_binary_channel_test(&ClearPacketOverrideTest)
}

pub struct ClearPacketTest;
pub struct ClearPacketRecoveryTest;

Expand Down Expand Up @@ -294,3 +300,124 @@ impl BinaryChannelTest for ClearPacketNoScanTest {
})
}
}
pub struct ClearPacketOverrideTest;

impl TestOverrides for ClearPacketOverrideTest {
fn modify_relayer_config(&self, config: &mut Config) {
// Disabling client workers and clear_on_start should make the relayer not
// relay any packet it missed before starting.
config.mode.clients.enabled = false;
config.mode.connections.enabled = false;
config.mode.channels.enabled = false;

config.mode.packets.enabled = true;
config.mode.packets.clear_on_start = false;
// Set a very high global clear interval
config.mode.packets.clear_interval = 10000;

for chain_config in config.chains.iter_mut() {
match chain_config {
// Use a small clear interval in the chain configurations to override the global high interval
ChainConfig::CosmosSdk(chain_config) => chain_config.clear_interval = Some(10),
}
}
}

fn should_spawn_supervisor(&self) -> bool {
false
}
}

impl BinaryChannelTest for ClearPacketOverrideTest {
fn run<ChainA: ChainHandle, ChainB: ChainHandle>(
&self,
config: &TestConfig,
relayer: RelayerDriver,
chains: ConnectedChains<ChainA, ChainB>,
channel: ConnectedChannel<ChainA, ChainB>,
) -> Result<(), Error> {
let denom_a = chains.node_a.denom();
let fee_denom_a = MonoTagged::new(Denom::base(&config.native_tokens[0]));

let wallet_a = chains.node_a.wallets().user1().cloned();
let wallet_b = chains.node_b.wallets().user1().cloned();

let amount1 = random_u128_range(1000, 5000);

let balance_a = chains
.node_a
.chain_driver()
.query_balance(&wallet_a.address(), &denom_a)?;

chains.node_a.chain_driver().ibc_transfer_token(
&channel.port_a.as_ref(),
&channel.channel_id_a.as_ref(),
&wallet_a.as_ref(),
&wallet_b.address(),
&denom_a.with_amount(amount1).as_ref(),
)?;

let denom_b2 = derive_ibc_denom(
&channel.port_b.as_ref(),
&channel.channel_id_b.as_ref(),
&denom_a,
)?;

thread::sleep(Duration::from_secs(5));

relayer.with_supervisor(|| {
info!("Assert clear on start does not trigger");
chains.node_a.chain_driver().assert_eventual_wallet_amount(
&wallet_a.address(),
&(balance_a.clone() - amount1).as_ref(),
)?;

// Clear on start is disabled, packets will not be cleared
chains.node_b.chain_driver().assert_eventual_wallet_amount(
&wallet_b.address(),
&denom_b2.with_amount(0u128).as_ref(),
)?;

// Wait for clear interval to trigger
thread::sleep(Duration::from_secs(20));

info!("Assert clear interval does not trigger");

// Channel is not discovered, packets won't be cleared
chains.node_b.chain_driver().assert_eventual_wallet_amount(
&wallet_b.address(),
&denom_b2.with_amount(0u128).as_ref(),
)?;

info!("Trigger a transfer");

let dst_height = chains.handle_b().query_latest_height()?;

// Trigger a transfer from chain
chains.node_a.chain_driver().transfer_from_chain(
&chains.node_a.wallets().user1(),
&chains.node_b.wallets().user1().address(),
&channel.port_a.0,
&channel.channel_id_a.0,
&denom_a.with_amount(amount1).as_ref(),
&fee_denom_a.with_amount(1200u64).as_ref(),
&dst_height,
)?;

info!("Assert clear interval correctly triggers");

chains.node_a.chain_driver().assert_eventual_wallet_amount(
&wallet_a.address(),
&(balance_a - amount1 - amount1).as_ref(),
)?;

// Wait for clear interval to clear packets
chains.node_b.chain_driver().assert_eventual_wallet_amount(
&wallet_b.address(),
&denom_b2.with_amount(amount1 + amount1).as_ref(),
)?;

Ok(())
})
}
}
1 change: 1 addition & 0 deletions tools/test-framework/src/types/single/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ impl FullNode {
extension_options: Default::default(),
sequential_batch_tx: false,
compat_mode,
clear_interval: None,
}))
}

Expand Down

0 comments on commit a09b556

Please sign in to comment.