From f0281a8482e42a379c5a05794f0910f6c3cdf89b Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Fri, 1 Sep 2023 14:42:32 -0500 Subject: [PATCH 01/29] Start scaffolding ica_ordered_channel test --- .../ica_ordered_channel.rs | 81 +++++++++++++++++++ .../src/tests/interchain_security/mod.rs | 1 + 2 files changed, 82 insertions(+) create mode 100644 tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs new file mode 100644 index 0000000000..9d8d2049dd --- /dev/null +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -0,0 +1,81 @@ +//! Verifies the behaviour of ordered channels with Interchain Accounts. +//! +//! In order to ensure that ordered channels correctly clear packets on ICA +//! channels, this test sends some sequential packets with the supervisor enabled, +//! sends the next packet without the supervisor enabled, then sends additional +//! packets with the supervisor enabled again. The pending packet that was sent +//! without the supervisor enabled should be relayed in order along with the +//! other packets, as expected of ordered channel behaviour. + +use ibc_test_framework::prelude::*; +use ibc_test_framework::chain::ext::ica::register_interchain_account; +use ibc_test_framework::relayer::channel::query_channel_end; + +#[test] +fn test_ica_ordered_channel() -> Result<(), Error> { + run_binary_interchain_security_channel_test(&IcaOrderedChannelTest) +} + +struct IcaOrderedChannelTest; + +impl TestOverrides for IcaOrderedChannelTest { + fn modify_genesis_file(&self, genesis: &mut serde_json::Value) -> Result<(), Error> { + use serde_json::Value; + + // Allow MsgSend messages over ICA + let allow_messages = genesis + .get_mut("app_state") + .and_then(|app_state| app_state.get_mut("interchainaccounts")) + .and_then(|ica| ica.get_mut("host_genesis_state")) + .and_then(|state| state.get_mut("params")) + .and_then(|params| params.get_mut("allow_messages")) + .and_then(|allow_messages| allow_messages.as_array_mut()); + + if let Some(allow_messages) = allow_messages { + allow_messages.push(Value::String("/cosmos.bank.v1beta1.MsgSend".to_string())); + } else { + return Err(Error::generic(eyre!("failed to update genesis file"))); + } + + update_genesis_for_consumer_chain(genesis)?; + + Ok(()) + } + + fn modify_relayer_config(&self, config: &mut Config) { + config.mode.channels.enabled = true; + } + + fn should_spawn_supervisor(&self) -> bool { + false + } +} + +impl BinaryChannelTest for IcaOrderedChannelTest { + fn run( + &self, + _config: &TestConfig, + _relayer: RelayerDriver, + chains: ConnectedChains, + channel: ConnectedChannel, + ) -> Result<(), Error> { + let connection_b_to_a = channel.connection.clone().flip(); + let (wallet, channel_id, port_id) = + register_interchain_account(&chains.node_b, chains.handle_b(), &connection_b_to_a)?; + + // Check that the corresponding ICA channel is eventually established. + let _counterparty_channel_id = assert_eventually_channel_established( + chains.handle_b(), + chains.handle_a(), + &channel_id.as_ref(), + &port_id.as_ref(), + )?; + + // Assert that the channel returned by `register_interchain_account` is an ordered channel + let channel_end = query_channel_end(chains.handle_b(), &channel_id.as_ref(), &port_id.as_ref())?; + + assert_eq!(channel_end.value().ordering(), ChannelOrder::Ordered); + + Ok(()) + } +} \ No newline at end of file diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index a2acc74d92..010402929e 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,5 +1,6 @@ #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; From 92c2ec78680d092e89b603e58f20c923e26ea2f4 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Fri, 8 Sep 2023 09:26:54 -0500 Subject: [PATCH 02/29] Disable packet clearing --- .../tests/interchain_security/ica_ordered_channel.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 9d8d2049dd..b3db4cb266 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -44,6 +44,12 @@ impl TestOverrides for IcaOrderedChannelTest { fn modify_relayer_config(&self, config: &mut Config) { config.mode.channels.enabled = true; + + // Disable packet clearing so that packets sent without the supervisor + // enabled enter a pending state. + config.mode.packets.enabled = true; + config.mode.packets.clear_on_start = false; + config.mode.packets.clear_interval = 0; } fn should_spawn_supervisor(&self) -> bool { @@ -55,7 +61,7 @@ impl BinaryChannelTest for IcaOrderedChannelTest { fn run( &self, _config: &TestConfig, - _relayer: RelayerDriver, + relayer: RelayerDriver, chains: ConnectedChains, channel: ConnectedChannel, ) -> Result<(), Error> { @@ -76,6 +82,10 @@ impl BinaryChannelTest for IcaOrderedChannelTest { assert_eq!(channel_end.value().ordering(), ChannelOrder::Ordered); + relayer.with_supervisor(|| { + + }); + Ok(()) } } \ No newline at end of file From 27f762a3b7234cf51dfc45f244550279e8923a52 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Wed, 13 Sep 2023 11:56:50 -0500 Subject: [PATCH 03/29] Add ica_ordered_channel test --- tools/integration-test/src/tests/ica.rs | 24 +--- .../ica_ordered_channel.rs | 105 ++++++++++++++++-- .../tests/interchain_security/ica_transfer.rs | 25 +---- .../src/tests/interchain_security/mod.rs | 2 +- tools/integration-test/src/tests/mod.rs | 2 + tools/integration-test/src/tests/utils.rs | 35 ++++++ 6 files changed, 139 insertions(+), 54 deletions(-) create mode 100644 tools/integration-test/src/tests/utils.rs diff --git a/tools/integration-test/src/tests/ica.rs b/tools/integration-test/src/tests/ica.rs index dfcda16cea..e77c140938 100644 --- a/tools/integration-test/src/tests/ica.rs +++ b/tools/integration-test/src/tests/ica.rs @@ -1,3 +1,4 @@ +use crate::utils::interchain_send_tx; use std::collections::HashMap; use std::str::FromStr; @@ -174,29 +175,6 @@ impl BinaryConnectionTest for IcaFilterTestAllow { } } -fn interchain_send_tx( - chain: &ChainA, - from: &Signer, - connection: &ConnectionId, - msg: InterchainAccountPacketData, - relative_timeout: Timestamp, -) -> Result, Error> { - let msg = MsgSendTx { - owner: from.clone(), - connection_id: connection.clone(), - packet_data: msg, - relative_timeout, - }; - - let msg_any = msg.to_any(); - - let tm = TrackedMsgs::new_static(vec![msg_any], "SendTx"); - - chain - .send_messages_and_wait_commit(tm) - .map_err(Error::relayer) -} - #[test] fn test_ica_filter_deny() -> Result<(), Error> { run_binary_connection_test(&IcaFilterTestDeny) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index b3db4cb266..4ff613a5d9 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -1,14 +1,14 @@ //! Verifies the behaviour of ordered channels with Interchain Accounts. -//! +//! //! In order to ensure that ordered channels correctly clear packets on ICA //! channels, this test sends some sequential packets with the supervisor enabled, //! sends the next packet without the supervisor enabled, then sends additional //! packets with the supervisor enabled again. The pending packet that was sent -//! without the supervisor enabled should be relayed in order along with the -//! other packets, as expected of ordered channel behaviour. +//! without the supervisor enabled should be relayed in order along with the +//! other packets, as expected of ordered channel behaviour. -use ibc_test_framework::prelude::*; use ibc_test_framework::chain::ext::ica::register_interchain_account; +use ibc_test_framework::prelude::*; use ibc_test_framework::relayer::channel::query_channel_end; #[test] @@ -45,7 +45,7 @@ impl TestOverrides for IcaOrderedChannelTest { fn modify_relayer_config(&self, config: &mut Config) { config.mode.channels.enabled = true; - // Disable packet clearing so that packets sent without the supervisor + // Disable packet clearing so that packets sent without the supervisor // enabled enter a pending state. config.mode.packets.enabled = true; config.mode.packets.clear_on_start = false; @@ -78,14 +78,105 @@ impl BinaryChannelTest for IcaOrderedChannelTest { )?; // Assert that the channel returned by `register_interchain_account` is an ordered channel - let channel_end = query_channel_end(chains.handle_b(), &channel_id.as_ref(), &port_id.as_ref())?; + let channel_end = + query_channel_end(chains.handle_b(), &channel_id.as_ref(), &port_id.as_ref())?; assert_eq!(channel_end.value().ordering(), ChannelOrder::Ordered); + // Query the controller chain for the address of the ICA wallet on the host chain. + let ica_address = chains.node_b.chain_driver().query_interchain_account( + &wallet.address(), + &channel.connection.connection_id_b.as_ref(), + )?; + + let stake_denom: MonoTagged = MonoTagged::new(Denom::base("stake")); + + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &ica_address.as_ref(), + &stake_denom.with_amount(0u64).as_ref(), + )?; + + // Send funds to the interchain account. + let ica_fund = 42000u64; + + chains.node_a.chain_driver().local_transfer_token( + &chains.node_a.wallets().user1(), + &ica_address.as_ref(), + &stake_denom.with_amount(ica_fund).as_ref(), + )?; + + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &ica_address.as_ref(), + &stake_denom.with_amount(ica_fund).as_ref(), + )?; + + let amount = 1200; + + let msg = MsgSend { + from_address: ica_address.to_string(), + to_address: chains.node_a.wallets().user2().address().to_string(), + amount: vec![Coin { + denom: stake_denom.to_string(), + amount: Amount(U256::from(amount)), + }], + }; + + let raw_msg = msg.to_any(); + + let cosmos_tx = CosmosTx { + messages: vec![raw_msg], + }; + + let raw_cosmos_tx = cosmos_tx.to_any(); + + let interchain_account_packet_data = InterchainAccountPacketData::new(raw_cosmos_tx.value); + + let signer = Signer::from_str(&wallet.address().to_string()).unwrap(); + relayer.with_supervisor(|| { + let ica_events = interchain_send_tx( + chains.handle_b(), + &signer, + &channel.connection.connection_id_b.0, + interchain_account_packet_data, + Timestamp::from_nanoseconds(120000000000).unwrap(), + )?; + + info!("First ICA transfer made with supervisor: {ica_events:#?}"); + + Ok(()) + }); + + let ica_events = interchain_send_tx( + chains.handle_b(), + &signer, + &channel.connection.connection_id_b.0, + interchain_account_packet_data, + Timestamp::from_nanoseconds(120000000000).unwrap(), + )?; + info!("Second ICA transfer made without supervisor: {ica_events:#?}"); + + relayer.with_supervisor(|| { + let ica_events = interchain_send_tx( + chains.handle_b(), + &signer, + &channel.connection.connection_id_b.0, + interchain_account_packet_data, + Timestamp::from_nanoseconds(120000000000).unwrap(), + )?; + + info!("Third ICA transfer made with supervisor: {ica_events:#?}"); + + Ok(()) }); + // Check that the ICA account's balance has been debited the sent amount. + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &ica_address.as_ref(), + &stake_denom.with_amount(ica_fund - 3 * amount).as_ref(), + )?; + Ok(()) } -} \ No newline at end of file +} diff --git a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs index fc44ff7fd2..86b2be9573 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs @@ -1,6 +1,7 @@ //! The following tests are for the Interchain Security. //! These tests require the first chain to be a Provider chain and //! the second chain a Consumer chain. +use crate::utils::interchain_send_tx; use std::str::FromStr; use ibc_relayer::chain::tracking::TrackedMsgs; @@ -151,29 +152,7 @@ impl BinaryChannelTest for InterchainSecurityIcaTransferTest { &ica_address.as_ref(), &stake_denom.with_amount(ica_fund - amount).as_ref(), )?; + Ok(()) } } - -fn interchain_send_tx( - chain: &ChainA, - from: &Signer, - connection: &ConnectionId, - msg: InterchainAccountPacketData, - relative_timeout: Timestamp, -) -> Result, Error> { - let msg = MsgSendTx { - owner: from.clone(), - connection_id: connection.clone(), - packet_data: msg, - relative_timeout, - }; - - let msg_any = msg.to_any(); - - let tm = TrackedMsgs::new_static(vec![msg_any], "SendTx"); - - chain - .send_messages_and_wait_commit(tm) - .map_err(Error::relayer) -} diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index 010402929e..a8286825b5 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,6 +1,6 @@ +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; diff --git a/tools/integration-test/src/tests/mod.rs b/tools/integration-test/src/tests/mod.rs index 57526ac08b..7d2f406e99 100644 --- a/tools/integration-test/src/tests/mod.rs +++ b/tools/integration-test/src/tests/mod.rs @@ -25,6 +25,8 @@ pub mod tendermint; pub mod ternary_transfer; pub mod transfer; +mod utils; + #[cfg(any(doc, feature = "ics29-fee"))] pub mod fee; diff --git a/tools/integration-test/src/tests/utils.rs b/tools/integration-test/src/tests/utils.rs new file mode 100644 index 0000000000..1f5779df0c --- /dev/null +++ b/tools/integration-test/src/tests/utils.rs @@ -0,0 +1,35 @@ +//! Utility functions for integration tests. + +use ibc_relayer::chain::tracking::TrackedMsgs; +use ibc_relayer::event::IbcEventWithHeight; +use ibc_relayer_types::applications::ics27_ica::msgs::send_tx::MsgSendTx; +use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; +use ibc_relayer_types::signer::Signer; +use ibc_relayer_types::timestamp::Timestamp; +use ibc_relayer_types::tx_msg::Msg; +use ibc_test_framework::prelude::*; + +/// Sends a message containing `InterchainAccountPacketData` from the `Signer` +/// to the `Chain`. +pub fn interchain_send_tx( + chain: &ChainA, + from: &Signer, + connection: &ConnectionId, + msg: InterchainAccountPacketData, + relative_timeout: Timestamp, +) -> Result, Error> { + let msg = MsgSendTx { + owner: from.clone(), + connection_id: connection.clone(), + packet_data: msg, + relative_timeout, + }; + + let msg_any = msg.to_any(); + + let tm = TrackedMsgs::new_static(vec![msg_any], "SendTx"); + + chain + .send_messages_and_wait_commit(tm) + .map_err(Error::relayer) +} From 54abddf177294ee4adcce0a2c3ad3d8c95861647 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Wed, 13 Sep 2023 15:56:22 -0500 Subject: [PATCH 04/29] Move some imports around --- .../src/tests/interchain_security/ica_transfer.rs | 1 - tools/integration-test/src/tests/interchain_security/mod.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs index 86b2be9573..4e10120cfa 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs @@ -14,7 +14,6 @@ use ibc_relayer_types::applications::transfer::{Amount, Coin}; use ibc_relayer_types::bigint::U256; use ibc_relayer_types::signer::Signer; use ibc_relayer_types::timestamp::Timestamp; -use ibc_relayer_types::tx_msg::Msg; use ibc_test_framework::chain::ext::ica::register_interchain_account; use ibc_test_framework::framework::binary::channel::run_binary_interchain_security_channel_test; use ibc_test_framework::prelude::*; diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index a8286825b5..010402929e 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,6 +1,6 @@ -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; From 7eefadca7bf539e28c0e48973fc6407437f8f055 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Thu, 14 Sep 2023 08:26:02 -0500 Subject: [PATCH 05/29] Clean up imports --- tools/integration-test/src/tests/ica.rs | 6 ++---- .../tests/interchain_security/ica_ordered_channel.rs | 12 +++++++++++- .../src/tests/interchain_security/ica_transfer.rs | 6 ++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/tools/integration-test/src/tests/ica.rs b/tools/integration-test/src/tests/ica.rs index e77c140938..e32f8685f8 100644 --- a/tools/integration-test/src/tests/ica.rs +++ b/tools/integration-test/src/tests/ica.rs @@ -1,15 +1,13 @@ -use crate::utils::interchain_send_tx; use std::collections::HashMap; use std::str::FromStr; +use crate::utils::interchain_send_tx; + use ibc_relayer::chain::handle::ChainHandle; -use ibc_relayer::chain::tracking::TrackedMsgs; use ibc_relayer::config::{ filter::{ChannelFilters, ChannelPolicy, FilterPattern}, PacketFilter, }; -use ibc_relayer::event::IbcEventWithHeight; -use ibc_relayer_types::applications::ics27_ica::msgs::send_tx::MsgSendTx; use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; use ibc_relayer_types::applications::{ ics27_ica::cosmos_tx::CosmosTx, diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 4ff613a5d9..0526d01f1c 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -7,9 +7,19 @@ //! without the supervisor enabled should be relayed in order along with the //! other packets, as expected of ordered channel behaviour. +use ibc_relayer_types::applications::ics27_ica::cosmos_tx::CosmosTx; +use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; +use ibc_relayer_types::signer::Signer; +use ibc_relayer_types::timestamp::Timestamp; +use ibc_relayer_types::applications::transfer::{Amount, Coin}; +use ibc_relayer_types::applications::transfer::msgs::send::MsgSend; +use ibc_relayer_types::bigint::U256; use ibc_test_framework::chain::ext::ica::register_interchain_account; use ibc_test_framework::prelude::*; use ibc_test_framework::relayer::channel::query_channel_end; +use ibc_test_framework::framework::binary::channel::run_binary_interchain_security_channel_test; +use ibc_test_framework::util::interchain_security::update_genesis_for_consumer_chain; +use ibc_test_framework::relayer::channel::assert_eventually_channel_established; #[test] fn test_ica_ordered_channel() -> Result<(), Error> { @@ -81,7 +91,7 @@ impl BinaryChannelTest for IcaOrderedChannelTest { let channel_end = query_channel_end(chains.handle_b(), &channel_id.as_ref(), &port_id.as_ref())?; - assert_eq!(channel_end.value().ordering(), ChannelOrder::Ordered); + assert_eq!(channel_end.value().ordering(), Ordering::Ordered); // Query the controller chain for the address of the ICA wallet on the host chain. let ica_address = chains.node_b.chain_driver().query_interchain_account( diff --git a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs index 4e10120cfa..ce365fd357 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs @@ -1,13 +1,11 @@ //! The following tests are for the Interchain Security. //! These tests require the first chain to be a Provider chain and //! the second chain a Consumer chain. -use crate::utils::interchain_send_tx; use std::str::FromStr; -use ibc_relayer::chain::tracking::TrackedMsgs; -use ibc_relayer::event::IbcEventWithHeight; +use crate::utils::interchain_send_tx; + use ibc_relayer_types::applications::ics27_ica::cosmos_tx::CosmosTx; -use ibc_relayer_types::applications::ics27_ica::msgs::send_tx::MsgSendTx; use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; use ibc_relayer_types::applications::transfer::msgs::send::MsgSend; use ibc_relayer_types::applications::transfer::{Amount, Coin}; From bc16d41a056b52e200e5620bc36af302abef1914 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Mon, 18 Sep 2023 14:59:46 -0500 Subject: [PATCH 06/29] Add sleep calls in between supervisor runs --- .../interchain_security/ica_ordered_channel.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 0526d01f1c..bb7aae4f6d 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -143,7 +143,12 @@ impl BinaryChannelTest for IcaOrderedChannelTest { let signer = Signer::from_str(&wallet.address().to_string()).unwrap(); + sleep(Duration::from_secs(1)); + relayer.with_supervisor(|| { + + sleep(Duration::from_secs(1)); + let ica_events = interchain_send_tx( chains.handle_b(), &signer, @@ -154,9 +159,13 @@ impl BinaryChannelTest for IcaOrderedChannelTest { info!("First ICA transfer made with supervisor: {ica_events:#?}"); + sleep(Duration::from_secs(1)); + Ok(()) }); + sleep(Duration::from_secs(1)); + let ica_events = interchain_send_tx( chains.handle_b(), &signer, @@ -167,7 +176,12 @@ impl BinaryChannelTest for IcaOrderedChannelTest { info!("Second ICA transfer made without supervisor: {ica_events:#?}"); + sleep(Duration::from_secs(1)); + relayer.with_supervisor(|| { + + sleep(Duration::from_secs(1)); + let ica_events = interchain_send_tx( chains.handle_b(), &signer, @@ -178,6 +192,8 @@ impl BinaryChannelTest for IcaOrderedChannelTest { info!("Third ICA transfer made with supervisor: {ica_events:#?}"); + sleep(Duration::from_secs(1)); + Ok(()) }); From 083b81a42c9fe735909f716d6822839d794a0977 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Mon, 2 Oct 2023 13:18:46 +0200 Subject: [PATCH 07/29] Formatting --- .../interchain_security/ica_ordered_channel.rs | 16 +++++++--------- .../src/tests/interchain_security/mod.rs | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index bb7aae4f6d..e3617d2307 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -9,17 +9,17 @@ use ibc_relayer_types::applications::ics27_ica::cosmos_tx::CosmosTx; use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; -use ibc_relayer_types::signer::Signer; -use ibc_relayer_types::timestamp::Timestamp; -use ibc_relayer_types::applications::transfer::{Amount, Coin}; use ibc_relayer_types::applications::transfer::msgs::send::MsgSend; +use ibc_relayer_types::applications::transfer::{Amount, Coin}; use ibc_relayer_types::bigint::U256; +use ibc_relayer_types::signer::Signer; +use ibc_relayer_types::timestamp::Timestamp; use ibc_test_framework::chain::ext::ica::register_interchain_account; +use ibc_test_framework::framework::binary::channel::run_binary_interchain_security_channel_test; use ibc_test_framework::prelude::*; +use ibc_test_framework::relayer::channel::assert_eventually_channel_established; use ibc_test_framework::relayer::channel::query_channel_end; -use ibc_test_framework::framework::binary::channel::run_binary_interchain_security_channel_test; use ibc_test_framework::util::interchain_security::update_genesis_for_consumer_chain; -use ibc_test_framework::relayer::channel::assert_eventually_channel_established; #[test] fn test_ica_ordered_channel() -> Result<(), Error> { @@ -146,7 +146,6 @@ impl BinaryChannelTest for IcaOrderedChannelTest { sleep(Duration::from_secs(1)); relayer.with_supervisor(|| { - sleep(Duration::from_secs(1)); let ica_events = interchain_send_tx( @@ -156,7 +155,7 @@ impl BinaryChannelTest for IcaOrderedChannelTest { interchain_account_packet_data, Timestamp::from_nanoseconds(120000000000).unwrap(), )?; - + info!("First ICA transfer made with supervisor: {ica_events:#?}"); sleep(Duration::from_secs(1)); @@ -179,7 +178,6 @@ impl BinaryChannelTest for IcaOrderedChannelTest { sleep(Duration::from_secs(1)); relayer.with_supervisor(|| { - sleep(Duration::from_secs(1)); let ica_events = interchain_send_tx( @@ -189,7 +187,7 @@ impl BinaryChannelTest for IcaOrderedChannelTest { interchain_account_packet_data, Timestamp::from_nanoseconds(120000000000).unwrap(), )?; - + info!("Third ICA transfer made with supervisor: {ica_events:#?}"); sleep(Duration::from_secs(1)); diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index 010402929e..a8286825b5 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,6 +1,6 @@ +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; From f649204f7abdbcfcc342f827761946ac235d0f6b Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Mon, 2 Oct 2023 13:23:02 +0200 Subject: [PATCH 08/29] Fix compilation issues --- tools/integration-test/src/tests/ica.rs | 4 ++-- .../ica_ordered_channel.rs | 23 +++++++++++-------- .../tests/interchain_security/ica_transfer.rs | 5 ++-- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tools/integration-test/src/tests/ica.rs b/tools/integration-test/src/tests/ica.rs index e32f8685f8..29d9a1d8e6 100644 --- a/tools/integration-test/src/tests/ica.rs +++ b/tools/integration-test/src/tests/ica.rs @@ -1,8 +1,6 @@ use std::collections::HashMap; use std::str::FromStr; -use crate::utils::interchain_send_tx; - use ibc_relayer::chain::handle::ChainHandle; use ibc_relayer::config::{ filter::{ChannelFilters, ChannelPolicy, FilterPattern}, @@ -26,6 +24,8 @@ use ibc_test_framework::relayer::channel::{ assert_eventually_channel_established, query_channel_end, }; +use crate::tests::utils::interchain_send_tx; + #[test] fn test_ica_filter_default() -> Result<(), Error> { run_binary_connection_test(&IcaFilterTestAllow::new(PacketFilter::default())) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index e3617d2307..332085fc6c 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -7,6 +7,8 @@ //! without the supervisor enabled should be relayed in order along with the //! other packets, as expected of ordered channel behaviour. +use std::str::FromStr; + use ibc_relayer_types::applications::ics27_ica::cosmos_tx::CosmosTx; use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; use ibc_relayer_types::applications::transfer::msgs::send::MsgSend; @@ -14,6 +16,7 @@ use ibc_relayer_types::applications::transfer::{Amount, Coin}; use ibc_relayer_types::bigint::U256; use ibc_relayer_types::signer::Signer; use ibc_relayer_types::timestamp::Timestamp; +use ibc_relayer_types::tx_msg::Msg; use ibc_test_framework::chain::ext::ica::register_interchain_account; use ibc_test_framework::framework::binary::channel::run_binary_interchain_security_channel_test; use ibc_test_framework::prelude::*; @@ -21,6 +24,8 @@ use ibc_test_framework::relayer::channel::assert_eventually_channel_established; use ibc_test_framework::relayer::channel::query_channel_end; use ibc_test_framework::util::interchain_security::update_genesis_for_consumer_chain; +use crate::tests::utils::interchain_send_tx; + #[test] fn test_ica_ordered_channel() -> Result<(), Error> { run_binary_interchain_security_channel_test(&IcaOrderedChannelTest) @@ -91,7 +96,7 @@ impl BinaryChannelTest for IcaOrderedChannelTest { let channel_end = query_channel_end(chains.handle_b(), &channel_id.as_ref(), &port_id.as_ref())?; - assert_eq!(channel_end.value().ordering(), Ordering::Ordered); + assert_eq!(channel_end.value().ordering(), &Ordering::Ordered); // Query the controller chain for the address of the ICA wallet on the host chain. let ica_address = chains.node_b.chain_driver().query_interchain_account( @@ -143,7 +148,7 @@ impl BinaryChannelTest for IcaOrderedChannelTest { let signer = Signer::from_str(&wallet.address().to_string()).unwrap(); - sleep(Duration::from_secs(1)); + sleep(Duration::from_secs(5)); relayer.with_supervisor(|| { sleep(Duration::from_secs(1)); @@ -152,16 +157,16 @@ impl BinaryChannelTest for IcaOrderedChannelTest { chains.handle_b(), &signer, &channel.connection.connection_id_b.0, - interchain_account_packet_data, + interchain_account_packet_data.clone(), Timestamp::from_nanoseconds(120000000000).unwrap(), )?; info!("First ICA transfer made with supervisor: {ica_events:#?}"); - sleep(Duration::from_secs(1)); + sleep(Duration::from_secs(5)); Ok(()) - }); + })?; sleep(Duration::from_secs(1)); @@ -169,13 +174,13 @@ impl BinaryChannelTest for IcaOrderedChannelTest { chains.handle_b(), &signer, &channel.connection.connection_id_b.0, - interchain_account_packet_data, + interchain_account_packet_data.clone(), Timestamp::from_nanoseconds(120000000000).unwrap(), )?; info!("Second ICA transfer made without supervisor: {ica_events:#?}"); - sleep(Duration::from_secs(1)); + sleep(Duration::from_secs(5)); relayer.with_supervisor(|| { sleep(Duration::from_secs(1)); @@ -190,10 +195,10 @@ impl BinaryChannelTest for IcaOrderedChannelTest { info!("Third ICA transfer made with supervisor: {ica_events:#?}"); - sleep(Duration::from_secs(1)); + sleep(Duration::from_secs(5)); Ok(()) - }); + })?; // Check that the ICA account's balance has been debited the sent amount. chains.node_a.chain_driver().assert_eventual_wallet_amount( diff --git a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs index ce365fd357..01ecb29255 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs @@ -3,8 +3,6 @@ //! the second chain a Consumer chain. use std::str::FromStr; -use crate::utils::interchain_send_tx; - use ibc_relayer_types::applications::ics27_ica::cosmos_tx::CosmosTx; use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; use ibc_relayer_types::applications::transfer::msgs::send::MsgSend; @@ -12,6 +10,7 @@ use ibc_relayer_types::applications::transfer::{Amount, Coin}; use ibc_relayer_types::bigint::U256; use ibc_relayer_types::signer::Signer; use ibc_relayer_types::timestamp::Timestamp; +use ibc_relayer_types::tx_msg::Msg; use ibc_test_framework::chain::ext::ica::register_interchain_account; use ibc_test_framework::framework::binary::channel::run_binary_interchain_security_channel_test; use ibc_test_framework::prelude::*; @@ -20,6 +19,8 @@ use ibc_test_framework::util::interchain_security::{ update_genesis_for_consumer_chain, update_relayer_config_for_consumer_chain, }; +use crate::tests::utils::interchain_send_tx; + #[test] fn test_ics_ica_transfer() -> Result<(), Error> { run_binary_interchain_security_channel_test(&InterchainSecurityIcaTransferTest) From 08b479d295d59594a2ef463f722a42ce6d537079 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 17 Oct 2023 11:14:24 -0500 Subject: [PATCH 09/29] Emphasize wording in documentation --- .../src/tests/interchain_security/ica_ordered_channel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 332085fc6c..9a42ee382d 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -2,7 +2,7 @@ //! //! In order to ensure that ordered channels correctly clear packets on ICA //! channels, this test sends some sequential packets with the supervisor enabled, -//! sends the next packet without the supervisor enabled, then sends additional +//! sends the next packet *without* the supervisor enabled, then sends additional //! packets with the supervisor enabled again. The pending packet that was sent //! without the supervisor enabled should be relayed in order along with the //! other packets, as expected of ordered channel behaviour. From e1d48d22cb789584de8c017ee901a83c7b552147 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Mon, 30 Oct 2023 13:26:13 -0500 Subject: [PATCH 10/29] Fill in code from discussion --- crates/relayer/src/chain/tracking.rs | 3 +++ crates/relayer/src/link/relay_path.rs | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/relayer/src/chain/tracking.rs b/crates/relayer/src/chain/tracking.rs index 62f89ca096..b0d8d621c8 100644 --- a/crates/relayer/src/chain/tracking.rs +++ b/crates/relayer/src/chain/tracking.rs @@ -32,6 +32,9 @@ impl TrackingId { pub fn new_cleared_uuid() -> Self { Self::ClearedUuid(Uuid::new_v4()) } + + /// Indicates whether a packet clearing process is currently in-progress. + pub fn is_clearing(&self) -> bool {} } impl Display for TrackingId { diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index b055bf1268..3647133164 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -673,10 +673,21 @@ impl RelayPath { } Err(LinkError(error::LinkErrorDetail::Send(e), _)) => { // This error means we could retry - error!("error {}", e.event); + error!("error {}", e.event); // TODO: better error message here + + // if on an ordered channel, perform a packet clearing, but only if we + // are not in the middle of another packet clearing process + if self.ordered_channel() && i == 0 && !odata.tracking_id.is_clearing() { + // Do we need to specify the height for the packet clearing? + // Probably not, since no progress will have been made on the clearing process + self.relay_pending_packets(None)?; + } + if i + 1 == MAX_RETRIES { - error!("{}/{} retries exhausted. giving up", i + 1, MAX_RETRIES) + error!("{}/{} retries exhausted. Giving up", i + 1, MAX_RETRIES) } else { + debug!("{}/{} retries exhausted. Retrying with newly-generated operational data", i + 1, MAX_RETRIES); + // If we haven't exhausted all retries, regenerate the op. data & retry match self.regenerate_operational_data(odata.clone()) { None => return Ok(S::Reply::empty()), // Nothing to retry From 7480c1fe7562f9d44fc066f3c255658a8d52e3b3 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Thu, 2 Nov 2023 09:23:45 -0500 Subject: [PATCH 11/29] Rename TrakingId::ClearId to TrackingId::PacketClearing --- crates/relayer/src/chain/tracking.rs | 12 +++++++----- crates/relayer/src/link/relay_path.rs | 8 +++++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/relayer/src/chain/tracking.rs b/crates/relayer/src/chain/tracking.rs index b0d8d621c8..589ad305f2 100644 --- a/crates/relayer/src/chain/tracking.rs +++ b/crates/relayer/src/chain/tracking.rs @@ -15,7 +15,7 @@ pub enum TrackingId { /// the CLI or during packet clearing. Static(&'static str), /// Random identifier used to track latency of packet clearing. - ClearedUuid(Uuid), + PacketClearing(Uuid), } impl TrackingId { @@ -29,12 +29,14 @@ impl TrackingId { Self::Static(s) } - pub fn new_cleared_uuid() -> Self { - Self::ClearedUuid(Uuid::new_v4()) + pub fn new_packet_clearing() -> Self { + Self::PacketClearing(Uuid::new_v4()) } /// Indicates whether a packet clearing process is currently in-progress. - pub fn is_clearing(&self) -> bool {} + pub fn is_clearing(&self) -> bool { + matches!(self, Self::PacketClearing(_)) + } } impl Display for TrackingId { @@ -46,7 +48,7 @@ impl Display for TrackingId { s.fmt(f) } TrackingId::Static(s) => s.fmt(f), - TrackingId::ClearedUuid(u) => { + TrackingId::PacketClearing(u) => { let mut uuid = "cleared/".to_owned(); let mut s = u.to_string(); s.truncate(8); diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 3647133164..a821eb8cb8 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -415,7 +415,7 @@ impl RelayPath { fn relay_pending_packets(&self, height: Option) -> Result<(), LinkError> { let _span = span!(Level::ERROR, "relay_pending_packets", ?height).entered(); - let tracking_id = TrackingId::new_cleared_uuid(); + let tracking_id = TrackingId::new_packet_clearing(); telemetry!(received_event_batch, tracking_id); for i in 1..=MAX_RETRIES { @@ -672,8 +672,10 @@ impl RelayPath { return Ok(reply); } Err(LinkError(error::LinkErrorDetail::Send(e), _)) => { - // This error means we could retry - error!("error {}", e.event); // TODO: better error message here + warn!( + "Determining whether we need to retry in response to: {}", + e.event + ); // if on an ordered channel, perform a packet clearing, but only if we // are not in the middle of another packet clearing process From 6fc48b47fbbc6b19a137713595d5a82f338e4372 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Thu, 2 Nov 2023 10:42:22 -0500 Subject: [PATCH 12/29] Compile ica ordered channel test under the ica feature flag --- tools/integration-test/src/tests/interchain_security/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index a8286825b5..010402929e 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,6 +1,6 @@ -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; From 53df7681ba52ec92f4742c589f9b3cd20961d2cd Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Thu, 2 Nov 2023 10:43:14 -0500 Subject: [PATCH 13/29] Cargo fmt --- tools/integration-test/src/tests/interchain_security/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index 010402929e..a8286825b5 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,6 +1,6 @@ +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; From f927d0fcb6591c65da321cb5f7ad8e33f22cb433 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Fri, 10 Nov 2023 09:50:20 -0600 Subject: [PATCH 14/29] Move interchain_send_tx fn to test-framework crate --- tools/integration-test/src/tests/ica.rs | 3 +- .../ica_ordered_channel.rs | 6 ++-- .../tests/interchain_security/ica_transfer.rs | 4 +-- .../src/tests/interchain_security/mod.rs | 2 +- tools/integration-test/src/tests/mod.rs | 2 -- tools/integration-test/src/tests/utils.rs | 35 ------------------ .../src/util/interchain_security.rs | 36 +++++++++++++++++-- 7 files changed, 40 insertions(+), 48 deletions(-) delete mode 100644 tools/integration-test/src/tests/utils.rs diff --git a/tools/integration-test/src/tests/ica.rs b/tools/integration-test/src/tests/ica.rs index bf58873e22..1eb46e146d 100644 --- a/tools/integration-test/src/tests/ica.rs +++ b/tools/integration-test/src/tests/ica.rs @@ -23,8 +23,7 @@ use ibc_test_framework::prelude::*; use ibc_test_framework::relayer::channel::{ assert_eventually_channel_established, query_channel_end, }; - -use crate::tests::utils::interchain_send_tx; +use ibc_test_framework::util::interchain_security::interchain_send_tx; #[test] fn test_ica_filter_default() -> Result<(), Error> { diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 9a42ee382d..1f16c35b55 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -22,9 +22,9 @@ use ibc_test_framework::framework::binary::channel::run_binary_interchain_securi use ibc_test_framework::prelude::*; use ibc_test_framework::relayer::channel::assert_eventually_channel_established; use ibc_test_framework::relayer::channel::query_channel_end; -use ibc_test_framework::util::interchain_security::update_genesis_for_consumer_chain; - -use crate::tests::utils::interchain_send_tx; +use ibc_test_framework::util::interchain_security::{ + interchain_send_tx, update_genesis_for_consumer_chain, +}; #[test] fn test_ica_ordered_channel() -> Result<(), Error> { diff --git a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs index 01ecb29255..5377f206da 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_transfer.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_transfer.rs @@ -16,11 +16,9 @@ use ibc_test_framework::framework::binary::channel::run_binary_interchain_securi use ibc_test_framework::prelude::*; use ibc_test_framework::relayer::channel::assert_eventually_channel_established; use ibc_test_framework::util::interchain_security::{ - update_genesis_for_consumer_chain, update_relayer_config_for_consumer_chain, + interchain_send_tx, update_genesis_for_consumer_chain, update_relayer_config_for_consumer_chain, }; -use crate::tests::utils::interchain_send_tx; - #[test] fn test_ics_ica_transfer() -> Result<(), Error> { run_binary_interchain_security_channel_test(&InterchainSecurityIcaTransferTest) diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index a8286825b5..010402929e 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,6 +1,6 @@ -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; diff --git a/tools/integration-test/src/tests/mod.rs b/tools/integration-test/src/tests/mod.rs index 888785f137..2badc7caae 100644 --- a/tools/integration-test/src/tests/mod.rs +++ b/tools/integration-test/src/tests/mod.rs @@ -27,8 +27,6 @@ pub mod tendermint; pub mod ternary_transfer; pub mod transfer; -mod utils; - #[cfg(any(doc, feature = "ics29-fee"))] pub mod fee; diff --git a/tools/integration-test/src/tests/utils.rs b/tools/integration-test/src/tests/utils.rs deleted file mode 100644 index 1f5779df0c..0000000000 --- a/tools/integration-test/src/tests/utils.rs +++ /dev/null @@ -1,35 +0,0 @@ -//! Utility functions for integration tests. - -use ibc_relayer::chain::tracking::TrackedMsgs; -use ibc_relayer::event::IbcEventWithHeight; -use ibc_relayer_types::applications::ics27_ica::msgs::send_tx::MsgSendTx; -use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; -use ibc_relayer_types::signer::Signer; -use ibc_relayer_types::timestamp::Timestamp; -use ibc_relayer_types::tx_msg::Msg; -use ibc_test_framework::prelude::*; - -/// Sends a message containing `InterchainAccountPacketData` from the `Signer` -/// to the `Chain`. -pub fn interchain_send_tx( - chain: &ChainA, - from: &Signer, - connection: &ConnectionId, - msg: InterchainAccountPacketData, - relative_timeout: Timestamp, -) -> Result, Error> { - let msg = MsgSendTx { - owner: from.clone(), - connection_id: connection.clone(), - packet_data: msg, - relative_timeout, - }; - - let msg_any = msg.to_any(); - - let tm = TrackedMsgs::new_static(vec![msg_any], "SendTx"); - - chain - .send_messages_and_wait_commit(tm) - .map_err(Error::relayer) -} diff --git a/tools/test-framework/src/util/interchain_security.rs b/tools/test-framework/src/util/interchain_security.rs index ec808a539c..817cfe62ed 100644 --- a/tools/test-framework/src/util/interchain_security.rs +++ b/tools/test-framework/src/util/interchain_security.rs @@ -1,8 +1,15 @@ -use ibc_relayer::config::ChainConfig; - use crate::chain::config::set_voting_period; use crate::prelude::*; +use ibc_relayer::config::ChainConfig; +use ibc_relayer::chain::tracking::TrackedMsgs; +use ibc_relayer::event::IbcEventWithHeight; +use ibc_relayer_types::applications::ics27_ica::msgs::send_tx::MsgSendTx; +use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; +use ibc_relayer_types::signer::Signer; +use ibc_relayer_types::timestamp::Timestamp; +use ibc_relayer_types::tx_msg::Msg; + pub fn update_genesis_for_consumer_chain(genesis: &mut serde_json::Value) -> Result<(), Error> { // Consumer chain doesn't have a gov key. if genesis @@ -32,3 +39,28 @@ pub fn update_relayer_config_for_consumer_chain(config: &mut Config) { } } } + +/// Sends a message containing `InterchainAccountPacketData` from the `Signer` +/// to the `Chain`. +pub fn interchain_send_tx( + chain: &ChainA, + from: &Signer, + connection: &ConnectionId, + msg: InterchainAccountPacketData, + relative_timeout: Timestamp, +) -> Result, Error> { + let msg = MsgSendTx { + owner: from.clone(), + connection_id: connection.clone(), + packet_data: msg, + relative_timeout, + }; + + let msg_any = msg.to_any(); + + let tm = TrackedMsgs::new_static(vec![msg_any], "SendTx"); + + chain + .send_messages_and_wait_commit(tm) + .map_err(Error::relayer) +} \ No newline at end of file From 3cd60e2e80a9345441a523a76240f63fd40cdf16 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Fri, 10 Nov 2023 10:00:17 -0600 Subject: [PATCH 15/29] Cargo fmt --- tools/integration-test/src/tests/interchain_security/mod.rs | 2 +- tools/test-framework/src/util/interchain_security.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index 010402929e..a8286825b5 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,6 +1,6 @@ +pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ics31"))] pub mod icq; pub mod simple_transfer; diff --git a/tools/test-framework/src/util/interchain_security.rs b/tools/test-framework/src/util/interchain_security.rs index 817cfe62ed..50c049ec8a 100644 --- a/tools/test-framework/src/util/interchain_security.rs +++ b/tools/test-framework/src/util/interchain_security.rs @@ -1,8 +1,8 @@ use crate::chain::config::set_voting_period; use crate::prelude::*; -use ibc_relayer::config::ChainConfig; use ibc_relayer::chain::tracking::TrackedMsgs; +use ibc_relayer::config::ChainConfig; use ibc_relayer::event::IbcEventWithHeight; use ibc_relayer_types::applications::ics27_ica::msgs::send_tx::MsgSendTx; use ibc_relayer_types::applications::ics27_ica::packet_data::InterchainAccountPacketData; @@ -63,4 +63,4 @@ pub fn interchain_send_tx( chain .send_messages_and_wait_commit(tm) .map_err(Error::relayer) -} \ No newline at end of file +} From fca4417cf0e5ffd837df765ebe18a7569cd3df08 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Mon, 13 Nov 2023 11:13:44 -0600 Subject: [PATCH 16/29] Update relayer config for consumer chain --- .../src/tests/interchain_security/ica_ordered_channel.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 1f16c35b55..7b832b55c8 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -23,7 +23,7 @@ use ibc_test_framework::prelude::*; use ibc_test_framework::relayer::channel::assert_eventually_channel_established; use ibc_test_framework::relayer::channel::query_channel_end; use ibc_test_framework::util::interchain_security::{ - interchain_send_tx, update_genesis_for_consumer_chain, + interchain_send_tx, update_genesis_for_consumer_chain, update_relayer_config_for_consumer_chain, }; #[test] @@ -65,6 +65,8 @@ impl TestOverrides for IcaOrderedChannelTest { config.mode.packets.enabled = true; config.mode.packets.clear_on_start = false; config.mode.packets.clear_interval = 0; + + update_relayer_config_for_consumer_chain(config); } fn should_spawn_supervisor(&self) -> bool { From ccf329f5435a53bf645574899e502431c0655fdf Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 14 Nov 2023 09:03:55 -0600 Subject: [PATCH 17/29] Move ica_ordered_channel test under the ica feature --- tools/integration-test/src/tests/interchain_security/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index a8286825b5..83136c9f88 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,5 +1,5 @@ -pub mod ica_ordered_channel; #[cfg(any(doc, feature = "ica"))] +pub mod ica_ordered_channel; pub mod ica_transfer; #[cfg(any(doc, feature = "ics31"))] pub mod icq; From 112c4f8b5fa37a9c43b2f3bbcb6b626f02ff7be5 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 14 Nov 2023 09:35:41 -0600 Subject: [PATCH 18/29] Move ica_transfer test under ica feature --- tools/integration-test/src/tests/interchain_security/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/integration-test/src/tests/interchain_security/mod.rs b/tools/integration-test/src/tests/interchain_security/mod.rs index 83136c9f88..0772d733db 100644 --- a/tools/integration-test/src/tests/interchain_security/mod.rs +++ b/tools/integration-test/src/tests/interchain_security/mod.rs @@ -1,5 +1,6 @@ #[cfg(any(doc, feature = "ica"))] pub mod ica_ordered_channel; +#[cfg(any(doc, feature = "ica"))] pub mod ica_transfer; #[cfg(any(doc, feature = "ics31"))] pub mod icq; From 1dbe79c46c36532710f305dca7c4f5f843627cb8 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 14 Nov 2023 10:32:38 -0600 Subject: [PATCH 19/29] Check that ICA channel is eventually established using the supervisor --- .../interchain_security/ica_ordered_channel.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 7b832b55c8..37999e478f 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -86,13 +86,15 @@ impl BinaryChannelTest for IcaOrderedChannelTest { let (wallet, channel_id, port_id) = register_interchain_account(&chains.node_b, chains.handle_b(), &connection_b_to_a)?; - // Check that the corresponding ICA channel is eventually established. - let _counterparty_channel_id = assert_eventually_channel_established( - chains.handle_b(), - chains.handle_a(), - &channel_id.as_ref(), - &port_id.as_ref(), - )?; + relayer.with_supervisor(|| { + // Check that the corresponding ICA channel is eventually established. + let _counterparty_channel_id = assert_eventually_channel_established( + chains.handle_b(), + chains.handle_a(), + &channel_id.as_ref(), + &port_id.as_ref(), + )?; + }); // Assert that the channel returned by `register_interchain_account` is an ordered channel let channel_end = From fdc352bb339033efe9930f9a7df2e80e631b511a Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 14 Nov 2023 10:43:31 -0600 Subject: [PATCH 20/29] Fix clippy warnings --- .../src/tests/interchain_security/ica_ordered_channel.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 37999e478f..42dd0877d5 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -94,7 +94,9 @@ impl BinaryChannelTest for IcaOrderedChannelTest { &channel_id.as_ref(), &port_id.as_ref(), )?; - }); + + Ok(()) + })?; // Assert that the channel returned by `register_interchain_account` is an ordered channel let channel_end = From cc819348335237fa0026356b00e59daa138c237f Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Fri, 12 Jan 2024 11:12:12 +0100 Subject: [PATCH 21/29] Improve logs --- crates/relayer/src/link/relay_path.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 3bf28f5494..c89f20be7f 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -676,24 +676,21 @@ impl RelayPath { return Ok(reply); } - Err(LinkError(error::LinkErrorDetail::Send(e), _)) => { - warn!( - "Determining whether we need to retry in response to: {}", - e.event - ); - - // if on an ordered channel, perform a packet clearing, but only if we + Err(LinkError(error::LinkErrorDetail::Send(_), _)) => { + // If on an ordered channel, perform a packet clearing, but only if we // are not in the middle of another packet clearing process if self.ordered_channel() && i == 0 && !odata.tracking_id.is_clearing() { - // Do we need to specify the height for the packet clearing? - // Probably not, since no progress will have been made on the clearing process + warn!("Failed to relay to ordered channel, attempting to recover by clearing packets"); + + // We do need to specify the height for the packet clearing, + // since no progress will have been made on the clearing process self.relay_pending_packets(None)?; } if i + 1 == MAX_RETRIES { - error!("{}/{} retries exhausted. Giving up", i + 1, MAX_RETRIES) + error!("{}/{} retries exhausted, giving up", i + 1, MAX_RETRIES) } else { - debug!("{}/{} retries exhausted. Retrying with newly-generated operational data", i + 1, MAX_RETRIES); + debug!("{}/{} retries exhausted, retrying with newly-generated operational data", i + 1, MAX_RETRIES); // If we haven't exhausted all retries, regenerate the op. data & retry match self.regenerate_operational_data(odata.clone()) { From 19d106518aa61a9da14c0f612b9ad4262950d761 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Fri, 12 Jan 2024 11:37:39 +0100 Subject: [PATCH 22/29] Add changelog entry --- .../ibc-relayer/3540-ordered-channels-resilience.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md diff --git a/.changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md b/.changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md new file mode 100644 index 0000000000..4b62d567fb --- /dev/null +++ b/.changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md @@ -0,0 +1,6 @@ +- Improve resilience when relaying on ordered channels. When a packet + fails to be relayed on an ordered channel, this used to block the + channel until either another relayer relayed the packet successfully or + until packet clearing kicked off. Hermes will now detect the failure + and attempt to clear packets on the channel in order to unblock it. + ([\#3540](https://github.com/informalsystems/hermes/issues/3540)) From a6506bf87a2f4678b8d7548328b6d4688296aa53 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 27 Feb 2024 15:55:48 +0100 Subject: [PATCH 23/29] Fix compilation of ICA tests --- tools/integration-test/src/tests/ica.rs | 28 ------------------------- 1 file changed, 28 deletions(-) diff --git a/tools/integration-test/src/tests/ica.rs b/tools/integration-test/src/tests/ica.rs index 2a68a64024..024e82a4d9 100644 --- a/tools/integration-test/src/tests/ica.rs +++ b/tools/integration-test/src/tests/ica.rs @@ -186,11 +186,6 @@ impl BinaryConnectionTest for IcaFilterTestAllow { } } -#[test] -fn test_ica_filter_deny() -> Result<(), Error> { - run_binary_connection_test(&IcaFilterTestDeny) -} - pub struct IcaFilterTestDeny; impl TestOverrides for IcaFilterTestDeny { @@ -377,26 +372,3 @@ impl BinaryConnectionTest for ICACloseChannelTest { }) } } - -fn interchain_send_tx( - chain: &ChainA, - from: &Signer, - connection: &ConnectionId, - msg: InterchainAccountPacketData, - relative_timeout: Timestamp, -) -> Result, Error> { - let msg = MsgSendTx { - owner: from.clone(), - connection_id: connection.clone(), - packet_data: msg, - relative_timeout, - }; - - let msg_any = msg.to_any(); - - let tm = TrackedMsgs::new_static(vec![msg_any], "SendTx"); - - chain - .send_messages_and_wait_commit(tm) - .map_err(Error::relayer) -} From 11abdd760d6abb95bccf94e19c5f021950334d87 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 27 Feb 2024 16:19:49 +0100 Subject: [PATCH 24/29] Add `force_disable_clear_on_start` config option, only available in test code --- crates/relayer/src/config.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 2f335e13d0..0eeab4840e 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -414,6 +414,24 @@ pub struct Packets { pub ics20_max_memo_size: Ics20FieldSizeLimit, #[serde(default = "default::ics20_max_receiver_size")] pub ics20_max_receiver_size: Ics20FieldSizeLimit, + + #[cfg(test)] + #[serde(default)] + pub force_disable_clear_on_start: bool, +} + +impl Packets { + pub fn force_disable_clear_on_start(&self) -> bool { + #[cfg(test)] + { + self.force_disable_clear_on_start + } + + #[cfg(not(test))] + { + false + } + } } impl Default for Packets { @@ -426,6 +444,9 @@ impl Default for Packets { auto_register_counterparty_payee: default::auto_register_counterparty_payee(), ics20_max_memo_size: default::ics20_max_memo_size(), ics20_max_receiver_size: default::ics20_max_receiver_size(), + + #[cfg(test)] + force_disable_clear_on_start: false, } } } From 7acbfa43c8ae0b2ae5bef4cfd116c6c2dcba053c Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 27 Feb 2024 16:20:23 +0100 Subject: [PATCH 25/29] Cleanup --- crates/relayer/src/link/error.rs | 2 +- crates/relayer/src/link/relay_sender.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/relayer/src/link/error.rs b/crates/relayer/src/link/error.rs index adf8142ec8..694bc22f70 100644 --- a/crates/relayer/src/link/error.rs +++ b/crates/relayer/src/link/error.rs @@ -73,7 +73,7 @@ define_error! { Send { event: IbcEvent } |e| { - format!("chain error when sending messages: {0}", e.event) + format!("failed to send message: {0}", e.event) }, MissingChannelId diff --git a/crates/relayer/src/link/relay_sender.rs b/crates/relayer/src/link/relay_sender.rs index c4d29fe91a..a8c70cb9f0 100644 --- a/crates/relayer/src/link/relay_sender.rs +++ b/crates/relayer/src/link/relay_sender.rs @@ -95,10 +95,11 @@ impl Submit for AsyncSender { type Reply = AsyncReply; fn submit(target: &impl ChainHandle, msgs: TrackedMsgs) -> Result { - let a = target + let responses = target .send_messages_and_wait_check_tx(msgs) .map_err(LinkError::relayer)?; - let reply = AsyncReply { responses: a }; + + let reply = AsyncReply { responses }; // Note: There may be errors in the reply, for example: // `Response { code: Err(11), data: Data([]), log: Log("Too much gas wanted: 35000000, maximum is 25000000: out of gas")` From 3a5acfc31b7d7d6ca717f46601feeb9305660767 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 27 Feb 2024 16:21:31 +0100 Subject: [PATCH 26/29] Check whether packet clear is needed instead of reacting to error when it fails --- crates/relayer/src/link/relay_path.rs | 10 ------ crates/relayer/src/worker.rs | 10 +++++- crates/relayer/src/worker/packet.rs | 49 +++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 8b2f7cbf4e..9dd3c570c0 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -711,16 +711,6 @@ impl RelayPath { return Ok(reply); } Err(LinkError(error::LinkErrorDetail::Send(_), _)) => { - // If on an ordered channel, perform a packet clearing, but only if we - // are not in the middle of another packet clearing process - if self.ordered_channel() && i == 0 && !odata.tracking_id.is_clearing() { - warn!("Failed to relay to ordered channel, attempting to recover by clearing packets"); - - // We do need to specify the height for the packet clearing, - // since no progress will have been made on the clearing process - self.relay_pending_packets(None)?; - } - if i + 1 == MAX_RETRIES { error!("{}/{} retries exhausted, giving up", i + 1, MAX_RETRIES) } else { diff --git a/crates/relayer/src/worker.rs b/crates/relayer/src/worker.rs index 400ff811af..08abb94810 100644 --- a/crates/relayer/src/worker.rs +++ b/crates/relayer/src/worker.rs @@ -128,7 +128,7 @@ pub fn spawn_worker_tasks( Ok(link) => { let channel_ordering = link.a_to_b.channel().ordering; let should_clear_on_start = - packets_config.clear_on_start || channel_ordering == Ordering::Ordered; + should_clear_on_start(&packets_config, channel_ordering); let (cmd_tx, cmd_rx) = crossbeam_channel::unbounded(); let link = Arc::new(Mutex::new(link)); @@ -217,3 +217,11 @@ pub fn spawn_worker_tasks( WorkerHandle::new(id, object, data, cmd_tx, task_handles) } + +fn should_clear_on_start(config: &crate::config::Packets, channel_ordering: Ordering) -> bool { + if config.force_disable_clear_on_start() { + false + } else { + config.clear_on_start || channel_ordering == Ordering::Ordered + } +} diff --git a/crates/relayer/src/worker/packet.rs b/crates/relayer/src/worker/packet.rs index 2eb85f633c..5daf122466 100644 --- a/crates/relayer/src/worker/packet.rs +++ b/crates/relayer/src/worker/packet.rs @@ -17,14 +17,17 @@ use ibc_proto::ibc::apps::fee::v1::{IdentifiedPacketFees, QueryIncentivizedPacke use ibc_proto::ibc::core::channel::v1::PacketId; use ibc_relayer_types::applications::ics29_fee::events::IncentivizedPacket; use ibc_relayer_types::applications::transfer::{Amount, Coin, RawCoin}; +use ibc_relayer_types::core::ics04_channel::channel::Ordering; use ibc_relayer_types::core::ics04_channel::events::WriteAcknowledgement; use ibc_relayer_types::core::ics04_channel::packet::Sequence; use ibc_relayer_types::events::{IbcEvent, IbcEventType}; use ibc_relayer_types::Height; use crate::chain::handle::ChainHandle; +use crate::chain::requests::QueryHeight; use crate::config::filter::FeePolicy; use crate::event::source::EventBatch; +use crate::event::IbcEventWithHeight; use crate::foreign_client::HasExpiredOrFrozenError; use crate::link::Resubmit; use crate::link::{error::LinkError, Link}; @@ -204,6 +207,24 @@ fn handle_packet_cmd( ) -> Result<(), TaskError> { // Handle packet clearing which is triggered from a command let (do_clear, maybe_height) = match &cmd { + WorkerCmd::IbcEvents { batch } if link.a_to_b.channel().ordering == Ordering::Ordered => { + let lowest_sequence = lowest_sequence(&batch.events); + + let next_sequence = query_next_sequence_receive( + link.a_to_b.dst_chain(), + link.a_to_b.dst_port_id(), + link.a_to_b.dst_channel_id(), + QueryHeight::Specific(batch.height), + ) + .ok(); + + if *should_clear_on_start || next_sequence < lowest_sequence { + (true, Some(batch.height)) + } else { + (false, None) + } + } + WorkerCmd::IbcEvents { batch } => { if *should_clear_on_start { (true, Some(batch.height)) @@ -439,6 +460,34 @@ fn handle_execute_schedule( Ok(()) } +fn query_next_sequence_receive( + chain: &Chain, + port_id: &PortId, + channel_id: &ChannelId, + height: QueryHeight, +) -> Result { + use crate::chain::requests::{IncludeProof, QueryNextSequenceReceiveRequest}; + + chain + .query_next_sequence_receive( + QueryNextSequenceReceiveRequest { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + height, + }, + IncludeProof::No, + ) + .map(|(seq, _height)| seq) + .map_err(|e| LinkError::query(chain.id(), e)) +} + +fn lowest_sequence(events: &[IbcEventWithHeight]) -> Option { + events + .iter() + .flat_map(|event| event.event.packet().map(|p| p.sequence)) + .min() +} + #[cfg(feature = "telemetry")] use crate::link::RelaySummary; From b2c7fea6c0abf6c06c35b892730d772bafccf89a Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 27 Feb 2024 16:27:33 +0100 Subject: [PATCH 27/29] Force disable clear on start in ICA ordered channel test --- crates/relayer/src/config.rs | 19 +------------------ crates/relayer/src/worker.rs | 2 +- .../ica_ordered_channel.rs | 2 ++ 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 0eeab4840e..ec0afdaf34 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -415,25 +415,10 @@ pub struct Packets { #[serde(default = "default::ics20_max_receiver_size")] pub ics20_max_receiver_size: Ics20FieldSizeLimit, - #[cfg(test)] - #[serde(default)] + #[serde(skip)] pub force_disable_clear_on_start: bool, } -impl Packets { - pub fn force_disable_clear_on_start(&self) -> bool { - #[cfg(test)] - { - self.force_disable_clear_on_start - } - - #[cfg(not(test))] - { - false - } - } -} - impl Default for Packets { fn default() -> Self { Self { @@ -444,8 +429,6 @@ impl Default for Packets { auto_register_counterparty_payee: default::auto_register_counterparty_payee(), ics20_max_memo_size: default::ics20_max_memo_size(), ics20_max_receiver_size: default::ics20_max_receiver_size(), - - #[cfg(test)] force_disable_clear_on_start: false, } } diff --git a/crates/relayer/src/worker.rs b/crates/relayer/src/worker.rs index 08abb94810..40f73b67de 100644 --- a/crates/relayer/src/worker.rs +++ b/crates/relayer/src/worker.rs @@ -219,7 +219,7 @@ pub fn spawn_worker_tasks( } fn should_clear_on_start(config: &crate::config::Packets, channel_ordering: Ordering) -> bool { - if config.force_disable_clear_on_start() { + if config.force_disable_clear_on_start { false } else { config.clear_on_start || channel_ordering == Ordering::Ordered diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 42dd0877d5..8e94a1d45a 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -65,6 +65,8 @@ impl TestOverrides for IcaOrderedChannelTest { config.mode.packets.enabled = true; config.mode.packets.clear_on_start = false; config.mode.packets.clear_interval = 0; + // This is needed for ordered channels + config.mode.packets.force_disable_clear_on_start = true; update_relayer_config_for_consumer_chain(config); } From 92fe20eb65c4ce011013283f4a1c12a19eceff45 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 27 Feb 2024 17:31:08 +0100 Subject: [PATCH 28/29] Update changelog entry --- .../ibc-relayer/3540-ordered-channels-resilience.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md b/.changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md index 4b62d567fb..88899c96f5 100644 --- a/.changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md +++ b/.changelog/unreleased/improvements/ibc-relayer/3540-ordered-channels-resilience.md @@ -1,6 +1,6 @@ -- Improve resilience when relaying on ordered channels. When a packet - fails to be relayed on an ordered channel, this used to block the - channel until either another relayer relayed the packet successfully or - until packet clearing kicked off. Hermes will now detect the failure - and attempt to clear packets on the channel in order to unblock it. +- Improve resilience when relaying on ordered channels. + When relaying packets on an ordered channel, Hermes will now attempt + to detect whether the next message to send has the sequence number + expected on that channel. If there is a mismatch, then Hermes will trigger a packet + clear on the channel to unblock it before resuming operations on that channel. ([\#3540](https://github.com/informalsystems/hermes/issues/3540)) From 00745dab153c9dfa9ea6c4098457f6bc9b5511ca Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 28 Feb 2024 14:36:54 +0100 Subject: [PATCH 29/29] Improve ICA ordered channel test asserts --- .../ica_ordered_channel.rs | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs index 8e94a1d45a..56a0075ff3 100644 --- a/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs +++ b/tools/integration-test/src/tests/interchain_security/ica_ordered_channel.rs @@ -58,6 +58,8 @@ impl TestOverrides for IcaOrderedChannelTest { } fn modify_relayer_config(&self, config: &mut Config) { + config.mode.clients.misbehaviour = false; + config.mode.channels.enabled = true; // Disable packet clearing so that packets sent without the supervisor @@ -156,11 +158,12 @@ impl BinaryChannelTest for IcaOrderedChannelTest { let signer = Signer::from_str(&wallet.address().to_string()).unwrap(); - sleep(Duration::from_secs(5)); + let user2_balance = chains.node_a.chain_driver().query_balance( + &chains.node_a.wallets().user2().address(), + &stake_denom.as_ref(), + )?; relayer.with_supervisor(|| { - sleep(Duration::from_secs(1)); - let ica_events = interchain_send_tx( chains.handle_b(), &signer, @@ -169,15 +172,22 @@ impl BinaryChannelTest for IcaOrderedChannelTest { Timestamp::from_nanoseconds(120000000000).unwrap(), )?; - info!("First ICA transfer made with supervisor: {ica_events:#?}"); + // Check that the ICA account's balance has been debited the sent amount. + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &ica_address.as_ref(), + &stake_denom.with_amount(ica_fund - amount).as_ref(), + )?; - sleep(Duration::from_secs(5)); + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &chains.node_a.wallets().user2().address(), + &(user2_balance.clone() + amount).as_ref(), + )?; + + info!("First ICA transfer made with supervisor: {ica_events:#?}"); Ok(()) })?; - sleep(Duration::from_secs(1)); - let ica_events = interchain_send_tx( chains.handle_b(), &signer, @@ -188,11 +198,7 @@ impl BinaryChannelTest for IcaOrderedChannelTest { info!("Second ICA transfer made without supervisor: {ica_events:#?}"); - sleep(Duration::from_secs(5)); - relayer.with_supervisor(|| { - sleep(Duration::from_secs(1)); - let ica_events = interchain_send_tx( chains.handle_b(), &signer, @@ -201,19 +207,20 @@ impl BinaryChannelTest for IcaOrderedChannelTest { Timestamp::from_nanoseconds(120000000000).unwrap(), )?; + // Check that the ICA account's balance has been debited the sent amount. + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &ica_address.as_ref(), + &stake_denom.with_amount(ica_fund - 3 * amount).as_ref(), + )?; + info!("Third ICA transfer made with supervisor: {ica_events:#?}"); - sleep(Duration::from_secs(5)); + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &chains.node_a.wallets().user2().address(), + &(user2_balance + (3 * amount)).as_ref(), + )?; Ok(()) - })?; - - // Check that the ICA account's balance has been debited the sent amount. - chains.node_a.chain_driver().assert_eventual_wallet_amount( - &ica_address.as_ref(), - &stake_denom.with_amount(ica_fund - 3 * amount).as_ref(), - )?; - - Ok(()) + }) } }