From 31856c7f446de69e9f5ac9bb2651fce1dd2fa7b8 Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Wed, 28 Jul 2021 19:08:57 +0200 Subject: [PATCH] Fix for #1245 --- relayer/src/chain/cosmos.rs | 36 ++++++++++---- relayer/src/chain/mock.rs | 4 +- relayer/src/config.rs | 8 ++- relayer/src/config/types.rs | 97 +++++++++++++++++++++++++++++++++++++ relayer/src/error.rs | 11 +++++ 5 files changed, 143 insertions(+), 13 deletions(-) create mode 100644 relayer/src/config/types.rs diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index b4cb7ddd1d..1431bc8e07 100644 --- a/relayer/src/chain/cosmos.rs +++ b/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,18 +112,20 @@ 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 /// further checks. fn health_checkup(&self) { - async fn do_health_checkup(chain: &CosmosSdkChain) -> Result<(), Error> { + async fn do_health_checkup(chain: &CosmosSdkChain, max_tx_size: f64) -> Result<(), Error> { let chain_id = chain.id(); let grpc_address = chain.grpc_addr.to_string(); let rpc_address = chain.config.rpc_addr.to_string(); @@ -161,6 +160,7 @@ impl CosmosSdkChain { ) })?; + // Construct a grpc client let mut client = ServiceClient::connect(chain.grpc_addr.clone()) .await .map_err(|e| { @@ -200,10 +200,28 @@ impl CosmosSdkChain { )); } + // Check on the configured max_tx_size against genesis block max_bytes parameter + let a = chain.rpc_client.genesis().await.map_err(|e| { + Error::health_check_json_rpc( + chain_id.clone(), + rpc_address.clone(), + "/genesis".to_string(), + e, + ) + })?; + let max_allowed = a.consensus_params.block.max_bytes as f64 * 0.9; + if max_tx_size >= max_allowed { + return Err(Error::health_check_tx_size_out_of_bounds( + chain_id.clone(), + max_tx_size, + a.consensus_params.block.max_bytes, + )); + } + Ok(()) } - if let Err(e) = self.block_on(do_health_checkup(self)) { + if let Err(e) = self.block_on(do_health_checkup(self, self.max_tx_size() as f64)) { warn!("{}", e); warn!("some Hermes features may not work in this mode!"); } @@ -378,12 +396,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 { diff --git a/relayer/src/chain/mock.rs b/relayer/src/chain/mock.rs index f88659b952..1e8c2b8c9b 100644 --- a/relayer/src/chain/mock.rs +++ b/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(), diff --git a/relayer/src/config.rs b/relayer/src/config.rs index 93d0adbcbb..cce08b279e 100644 --- a/relayer/src/config.rs +++ b/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, pub gas_adjustment: Option, - pub max_msg_num: Option, - pub max_tx_size: Option, + #[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")] diff --git a/relayer/src/config/types.rs b/relayer/src/config/types.rs new file mode 100644 index 0000000000..5b991fa3e5 --- /dev/null +++ b/relayer/src/config/types.rs @@ -0,0 +1,97 @@ +//! 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}; + +const DEFAULT_MAX_MSG_NUM: usize = 30; +const DEFAULT_MAX_TX_SIZE: usize = 2 * 1048576; // 2 MBytes + +const BOUND_MAX_MSG_NUM: usize = 100; +const BOUND_MAX_TX_SIZE: usize = 8 * 1048576; // 8 MBytes + +#[derive(Debug, Clone, Copy)] +pub struct MaxMsgNum(usize); + +impl Default for MaxMsgNum { + fn default() -> Self { + Self(DEFAULT_MAX_MSG_NUM) + } +} + +impl<'de> Deserialize<'de> for MaxMsgNum { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let u = usize::deserialize(deserializer)?; + + if u > BOUND_MAX_MSG_NUM { + return Err(D::Error::invalid_value( + Unexpected::Unsigned(u as u64), + &format!("a usize less than {}", BOUND_MAX_MSG_NUM).as_str(), + )); + } + + Ok(MaxMsgNum(u)) + } +} + +impl Serialize for MaxMsgNum { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.to_string().serialize(serializer) + } +} + +impl From for usize { + fn from(m: MaxMsgNum) -> Self { + m.0 + } +} + +#[derive(Debug, Clone, Copy)] +pub struct MaxTxSize(usize); + +impl Default for MaxTxSize { + fn default() -> Self { + Self(DEFAULT_MAX_TX_SIZE) + } +} + +impl<'de> Deserialize<'de> for MaxTxSize { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let u = usize::deserialize(deserializer)?; + + if u > BOUND_MAX_TX_SIZE { + return Err(D::Error::invalid_value( + Unexpected::Unsigned(u as u64), + &format!("a usize less than {}", BOUND_MAX_TX_SIZE).as_str(), + )); + } + + Ok(MaxTxSize(u)) + } +} + +impl Serialize for MaxTxSize { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.to_string().serialize(serializer) + } +} + +impl From for usize { + fn from(m: MaxTxSize) -> Self { + m.0 + } +} diff --git a/relayer/src/error.rs b/relayer/src/error.rs index b3cd9f61a3..fda36cb3c2 100644 --- a/relayer/src/error.rs +++ b/relayer/src/error.rs @@ -373,6 +373,17 @@ define_error! { e.endpoint, e.chain_id, e.address) }, + HealthCheckTxSizeOutOfBounds + { + chain_id: ChainId, + configured_bound: f64, + 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,