Infer compatibility mode from version specs and fallback to node info…

… only if necessary (#4181)

* Infer compatibility mode from version specs and fallback to node info only if necessary

* Improve min gas price check error message

* Fix integration tests

* Fix clippy warnings

* Add changelog entry

* Show collated excluded sequences

.changelog/unreleased/improvements/4178-compat-mode-infer.md

@@ -0,0 +1,3 @@
+- Improve detection of Tendermint/CometBFT compatibility
+  mode when a chain runs a non-standard version
+  ([\#4178](https://github.com/informalsystems/hermes/issues/4178))

crates/relayer-cli/src/commands/listen.rs

@@ -1,27 +1,24 @@
 use alloc::sync::Arc;
-use core::{
-    fmt::{Display, Error as FmtError, Formatter},
-    str::FromStr,
-};
+use core::fmt::{Display, Error as FmtError, Formatter};
+use core::str::FromStr;
 use std::thread;
 
 use abscissa_core::application::fatal_error;
 use abscissa_core::clap::Parser;
 use eyre::eyre;
 use itertools::Itertools;
-use tendermint_rpc::{client::CompatMode, Client, HttpClient};
+use tendermint_rpc::{client::CompatMode, HttpClient};
 use tokio::runtime::Runtime as TokioRuntime;
 use tracing::{error, info, instrument};
 
-use ibc_relayer::{
-    chain::handle::Subscription,
-    config::{ChainConfig, EventSourceMode},
-    error::Error,
-    event::source::EventSource,
-    util::compat_mode::compat_mode_from_version,
-    HERMES_VERSION,
-};
-use ibc_relayer_types::{core::ics24_host::identifier::ChainId, events::IbcEvent};
+use ibc_relayer::chain::cosmos::fetch_compat_mode;
+use ibc_relayer::chain::handle::Subscription;
+use ibc_relayer::config::{ChainConfig, EventSourceMode};
+use ibc_relayer::error::Error;
+use ibc_relayer::event::source::EventSource;
+use ibc_relayer::HERMES_VERSION;
+use ibc_relayer_types::core::ics24_host::identifier::ChainId;
+use ibc_relayer_types::events::IbcEvent;
 
 use crate::prelude::*;
 
@@ -194,16 +191,15 @@ fn detect_compatibility_mode(
     let rpc_addr = match config {
         ChainConfig::CosmosSdk(config) => config.rpc_addr.clone(),
     };
+
     let client = HttpClient::builder(rpc_addr.try_into()?)
         .user_agent(format!("hermes/{}", HERMES_VERSION))
         .build()?;
 
-    let status = rt.block_on(client.status())?;
     let compat_mode = match config {
-        ChainConfig::CosmosSdk(config) => {
-            compat_mode_from_version(&config.compat_mode, status.node_info.version)?.into()
-        }
+        ChainConfig::CosmosSdk(config) => rt.block_on(fetch_compat_mode(&client, config))?,
     };
+
     Ok(compat_mode)
 }
 

crates/relayer/src/chain/cosmos.rs

@@ -1,8 +1,10 @@
 use alloc::sync::Arc;
 use bytes::Buf;
 use bytes::Bytes;
+use config::CosmosSdkConfig;
 use core::{future::Future, str::FromStr, time::Duration};
 use futures::future::join_all;
+use itertools::Itertools;
 use num_bigint::BigInt;
 use prost::Message;
 use std::cmp::Ordering;
@@ -106,9 +108,8 @@ use crate::keyring::{KeyRing, Secp256k1KeyPair, SigningKeyPair};
 use crate::light_client::tendermint::LightClient as TmLightClient;
 use crate::light_client::{LightClient, Verified};
 use crate::misbehaviour::MisbehaviourEvidence;
-use crate::util::compat_mode::compat_mode_from_version;
+use crate::util::collate::CollatedIterExt;
 use crate::util::create_grpc_client;
-use crate::util::pretty::PrettySlice;
 use crate::util::pretty::{
     PrettyIdentifiedChannel, PrettyIdentifiedClientState, PrettyIdentifiedConnection,
 };
@@ -917,11 +918,10 @@ impl ChainEndpoint for CosmosSdkChain {
             .build()
             .map_err(|e| Error::rpc(config.rpc_addr.clone(), e))?;
 
-        let node_info = rt.block_on(fetch_node_info(&rpc_client, &config))?;
-
-        let compat_mode = compat_mode_from_version(&config.compat_mode, node_info.version)?.into();
+        let compat_mode = rt.block_on(fetch_compat_mode(&rpc_client, &config))?;
         rpc_client.set_compat_mode(compat_mode);
 
+        let node_info = rt.block_on(fetch_node_info(&rpc_client, &config))?;
         let light_client = TmLightClient::from_cosmos_sdk_config(&config, node_info.id)?;
 
         // Initialize key store and load key
@@ -1116,6 +1116,7 @@ impl ChainEndpoint for CosmosSdkChain {
             &self.rpc_client,
             &self.config.rpc_addr,
         ))?;
+
         Ok(version_specs)
     }
 
@@ -2682,7 +2683,7 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
             if !seqs.is_empty() {
                 warn!(
                     "chain '{chain_id}' will not clear packets on channel '{channel_id}' with sequences: {}. \
-                    Ignore this warning if this configuration is correct.", PrettySlice(seqs)
+                    Ignore this warning if this configuration is correct.", seqs.iter().copied().collated().format(", ")
                 );
             }
         }
@@ -2732,17 +2733,16 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
 
             if !found_matching_denom {
                 warn!(
-                        "chain '{}' has no minimum gas price of denomination '{}' \
-                        that is strictly less than the `gas_price` specified for that chain in the Hermes configuration. \
-                        This is usually a sign of misconfiguration, please check your chain and Hermes configurations",
-                        chain_id, relayer_gas_price.denom
-                    );
+                    "chain '{}' does not provide a minimum gas price for denomination '{}'.\
+                    This is usually a sign of misconfiguration, please check your chain configuration",
+                    chain_id, relayer_gas_price.denom
+                );
             }
         }
 
         Some(_) => warn!(
-            "chain '{}' has no minimum gas price value configured for denomination '{}'. \
-            This is usually a sign of misconfiguration, please check your chain and relayer configurations",
+            "chain '{}' does not provide a minimum gas price for denomination '{}'. \
+            This is usually a sign of misconfiguration, please check your chain configuration",
             chain_id, relayer_gas_price.denom
         ),
 
