From 095d1f93813467a2cd3b4201d56cc451eff0f483 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 10 Jul 2023 10:34:24 +0200 Subject: [PATCH 1/4] Use max_block_time queried from /genesis --- Cargo.lock | 67 ++++++++++++++++++++++++++++ crates/relayer/Cargo.toml | 1 + crates/relayer/src/chain/cosmos.rs | 55 +++++++++++++++++++++-- crates/relayer/src/chain/endpoint.rs | 2 +- 4 files changed, 121 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6bfebe8b07..5ba81a428e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -417,6 +417,7 @@ dependencies = [ "iana-time-zone", "js-sys", "num-traits", + "serde", "time 0.1.45", "wasm-bindgen", "winapi", @@ -668,6 +669,41 @@ dependencies = [ "zeroize", ] +[[package]] +name = "darling" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0558d22a7b463ed0241e993f76f09f30b126687447751a8638587b864e4b3944" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab8bfa2e259f8ee1ce5e97824a3c55ec4404a0d772ca7fa96bf19f0752a046eb" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim", + "syn 2.0.23", +] + +[[package]] +name = "darling_macro" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29a358ff9f12ec09c3e61fef9b5a9902623a695a46a917b07f269bff1445611a" +dependencies = [ + "darling_core", + "quote", + "syn 2.0.23", +] + [[package]] name = "dashmap" version = "5.4.0" @@ -1542,6 +1578,7 @@ dependencies = [ "serde", "serde_derive", "serde_json", + "serde_with", "serial_test", "sha2 0.10.6", "signature 1.6.4", @@ -1763,6 +1800,7 @@ checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" dependencies = [ "autocfg", "hashbrown", + "serde", ] [[package]] @@ -3038,6 +3076,34 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_with" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f02d8aa6e3c385bf084924f660ce2a3a6bd333ba55b35e8590b321f35d88513" +dependencies = [ + "base64 0.21.2", + "chrono", + "hex", + "indexmap", + "serde", + "serde_json", + "serde_with_macros", + "time 0.3.22", +] + +[[package]] +name = "serde_with_macros" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "edc7d5d3932fb12ce722ee5e64dd38c504efba37567f0c402f6ca728c3b8b070" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn 2.0.23", +] + [[package]] name = "serde_yaml" version = "0.9.21" @@ -3598,6 +3664,7 @@ version = "0.3.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea9e1b3cf1243ae005d9e74085d4d542f3125458f3a81af210d901dcd7411efd" dependencies = [ + "itoa", "serde", "time-core", "time-macros", diff --git a/crates/relayer/Cargo.toml b/crates/relayer/Cargo.toml index 5bf755ba2e..c19e9f92ed 100644 --- a/crates/relayer/Cargo.toml +++ b/crates/relayer/Cargo.toml @@ -28,6 +28,7 @@ subtle-encoding = "0.5" humantime-serde = "1.1.1" serde = "1.0" serde_derive = "1.0" +serde_with = "3.0.0" thiserror = "1.0.40" toml = "0.7" tracing = "0.1.36" diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index 94bd7bf8dd..5e2c7ae5da 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -8,12 +8,14 @@ use core::{ }; use futures::future::join_all; use num_bigint::BigInt; +use serde_derive::{Deserialize, Serialize}; +use serde_with::{serde_as, DisplayFromStr}; 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, @@ -153,6 +155,28 @@ pub struct CosmosSdkChain { tx_monitor_cmd: Option, } +#[derive(Debug, Deserialize, Serialize)] +struct GenesisAppState { + ibc: IbcConfig, +} + +#[derive(Debug, Deserialize, Serialize)] +struct IbcConfig { + connection_genesis: ConnectionGenesisConfig, +} + +#[derive(Debug, Deserialize, Serialize)] +struct ConnectionGenesisConfig { + params: ConnectionGenesisParams, +} + +#[serde_as] +#[derive(Debug, Deserialize, Serialize)] +struct ConnectionGenesisParams { + #[serde_as(as = "DisplayFromStr")] + max_expected_time_per_block: u64, +} + impl CosmosSdkChain { /// Get a reference to the configuration for this chain. pub fn config(&self) -> &ChainConfig { @@ -190,7 +214,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 +307,31 @@ impl CosmosSdkChain { )); } + 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 + .ibc + .connection_genesis + .params + .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(()) } @@ -931,7 +980,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/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; From 436a0e506a94a23bbfe424ebc78a9f9dbc1c7d02 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 10 Jul 2023 15:57:03 +0200 Subject: [PATCH 2/4] Clean solution --- config.toml | 5 ++- crates/relayer/src/chain/cosmos.rs | 39 ++++--------------- .../src/chain/cosmos/types/app_state.rs | 38 ++++++++++++++++++ crates/relayer/src/chain/cosmos/types/mod.rs | 1 + 4 files changed, 50 insertions(+), 33 deletions(-) create mode 100644 crates/relayer/src/chain/cosmos/types/app_state.rs diff --git a/config.toml b/config.toml index 949d67d2c7..e2a0720448 100644 --- a/config.toml +++ b/config.toml @@ -284,9 +284,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 5e2c7ae5da..29c89cc7bf 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -8,8 +8,6 @@ use core::{ }; use futures::future::join_all; use num_bigint::BigInt; -use serde_derive::{Deserialize, Serialize}; -use serde_with::{serde_as, DisplayFromStr}; use std::{cmp::Ordering, thread}; use tokio::runtime::Runtime as TokioRuntime; @@ -109,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; @@ -155,28 +155,6 @@ pub struct CosmosSdkChain { tx_monitor_cmd: Option, } -#[derive(Debug, Deserialize, Serialize)] -struct GenesisAppState { - ibc: IbcConfig, -} - -#[derive(Debug, Deserialize, Serialize)] -struct IbcConfig { - connection_genesis: ConnectionGenesisConfig, -} - -#[derive(Debug, Deserialize, Serialize)] -struct ConnectionGenesisConfig { - params: ConnectionGenesisParams, -} - -#[serde_as] -#[derive(Debug, Deserialize, Serialize)] -struct ConnectionGenesisParams { - #[serde_as(as = "DisplayFromStr")] - max_expected_time_per_block: u64, -} - impl CosmosSdkChain { /// Get a reference to the configuration for this chain. pub fn config(&self) -> &ChainConfig { @@ -307,17 +285,14 @@ 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 - .ibc - .connection_genesis - .params - .max_expected_time_per_block, - ); + 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(), 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..30df52c895 --- /dev/null +++ b/crates/relayer/src/chain/cosmos/types/app_state.rs @@ -0,0 +1,38 @@ +//! Structure used to parse queried Genesis state using +//! /genesis RPC endpoint. + +use serde_derive::Deserialize; +use serde_derive::Serialize; +use serde_with::serde_as; +use serde_with::DisplayFromStr; + +#[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, +} + +#[serde_as] +#[derive(Debug, Deserialize, Serialize)] +struct ConnectionGenesisParams { + #[serde_as(as = "DisplayFromStr")] + max_expected_time_per_block: u64, +} 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; From 99e7dbf73afb934780beaf19fb0eab90d563a5ea Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 10 Jul 2023 16:01:25 +0200 Subject: [PATCH 3/4] Add unclog entry --- .../improvements/ibc-relayer/3211-query-max-block-time.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/improvements/ibc-relayer/3211-query-max-block-time.md 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 From 389ceae0ac6b21793b06bceeedbbf15b42d68e3e Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Tue, 11 Jul 2023 08:21:16 +0200 Subject: [PATCH 4/4] Remove serde-with dependency --- Cargo.lock | 67 ------------------- crates/relayer/Cargo.toml | 1 - .../src/chain/cosmos/types/app_state.rs | 17 +++-- 3 files changed, 13 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5ba81a428e..6bfebe8b07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -417,7 +417,6 @@ dependencies = [ "iana-time-zone", "js-sys", "num-traits", - "serde", "time 0.1.45", "wasm-bindgen", "winapi", @@ -669,41 +668,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "darling" -version = "0.20.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0558d22a7b463ed0241e993f76f09f30b126687447751a8638587b864e4b3944" -dependencies = [ - "darling_core", - "darling_macro", -] - -[[package]] -name = "darling_core" -version = "0.20.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab8bfa2e259f8ee1ce5e97824a3c55ec4404a0d772ca7fa96bf19f0752a046eb" -dependencies = [ - "fnv", - "ident_case", - "proc-macro2", - "quote", - "strsim", - "syn 2.0.23", -] - -[[package]] -name = "darling_macro" -version = "0.20.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29a358ff9f12ec09c3e61fef9b5a9902623a695a46a917b07f269bff1445611a" -dependencies = [ - "darling_core", - "quote", - "syn 2.0.23", -] - [[package]] name = "dashmap" version = "5.4.0" @@ -1578,7 +1542,6 @@ dependencies = [ "serde", "serde_derive", "serde_json", - "serde_with", "serial_test", "sha2 0.10.6", "signature 1.6.4", @@ -1800,7 +1763,6 @@ checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" dependencies = [ "autocfg", "hashbrown", - "serde", ] [[package]] @@ -3076,34 +3038,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_with" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f02d8aa6e3c385bf084924f660ce2a3a6bd333ba55b35e8590b321f35d88513" -dependencies = [ - "base64 0.21.2", - "chrono", - "hex", - "indexmap", - "serde", - "serde_json", - "serde_with_macros", - "time 0.3.22", -] - -[[package]] -name = "serde_with_macros" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "edc7d5d3932fb12ce722ee5e64dd38c504efba37567f0c402f6ca728c3b8b070" -dependencies = [ - "darling", - "proc-macro2", - "quote", - "syn 2.0.23", -] - [[package]] name = "serde_yaml" version = "0.9.21" @@ -3664,7 +3598,6 @@ version = "0.3.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea9e1b3cf1243ae005d9e74085d4d542f3125458f3a81af210d901dcd7411efd" dependencies = [ - "itoa", "serde", "time-core", "time-macros", diff --git a/crates/relayer/Cargo.toml b/crates/relayer/Cargo.toml index c19e9f92ed..5bf755ba2e 100644 --- a/crates/relayer/Cargo.toml +++ b/crates/relayer/Cargo.toml @@ -28,7 +28,6 @@ subtle-encoding = "0.5" humantime-serde = "1.1.1" serde = "1.0" serde_derive = "1.0" -serde_with = "3.0.0" thiserror = "1.0.40" toml = "0.7" tracing = "0.1.36" diff --git a/crates/relayer/src/chain/cosmos/types/app_state.rs b/crates/relayer/src/chain/cosmos/types/app_state.rs index 30df52c895..256a1017a2 100644 --- a/crates/relayer/src/chain/cosmos/types/app_state.rs +++ b/crates/relayer/src/chain/cosmos/types/app_state.rs @@ -1,10 +1,9 @@ //! Structure used to parse queried Genesis state using //! /genesis RPC endpoint. +use serde::Deserialize as SerdeDeserialize; use serde_derive::Deserialize; use serde_derive::Serialize; -use serde_with::serde_as; -use serde_with::DisplayFromStr; #[derive(Debug, Deserialize, Serialize)] pub struct GenesisAppState { @@ -30,9 +29,19 @@ struct ConnectionGenesisConfig { params: ConnectionGenesisParams, } -#[serde_as] #[derive(Debug, Deserialize, Serialize)] struct ConnectionGenesisParams { - #[serde_as(as = "DisplayFromStr")] + #[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) +}