From 71d5f30ed28f8bb7b16dd91f3de4cce27c506852 Mon Sep 17 00:00:00 2001 From: Luca Joss <43531661+ljoss17@users.noreply.github.com> Date: Tue, 11 Jul 2023 11:02:56 +0200 Subject: [PATCH] Use `max_expected_time_per_block` value for the `max_block_time` (#3467) * Use max_block_time queried from /genesis * Clean solution * Add unclog entry * Remove serde-with dependency --- .../ibc-relayer/3211-query-max-block-time.md | 3 ++ config.toml | 5 +- crates/relayer/src/chain/cosmos.rs | 30 ++++++++++-- .../src/chain/cosmos/types/app_state.rs | 47 +++++++++++++++++++ crates/relayer/src/chain/cosmos/types/mod.rs | 1 + crates/relayer/src/chain/endpoint.rs | 2 +- 6 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 .changelog/unreleased/improvements/ibc-relayer/3211-query-max-block-time.md create mode 100644 crates/relayer/src/chain/cosmos/types/app_state.rs diff --git a/.changelog/unreleased/improvements/ibc-relayer/3211-query-max-block-time.md b/.changelog/unreleased/improvements/ibc-relayer/3211-query-max-block-time.md new file mode 100644 index 0000000000..f346256cfc --- /dev/null +++ b/.changelog/unreleased/improvements/ibc-relayer/3211-query-max-block-time.md @@ -0,0 +1,3 @@ +- Query the /genesis RPC endpoint to retrieve the value of + `max_expected_time_per_block` and use it for `max_block_time`. + ([\#3211](https://github.com/informalsystems/hermes/issues/3211)) \ No newline at end of file diff --git a/config.toml b/config.toml index 276a221ccd..f29ef2ceef 100644 --- a/config.toml +++ b/config.toml @@ -285,9 +285,12 @@ max_tx_size = 2097152 # can drift into the future. Default: 5s clock_drift = '5s' -# Specify the maximum time per block for this chain. +# Specify the fallback value for the maximum time per block for this chain. # The block time together with the clock drift are added to the source drift to estimate # the maximum clock drift when creating a client on this chain. Default: 30s +# When validating the configuration, Hermes will query the /genesis RPC endpoint in order +# to retrieve the `max_expected_time_per_block` and use it as the `max_block_time`. +# If the query fails, the configured `max_block_time` is used instead. # For cosmos-SDK chains a good approximation is `timeout_propose` + `timeout_commit` # Note: This MUST be the same as the `max_expected_time_per_block` genesis parameter for Tendermint chains. max_block_time = '30s' diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index 04dabeb01c..f3e5c0f3b9 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -13,7 +13,7 @@ use std::{cmp::Ordering, thread}; use tokio::runtime::Runtime as TokioRuntime; use tonic::codegen::http::Uri; use tonic::metadata::AsciiMetadataValue; -use tracing::{error, instrument, trace, warn}; +use tracing::{error, info, instrument, trace, warn}; use ibc_proto::cosmos::{ base::node::v1beta1::ConfigResponse, staking::v1beta1::Params as StakingParams, @@ -107,6 +107,8 @@ use crate::util::pretty::{ PrettyIdentifiedChannel, PrettyIdentifiedClientState, PrettyIdentifiedConnection, }; +use self::types::app_state::GenesisAppState; + pub mod batch; pub mod client; pub mod compatibility; @@ -190,7 +192,7 @@ impl CosmosSdkChain { /// /// Emits a log warning in case any error is encountered and /// exits early without doing subsequent validations. - pub fn validate_params(&self) -> Result<(), Error> { + pub fn validate_params(&mut self) -> Result<(), Error> { let unbonding_period = self.unbonding_period()?; let trusting_period = self.trusting_period(unbonding_period); @@ -283,6 +285,28 @@ impl CosmosSdkChain { )); } + // Query /genesis RPC endpoint to retrieve the `max_expected_time_per_block` value + // to use as `max_block_time`. + // If it is not found, keep the configured `max_block_time`. + match self.block_on(self.rpc_client.genesis::()) { + Ok(genesis_reponse) => { + let old_max_block_time = self.config.max_block_time; + self.config.max_block_time = + Duration::from_nanos(genesis_reponse.app_state.max_expected_time_per_block()); + info!( + "Updated `max_block_time` using /genesis endpoint. Old value: `{}s`, new value: `{}s`", + old_max_block_time.as_secs(), + self.config.max_block_time.as_secs() + ); + } + Err(e) => { + warn!( + "Will use fallback value for max_block_time: `{}s`. Error: {e}", + self.config.max_block_time.as_secs() + ); + } + } + Ok(()) } @@ -965,7 +989,7 @@ impl ChainEndpoint for CosmosSdkChain { /// Emits a log warning in case anything is amiss. /// Exits early if any health check fails, without doing any /// further checks. - fn health_check(&self) -> Result { + fn health_check(&mut self) -> Result { if let Err(e) = do_health_check(self) { warn!("Health checkup for chain '{}' failed", self.id()); warn!(" Reason: {}", e.detail()); diff --git a/crates/relayer/src/chain/cosmos/types/app_state.rs b/crates/relayer/src/chain/cosmos/types/app_state.rs new file mode 100644 index 0000000000..256a1017a2 --- /dev/null +++ b/crates/relayer/src/chain/cosmos/types/app_state.rs @@ -0,0 +1,47 @@ +//! Structure used to parse queried Genesis state using +//! /genesis RPC endpoint. + +use serde::Deserialize as SerdeDeserialize; +use serde_derive::Deserialize; +use serde_derive::Serialize; + +#[derive(Debug, Deserialize, Serialize)] +pub struct GenesisAppState { + ibc: IbcConfig, +} + +impl GenesisAppState { + pub fn max_expected_time_per_block(&self) -> u64 { + self.ibc + .connection_genesis + .params + .max_expected_time_per_block + } +} + +#[derive(Debug, Deserialize, Serialize)] +struct IbcConfig { + connection_genesis: ConnectionGenesisConfig, +} + +#[derive(Debug, Deserialize, Serialize)] +struct ConnectionGenesisConfig { + params: ConnectionGenesisParams, +} + +#[derive(Debug, Deserialize, Serialize)] +struct ConnectionGenesisParams { + #[serde(deserialize_with = "deserialize_max_expected_per_block")] + max_expected_time_per_block: u64, +} + +fn deserialize_max_expected_per_block<'de, T, D>(de: D) -> Result +where + D: serde::Deserializer<'de>, + T: std::str::FromStr, + ::Err: std::fmt::Display, +{ + String::deserialize(de)? + .parse() + .map_err(serde::de::Error::custom) +} diff --git a/crates/relayer/src/chain/cosmos/types/mod.rs b/crates/relayer/src/chain/cosmos/types/mod.rs index 8877d47c36..9951b58784 100644 --- a/crates/relayer/src/chain/cosmos/types/mod.rs +++ b/crates/relayer/src/chain/cosmos/types/mod.rs @@ -1,4 +1,5 @@ pub mod account; +pub mod app_state; pub mod config; pub mod events; pub mod gas; diff --git a/crates/relayer/src/chain/endpoint.rs b/crates/relayer/src/chain/endpoint.rs index 3752f25a51..5d2f70ab10 100644 --- a/crates/relayer/src/chain/endpoint.rs +++ b/crates/relayer/src/chain/endpoint.rs @@ -98,7 +98,7 @@ pub trait ChainEndpoint: Sized { fn shutdown(self) -> Result<(), Error>; /// Perform a health check - fn health_check(&self) -> Result; + fn health_check(&mut self) -> Result; // Events fn subscribe(&mut self) -> Result;