Skip to content

Commit

Permalink
Fix interchain-security test bootstrap flakiness and add helper metho…
Browse files Browse the repository at this point in the history
…ds to overwrite configs (#3518)
  • Loading branch information
ljoss17 authored Aug 2, 2023
1 parent 44e8bdb commit dfae384
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ 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::config::set_voting_period;
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::util::interchain_security::{
update_genesis_for_consumer_chain, update_relayer_config_for_consumer_chain,
};

#[test]
fn test_ics_ica_transfer() -> Result<(), Error> {
Expand Down Expand Up @@ -46,14 +48,8 @@ impl TestOverrides for InterchainSecurityIcaTransferTest {
return Err(Error::generic(eyre!("failed to update genesis file")));
}

// Consumer chain doesn't have a gov key.
if genesis
.get_mut("app_state")
.and_then(|app_state| app_state.get("gov"))
.is_some()
{
set_voting_period(genesis, "10s")?;
}
update_genesis_for_consumer_chain(genesis)?;

Ok(())
}

Expand All @@ -64,12 +60,7 @@ impl TestOverrides for InterchainSecurityIcaTransferTest {
fn modify_relayer_config(&self, config: &mut Config) {
config.mode.channels.enabled = true;

for chain_config in config.chains.iter_mut() {
if chain_config.id == ChainId::from_string("ibcconsumer") {
chain_config.ccv_consumer_chain = true;
chain_config.trusting_period = Some(Duration::from_secs(99));
}
}
update_relayer_config_for_consumer_chain(config);
}
}

Expand Down
24 changes: 7 additions & 17 deletions tools/integration-test/src/tests/interchain_security/icq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
use ibc_relayer::config::{self, ModeConfig};

use ibc_test_framework::prelude::*;
use ibc_test_framework::util::interchain_security::{
update_genesis_for_consumer_chain, update_relayer_config_for_consumer_chain,
};
use ibc_test_framework::util::random::random_u128_range;
use ibc_test_framework::{
chain::{
Expand Down Expand Up @@ -61,13 +64,8 @@ impl TestOverrides for InterchainSecurityIcqTest {
set_crisis_denom(genesis, "stake")?;
}

if genesis
.get_mut("app_state")
.and_then(|app_state| app_state.get("gov"))
.is_some()
{
set_voting_period(genesis, "10s")?;
}
update_genesis_for_consumer_chain(genesis)?;

Ok(())
}

Expand All @@ -79,16 +77,8 @@ impl TestOverrides for InterchainSecurityIcqTest {
channels: config::Channels { enabled: true },
..Default::default()
};
// The `ccv_consumer_chain` must be `true` for the Consumer chain.
// The `trusting_period` must be strictly smaller than the `unbonding_period`
// specified in the Consumer chain proposal. The test framework uses 100s in
// the proposal.
for chain_config in config.chains.iter_mut() {
if chain_config.id == ChainId::from_string("ibcconsumer") {
chain_config.ccv_consumer_chain = true;
chain_config.trusting_period = Some(Duration::from_secs(99));
}
}

update_relayer_config_for_consumer_chain(config);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +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 ibc_test_framework::chain::config::set_voting_period;
use ibc_test_framework::framework::binary::channel::run_binary_interchain_security_channel_test;
use ibc_test_framework::prelude::*;
use ibc_test_framework::util::interchain_security::{
update_genesis_for_consumer_chain, update_relayer_config_for_consumer_chain,
};
use ibc_test_framework::util::random::random_u128_range;

#[test]
Expand All @@ -15,28 +17,15 @@ struct InterchainSecurityTransferTest;

impl TestOverrides for InterchainSecurityTransferTest {
fn modify_genesis_file(&self, genesis: &mut serde_json::Value) -> Result<(), Error> {
// Consumer chain doesn't have a gov key.
if genesis
.get_mut("app_state")
.and_then(|app_state| app_state.get("gov"))
.is_some()
{
set_voting_period(genesis, "10s")?;
}
Ok(())
update_genesis_for_consumer_chain(genesis)
}

// The `ccv_consumer_chain` must be `true` for the Consumer chain.
// The `trusting_period` must be strictly smaller than the `unbonding_period`
// specified in the Consumer chain proposal. The test framework uses 100s in
// the proposal.
fn modify_relayer_config(&self, config: &mut Config) {
for chain_config in config.chains.iter_mut() {
if chain_config.id == ChainId::from_string("ibcconsumer") {
chain_config.ccv_consumer_chain = true;
chain_config.trusting_period = Some(Duration::from_secs(99));
}
}
update_relayer_config_for_consumer_chain(config);
}
}

Expand Down
2 changes: 1 addition & 1 deletion tools/test-framework/src/bootstrap/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub fn bootstrap_consumer_node(
chain_driver.add_genesis_account(&user2.address, &[&initial_stake, &initial_coin])?;

// Wait for the consumer chain to be initialized before querying the genesis
thread::sleep(Duration::from_secs(10));
thread::sleep(Duration::from_secs(15));

node_a
.chain_driver
Expand Down
27 changes: 27 additions & 0 deletions tools/test-framework/src/util/interchain_security.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
use crate::chain::config::set_voting_period;
use crate::prelude::*;

pub fn update_genesis_for_consumer_chain(genesis: &mut serde_json::Value) -> Result<(), Error> {
// Consumer chain doesn't have a gov key.
if genesis
.get_mut("app_state")
.and_then(|app_state| app_state.get("gov"))
.is_some()
{
set_voting_period(genesis, "10s")?;
}
Ok(())
}

pub fn update_relayer_config_for_consumer_chain(config: &mut Config) {
// The `ccv_consumer_chain` must be `true` for the Consumer chain.
// The `trusting_period` must be strictly smaller than the `unbonding_period`
// specified in the Consumer chain proposal. The test framework uses 100s in
// the proposal.
for chain_config in config.chains.iter_mut() {
if chain_config.id == ChainId::from_string("ibcconsumer") {
chain_config.ccv_consumer_chain = true;
chain_config.trusting_period = Some(Duration::from_secs(99));
}
}
}
1 change: 1 addition & 0 deletions tools/test-framework/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
pub mod array;
pub mod assert;
pub mod file;
pub mod interchain_security;
pub mod random;
pub mod retry;
pub mod suspend;

0 comments on commit dfae384

Please sign in to comment.