Add a check for min_gas_price in the health check process

.changelog/unreleased/improvements/ibc-relayer/2776-check-min-gas-price.md

@@ -0,0 +1,2 @@
+- The health check process now compares the configured `gas_price` config setting against the full node's `min_gas_price` setting, ensuring that the former is at least equal to the latter
+  ([#2776](https://github.com/informalsystems/hermes/issues/2776))

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

@@ -62,7 +62,7 @@ fn register_signals(tx_cmd: Sender<SupervisorCmd>) -> Result<(), io::Error> {
         SIGUSR1, // Dump state
     ];
 
-    let mut signals = Signals::new(&sigs)?;
+    let mut signals = Signals::new(sigs)?;
 
     std::thread::spawn(move || {
         for signal in &mut signals {

crates/relayer/src/chain/cosmos.rs

@@ -13,6 +13,7 @@ use tokio::runtime::Runtime as TokioRuntime;
 use tonic::{codegen::http::Uri, metadata::AsciiMetadataValue};
 use tracing::{error, instrument, trace, warn};
 
+use ibc_proto::cosmos::base::node::v1beta1::ConfigResponse;
 use ibc_proto::cosmos::staking::v1beta1::Params as StakingParams;
 use ibc_proto::protobuf::Protobuf;
 use ibc_relayer_types::clients::ics07_tendermint::client_state::{
@@ -77,7 +78,7 @@ use crate::chain::handle::Subscription;
 use crate::chain::requests::*;
 use crate::chain::tracking::TrackedMsgs;
 use crate::client_state::{AnyClientState, IdentifiedAnyClientState};
-use crate::config::ChainConfig;
+use crate::config::{parse_gas_prices, ChainConfig, GasPrice};
 use crate::consensus_state::{AnyConsensusState, AnyConsensusStateWithHeight};
 use crate::denom::DenomTrace;
 use crate::error::Error;
@@ -318,6 +319,68 @@ impl CosmosSdkChain {
         Ok(params)
     }
 
+    /// Query the node for its configuration parameters.
+    ///
+    /// ### Note: This query endpoint was introduced in SDK v0.46.3/v0.45.10. Not available before that.
+    ///
+    /// Returns:
+    ///     - `Ok(Some(..))` if the query was successful.
+    ///     - `Ok(None) in case the query endpoint is not available.
+    ///     - `Err` for any other error.
+    pub fn query_config_params(&self) -> Result<Option<ConfigResponse>, Error> {
+        crate::time!("query_config_params");
+        crate::telemetry!(query, self.id(), "query_config_params");
+
+        // Helper function to diagnose if the node config query is unimplemented
+        // by matching on the error details.
+        fn is_unimplemented_node_query(err_status: &tonic::Status) -> bool {
+            if err_status.code() != tonic::Code::Unimplemented {
+                return false;
+            }
+
+            err_status
+                .message()
+                .contains("unknown service cosmos.base.node.v1beta1.Service")
+        }
+
+        let mut client = self
+            .block_on(
+                ibc_proto::cosmos::base::node::v1beta1::service_client::ServiceClient::connect(
+                    self.grpc_addr.clone(),
+                ),
+            )
+            .map_err(Error::grpc_transport)?;
+
+        let request = tonic::Request::new(ibc_proto::cosmos::base::node::v1beta1::ConfigRequest {});
+
+        match self.block_on(client.config(request)) {
+            Ok(response) => {
+                let params = response.into_inner();
+
+                Ok(Some(params))
+            }
+            Err(e) => {
+                if is_unimplemented_node_query(&e) {
+                    Ok(None)
+                } else {
+                    Err(Error::grpc_status(e))
+                }
+            }
+        }
+    }
+
+    /// The minimum gas price that this node accepts
+    pub fn min_gas_price(&self) -> Result<Vec<GasPrice>, Error> {
+        crate::time!("min_gas_price");
+
+        let min_gas_price: Vec<GasPrice> =
+            self.query_config_params()?.map_or(vec![], |cfg_response| {
+                parse_gas_prices(cfg_response.minimum_gas_price)
+            });
+
+        Ok(min_gas_price)
+    }
+
     /// The unbonding period of this chain
     pub fn unbonding_period(&self) -> Result<Duration, Error> {
         crate::time!("unbonding_period");
@@ -1873,12 +1936,23 @@ fn client_id_suffix(client_id: &ClientId) -> Option<u64> {
         .and_then(|e| e.parse::<u64>().ok())
 }
 
+/// Performs a health check on a Cosmos chain.
+///
+/// This health check checks on the following in this order:
+/// 1. Checks on the self-reported health endpoint.
+/// 2. Checks that the staking module maintains some historical entries such
+///    that local header information is stored in the IBC state and thus
+///    client proofs that are part of the connection handshake can be verified.
+/// 3. Checks that transaction indexing is enabled.
+/// 4. Checks that the chain identifier matches the network name.
+/// 5. Checks that the underlying SDK and ibc-go versions are compatible.
+/// 6. Checks that the `gas_price` parameter in Hermes is >= the `min_gas_price`
+///    advertised by the node Hermes is connected to.
 fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
     let chain_id = chain.id();
     let grpc_address = chain.grpc_addr.to_string();
     let rpc_address = chain.config.rpc_addr.to_string();
 
-    // Checkup on the self-reported health endpoint
     chain.block_on(chain.rpc_client.health()).map_err(|e| {
         Error::health_check_json_rpc(
             chain_id.clone(),
@@ -1888,21 +1962,16 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
         )
     })?;
 
-    // Check that the staking module maintains some historical entries, meaning that
-    // local header information is stored in the IBC state and therefore client
-    // proofs that are part of the connection handshake messages can be verified.
     if chain.historical_entries()? == 0 {
         return Err(Error::no_historical_entries(chain_id.clone()));
     }
 
     let status = chain.chain_status()?;
 
-    // Check that transaction indexing is enabled
     if status.node_info.other.tx_index != TxIndexStatus::On {
         return Err(Error::tx_indexing_disabled(chain_id.clone()));
     }
 
-    // Check that the chain identifier matches the network name
     if status.node_info.network.as_str() != chain_id.as_str() {
         // Log the error, continue optimistically
         error!(
@@ -1912,9 +1981,31 @@ fn do_health_check(chain: &CosmosSdkChain) -> Result<(), Error> {
         );
     }
 
+    let relayer_gas_price = &chain.config.gas_price;
+    let node_min_gas_prices = chain.min_gas_price()?;
+    let mut found_matching_denom = false;
+
+    for price in node_min_gas_prices {
+        match relayer_gas_price.partial_cmp(&price) {
+            Some(Ordering::Less) => return Err(Error::gas_price_too_low(chain_id.clone())),
+            Some(_) => {
+                found_matching_denom = true;
+                break;
+            }
+            None => continue,
+        }
+    }
+
+    if !found_matching_denom {
+        warn!(
+            "Chain '{}' has no minimum gas price value configured for denomination '{}'. \
+            This is usually a sign of misconfiguration, please check your config.toml",
+            chain_id, relayer_gas_price.denom
+        );
+    }
+
     let version_specs = chain.block_on(fetch_version_specs(&chain.config.id, &chain.grpc_addr))?;
 
-    // Checkup on the underlying SDK & IBC-go versions
     if let Err(diagnostic) = compatibility::run_diagnostic(&version_specs) {
         return Err(Error::sdk_module_version(
             chain_id.clone(),

crates/relayer/src/config.rs

@@ -8,7 +8,9 @@ pub mod types;
 
 use alloc::collections::BTreeMap;
 use core::{
+    cmp::Ordering,
     fmt::{Display, Error as FmtError, Formatter},
+    str::FromStr,
     time::Duration,
 };
 use std::{fs, fs::File, io::Write, path::Path};
@@ -28,6 +30,7 @@ use crate::error::Error as RelayerError;
 use crate::extension_options::ExtensionOptionDynamicFeeTx;
 use crate::keyring::Store;
 
+pub use crate::config::Error as ConfigError;
 pub use error::Error;
 
 pub use filter::PacketFilter;
@@ -50,6 +53,56 @@ impl Display for GasPrice {
     }
 }
 
+impl FromStr for GasPrice {
+    type Err = ConfigError;
+
+    fn from_str(price_in: &str) -> Result<Self, Self::Err> {
+        // TODO: We split by `char::is_alphabetic` delimiter.
+        //       More robust parsing methods might be needed.
+        let spos = price_in.find(char::is_alphabetic);
+
+        match spos {
+            Some(position) => {
+                let (price_str, denom) = price_in.split_at(position);
+
+                let price = price_str
+                    .parse::<f64>()
+                    .map_err(|_| Error::invalid_gas_price(price_in.to_string()))?;
+
+                Ok(GasPrice {
+                    price,
+                    denom: denom.to_owned(),
+                })
+            }
+
+            None => Err(Error::invalid_gas_price(price_in.to_string())),
+        }
+    }
+}
+
+// Note: Only `PartialOrd` is implemented for `GasPrice` because gas
+// prices must be of the same denomination in order to be compared.
+impl PartialOrd for GasPrice {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        if self.denom == other.denom {
+            self.price.partial_cmp(&other.price)
+        } else {
+            None
+        }
+    }
+}
+
+/// Attempts to parse 0 or more `GasPrice`s from a String,
+/// returning the successfully parsed prices in a Vec. Any
+/// single price that fails to be parsed does not affect
+/// the parsing of other prices.
+pub fn parse_gas_prices(prices: String) -> Vec<GasPrice> {
+    prices
+        .split(';')
+        .filter_map(|gp| GasPrice::from_str(gp).ok())
+        .collect()
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(
     rename_all = "snake_case",
@@ -482,7 +535,10 @@ pub(crate) fn store_writer(config: &Config, mut writer: impl Write) -> Result<()
 
 #[cfg(test)]
 mod tests {
-    use super::{load, store_writer};
+    use core::str::FromStr;
+
+    use super::{load, parse_gas_prices, store_writer};
+    use crate::config::GasPrice;
     use test_log::test;
 
     #[test]
@@ -509,4 +565,54 @@ mod tests {
         let mut buffer = Vec::new();
         store_writer(&config, &mut buffer).unwrap();
     }
+
+    #[test]
+    fn gas_price_from_str() {
+        let gp_original = GasPrice::new(10.0, "atom".to_owned());
+
+        let gp_raw = gp_original.to_string();
+        let gp = GasPrice::from_str(&gp_raw).expect("could not parse String into GasPrice");
+
+        assert_eq!(gp, gp_original);
+    }
+
+    #[test]
+    fn parse_multiple_gas_prices() {
+        let gas_prices = "0.25token1;0.0001token2";
+        let parsed = parse_gas_prices(gas_prices.to_string());
+
+        let expected = vec![
+            GasPrice {
+                price: 0.25,
+                denom: "token1".to_owned(),
+            },
+            GasPrice {
+                price: 0.0001,
+                denom: "token2".to_owned(),
+            },
+        ];
+
+        assert_eq!(expected, parsed);
+    }
+
+    #[test]
+    fn parse_empty_gas_price() {
+        let empty_price = "";
+        let parsed = parse_gas_prices(empty_price.to_string());
+
+        assert_eq!(parsed, vec![]);
+    }
+
+    #[test]
+    fn malformed_gas_prices_do_not_get_parsed() {
+        let malformed_prices = "token1;.token2;0.25token3";
+        let parsed = parse_gas_prices(malformed_prices.to_string());
+
+        let expected = vec![GasPrice {
+            price: 0.25,
+            denom: "token3".to_owned(),
+        }];
+
+        assert_eq!(expected, parsed);
+    }
 }

crates/relayer/src/config/error.rs

@@ -13,5 +13,9 @@ define_error! {
         Encode
             [ TraceError<toml::ser::Error> ]
             |_| { "invalid configuration" },
+
+        InvalidGasPrice
+            { price: String }
+            |e| { format!("invalid gas price: {}", e.price) },
     }
 }

crates/relayer/src/error.rs

@@ -37,6 +37,7 @@ use ibc_relayer_types::{
 
 use crate::chain::cosmos::version;
 use crate::chain::cosmos::BLOCK_MAX_BYTES_MAX_FRACTION;
+use crate::config::Error as ConfigError;
 use crate::event::monitor;
 use crate::keyring::errors::Error as KeyringError;
 use crate::sdk_error::SdkError;
@@ -56,6 +57,10 @@ define_error! {
             { query: AbciQuery }
             |e| { format!("ABCI query returned an error: {:?}", e.query) },
 
+        Config
+            [ ConfigError ]
+            |_| { "Configuration error" },
+
         CheckTx
             {
                 response: TxSyncResponse,
@@ -523,6 +528,9 @@ define_error! {
                 )
             },
 
+        GasPriceTooLow
+            { chain_id: ChainId }
+            |e| { format!("Hermes gas price is lower than the minimum gas price set by node operator'{}'", e.chain_id) },
 
         TxIndexingDisabled
             { chain_id: ChainId }