@@ -2774,6 +2774,40 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
     Ok(())
 }
 
+pub async fn fetch_compat_mode(
+    client: &HttpClient,
+    config: &CosmosSdkConfig,
+) -> Result<CompatMode, Error> {
+    use crate::util::compat_mode::compat_mode_from_node_version;
+    use crate::util::compat_mode::compat_mode_from_version_specs;
+
+    let version_specs = fetch_version_specs(&config.id, client, &config.rpc_addr).await;
+
+    let compat_mode = match version_specs {
+        Ok(specs) => compat_mode_from_version_specs(&config.compat_mode, specs.consensus),
+        Err(e) => {
+            warn!(
+                "Failed to fetch version specs for chain '{}': {e}",
+                config.id
+            );
+
+            let status = client
+                .status()
+                .await
+                .map_err(|e| Error::rpc(config.rpc_addr.clone(), e))?;
+
+            warn!(
+                "Will fall back on using the node version: {}",
+                status.node_info.version
+            );
+
+            compat_mode_from_node_version(&config.compat_mode, status.node_info.version)
+        }
+    }?;
+
+    Ok(compat_mode.into())
+}
+
 #[cfg(test)]
 mod tests {
     use super::calculate_fee;

crates/relayer/src/config/compat_mode.rs

@@ -10,14 +10,11 @@ use tendermint_rpc::client::CompatMode as TmCompatMode;
 use crate::config::Error;
 
 /// CometBFT RPC compatibility mode
-///
-/// Can be removed in favor of the one in tendermint-rs, once
-/// <https://github.com/informalsystems/tendermint-rs/pull/1367> is merged.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum CompatMode {
     /// Use version 0.34 of the protocol.
     V0_34,
-    /// Use version 0.37 of the protocol.
+    /// Use version 0.37+ of the protocol.
     V0_37,
 }
 
