Skip to content

Commit

Permalink
Fix issue with nested calls to block_on (#3789)
Browse files Browse the repository at this point in the history
  • Loading branch information
romac authored Jan 17, 2024
1 parent 010d5c1 commit 752ab4d
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 57 deletions.
10 changes: 9 additions & 1 deletion crates/relayer/src/chain/cosmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ use cosmwasm_std::{Decimal, Uint128};
use osmosis_std::types::cosmos::base::v1beta1::DecProto;

use self::types::app_state::GenesisAppState;
use self::{gas::dynamic_gas_price, types::gas::GasConfig};

pub mod batch;
pub mod client;
Expand Down Expand Up @@ -490,6 +491,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<Duration, Error> {
crate::time!(
Expand Down Expand Up @@ -2384,7 +2392,7 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
);
}

let relayer_gas_price = &chain.config.dynamic_gas_price();
let relayer_gas_price = &chain.dynamic_gas_price();
let node_min_gas_prices = chain.min_gas_price()?;

if !node_min_gas_prices.is_empty() {
Expand Down
52 changes: 32 additions & 20 deletions crates/relayer/src/chain/cosmos/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -238,7 +238,7 @@ fn response_to_tx_sync_result(
}
}

fn batch_messages(
async fn batch_messages(
config: &TxConfig,
key_pair: &Secp256k1KeyPair,
account: &Account,
Expand All @@ -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;
Expand Down Expand Up @@ -364,14 +366,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],
Expand Down Expand Up @@ -408,6 +411,7 @@ mod tests {
&memo,
messages.clone(),
)
.await
.unwrap();

assert_eq!(batches.len(), 2);
Expand All @@ -424,8 +428,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();
Expand All @@ -446,6 +450,7 @@ mod tests {
&memo,
messages.clone(),
)
.await
.unwrap();

assert_eq!(batches.len(), 1);
Expand All @@ -455,20 +460,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
Expand Down Expand Up @@ -507,6 +513,7 @@ mod tests {
&Memo::new("").unwrap(),
messages.clone(),
)
.await
.unwrap();

assert_eq!(batches.len(), 5);
Expand All @@ -525,14 +532,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();
Expand Down Expand Up @@ -573,6 +581,7 @@ mod tests {
&memo,
messages.clone(),
)
.await
.unwrap();

assert_eq!(batches.len(), 5);
Expand All @@ -581,7 +590,8 @@ mod tests {
&config.gas_config,
config.gas_config.max_gas,
&config.rpc_address,
);
)
.await;

for batch in batches {
assert_eq!(batch.len(), 1);
Expand All @@ -601,15 +611,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(
Expand All @@ -618,6 +629,7 @@ mod tests {
&account,
&Memo::new("").unwrap(),
vec![],
);
)
.await;
}
}
2 changes: 1 addition & 1 deletion crates/relayer/src/chain/cosmos/estimate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 13 additions & 13 deletions crates/relayer/src/chain/cosmos/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,19 @@ 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,
gas_amount,
});

// 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 {
Expand All @@ -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()
}
Expand Down
4 changes: 2 additions & 2 deletions crates/relayer/src/chain/cosmos/types/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl<'a> From<&'a ChainConfig> for GasConfig {
default_gas: default_gas_from_config(config),
max_gas: max_gas_from_config(config),
gas_multiplier: gas_multiplier_from_config(config),
gas_price: config.dynamic_gas_price(),
gas_price: config.gas_price.clone(),
max_fee: max_fee_from_config(config),
fee_granter: fee_granter_from_config(config),
dynamic_gas_price_multiplier: config.dynamic_gas.dynamic_gas_price(),
Expand Down Expand Up @@ -64,7 +64,7 @@ fn max_fee_from_config(config: &ChainConfig) -> Fee {
let max_gas = max_gas_from_config(config);

// The maximum fee the relayer pays for a transaction
let max_fee_in_coins = calculate_fee(max_gas, &config.dynamic_gas_price());
let max_fee_in_coins = calculate_fee(max_gas, &config.gas_price);

let fee_granter = fee_granter_from_config(config);

Expand Down
18 changes: 0 additions & 18 deletions crates/relayer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ use crate::keyring::Store;
pub use crate::config::Error as ConfigError;
pub use error::Error;

use crate::chain::cosmos::query_eip_base_fee;
use crate::util::block_on;
pub use filter::PacketFilter;

use crate::config::dynamic_gas::DynamicGas;
Expand Down Expand Up @@ -677,22 +675,6 @@ pub struct ChainConfig {
pub clear_interval: Option<u64>,
}

impl ChainConfig {
pub fn dynamic_gas_price(&self) -> GasPrice {
if let Some(dynamic_gas_price) = self.dynamic_gas.dynamic_gas_price() {
let new_price = block_on(query_eip_base_fee(&self.rpc_addr.to_string())).unwrap()
* dynamic_gas_price;

GasPrice {
price: new_price,
denom: self.gas_price.denom.clone(),
}
} else {
self.gas_price.clone()
}
}
}

/// Attempt to load and parse the TOML config file as a `Config`.
pub fn load(path: impl AsRef<Path>) -> Result<Config, Error> {
let config_toml = std::fs::read_to_string(&path).map_err(Error::io)?;
Expand Down
1 change: 0 additions & 1 deletion crates/relayer/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![forbid(unsafe_code)]
#![deny(
warnings,
trivial_casts,
trivial_numeric_casts,
unused_import_braces,
Expand Down
3 changes: 2 additions & 1 deletion tools/integration-test/src/tests/dynamic_gas_fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ impl BinaryChannelTest for DynamicGasTest {
) -> Result<(), Error> {
let fee_denom_a = MonoTagged::new(Denom::base(&config.native_tokens[0]));

let driver_a = chains.node_a.chain_driver().value().clone();
let driver_a = chains.node_a.chain_driver();
let driver_a = driver_a.value();

// Create a separate chain driver to try and avoid the block_on panic.
// And enable dynamic gas
Expand Down

0 comments on commit 752ab4d

Please sign in to comment.