Domain type & semantic validation of max_tx_size and max_num_msg config options

relayer/src/chain/cosmos.rs

@@ -89,9 +89,6 @@ mod compatibility;
 const DEFAULT_MAX_GAS: u64 = 300_000;
 const DEFAULT_GAS_PRICE_ADJUSTMENT: f64 = 0.1;
 
-const DEFAULT_MAX_MSG_NUM: usize = 30;
-const DEFAULT_MAX_TX_SIZE: usize = 2 * 1048576; // 2 MBytes
-
 mod retry_strategy {
     use crate::util::retry::Fixed;
     use std::time::Duration;
@@ -115,12 +112,14 @@ pub struct CosmosSdkChain {
 
 impl CosmosSdkChain {
     /// Does multiple RPC calls to the full node, to check for
-    /// reachability and that some basic APIs are available.
+    /// reachability, some basic APIs are available, and that
+    /// Hermes configuration is appropriate.
     ///
     /// Currently this checks that:
     ///     - the node responds OK to `/health` RPC call;
     ///     - the node has transaction indexing enabled;
-    ///     - the SDK version is supported.
+    ///     - the SDK version is supported;
+    ///     - the configured `max_tx_size` is appropriate.
     ///
     /// Emits a log warning in case anything is amiss.
     /// Exits early if any health check fails, without doing any
@@ -161,6 +160,7 @@ impl CosmosSdkChain {
                     )
                 })?;
 
+            // Construct a grpc client
             let mut client = ServiceClient::connect(chain.grpc_addr.clone())
                 .await
                 .map_err(|e| {
@@ -204,8 +204,42 @@ impl CosmosSdkChain {
         }
 
         if let Err(e) = self.block_on(do_health_checkup(self)) {
-            warn!("{}", e);
-            warn!("some Hermes features may not work in this mode!");
+            warn!("Health checkup for chain '{}' failed", self.id());
+            warn!("Reason: {}", e);
+            warn!("Some Hermes features may not work in this mode!");
+        }
+    }
+
+    pub fn validate_params(&self) {
+        fn do_validate_params(chain: &CosmosSdkChain) -> Result<(), Error> {
+            // Check on the configured max_tx_size against genesis block max_bytes parameter
+            let genesis = chain.block_on(chain.rpc_client.genesis()).map_err(|e| {
+                Error::health_check_json_rpc(
+                    chain.id().clone(),
+                    chain.config.rpc_addr.to_string(),
+                    "/genesis".to_string(),
+                    e,
+                )
+            })?;
+
+            let genesis_max_bound = genesis.consensus_params.block.max_bytes;
+            let max_allowed = mul_ceil(genesis_max_bound, 0.9) as usize;
+
+            if chain.max_tx_size() > max_allowed {
+                return Err(Error::tx_size_out_of_bounds(
+                    chain.id().clone(),
+                    chain.max_tx_size(),
+                    genesis_max_bound,
+                ));
+            }
+
+            Ok(())
+        }
+
+        if let Err(e) = do_validate_params(self) {
+            warn!("Hermes might be misconfigured for chain '{}'", self.id());
+            warn!("Reason: {}", e);
+            warn!("Some Hermes features may not work in this mode!");
         }
     }
 
@@ -378,12 +412,12 @@ impl CosmosSdkChain {
 
     /// The maximum number of messages included in a transaction
     fn max_msg_num(&self) -> usize {
-        self.config.max_msg_num.unwrap_or(DEFAULT_MAX_MSG_NUM)
+        self.config.max_msg_num.into()
     }
 
     /// The maximum size of any transaction sent by the relayer to this chain
     fn max_tx_size(&self) -> usize {
-        self.config.max_tx_size.unwrap_or(DEFAULT_MAX_TX_SIZE)
+        self.config.max_tx_size.into()
     }
 
     fn query(&self, data: Path, height: ICSHeight, prove: bool) -> Result<QueryResponse, Error> {
@@ -677,6 +711,7 @@ impl Chain for CosmosSdkChain {
         };
 
         chain.health_checkup();
+        chain.validate_params();
 
         Ok(chain)
     }
@@ -1964,6 +1999,7 @@ fn calculate_fee(adjusted_gas_amount: u64, gas_price: &GasPrice) -> Coin {
     }
 }
 
+/// Multiply `a` with `f` and round to result up to the nearest integer.
 fn mul_ceil(a: u64, f: f64) -> u64 {
     use fraction::Fraction as F;
 

relayer/src/chain/mock.rs

@@ -406,8 +406,8 @@ pub mod test_utils {
             max_gas: None,
             gas_price: GasPrice::new(0.001, "uatom".to_string()),
             gas_adjustment: None,
-            max_msg_num: None,
-            max_tx_size: None,
+            max_msg_num: Default::default(),
+            max_tx_size: Default::default(),
             clock_drift: Duration::from_secs(5),
             trusting_period: Duration::from_secs(14 * 24 * 60 * 60), // 14 days
             trust_threshold: Default::default(),

relayer/src/config.rs

@@ -1,6 +1,7 @@
 //! Relayer configuration
 
 pub mod reload;
+pub mod types;
 
 use std::collections::{HashMap, HashSet};
 use std::{fmt, fs, fs::File, io::Write, path::Path, time::Duration};
@@ -11,6 +12,7 @@ use tendermint_light_client::types::TrustThreshold;
 use ibc::ics24_host::identifier::{ChainId, ChannelId, PortId};
 use ibc::timestamp::ZERO_DURATION;
 
+use crate::config::types::{MaxMsgNum, MaxTxSize};
 use crate::error::Error;
 
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
@@ -254,8 +256,10 @@ pub struct ChainConfig {
     pub store_prefix: String,
     pub max_gas: Option<u64>,
     pub gas_adjustment: Option<f64>,
-    pub max_msg_num: Option<usize>,
-    pub max_tx_size: Option<usize>,
+    #[serde(default)]
+    pub max_msg_num: MaxMsgNum,
+    #[serde(default)]
+    pub max_tx_size: MaxTxSize,
     #[serde(default = "default::clock_drift", with = "humantime_serde")]
     pub clock_drift: Duration,
     #[serde(default = "default::trusting_period", with = "humantime_serde")]

relayer/src/config/types.rs

@@ -0,0 +1,101 @@
+//! Configuration-related types.
+//!
+//! Implements defaults, as well as serializing and
+//! deserializing with upper-bound verification.
+
+use serde::de::Unexpected;
+use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer};
+
+#[derive(Debug, Clone, Copy)]
+pub struct MaxMsgNum(usize);
+
+impl MaxMsgNum {
+    const DEFAULT: usize = 30;
+    const MAX_BOUND: usize = 100;
+}
+
+impl Default for MaxMsgNum {
+    fn default() -> Self {
+        Self(Self::DEFAULT)
+    }
+}
+
+impl<'de> Deserialize<'de> for MaxMsgNum {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let u = usize::deserialize(deserializer)?;
+
+        if u > Self::MAX_BOUND {
+            return Err(D::Error::invalid_value(
+                Unexpected::Unsigned(u as u64),
+                &format!("a usize less than {}", Self::MAX_BOUND).as_str(),
+            ));
+        }
+
+        Ok(MaxMsgNum(u))
+    }
+}
+
+impl Serialize for MaxMsgNum {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        self.0.serialize(serializer)
+    }
+}
+
+impl From<MaxMsgNum> for usize {
+    fn from(m: MaxMsgNum) -> Self {
+        m.0
+    }
+}
+
+#[derive(Debug, Clone, Copy)]
+pub struct MaxTxSize(usize);
+
+impl MaxTxSize {
+    const DEFAULT: usize = 2 * 1048576; // 2 MBytes
+    const MAX_BOUND: usize = 8 * 1048576; // 8 MBytes
+}
+
+impl Default for MaxTxSize {
+    fn default() -> Self {
+        Self(Self::DEFAULT)
+    }
+}
+
+impl<'de> Deserialize<'de> for MaxTxSize {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let u = usize::deserialize(deserializer)?;
+
+        if u > Self::MAX_BOUND {
+            return Err(D::Error::invalid_value(
+                Unexpected::Unsigned(u as u64),
+                &format!("a usize less than {}", Self::MAX_BOUND).as_str(),
+            ));
+        }
+
+        Ok(MaxTxSize(u))
+    }
+}
+
+impl Serialize for MaxTxSize {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        self.0.serialize(serializer)
+    }
+}
+
+impl From<MaxTxSize> for usize {
+    fn from(m: MaxTxSize) -> Self {
+        m.0
+    }
+}

relayer/src/error.rs

@@ -373,6 +373,17 @@ define_error! {
                     e.endpoint, e.chain_id, e.address)
             },
 
+        TxSizeOutOfBounds
+            {
+                chain_id: ChainId,
+                configured_bound: usize,
+                genesis_bound: u64,
+            }
+            |e| {
+                format!("Hermes health check failed for max_tx_size bound of chain id {}: Hermes configured with 'max_tx_size'={} which is over 90% of genesis block param '.max_bytes'={}",
+                    e.chain_id, e.configured_bound, e.genesis_bound)
+            },
+
         SdkModuleVersion
             {
                 chain_id: ChainId,