From bb9d42e30c7ebe7ebe4e9e5907e2333f1617b1e2 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 12 Mar 2024 12:33:17 +0100 Subject: [PATCH] Fix parsing of IBC-Go version in health check and improve health check reporting (#3888) --- .../ibc-relayer/3880-health-check-fix.md | 2 + crates/relayer/src/chain/cosmos.rs | 21 +- crates/relayer/src/chain/cosmos/estimate.rs | 7 +- crates/relayer/src/chain/cosmos/version.rs | 242 +++++++++++++++--- crates/relayer/src/worker/packet.rs | 8 +- 5 files changed, 228 insertions(+), 52 deletions(-) create mode 100644 .changelog/unreleased/bug-fixes/ibc-relayer/3880-health-check-fix.md diff --git a/.changelog/unreleased/bug-fixes/ibc-relayer/3880-health-check-fix.md b/.changelog/unreleased/bug-fixes/ibc-relayer/3880-health-check-fix.md new file mode 100644 index 0000000000..e6106fa10a --- /dev/null +++ b/.changelog/unreleased/bug-fixes/ibc-relayer/3880-health-check-fix.md @@ -0,0 +1,2 @@ +- Fix parsing of IBC-Go version in health check and improve health check + reporting ([\#3880](https://github.com/informalsystems/hermes/issues/3880)) \ No newline at end of file diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index 8b2ccb0b7e..53505d2b0d 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -146,6 +146,7 @@ pub mod wait; /// /// [tm-37-max]: https://github.com/tendermint/tendermint/blob/v0.37.0-rc1/types/params.go#L79 pub const BLOCK_MAX_BYTES_MAX_FRACTION: f64 = 0.9; + pub struct CosmosSdkChain { config: config::CosmosSdkConfig, tx_config: TxConfig, @@ -292,19 +293,23 @@ impl CosmosSdkChain { )); } - // Query /genesis RPC endpoint to retrieve the `max_expected_time_per_block` value - // to use as `max_block_time`. + // 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 = + let new_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() - ); + + if old_max_block_time.as_secs() != new_max_block_time.as_secs() { + self.config.max_block_time = new_max_block_time; + + 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!( diff --git a/crates/relayer/src/chain/cosmos/estimate.rs b/crates/relayer/src/chain/cosmos/estimate.rs index 4223e39068..dff1a5cdff 100644 --- a/crates/relayer/src/chain/cosmos/estimate.rs +++ b/crates/relayer/src/chain/cosmos/estimate.rs @@ -131,7 +131,7 @@ async fn estimate_gas_with_tx( gas_config: &GasConfig, grpc_address: &Uri, tx: Tx, - account: &Account, + _account: &Account, ) -> Result { let simulated_gas = send_tx_simulate(grpc_address, tx) .await @@ -169,7 +169,7 @@ async fn estimate_gas_with_tx( telemetry!( simulate_errors, - &account.address.to_string(), + &_account.address.to_string(), true, get_error_text(&e), ); @@ -185,7 +185,7 @@ async fn estimate_gas_with_tx( telemetry!( simulate_errors, - &account.address.to_string(), + &_account.address.to_string(), false, get_error_text(&e), ); @@ -212,6 +212,7 @@ fn can_recover_from_simulation_failure(e: &Error) -> bool { } } +#[cfg(feature = "telemetry")] fn get_error_text(e: &Error) -> String { use crate::error::ErrorDetail::*; diff --git a/crates/relayer/src/chain/cosmos/version.rs b/crates/relayer/src/chain/cosmos/version.rs index a75c4ed822..5db1b56961 100644 --- a/crates/relayer/src/chain/cosmos/version.rs +++ b/crates/relayer/src/chain/cosmos/version.rs @@ -7,7 +7,7 @@ use core::fmt::{Display, Error as FmtError, Formatter}; use flex_error::define_error; use tracing::trace; -use ibc_proto::cosmos::base::tendermint::v1beta1::VersionInfo; +use ibc_proto::cosmos::base::tendermint::v1beta1::{Module, VersionInfo}; /// Specifies the SDK, IBC-go, and Tendermint modules path, as expected /// to appear in the application version information of a @@ -23,17 +23,22 @@ use ibc_proto::cosmos::base::tendermint::v1beta1::VersionInfo; /// }, /// ``` const SDK_MODULE_NAME: &str = "github.com/cosmos/cosmos-sdk"; -const IBC_GO_MODULE_NAME: &str = "github.com/cosmos/ibc-go"; +const IBC_GO_MODULE_PREFIX: &str = "github.com/cosmos/ibc-go/v"; const TENDERMINT_MODULE_NAME: &str = "github.com/tendermint/tendermint"; const COMET_MODULE_NAME: &str = "github.com/cometbft/cometbft"; +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ConsensusVersion { + Tendermint(semver::Version), + Comet(semver::Version), +} + /// Captures the version(s) specification of different modules of a network. -#[derive(Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct Specs { pub cosmos_sdk: Option, pub ibc_go: Option, - pub tendermint: Option, - pub comet: Option, + pub consensus: Option, } impl Display for Specs { @@ -50,22 +55,16 @@ impl Display for Specs { .map(|v| v.to_string()) .unwrap_or_else(|| "UNKNOWN".to_string()); - let tendermint = self - .tendermint - .as_ref() - .map(|v| v.to_string()) - .unwrap_or_else(|| "UNKNOWN".to_string()); - - let comet = self - .comet - .as_ref() - .map(|v| v.to_string()) - .unwrap_or_else(|| "UNKNOWN".to_string()); + let consensus = match self.consensus { + Some(ConsensusVersion::Tendermint(ref v)) => format!("Tendermint {v}"), + Some(ConsensusVersion::Comet(ref v)) => format!("CometBFT {v}"), + None => "Tendermint: UNKNOWN, CometBFT: UNKNOWN".to_string(), + }; write!( f, - "Cosmos SDK {}, IBC-Go {}, Tendermint {}, CometBFT {}", - cosmos_sdk, ibc_go, tendermint, comet + "Cosmos SDK {}, IBC-Go {}, {}", + cosmos_sdk, ibc_go, consensus ) } } @@ -78,7 +77,8 @@ define_error! { comet: String, app: AppInfo, } - |e| { format!("failed to find the Tendermint ('{}') or CometBFT ('{}') dependency for application {}", e.tendermint, e.comet, e.app) }, + |e| { format!("failed to find the Tendermint ('{}') or CometBFT ('{}') dependency for application {}", + e.tendermint, e.comet, e.app) }, VersionParsingFailed { @@ -102,8 +102,14 @@ impl TryFrom for Specs { let tendermint_version = parse_tendermint_version(&raw_version)?; let comet_version = parse_comet_version(&raw_version)?; + let consensus_version = match (tendermint_version, comet_version) { + (_, Some(comet)) => Some(ConsensusVersion::Comet(comet)), + (Some(tendermint), _) => Some(ConsensusVersion::Tendermint(tendermint)), + _ => None, + }; + // Ensure that either Tendermint or CometBFT are being used. - if tendermint_version.is_none() && comet_version.is_none() { + if consensus_version.is_none() { return Err(Error::consensus_module_not_found( TENDERMINT_MODULE_NAME.to_string(), COMET_MODULE_NAME.to_string(), @@ -117,50 +123,48 @@ impl TryFrom for Specs { git_commit = %raw_version.git_commit, sdk_version = ?sdk_version, ibc_go_status = ?ibc_go_version, - tendermint_version = ?tendermint_version, - comet_version = ?comet_version, + consensus_version = ?consensus_version, "parsed version specification" ); Ok(Self { cosmos_sdk: sdk_version, ibc_go: ibc_go_version, - tendermint: tendermint_version, - comet: comet_version, + consensus: consensus_version, }) } } fn parse_sdk_version(version_info: &VersionInfo) -> Result, Error> { - parse_optional_version(version_info, SDK_MODULE_NAME) + parse_optional_version(version_info, |m| m.path == SDK_MODULE_NAME) } fn parse_ibc_go_version(version_info: &VersionInfo) -> Result, Error> { - parse_optional_version(version_info, IBC_GO_MODULE_NAME) + parse_optional_version(version_info, |m| m.path.starts_with(IBC_GO_MODULE_PREFIX)) } fn parse_tendermint_version(version_info: &VersionInfo) -> Result, Error> { - parse_optional_version(version_info, TENDERMINT_MODULE_NAME) + parse_optional_version(version_info, |m| m.path == TENDERMINT_MODULE_NAME) } fn parse_comet_version(version_info: &VersionInfo) -> Result, Error> { - parse_optional_version(version_info, COMET_MODULE_NAME) + parse_optional_version(version_info, |m| m.path == COMET_MODULE_NAME) } fn parse_optional_version( version_info: &VersionInfo, - module_name: &str, + predicate: impl Fn(&Module) -> bool, ) -> Result, Error> { - match version_info - .build_deps - .iter() - .find(|&m| m.path == module_name) - { + let module = version_info.build_deps.iter().find(|&m| predicate(m)); + + match module { None => Ok(None), + Some(module) => { let plain_version = module.version.trim_start_matches('v'); semver::Version::parse(plain_version) + // Discard the pre-release version, if any .map(|mut version| { version.pre = semver::Prerelease::EMPTY; Some(version) @@ -179,7 +183,7 @@ fn parse_optional_version( /// Helper struct to capture all the reported information of an /// IBC application, e.g., `gaiad`. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct AppInfo { app_name: String, version: String, @@ -201,3 +205,171 @@ impl From<&VersionInfo> for AppInfo { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cosmoshub() { + let version_info = VersionInfo { + name: "gaia".to_string(), + app_name: "gaiad".to_string(), + version: "v14.2.0".to_string(), + git_commit: "3aa6e5058b4cbb4729300b239abfabd9a5b5691f".to_string(), + build_tags: "netgo,ledger".to_string(), + go_version: "go version go1.20.3 linux/amd64".to_string(), + cosmos_sdk_version: "v0.45.16".to_string(), + build_deps: vec![ + Module { + path: "github.com/cometbft/cometbft-db".to_string(), + version: "v0.7.0".to_string(), + sum: "h1:uBjbrBx4QzU0zOEnU8KxoDl18dMNgDh+zZRUE0ucsbo=".to_string(), + }, + Module { + path: "github.com/confio/ics23/go".to_string(), + version: "v0.9.0".to_string(), + sum: "h1:cWs+wdbS2KRPZezoaaj+qBleXgUk5WOQFMP3CQFGTr4=".to_string(), + }, + Module { + path: "github.com/cosmos/cosmos-db".to_string(), + version: "v0.0.0-20221226095112-f3c38ecb5e32".to_string(), + sum: "h1:zlCp9n3uwQieELltZWHRmwPmPaZ8+XoL2Sj+A2YJlr8=".to_string(), + }, + Module { + path: "github.com/cosmos/cosmos-proto".to_string(), + version: "v1.0.0-beta.1".to_string(), + sum: "h1:iDL5qh++NoXxG8hSy93FdYJut4XfgbShIocllGaXx/0=".to_string(), + }, + Module { + path: "github.com/cosmos/cosmos-sdk".to_string(), + version: "v0.45.16".to_string(), + sum: "".to_string(), + }, + Module { + path: "github.com/cosmos/go-bip39".to_string(), + version: "v1.0.0".to_string(), + sum: "h1:pcomnQdrdH22njcAatO0yWojsUnCO3y2tNoV1cb6hHY=".to_string(), + }, + Module { + path: "github.com/cosmos/iavl".to_string(), + version: "v0.19.5".to_string(), + sum: "h1:rGA3hOrgNxgRM5wYcSCxgQBap7fW82WZgY78V9po/iY=".to_string(), + }, + Module { + path: "github.com/tendermint/tendermint".to_string(), + version: "v0.34.27".to_string(), + sum: "".to_string(), + }, + Module { + path: "github.com/cosmos/ibc-go/v4".to_string(), + version: "v4.4.2".to_string(), + sum: "h1:PG4Yy0/bw6Hvmha3RZbc53KYzaCwuB07Ot4GLyzcBvo=".to_string(), + }, + Module { + path: "github.com/cosmos/interchain-security/v2".to_string(), + version: "v2.0.0".to_string(), + sum: "".to_string(), + }, + ], + }; + + let app_info = AppInfo::from(&version_info); + + assert_eq!( + app_info, + AppInfo { + app_name: "gaiad".to_string(), + version: "v14.2.0".to_string(), + git_commit: "3aa6e5058b4cbb4729300b239abfabd9a5b5691f".to_string() + } + ); + + let specs = Specs::try_from(version_info).unwrap(); + + assert_eq!( + specs, + Specs { + cosmos_sdk: Some(semver::Version::parse("0.45.16").unwrap()), + ibc_go: Some(semver::Version::parse("4.4.2").unwrap()), + consensus: Some(ConsensusVersion::Tendermint( + semver::Version::parse("0.34.27").unwrap() + )) + } + ); + } + + #[test] + fn phoenix() { + let version_info = VersionInfo { + name: "terra".to_string(), + app_name: "terrad".to_string(), + version: "20210603".to_string(), + git_commit: "7cbb1f555b661a6ebec55231e563d2f94effc40e".to_string(), + build_tags: "netgo,ledger".to_string(), + go_version: "go version go1.20 linux/amd64".to_string(), + cosmos_sdk_version: "v0.47.5".to_string(), + build_deps: vec![ + Module { + path: "github.com/confio/ics23/go".to_string(), + version: "v0.9.0".to_string(), + sum: "h1:cWs+wdbS2KRPZezoaaj+qBleXgUk5WOQFMP3CQFGTr4=".to_string(), + }, + Module { + path: "github.com/cometbft/cometbft-db".to_string(), + version: "v0.8.0".to_string(), + sum: "h1:vUMDaH3ApkX8m0KZvOFFy9b5DZHBAjsnEuo9AKVZpjo=".to_string(), + }, + Module { + path: "github.com/cosmos/cosmos-proto".to_string(), + version: "v1.0.0-beta.3".to_string(), + sum: "h1:VitvZ1lPORTVxkmF2fAp3IiA61xVwArQYKXTdEcpW6o=".to_string(), + }, + Module { + path: "github.com/cosmos/cosmos-sdk".to_string(), + version: "v0.47.5".to_string(), + sum: "".to_string(), + }, + Module { + path: "github.com/cosmos/ibc-go/v7".to_string(), + version: "v7.3.0".to_string(), + sum: "".to_string(), + }, + Module { + path: "github.com/cosmos/ics23/go".to_string(), + version: "v0.10.0".to_string(), + sum: "h1:iXqLLgp2Lp+EdpIuwXTYIQU+AiHj9mOC2X9ab++bZDM=".to_string(), + }, + Module { + path: "github.com/cometbft/cometbft".to_string(), + version: "v0.37.2".to_string(), + sum: "h1:XB0yyHGT0lwmJlFmM4+rsRnczPlHoAKFX6K8Zgc2/Jc=".to_string(), + }, + ], + }; + + let app_info = AppInfo::from(&version_info); + + assert_eq!( + app_info, + AppInfo { + app_name: "terrad".to_string(), + version: "20210603".to_string(), + git_commit: "7cbb1f555b661a6ebec55231e563d2f94effc40e".to_string() + } + ); + + let specs = Specs::try_from(version_info).unwrap(); + + assert_eq!( + specs, + Specs { + cosmos_sdk: Some(semver::Version::parse("0.47.5").unwrap()), + ibc_go: Some(semver::Version::parse("7.3.0").unwrap()), + consensus: Some(ConsensusVersion::Comet( + semver::Version::parse("0.37.2").unwrap() + )) + } + ); + } +} diff --git a/crates/relayer/src/worker/packet.rs b/crates/relayer/src/worker/packet.rs index 5daf122466..7439301828 100644 --- a/crates/relayer/src/worker/packet.rs +++ b/crates/relayer/src/worker/packet.rs @@ -1,9 +1,3 @@ -#[cfg(feature = "telemetry")] -use { - ibc_relayer_types::core::ics24_host::identifier::ChannelId, - ibc_relayer_types::core::ics24_host::identifier::PortId, -}; - use core::time::Duration; use std::borrow::BorrowMut; use std::sync::{Arc, Mutex}; @@ -20,6 +14,8 @@ use ibc_relayer_types::applications::transfer::{Amount, Coin, RawCoin}; use ibc_relayer_types::core::ics04_channel::channel::Ordering; use ibc_relayer_types::core::ics04_channel::events::WriteAcknowledgement; use ibc_relayer_types::core::ics04_channel::packet::Sequence; +use ibc_relayer_types::core::ics24_host::identifier::ChannelId; +use ibc_relayer_types::core::ics24_host::identifier::PortId; use ibc_relayer_types::events::{IbcEvent, IbcEventType}; use ibc_relayer_types::Height;