From 13df419dd57fa4d87fff0c54225eb337e9b5da5b Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 18 Jan 2024 12:49:23 +0100 Subject: [PATCH] dynamic gas price: Add integration test (#3788) * Dynamic gas fee test * Add dynamic gas fee test to CI * Fix issue with nested calls to `block_on` (#3789) * Fix test for dynamic gas price --------- Co-authored-by: Luca Joss --- .github/workflows/integration.yaml | 2 +- crates/relayer/src/chain/cosmos.rs | 8 + crates/relayer/src/chain/cosmos/batch.rs | 52 +++-- crates/relayer/src/chain/cosmos/estimate.rs | 2 +- crates/relayer/src/chain/cosmos/gas.rs | 26 +-- crates/relayer/src/chain/cosmos/types/gas.rs | 2 +- crates/relayer/src/lib.rs | 1 - tools/integration-test/Cargo.toml | 1 + .../src/tests/dynamic_gas_fee.rs | 213 ++++++++++++++++++ tools/integration-test/src/tests/mod.rs | 3 + 10 files changed, 273 insertions(+), 37 deletions(-) create mode 100644 tools/integration-test/src/tests/dynamic_gas_fee.rs diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 78ef604845..bc9f4a5427 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -79,7 +79,7 @@ jobs: command: osmosisd account_prefix: osmo native_token: stake - features: '' + features: dynamic-gas-fee - package: juno command: junod account_prefix: juno diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index b389618c2d..cbf014ba3a 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -109,6 +109,7 @@ use crate::{chain::client::ClientSettings, config::Error as ConfigError}; use self::types::app_state::GenesisAppState; use self::version::Specs; +use self::{gas::dynamic_gas_price, types::gas::GasConfig}; pub mod batch; pub mod client; @@ -492,6 +493,13 @@ impl CosmosSdkChain { Ok(min_gas_price) } + pub fn dynamic_gas_price(&self) -> GasPrice { + let gas_config = GasConfig::from(self.config()); + + self.rt + .block_on(dynamic_gas_price(&gas_config, &self.config.rpc_addr)) + } + /// The unbonding period of this chain pub fn unbonding_period(&self) -> Result { crate::time!( diff --git a/crates/relayer/src/chain/cosmos/batch.rs b/crates/relayer/src/chain/cosmos/batch.rs index d1291c14d3..9e39da9679 100644 --- a/crates/relayer/src/chain/cosmos/batch.rs +++ b/crates/relayer/src/chain/cosmos/batch.rs @@ -102,7 +102,7 @@ pub async fn send_batched_messages_and_wait_check_tx( return Ok(Vec::new()); } - let batches = batch_messages(config, key_pair, account, tx_memo, messages)?; + let batches = batch_messages(config, key_pair, account, tx_memo, messages).await?; let mut responses = Vec::new(); @@ -132,7 +132,7 @@ async fn send_messages_as_batches( let message_count = messages.len(); - let batches = batch_messages(config, key_pair, account, tx_memo, messages)?; + let batches = batch_messages(config, key_pair, account, tx_memo, messages).await?; debug!( "sending {} messages as {} batches to chain {} in parallel", @@ -173,7 +173,7 @@ async fn sequential_send_messages_as_batches( let message_count = messages.len(); - let batches = batch_messages(config, key_pair, account, tx_memo, messages)?; + let batches = batch_messages(config, key_pair, account, tx_memo, messages).await?; debug!( "sending {} messages as {} batches to chain {} in serial", @@ -238,7 +238,7 @@ fn response_to_tx_sync_result( } } -fn batch_messages( +async fn batch_messages( config: &TxConfig, key_pair: &Secp256k1KeyPair, account: &Account, @@ -257,7 +257,9 @@ fn batch_messages( &config.gas_config, config.gas_config.max_gas, &config.rpc_address, - ); + ) + .await; + let tx_metrics = encoded_tx_metrics(config, key_pair, account, tx_memo, &[], &max_fee)?; let tx_envelope_len = tx_metrics.envelope_len; let empty_body_len = tx_metrics.body_bytes_len; @@ -368,14 +370,15 @@ mod tests { (tx_config, key_pair, account) } - #[test] - fn batch_does_not_exceed_max_tx_size() { + #[tokio::test] + async fn batch_does_not_exceed_max_tx_size() { let (config, key_pair, account) = test_fixture(); let max_fee = gas_amount_to_fee( &config.gas_config, config.gas_config.max_gas, &config.rpc_address, - ); + ) + .await; let mut messages = vec![Any { type_url: "/example.Baz".into(), value: vec![0; 2], @@ -412,6 +415,7 @@ mod tests { &memo, messages.clone(), ) + .await .unwrap(); assert_eq!(batches.len(), 2); @@ -428,8 +432,8 @@ mod tests { } } - #[test] - fn batch_error_on_oversized_message() { + #[tokio::test] + async fn batch_error_on_oversized_message() { const MAX_TX_SIZE: usize = 203; let (config, key_pair, account) = test_fixture(); @@ -450,6 +454,7 @@ mod tests { &memo, messages.clone(), ) + .await .unwrap(); assert_eq!(batches.len(), 1); @@ -459,20 +464,21 @@ mod tests { &config.gas_config, config.gas_config.max_gas, &config.rpc_address, - ); + ) + .await; let tx_bytes = sign_and_encode_tx(&config, &key_pair, &account, &memo, &batches[0], &max_fee).unwrap(); assert_eq!(tx_bytes.len(), MAX_TX_SIZE); limited_config.max_tx_size = MaxTxSize::new(MAX_TX_SIZE - 1).unwrap(); - let res = batch_messages(&limited_config, &key_pair, &account, &memo, messages); + let res = batch_messages(&limited_config, &key_pair, &account, &memo, messages).await; assert!(res.is_err()); } - #[test] - fn test_batches_are_structured_appropriately_per_max_msg_num() { + #[tokio::test] + async fn test_batches_are_structured_appropriately_per_max_msg_num() { let (config, key_pair, account) = test_fixture(); // Ensure that when MaxMsgNum is 1, the resulting batch @@ -511,6 +517,7 @@ mod tests { &Memo::new("").unwrap(), messages.clone(), ) + .await .unwrap(); assert_eq!(batches.len(), 5); @@ -529,14 +536,15 @@ mod tests { &Memo::new("").unwrap(), messages, ) + .await .unwrap(); assert_eq!(batches.len(), 1); assert_eq!(batches[0].len(), 5); } - #[test] - fn test_batches_are_structured_appropriately_per_max_tx_size() { + #[tokio::test] + async fn test_batches_are_structured_appropriately_per_max_tx_size() { const MAX_TX_SIZE: usize = 198; let (config, key_pair, account) = test_fixture(); @@ -577,6 +585,7 @@ mod tests { &memo, messages.clone(), ) + .await .unwrap(); assert_eq!(batches.len(), 5); @@ -585,7 +594,8 @@ mod tests { &config.gas_config, config.gas_config.max_gas, &config.rpc_address, - ); + ) + .await; for batch in batches { assert_eq!(batch.len(), 1); @@ -605,15 +615,16 @@ mod tests { &Memo::new("").unwrap(), messages, ) + .await .unwrap(); assert_eq!(batches.len(), 1); assert_eq!(batches[0].len(), 5); } - #[test] + #[tokio::test] #[should_panic(expected = "`max_msg_num` must be greater than or equal to 1, found 0")] - fn test_max_msg_num_of_zero_panics() { + async fn test_max_msg_num_of_zero_panics() { let (mut config, key_pair, account) = test_fixture(); config.max_msg_num = MaxMsgNum::new(0).unwrap(); let _batches = batch_messages( @@ -622,6 +633,7 @@ mod tests { &account, &Memo::new("").unwrap(), vec![], - ); + ) + .await; } } diff --git a/crates/relayer/src/chain/cosmos/estimate.rs b/crates/relayer/src/chain/cosmos/estimate.rs index 0c28a7d658..c8717630be 100644 --- a/crates/relayer/src/chain/cosmos/estimate.rs +++ b/crates/relayer/src/chain/cosmos/estimate.rs @@ -88,7 +88,7 @@ async fn estimate_fee_with_tx( )); } - let adjusted_fee = gas_amount_to_fee(gas_config, estimated_gas, rpc_address); + let adjusted_fee = gas_amount_to_fee(gas_config, estimated_gas, rpc_address).await; debug!( id = %chain_id, diff --git a/crates/relayer/src/chain/cosmos/gas.rs b/crates/relayer/src/chain/cosmos/gas.rs index 2bcc20f4e9..7c5a5a2872 100644 --- a/crates/relayer/src/chain/cosmos/gas.rs +++ b/crates/relayer/src/chain/cosmos/gas.rs @@ -5,13 +5,11 @@ use num_bigint::BigInt; use num_rational::BigRational; use tendermint_rpc::Url; +use super::query_eip_base_fee; use crate::chain::cosmos::types::gas::GasConfig; use crate::config::GasPrice; -use crate::util::block_on; - -use super::query_eip_base_fee; -pub fn gas_amount_to_fee(config: &GasConfig, gas_amount: u64, rpc_address: &Url) -> Fee { +pub async fn gas_amount_to_fee(config: &GasConfig, gas_amount: u64, rpc_address: &Url) -> Fee { let adjusted_gas_limit = adjust_estimated_gas(AdjustGas { gas_multiplier: config.gas_multiplier, max_gas: config.max_gas, @@ -19,7 +17,7 @@ pub fn gas_amount_to_fee(config: &GasConfig, gas_amount: u64, rpc_address: &Url) }); // The fee in coins based on gas amount - let dynamic_gas_price = dynamic_gas_price(config, rpc_address); + let dynamic_gas_price = dynamic_gas_price(config, rpc_address).await; let amount = calculate_fee(adjusted_gas_limit, &dynamic_gas_price); Fee { @@ -29,15 +27,17 @@ pub fn gas_amount_to_fee(config: &GasConfig, gas_amount: u64, rpc_address: &Url) granter: config.fee_granter.clone(), } } -pub fn dynamic_gas_price(config: &GasConfig, rpc_address: &Url) -> GasPrice { - if let Some(dynamic_gas_price_multiplier) = config.dynamic_gas_price_multiplier { - let new_price = block_on(query_eip_base_fee(&rpc_address.to_string())).unwrap() - * dynamic_gas_price_multiplier; - GasPrice { - price: new_price, - denom: config.gas_price.denom.clone(), - } +pub async fn dynamic_gas_price(config: &GasConfig, rpc_address: &Url) -> GasPrice { + if let Some(dynamic_gas_price_multiplier) = config.dynamic_gas_price_multiplier { + query_eip_base_fee(&rpc_address.to_string()) + .await + .map(|base_fee| base_fee * dynamic_gas_price_multiplier) + .map(|new_price| GasPrice { + price: new_price, + denom: config.gas_price.denom.clone(), + }) + .unwrap_or_else(|_| config.gas_price.clone()) } else { config.gas_price.clone() } diff --git a/crates/relayer/src/chain/cosmos/types/gas.rs b/crates/relayer/src/chain/cosmos/types/gas.rs index c721f38d6b..10d0aa19b0 100644 --- a/crates/relayer/src/chain/cosmos/types/gas.rs +++ b/crates/relayer/src/chain/cosmos/types/gas.rs @@ -29,7 +29,7 @@ impl<'a> From<&'a CosmosSdkConfig> for GasConfig { gas_price: config.gas_price.clone(), max_fee: max_fee_from_config(config), fee_granter: fee_granter_from_config(config), - dynamic_gas_price_multiplier: None, + dynamic_gas_price_multiplier: config.dynamic_gas.dynamic_gas_price(), } } } diff --git a/crates/relayer/src/lib.rs b/crates/relayer/src/lib.rs index 0bca2140c0..16744ad6aa 100644 --- a/crates/relayer/src/lib.rs +++ b/crates/relayer/src/lib.rs @@ -1,6 +1,5 @@ #![forbid(unsafe_code)] #![deny( - warnings, trivial_casts, trivial_numeric_casts, unused_import_braces, diff --git a/tools/integration-test/Cargo.toml b/tools/integration-test/Cargo.toml index 3b2327aff1..640b0871a9 100644 --- a/tools/integration-test/Cargo.toml +++ b/tools/integration-test/Cargo.toml @@ -42,6 +42,7 @@ interchain-security = [] celestia = [] async-icq = [] juno = [] +dynamic-gas-fee = [] [[bin]] name = "test_setup_with_binary_channel" diff --git a/tools/integration-test/src/tests/dynamic_gas_fee.rs b/tools/integration-test/src/tests/dynamic_gas_fee.rs new file mode 100644 index 0000000000..378ed8ba3c --- /dev/null +++ b/tools/integration-test/src/tests/dynamic_gas_fee.rs @@ -0,0 +1,213 @@ +//! The [`DynamicGasTest`] test ensures that the [`DynamicGas`] +//! configuration works correctly. The test can enable or disable the dynamic +//! gas price for the second chain. +//! +//! To test dynamic gas configuration, it will enable dynamic gas price on the +//! second chain only. It will then create and relay first IBC transfer with a +//! big memo. The gas fee paid is then recorded. +//! A second IBC transfer without memo will then be relayed. The gas fee paid +//! will also be recorded. The test will assert that the Tx with a big memo +//! and dynamic gas enabled is lower than the Tx without memo and dynamic gas +//! disabled. +//! +//! The [`DynamicGasTest`] test can be configured to disable the dynamic gas +//! price on both chains in order to ensure that the first IBC transfer will +//! cost more if dynamic gas is disabled. + +use ibc_relayer::config::dynamic_gas::DynamicGas; +use ibc_relayer::config::ChainConfig; +use ibc_relayer::config::GasPrice; +use ibc_test_framework::prelude::*; + +#[test] +fn test_dynamic_gas_transfer() -> Result<(), Error> { + run_binary_channel_test(&DynamicGasTest { + dynamic_gas_enabled: true, + }) +} + +#[test] +fn test_static_gas_transfer() -> Result<(), Error> { + run_binary_channel_test(&DynamicGasTest { + dynamic_gas_enabled: false, + }) +} + +const MEMO_CHAR: &str = "a"; +const MEMO_SIZE: usize = 10000; + +pub struct DynamicGasTest { + dynamic_gas_enabled: bool, +} + +impl TestOverrides for DynamicGasTest { + fn modify_relayer_config(&self, config: &mut Config) { + config.mode.clients.misbehaviour = false; + config.mode.clients.refresh = false; + config.mode.packets.clear_interval = 0; + + match &mut config.chains[0] { + ChainConfig::CosmosSdk(chain_config_a) => { + chain_config_a.gas_price = + GasPrice::new(0.1, chain_config_a.gas_price.denom.clone()); + chain_config_a.dynamic_gas = DynamicGas::unsafe_new(false, 1.1); + } + } + + match &mut config.chains[1] { + ChainConfig::CosmosSdk(chain_config_b) => { + chain_config_b.gas_price = + GasPrice::new(0.1, chain_config_b.gas_price.denom.clone()); + chain_config_b.dynamic_gas = DynamicGas::unsafe_new(self.dynamic_gas_enabled, 1.1); + } + } + } + + fn should_spawn_supervisor(&self) -> bool { + false + } +} + +impl BinaryChannelTest for DynamicGasTest { + fn run( + &self, + _config: &TestConfig, + relayer: RelayerDriver, + chains: ConnectedChains, + channel: ConnectedChannel, + ) -> Result<(), Error> { + let denom_a = chains.node_a.denom(); + let wallet_a = chains.node_a.wallets().user1().cloned(); + let wallet_b = chains.node_b.wallets().user1().cloned(); + + let a_to_b_amount = 12345u64; + + let denom_a_to_b = derive_ibc_denom( + &channel.port_b.as_ref(), + &channel.channel_id_b.as_ref(), + &denom_a, + )?; + + let gas_denom_a: MonoTagged = + MonoTagged::new(Denom::Base("stake".to_owned())); + let gas_denom_b: MonoTagged = + MonoTagged::new(Denom::Base("stake".to_owned())); + + let balance_relayer_b_before = chains.node_b.chain_driver().query_balance( + &chains.node_b.wallets().relayer().address(), + &gas_denom_b.as_ref(), + )?; + + let memo: String = MEMO_CHAR.repeat(MEMO_SIZE); + + chains + .node_a + .chain_driver() + .ibc_transfer_token_with_memo_and_timeout( + &channel.port_a.as_ref(), + &channel.channel_id_a.as_ref(), + &wallet_a.as_ref(), + &wallet_b.address(), + &denom_a.with_amount(a_to_b_amount).as_ref(), + Some(memo), + None, + )?; + + // Do a simple IBC transfer with the dynamic gas configuration + let tx1_paid_gas_relayer = relayer.with_supervisor(|| { + // Assert that user on chain B received the tokens + chains.node_b.chain_driver().assert_eventual_wallet_amount( + &wallet_b.address(), + &denom_a_to_b.with_amount(a_to_b_amount).as_ref(), + )?; + + // Wait for a bit before querying the new balance + sleep(Duration::from_secs(5)); + + let balance_relayer_b_after = chains.node_b.chain_driver().query_balance( + &chains.node_b.wallets().relayer().address(), + &gas_denom_b.as_ref(), + )?; + + let paid_fees_relayer_b = balance_relayer_b_before + .amount() + .checked_sub(balance_relayer_b_after.amount()); + + assert!( + paid_fees_relayer_b.is_some(), + "subtraction between queried amounts for relayer should be Some" + ); + + info!("IBC transfer with memo was successful"); + + Ok(paid_fees_relayer_b.unwrap()) + })?; + + let b_to_a_amount = 23456u64; + let denom_b = chains.node_b.denom(); + + let denom_b_to_a = derive_ibc_denom( + &channel.port_a.as_ref(), + &channel.channel_id_a.as_ref(), + &denom_b, + )?; + + let balance_relayer_a_before = chains.node_a.chain_driver().query_balance( + &chains.node_a.wallets().relayer().address(), + &gas_denom_a.as_ref(), + )?; + + chains.node_b.chain_driver().ibc_transfer_token( + &channel.port_b.as_ref(), + &channel.channel_id_b.as_ref(), + &chains.node_b.wallets().user1(), + &chains.node_a.wallets().user1().address(), + &denom_b.with_amount(b_to_a_amount).as_ref(), + )?; + + let tx2_paid_gas_relayer = relayer.with_supervisor(|| { + // Assert that user on chain B received the tokens + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &chains.node_a.wallets().user1().address(), + &denom_b_to_a.with_amount(b_to_a_amount).as_ref(), + )?; + + // Wait for a bit before querying the new balance + sleep(Duration::from_secs(5)); + + let balance_relayer_a_after = chains.node_a.chain_driver().query_balance( + &chains.node_a.wallets().relayer().address(), + &gas_denom_a.as_ref(), + )?; + + let paid_fees_relayer_a = balance_relayer_a_before + .amount() + .checked_sub(balance_relayer_a_after.amount()); + + assert!( + paid_fees_relayer_a.is_some(), + "subtraction between queried amounts for relayer should be Some" + ); + + info!("IBC transfer without memo was successful"); + + Ok(paid_fees_relayer_a.unwrap()) + })?; + + info!("paid gas fees for Tx with memo `{tx1_paid_gas_relayer}`, without memo `{tx2_paid_gas_relayer}`"); + + if self.dynamic_gas_enabled { + assert!( + tx1_paid_gas_relayer < tx2_paid_gas_relayer, + "with dynamic gas enabled, gas paid for the first TX should be lower" + ); + } else { + assert!( + tx1_paid_gas_relayer > tx2_paid_gas_relayer, + "with dynamic gas disabled, gas paid for the second TX should be lower" + ); + } + + Ok(()) + } +} diff --git a/tools/integration-test/src/tests/mod.rs b/tools/integration-test/src/tests/mod.rs index b56c22b49d..373cf4d276 100644 --- a/tools/integration-test/src/tests/mod.rs +++ b/tools/integration-test/src/tests/mod.rs @@ -63,3 +63,6 @@ pub mod fee_grant; #[cfg(any(doc, feature = "interchain-security"))] pub mod interchain_security; + +#[cfg(any(doc, feature = "dynamic-gas-fee"))] +pub mod dynamic_gas_fee;