@@ -34,12 +31,13 @@ impl FromStr for CompatMode {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        const VALID_COMPAT_MODES: &str = "0.34, 0.37";
+        const VALID_COMPAT_MODES: &str = "0.34, 0.37, 0.38";
 
         // Trim leading 'v', if present
         match s.trim_start_matches('v') {
             "0.34" => Ok(CompatMode::V0_34),
             "0.37" => Ok(CompatMode::V0_37),
+            "0.38" => Ok(CompatMode::V0_37), // v0.38 is compatible with v0.37
             _ => Err(Error::invalid_compat_mode(
                 s.to_string(),
                 VALID_COMPAT_MODES,

crates/relayer/src/error.rs

@@ -591,7 +591,8 @@ define_error! {
 
         InvalidCompatMode
             [ TendermintRpcError ]
-            |_| { "Invalid CompatMode queried from chain and no `compat_mode` configured in Hermes. This can be fixed by specifying a `compat_mode` in Hermes config.toml" },
+            |_| { "Invalid compatibility mode queried from chain and no `compat_mode` configured in Hermes. \
+                   This can be fixed by specifying a `compat_mode` for chain '{}' in Hermes config.toml" },
 
         HttpRequest
             [ TraceError<reqwest::Error> ]

crates/relayer/src/util/compat_mode.rs

@@ -3,12 +3,11 @@ use tracing::warn;
 use tendermint::Version;
 use tendermint_rpc::client::CompatMode as TmCompatMode;
 
+use crate::chain::cosmos::version::ConsensusVersion;
 use crate::config::compat_mode::CompatMode;
 use crate::error::Error;
 
-/// This is a wrapper around tendermint-rs CompatMode::from_version() method.
-///
-pub fn compat_mode_from_version(
+pub fn compat_mode_from_node_version(
     configured_version: &Option<CompatMode>,
     version: Version,
 ) -> Result<CompatMode, Error> {
@@ -17,11 +16,68 @@ pub fn compat_mode_from_version(
     // This will prioritize the use of the CompatMode specified in Hermes configuration file
     match (configured_version, queried_version) {
         (Some(configured), Ok(queried)) if !configured.equal_to_tm_compat_mode(queried) => {
-            warn!("be wary of potential `compat_mode` misconfiguration. Configured version: {}, chain version: {}. Hermes will use the configured `compat_mode` version `{}`. If this configuration is done on purpose this message can be ignored.", configured, queried, configured);
+            warn!(
+                "potential `compat_mode` misconfiguration! Configured version '{configured}' does not match chain version '{queried}'. \
+                Hermes will use the configured `compat_mode` version '{configured}'. \
+                If this configuration is done on purpose this message can be ignored.",
+            );
+
             Ok(configured.clone())
         }
         (Some(configured), _) => Ok(configured.clone()),
         (_, Ok(queried)) => Ok(queried.into()),
         (_, Err(e)) => Err(Error::invalid_compat_mode(e)),
     }
 }
+
+pub fn compat_mode_from_version_specs(
+    configured_mode: &Option<CompatMode>,
+    version: Option<ConsensusVersion>,
+) -> Result<CompatMode, Error> {
+    let queried_mode = match version {
+        Some(ConsensusVersion::Tendermint(v) | ConsensusVersion::Comet(v)) => {
+            compat_mode_from_semver(v)
+        }
+        None => None,
+    };
+
+    match (configured_mode, queried_mode) {
+        (Some(configured), Some(queried)) if configured == &queried => Ok(queried),
+        (Some(configured), Some(queried)) => {
+            warn!(
+                "potential `compat_mode` misconfiguration! Configured version: {configured}, chain version: {queried}. \
+                Hermes will use the configured `compat_mode` version `{configured}`. \
+                If this configuration is done on purpose this message can be ignored."
+            );
+
+            Ok(configured.clone())
+        }
+        (Some(configured), None) => {
+            warn!(
+                "Hermes could not infer the compatibility mode for this chain, \
+                and will use the configured `compat_mode` version `{configured}`."
+            );
+
+            Ok(configured.clone())
+        }
+        (None, Some(queried)) => Ok(queried),
+        (None, None) => {
+            warn!(
+                "Hermes could not infer the compatibility mode for this chain, and no `compat_mode` was configured, \
+                and will use the default compatibility mode `0.37`. \
+                Please consider configuring the `compat_mode` in the Hermes configuration file."
+            );
+
+            Ok(CompatMode::V0_37)
+        }
+    }
+}
+
+fn compat_mode_from_semver(v: semver::Version) -> Option<CompatMode> {
+    match (v.major, v.minor) {
+        (0, 34) => Some(CompatMode::V0_34),
+        (0, 37) => Some(CompatMode::V0_37),
+        (0, 38) => Some(CompatMode::V0_37),
+        _ => None,
+    }
+}

tools/test-framework/src/chain/tagged.rs

@@ -5,10 +5,13 @@
 use ibc_proto::google::protobuf::Any;
 use ibc_relayer::chain::cosmos::tx::simple_send_tx;
 use ibc_relayer::chain::cosmos::types::config::TxConfig;
+use ibc_relayer::config::compat_mode::CompatMode;
 use ibc_relayer::event::IbcEventWithHeight;
-use ibc_relayer::util::compat_mode::compat_mode_from_version;
+use ibc_relayer_types::core::ics24_host::identifier::ChainId;
 use serde_json as json;
 use tendermint_rpc::client::{Client, HttpClient};
+use tendermint_rpc::Url;
+use tracing::warn;
 
 use crate::chain::cli::query::query_auth_module;
 use crate::chain::cli::query::query_recipient_transactions;
@@ -120,12 +123,16 @@ impl<'a, Chain: Send> TaggedChainDriverExt<Chain> for MonoTagged<Chain, &'a Chai
         let rpc_address = self.value().tx_config.rpc_address.clone();
         let rt = &self.value().runtime;
 
-        let mut client = HttpClient::new(rpc_address).map_err(handle_generic_error)?;
+        let mut client = HttpClient::new(rpc_address.clone()).map_err(handle_generic_error)?;
 
-        let status = rt.block_on(client.status()).map_err(handle_generic_error)?;
-        let compat_mode =
-            compat_mode_from_version(&self.value().compat_mode, status.node_info.version)?.into();
-        client.set_compat_mode(compat_mode);
+        let compat_mode = rt.block_on(fetch_compat_mode(
+            &client,
+            &self.value().chain_id,
+            &rpc_address,
+            &self.value().compat_mode,
+        ))?;
+
+        client.set_compat_mode(compat_mode.into());
 
         Ok(MonoTagged::new(client))
     }
@@ -209,3 +216,34 @@ impl<'a, Chain: Send> TaggedChainDriverExt<Chain> for MonoTagged<Chain, &'a Chai
         )
     }
 }
+
+pub async fn fetch_compat_mode(
+    client: &HttpClient,
+    id: &ChainId,
+    rpc_addr: &Url,
+    configured_mode: &Option<CompatMode>,
+) -> Result<CompatMode, Error> {
+    use ibc_relayer::chain::cosmos::query::fetch_version_specs;
+    use ibc_relayer::util::compat_mode::compat_mode_from_node_version;
+    use ibc_relayer::util::compat_mode::compat_mode_from_version_specs;
+
+    let version_specs = fetch_version_specs(id, client, rpc_addr).await;
+
+    let compat_mode = match version_specs {
+        Ok(specs) => compat_mode_from_version_specs(configured_mode, specs.consensus),
+        Err(e) => {
+            warn!("Failed to fetch version specs for chain '{id}': {e}");
+
+            let status = client.status().await.map_err(handle_generic_error)?;
+
+            warn!(
+                "Will fall back on using the node version: {}",
+                status.node_info.version
+            );
+
+            compat_mode_from_node_version(configured_mode, status.node_info.version)
+        }
+    }?;
+
+    Ok(compat_mode)
+}