Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dynamic gas price: Fix issue with nested calls to block_on #3789

Merged
merged 4 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading