Test ordered channel relaying with concurrent relayers

tools/integration-test/src/tests/ordered_channel.rs

@@ -1,7 +1,10 @@
+use ibc_relayer::keyring::Store;
 use ibc_test_framework::ibc::denom::derive_ibc_denom;
 use ibc_test_framework::prelude::*;
 use ibc_test_framework::util::random::random_u64_range;
 
+const TRANSFER_COUNT: u64 = 10;
+
 #[test]
 fn test_ordered_channel() -> Result<(), Error> {
     run_binary_channel_test(&OrderedChannelTest)
@@ -15,6 +18,12 @@ impl TestOverrides for OrderedChannelTest {
         // relay any packet it missed before starting.
         config.mode.packets.clear_on_start = false;
         config.mode.packets.clear_interval = 0;
+
+        for mut chain in config.chains.iter_mut() {
+            // We currently need the Test key store type to persist the keys
+            // across relayer driver forks.
+            chain.key_store_type = Store::Test;
+        }
     }
 
     fn channel_order(&self) -> Order {
@@ -43,63 +52,87 @@ impl BinaryChannelTest for OrderedChannelTest {
         let amount1 = random_u64_range(1000, 5000);
 
         info!(
-            "Performing IBC transfer with amount {}, which should be relayed because its an ordered channel",
-            amount1
+            "Performing IBC transfer with amount {} times {}, which should be relayed because its an ordered channel",
+            amount1, TRANSFER_COUNT
+        );
+
+        for _ in 0..TRANSFER_COUNT {
+            chains.node_a.chain_driver().transfer_token(
+                &channel.port_a.as_ref(),
+                &channel.channel_id_a.as_ref(),
+                &wallet_a.address(),
+                &wallet_b.address(),
+                amount1,
+                &denom_a,
+            )?;
+
+            // Have to sleep 1 second to wait for the transfer to be
+            // committed before doing another one.
+            std::thread::sleep(Duration::from_secs(1));
+        }
+
+        // Fork the relayer driver with different keys, so that we can spawn
+        // two racing relayers.
+        let relayer2 = {
+            let chain_id_a = chains.node_a.chain_id().into_value();
+            let chain_id_b = chains.node_b.chain_id().into_value();
+            let relayer_wallet_a = chains.node_a.wallets().relayer2().id().cloned_value().0;
+            let relayer_wallet_b = chains.node_b.wallets().relayer2().id().cloned_value().0;
+
+            relayer.fork(|config| {
+                for mut chain in config.chains.iter_mut() {
+                    if &chain.id == chain_id_a {
+                        chain.key_name = relayer_wallet_a.clone();
+                    } else if &chain.id == chain_id_b {
+                        chain.key_name = relayer_wallet_b.clone();
+                    }
+                }
+            })
+        };
+
+        let _relayer1 = relayer.spawn_supervisor()?;
+        let _relayer2 = relayer2.spawn_supervisor()?;
+
+        sleep(Duration::from_secs(3));
+
+        let amount2 = random_u64_range(1000, 5000);
+
+        info!(
+            "Performing IBC transfer with amount {}, which should be relayed",
+            amount2
         );
 
         chains.node_a.chain_driver().transfer_token(
             &channel.port_a.as_ref(),
             &channel.channel_id_a.as_ref(),
             &wallet_a.address(),
             &wallet_b.address(),
-            amount1,
+            amount2,
             &denom_a,
         )?;
 
         sleep(Duration::from_secs(1));
 
-        relayer.with_supervisor(|| {
-            sleep(Duration::from_secs(1));
-
-            let amount2 = random_u64_range(1000, 5000);
-
-            info!(
-                "Performing IBC transfer with amount {}, which should be relayed",
-                amount2
-            );
-
-            chains.node_a.chain_driver().transfer_token(
-                &channel.port_a.as_ref(),
-                &channel.channel_id_a.as_ref(),
-                &wallet_a.address(),
-                &wallet_b.address(),
-                amount2,
-                &denom_a,
-            )?;
-
-            sleep(Duration::from_secs(1));
-
-            let denom_b = derive_ibc_denom(
-                &channel.port_b.as_ref(),
-                &channel.channel_id_b.as_ref(),
-                &denom_a,
-            )?;
+        let denom_b = derive_ibc_denom(
+            &channel.port_b.as_ref(),
+            &channel.channel_id_b.as_ref(),
+            &denom_a,
+        )?;
 
-            // Wallet on chain A should have both amount deducted.
-            chains.node_a.chain_driver().assert_eventual_wallet_amount(
-                &wallet_a.address(),
-                balance_a - amount1 - amount2,
-                &denom_a,
-            )?;
+        // Wallet on chain A should have both amount deducted.
+        chains.node_a.chain_driver().assert_eventual_wallet_amount(
+            &wallet_a.address(),
+            balance_a - (amount1 * TRANSFER_COUNT) - amount2,
+            &denom_a,
+        )?;
 
-            // Wallet on chain B should receive both IBC transfers
-            chains.node_b.chain_driver().assert_eventual_wallet_amount(
-                &wallet_b.address(),
-                amount1 + amount2,
-                &denom_b.as_ref(),
-            )?;
+        // Wallet on chain B should receive both IBC transfers
+        chains.node_b.chain_driver().assert_eventual_wallet_amount(
+            &wallet_b.address(),
+            (amount1 * TRANSFER_COUNT) + amount2,
+            &denom_b.as_ref(),
+        )?;
 
-            Ok(())
-        })
+        Ok(())
     }
 }

tools/integration-test/src/tests/supervisor.rs

@@ -96,14 +96,14 @@ impl BinaryChainTest for SupervisorTest {
             // wallet to mess up the account sequence number on both sides.
 
             chains.node_a.chain_driver().local_transfer_token(
-                &chains.node_a.wallets().relayer().address(),
+                &chains.node_a.wallets().relayer1().address(),
                 &chains.node_a.wallets().user2().address(),
                 1000,
                 &denom_a,
             )?;
 
             chains.node_b.chain_driver().local_transfer_token(
-                &chains.node_b.wallets().relayer().address(),
+                &chains.node_b.wallets().relayer1().address(),
                 &chains.node_b.wallets().user2().address(),
                 1000,
                 &chains.node_b.denom(),

tools/test-framework/src/bootstrap/binary/chain.rs

@@ -5,25 +5,21 @@
 
 use eyre::Report as Error;
 use ibc::core::ics24_host::identifier::ClientId;
-use ibc_relayer::chain::handle::{ChainHandle, CountingAndCachingChainHandle};
-use ibc_relayer::config::{Config, SharedConfig};
-use ibc_relayer::error::ErrorDetail as RelayerErrorDetail;
+use ibc_relayer::chain::handle::ChainHandle;
+use ibc_relayer::config::Config;
 use ibc_relayer::foreign_client::{extract_client_id, ForeignClient};
-use ibc_relayer::keyring::errors::ErrorDetail as KeyringErrorDetail;
-use ibc_relayer::registry::SharedRegistry;
-use std::fs;
-use std::path::Path;
-use std::sync::Arc;
-use std::sync::RwLock;
+use std::sync::{Arc, RwLock};
 use tracing::{debug, info};
 
 use crate::relayer::driver::RelayerDriver;
+use crate::relayer::spawn::{
+    add_chain_config, new_registry, save_relayer_config, spawn_chain_handle,
+};
 use crate::types::binary::chains::ConnectedChains;
 use crate::types::binary::foreign_client::ForeignClientPair;
 use crate::types::config::TestConfig;
 use crate::types::single::node::FullNode;
 use crate::types::tagged::*;
-use crate::types::wallet::{TestWallets, Wallet};
 use crate::util::random::random_u64_range;
 
 /**
@@ -177,127 +173,3 @@ pub fn bootstrap_foreign_client<ChainA: ChainHandle, ChainB: ChainHandle>(
         chain_a.clone(),
     ))
 }
-
-/**
-   Spawn a new chain handle using the given [`SharedRegistry`] and
-   [`FullNode`].
-
-   The function accepts a proxy type `Seed` that should be unique
-   accross multiple calls so that the returned [`ChainHandle`]
-   have a unique type.
-
-   For example, the following test should fail to compile:
-
-   ```rust,compile_fail
-   # use ibc_test_framework::bootstrap::binary::chain::spawn_chain_handle;
-   fn same<T>(_: T, _: T) {}
-
-   let chain_a = spawn_chain_handle(|| {}, todo!(), todo!()).unwrap();
-   let chain_b = spawn_chain_handle(|| {}, todo!(), todo!()).unwrap();
-   same(chain_a, chain_b); // error: chain_a and chain_b have different types
-   ```
-
-   The reason is that Rust would give each closure expression `||{}` a
-   [unique anonymous type](https://doc.rust-lang.org/reference/types/closure.html).
-   When we instantiate two chains with different closure types,
-   the resulting values would be considered by Rust to have different types.
-
-   With this we can treat `chain_a` and `chain_b` having different types
-   so that we do not accidentally mix them up later in the code.
-*/
-pub fn spawn_chain_handle<Seed>(
-    _: Seed,
-    registry: &SharedRegistry<impl ChainHandle>,
-    node: &FullNode,
-) -> Result<impl ChainHandle, Error> {
-    let chain_id = &node.chain_driver.chain_id;
-    let handle = registry.get_or_spawn(chain_id)?;
-
-    add_keys_to_chain_handle(&handle, &node.wallets)?;
-
-    Ok(handle)
-}
-
-/**
-   Add a wallet key to a [`ChainHandle`]'s key store.
-
-   Note that if the [`ChainConfig`](ibc_relayer::config::ChainConfig) is
-   configured to use in-memory store only, the added key would not be
-   accessible through external CLI.
-*/
-pub fn add_key_to_chain_handle<Chain: ChainHandle>(
-    chain: &Chain,
-    wallet: &Wallet,
-) -> Result<(), Error> {
-    let res = chain.add_key(wallet.id.0.clone(), wallet.key.clone());
-
-    // Ignore error if chain handle already have the given key
-    match res {
-        Err(e) => match e.detail() {
-            RelayerErrorDetail::KeyBase(e2) => match e2.source {
-                KeyringErrorDetail::KeyAlreadyExist(_) => Ok(()),
-                _ => Err(e.into()),
-            },
-            _ => Err(e.into()),
-        },
-        Ok(()) => Ok(()),
-    }
-}
-
-/**
-   Add multiple wallets provided in [`TestWallets`] into the
-   [`ChainHandle`]'s key store.
-*/
-pub fn add_keys_to_chain_handle<Chain: ChainHandle>(
-    chain: &Chain,
-    wallets: &TestWallets,
-) -> Result<(), Error> {
-    add_key_to_chain_handle(chain, &wallets.relayer)?;
-    add_key_to_chain_handle(chain, &wallets.user1)?;
-    add_key_to_chain_handle(chain, &wallets.user2)?;
-
-    Ok(())
-}
-
-/**
-   Create a new [`SharedRegistry`] that uses [`CountingAndCachingChainHandle`]
-   as the [`ChainHandle`] implementation.
-*/
-pub fn new_registry(config: SharedConfig) -> SharedRegistry<CountingAndCachingChainHandle> {
-    <SharedRegistry<CountingAndCachingChainHandle>>::new(config)
-}
-
-/**
-   Generate [`ChainConfig`](ibc_relayer::config::ChainConfig) from a running
-   [`FullNode`] and add it to the relayer's [`Config`].
-*/
-pub fn add_chain_config(config: &mut Config, running_node: &FullNode) -> Result<(), Error> {
-    let chain_config = running_node.generate_chain_config()?;
-
-    config.chains.push(chain_config);
-    Ok(())
-}
-
-/**
-   Save a relayer's [`Config`] to the filesystem to make it accessible
-   through external CLI.
-
-   Note that the saved config file will not be updated if the
-   [`SharedConfig`] is reloaded within the test. So test authors that
-   test on the config reloading functionality of the relayer would have to
-   call this function again to save the updated relayer config to the
-   filesystem.
-*/
-pub fn save_relayer_config(config: &Config, config_path: &Path) -> Result<(), Error> {
-    let config_str = toml::to_string_pretty(&config)?;
-
-    fs::write(&config_path, &config_str)?;
-
-    info!(
-        "written hermes config.toml to {}:\n{}",
-        config_path.display(),
-        config_str
-    );
-
-    Ok(())
-}

tools/test-framework/src/bootstrap/nary/chain.rs

@@ -10,12 +10,12 @@ use ibc_relayer::registry::SharedRegistry;
 use std::sync::Arc;
 use std::sync::RwLock;
 
-use crate::bootstrap::binary::chain::{
-    add_chain_config, add_keys_to_chain_handle, bootstrap_foreign_client, new_registry,
-    save_relayer_config,
-};
+use crate::bootstrap::binary::chain::bootstrap_foreign_client;
 use crate::error::{handle_generic_error, Error};
 use crate::relayer::driver::RelayerDriver;
+use crate::relayer::spawn::{
+    add_chain_config, add_keys_to_chain_handle, new_registry, save_relayer_config,
+};
 use crate::types::config::TestConfig;
 use crate::types::nary::chains::{DynamicConnectedChains, NaryConnectedChains};
 use crate::types::single::node::FullNode;

tools/test-framework/src/bootstrap/single.rs

@@ -50,7 +50,8 @@ pub fn bootstrap_single_node(
     chain_driver.update_genesis_file("genesis.json", genesis_modifier)?;
 
     let validator = chain_driver.add_random_wallet("validator")?;
-    let relayer = chain_driver.add_random_wallet("relayer")?;
+    let relayer1 = chain_driver.add_random_wallet("relayer1")?;
+    let relayer2 = chain_driver.add_random_wallet("relayer2")?;
     let user1 = chain_driver.add_random_wallet("user1")?;
     let user2 = chain_driver.add_random_wallet("user2")?;
 
@@ -69,7 +70,12 @@ pub fn bootstrap_single_node(
     )?;
 
     chain_driver.add_genesis_account(
-        &relayer.address,
+        &relayer1.address,
+        &[(&stake_denom, initial_amount), (&denom, initial_amount)],
+    )?;
+
+    chain_driver.add_genesis_account(
+        &relayer2.address,
         &[(&stake_denom, initial_amount), (&denom, initial_amount)],
     )?;
 
@@ -91,7 +97,7 @@ pub fn bootstrap_single_node(
 
     let process = chain_driver.start()?;
 
-    chain_driver.assert_eventual_wallet_amount(&relayer.address, initial_amount, &denom)?;
+    chain_driver.assert_eventual_wallet_amount(&relayer1.address, initial_amount, &denom)?;
 
     info!(
         "started new chain {} at with home path {} and RPC address {}.",
@@ -114,7 +120,8 @@ pub fn bootstrap_single_node(
 
     let wallets = TestWallets {
         validator,
-        relayer,
+        relayer1,
+        relayer2,
         user1,
         user2,
     };