From 31856c7f446de69e9f5ac9bb2651fce1dd2fa7b8 Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Wed, 28 Jul 2021 19:08:57 +0200 Subject: [PATCH 01/10] 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, From c62b5cb62050d3ac4100c08e45b7ce1169eec8e2 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 13:30:26 +0200 Subject: [PATCH 02/10] Split out params semantic validation from health check --- relayer/src/chain/cosmos.rs | 44 ++++++++++++++++++++++++++----------- relayer/src/config/types.rs | 32 +++++++++++++++------------ relayer/src/error.rs | 4 ++-- 3 files changed, 51 insertions(+), 29 deletions(-) diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 1431bc8e07..743f15fc73 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -125,7 +125,7 @@ impl CosmosSdkChain { /// Exits early if any health check fails, without doing any /// further checks. fn health_checkup(&self) { - async fn do_health_checkup(chain: &CosmosSdkChain, max_tx_size: f64) -> Result<(), Error> { + async fn do_health_checkup(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(); @@ -200,30 +200,46 @@ impl CosmosSdkChain { )); } + Ok(()) + } + + if let Err(e) = self.block_on(do_health_checkup(self)) { + 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 a = chain.rpc_client.genesis().await.map_err(|e| { + let genesis = chain.block_on(chain.rpc_client.genesis()).map_err(|e| { Error::health_check_json_rpc( - chain_id.clone(), - rpc_address.clone(), + chain.id().clone(), + chain.config.rpc_addr.to_string(), "/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, + + 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) = 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!"); + 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!"); } } @@ -695,6 +711,7 @@ impl Chain for CosmosSdkChain { }; chain.health_checkup(); + chain.validate_params(); Ok(chain) } @@ -1982,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; diff --git a/relayer/src/config/types.rs b/relayer/src/config/types.rs index 5b991fa3e5..0f2b58c6cd 100644 --- a/relayer/src/config/types.rs +++ b/relayer/src/config/types.rs @@ -6,18 +6,17 @@ 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 MaxMsgNum { + const DEFAULT: usize = 30; + const MAX_BOUND: usize = 100; +} + impl Default for MaxMsgNum { fn default() -> Self { - Self(DEFAULT_MAX_MSG_NUM) + Self(Self::DEFAULT) } } @@ -28,10 +27,10 @@ impl<'de> Deserialize<'de> for MaxMsgNum { { let u = usize::deserialize(deserializer)?; - if u > BOUND_MAX_MSG_NUM { + if u > Self::MAX_BOUND { return Err(D::Error::invalid_value( Unexpected::Unsigned(u as u64), - &format!("a usize less than {}", BOUND_MAX_MSG_NUM).as_str(), + &format!("a usize less than {}", Self::MAX_BOUND).as_str(), )); } @@ -44,7 +43,7 @@ impl Serialize for MaxMsgNum { where S: Serializer, { - self.0.to_string().serialize(serializer) + self.0.serialize(serializer) } } @@ -57,9 +56,14 @@ impl From for usize { #[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(DEFAULT_MAX_TX_SIZE) + Self(Self::DEFAULT) } } @@ -70,10 +74,10 @@ impl<'de> Deserialize<'de> for MaxTxSize { { let u = usize::deserialize(deserializer)?; - if u > BOUND_MAX_TX_SIZE { + if u > Self::MAX_BOUND { return Err(D::Error::invalid_value( Unexpected::Unsigned(u as u64), - &format!("a usize less than {}", BOUND_MAX_TX_SIZE).as_str(), + &format!("a usize less than {}", Self::MAX_BOUND).as_str(), )); } @@ -86,7 +90,7 @@ impl Serialize for MaxTxSize { where S: Serializer, { - self.0.to_string().serialize(serializer) + self.0.serialize(serializer) } } diff --git a/relayer/src/error.rs b/relayer/src/error.rs index fda36cb3c2..165a89eb08 100644 --- a/relayer/src/error.rs +++ b/relayer/src/error.rs @@ -373,10 +373,10 @@ define_error! { e.endpoint, e.chain_id, e.address) }, - HealthCheckTxSizeOutOfBounds + TxSizeOutOfBounds { chain_id: ChainId, - configured_bound: f64, + configured_bound: usize, genesis_bound: u64, } |e| { From 5b2f323890bfbfdecbae89e71ae0203fe445b7e9 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 13:36:31 +0200 Subject: [PATCH 03/10] Fixup changelog entry --- .changelog/unreleased/features/988-flex-error.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.changelog/unreleased/features/988-flex-error.md b/.changelog/unreleased/features/988-flex-error.md index 402a1a4e03..d89a1b6525 100644 --- a/.changelog/unreleased/features/988-flex-error.md +++ b/.changelog/unreleased/features/988-flex-error.md @@ -1,2 +1,4 @@ -Use the [`flex-error`](https://docs.rs/flex-error/) crate to define and -handle errors. +- Use the [`flex-error`](https://docs.rs/flex-error/) crate to define and +handle errors ([#1158]) + +[#1158]: https://github.com/informalsystems/ibc-rs/issues/1158 From 0913d9cb04b08764d49549c5f28e4e70ef7a3309 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 13:36:38 +0200 Subject: [PATCH 04/10] Add .changelog entry --- .../unreleased/improvements/1245-max-params-validation.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/improvements/1245-max-params-validation.md diff --git a/.changelog/unreleased/improvements/1245-max-params-validation.md b/.changelog/unreleased/improvements/1245-max-params-validation.md new file mode 100644 index 0000000000..f0205f6b39 --- /dev/null +++ b/.changelog/unreleased/improvements/1245-max-params-validation.md @@ -0,0 +1,3 @@ +- Add semantic validation of of `max_tx_size` and `max_num_msg` config options ([#1245]) + +[#1245]: https://github.com/informalsystems/ibc-rs/issues/1245 From 6602a902c7f423e27c0de1c7659d024fe02fdb3e Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 13:38:14 +0200 Subject: [PATCH 05/10] Extract max fraction of genesis block max size into a consant --- relayer/src/chain/cosmos.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 743f15fc73..32fb340da3 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -89,6 +89,8 @@ mod compatibility; const DEFAULT_MAX_GAS: u64 = 300_000; const DEFAULT_GAS_PRICE_ADJUSTMENT: f64 = 0.1; +const GENESIS_MAX_BYTES_MAX_FRACTION: f64 = 0.9; + mod retry_strategy { use crate::util::retry::Fixed; use std::time::Duration; @@ -223,7 +225,7 @@ impl CosmosSdkChain { })?; let genesis_max_bound = genesis.consensus_params.block.max_bytes; - let max_allowed = mul_ceil(genesis_max_bound, 0.9) as usize; + let max_allowed = mul_ceil(genesis_max_bound, GENESIS_MAX_BYTES_MAX_FRACTION) as usize; if chain.max_tx_size() > max_allowed { return Err(Error::tx_size_out_of_bounds( From 33183a1c2be84f26877fd380abe7360a2ab4cf68 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 14:18:52 +0200 Subject: [PATCH 06/10] Doc comments for constants --- relayer/src/chain/cosmos.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 32fb340da3..e7dcc49cc2 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -86,9 +86,14 @@ use super::Chain; mod compatibility; +/// Default gas limit when submitting a transaction. const DEFAULT_MAX_GAS: u64 = 300_000; + +/// Fraction of the estimated gas to add to the gas limit when submitting a transaction. const DEFAULT_GAS_PRICE_ADJUSTMENT: f64 = 0.1; +/// Upper limit on the size of transactions submitted by Hermes, expressed as a +/// fraction of the maximum block size defined in the Tendermint core consensus parameters. const GENESIS_MAX_BYTES_MAX_FRACTION: f64 = 0.9; mod retry_strategy { From 55105b5ef33725aaee5ae93ac67c0113125d7025 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 14:34:22 +0200 Subject: [PATCH 07/10] Harmonize error messages --- relayer/src/chain/cosmos.rs | 8 ++++---- relayer/src/error.rs | 28 ++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index e7dcc49cc2..c79aabce8d 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -171,7 +171,7 @@ impl CosmosSdkChain { let mut client = ServiceClient::connect(chain.grpc_addr.clone()) .await .map_err(|e| { - Error::health_check_json_grpc_transport( + Error::health_check_grpc_transport( chain_id.clone(), rpc_address.clone(), "tendermint::ServiceClient".to_string(), @@ -182,7 +182,7 @@ impl CosmosSdkChain { let request = tonic::Request::new(GetNodeInfoRequest {}); let response = client.get_node_info(request).await.map_err(|e| { - Error::health_check_json_grpc_status( + Error::health_check_grpc_status( chain_id.clone(), rpc_address.clone(), "tendermint::ServiceClient".to_string(), @@ -221,7 +221,7 @@ impl CosmosSdkChain { 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( + Error::config_validation_json_rpc( chain.id().clone(), chain.config.rpc_addr.to_string(), "/genesis".to_string(), @@ -233,7 +233,7 @@ impl CosmosSdkChain { let max_allowed = mul_ceil(genesis_max_bound, GENESIS_MAX_BYTES_MAX_FRACTION) as usize; if chain.max_tx_size() > max_allowed { - return Err(Error::tx_size_out_of_bounds( + return Err(Error::config_validation_tx_size_out_of_bounds( chain.id().clone(), chain.max_tx_size(), genesis_max_bound, diff --git a/relayer/src/error.rs b/relayer/src/error.rs index 165a89eb08..96ec7b0cec 100644 --- a/relayer/src/error.rs +++ b/relayer/src/error.rs @@ -334,11 +334,11 @@ define_error! { } [ DisplayOnly ] |e| { - format!("Hermes health check failed for endpoint {0} on the Json RPC interface of chain {1}:{2}", + format!("health check failed for endpoint {0} on the JSON-RPC interface of chain {1}:{2}", e.endpoint, e.chain_id, e.address) }, - HealthCheckJsonGrpcTransport + HealthCheckGrpcTransport { chain_id: ChainId, address: String, @@ -346,11 +346,11 @@ define_error! { } [ DisplayOnly ] |e| { - format!("Hermes health check failed for endpoint {0} on the Json RPC interface of chain {1}:{2}", + format!("health check failed for endpoint {0} on the gRPC interface of chain {1}:{2}", e.endpoint, e.chain_id, e.address) }, - HealthCheckJsonGrpcStatus + HealthCheckGrpcStatus { chain_id: ChainId, address: String, @@ -358,7 +358,7 @@ define_error! { status: tonic::Status } |e| { - format!("Hermes health check failed for endpoint {0} on the Json RPC interface of chain {1}:{2}; caused by: {3}", + format!("health check failed for endpoint {0} on the gRPC interface of chain {1}:{2}; caused by: {3}", e.endpoint, e.chain_id, e.address, e.status) }, @@ -369,18 +369,30 @@ define_error! { endpoint: String, } |e| { - format!("Hermes health check failed for endpoint {0} on the Json RPC interface of chain {1}:{2}; the gRPC response contains no application version information", + format!("health check failed for endpoint {0} on the Json RPC interface of chain {1}:{2}; the gRPC response contains no application version information", e.endpoint, e.chain_id, e.address) }, - TxSizeOutOfBounds + ConfigValidationJsonRpc + { + chain_id: ChainId, + address: String, + endpoint: String, + } + [ DisplayOnly ] + |e| { + format!("semantic config validation failed for endpoint {0} on the JSON-RPC interface of chain {1}:{2}", + e.endpoint, e.chain_id, e.address) + }, + + ConfigValidationTxSizeOutOfBounds { 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'={}", + format!("semantic config validation failed for option `max_tx_size` chain '{}', reason: `max_tx_size` = {} which is over 90% of genesis block param .`max_size` = {}", e.chain_id, e.configured_bound, e.genesis_bound) }, From 43173d9ae91317f20661ac4e7808b999fbf22f87 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 14:58:38 +0200 Subject: [PATCH 08/10] Improve error message layout --- relayer/src/chain/cosmos.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index c79aabce8d..806ac20f3c 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -212,8 +212,8 @@ impl CosmosSdkChain { if let Err(e) = self.block_on(do_health_checkup(self)) { warn!("Health checkup for chain '{}' failed", self.id()); - warn!("Reason: {}", e); - warn!("Some Hermes features may not work in this mode!"); + warn!(" Reason: {}", e); + warn!(" Some Hermes features may not work in this mode!"); } } @@ -245,8 +245,8 @@ impl CosmosSdkChain { 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!"); + warn!(" Reason: {}", e); + warn!(" Some Hermes features may not work in this mode!"); } } From 37aae44810970b32a7562a1655b2502d87f22bc3 Mon Sep 17 00:00:00 2001 From: Adi Seredinschi Date: Thu, 29 Jul 2021 15:44:41 +0200 Subject: [PATCH 09/10] Comments & output consistent with parameters --- relayer/src/chain/cosmos.rs | 14 ++++++++++---- relayer/src/error.rs | 7 ++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 806ac20f3c..b61eb8979c 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -94,7 +94,7 @@ const DEFAULT_GAS_PRICE_ADJUSTMENT: f64 = 0.1; /// Upper limit on the size of transactions submitted by Hermes, expressed as a /// fraction of the maximum block size defined in the Tendermint core consensus parameters. -const GENESIS_MAX_BYTES_MAX_FRACTION: f64 = 0.9; +pub const GENESIS_MAX_BYTES_MAX_FRACTION: f64 = 0.9; mod retry_strategy { use crate::util::retry::Fixed; @@ -119,14 +119,12 @@ pub struct CosmosSdkChain { impl CosmosSdkChain { /// Does multiple RPC calls to the full node, to check for - /// reachability, some basic APIs are available, and that - /// Hermes configuration is appropriate. + /// reachability and some basic APIs are available. /// /// Currently this checks that: /// - the node responds OK to `/health` RPC call; /// - the node has transaction indexing enabled; /// - 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 @@ -217,6 +215,14 @@ impl CosmosSdkChain { } } + /// Performs validation of chain-specific configuration + /// parameters against the chain's genesis configuration. + /// + /// Currently, validates the following: + /// - the configured `max_tx_size` is appropriate. + /// + /// Emits a log warning in case any error is encountered and + /// exits early without doing subsequent validations. 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 diff --git a/relayer/src/error.rs b/relayer/src/error.rs index 96ec7b0cec..1bf60641d3 100644 --- a/relayer/src/error.rs +++ b/relayer/src/error.rs @@ -27,6 +27,7 @@ use ibc::{ proofs::ProofError, }; +use crate::chain::cosmos::GENESIS_MAX_BYTES_MAX_FRACTION; use crate::event::monitor; define_error! { @@ -381,7 +382,7 @@ define_error! { } [ DisplayOnly ] |e| { - format!("semantic config validation failed for endpoint {0} on the JSON-RPC interface of chain {1}:{2}", + format!("semantic config validation: failed to reach endpoint {0} on the JSON-RPC interface of chain {1}:{2}", e.endpoint, e.chain_id, e.address) }, @@ -392,8 +393,8 @@ define_error! { genesis_bound: u64, } |e| { - format!("semantic config validation failed for option `max_tx_size` chain '{}', reason: `max_tx_size` = {} which is over 90% of genesis block param .`max_size` = {}", - e.chain_id, e.configured_bound, e.genesis_bound) + format!("semantic config validation failed for option `max_tx_size` chain '{}', reason: `max_tx_size` = {} is over {} of genesis block param .`max_size` = {}", + e.chain_id, e.configured_bound, GENESIS_MAX_BYTES_MAX_FRACTION, e.genesis_bound) }, SdkModuleVersion From 8283bd0e14b68a0ceb5a1fc44b790dd7e971b588 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 29 Jul 2021 15:50:36 +0200 Subject: [PATCH 10/10] Final aestetic change --- relayer/src/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/relayer/src/error.rs b/relayer/src/error.rs index 1bf60641d3..07c7686116 100644 --- a/relayer/src/error.rs +++ b/relayer/src/error.rs @@ -393,8 +393,8 @@ define_error! { genesis_bound: u64, } |e| { - format!("semantic config validation failed for option `max_tx_size` chain '{}', reason: `max_tx_size` = {} is over {} of genesis block param .`max_size` = {}", - e.chain_id, e.configured_bound, GENESIS_MAX_BYTES_MAX_FRACTION, e.genesis_bound) + format!("semantic config validation failed for option `max_tx_size` chain '{}', reason: `max_tx_size` = {} is greater than {}% of the genesis block param `max_size` = {}", + e.chain_id, e.configured_bound, GENESIS_MAX_BYTES_MAX_FRACTION * 100.0, e.genesis_bound) }, SdkModuleVersion