diff --git a/.changelog/unreleased/bug-fixes/ibc-relayer/1971-non-batch-fix.md b/.changelog/unreleased/bug-fixes/ibc-relayer/1971-non-batch-fix.md new file mode 100644 index 0000000000..acf9f7a16f --- /dev/null +++ b/.changelog/unreleased/bug-fixes/ibc-relayer/1971-non-batch-fix.md @@ -0,0 +1,2 @@ +- Fix a bug where connection and channel handshakes would fail with non-batching transactions + ([#1971](https://github.com/informalsystems/ibc-rs/issues/1971)) diff --git a/.changelog/unreleased/improvements/ibc-relayer/1971-max-msg-num-min-bound.md b/.changelog/unreleased/improvements/ibc-relayer/1971-max-msg-num-min-bound.md new file mode 100644 index 0000000000..b416ad6d92 --- /dev/null +++ b/.changelog/unreleased/improvements/ibc-relayer/1971-max-msg-num-min-bound.md @@ -0,0 +1,2 @@ +- Ensure `max_msg_num` is between 1 and 100 with a default of 30 + ([#1971](https://github.com/informalsystems/ibc-rs/issues/1971)) diff --git a/.changelog/unreleased/improvements/relayer/2031-misleading-misbehavior-error.md b/.changelog/unreleased/improvements/ibc-relayer/2031-misleading-misbehavior-error.md similarity index 100% rename from .changelog/unreleased/improvements/relayer/2031-misleading-misbehavior-error.md rename to .changelog/unreleased/improvements/ibc-relayer/2031-misleading-misbehavior-error.md diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 9c0b84a87a..867a1989b3 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -121,7 +121,7 @@ fn batch_messages( max_tx_size: MaxTxSize, messages: Vec, ) -> Result>, Error> { - let max_message_count = max_msg_num.0; + let max_message_count = max_msg_num.to_usize(); let max_tx_size = max_tx_size.into(); let mut batches = vec![]; diff --git a/relayer/src/chain/cosmos/estimate.rs b/relayer/src/chain/cosmos/estimate.rs index 15fba52d22..5b6379cc14 100644 --- a/relayer/src/chain/cosmos/estimate.rs +++ b/relayer/src/chain/cosmos/estimate.rs @@ -121,7 +121,7 @@ async fn estimate_gas_with_tx( } // If there is a chance that the tx will be accepted once actually submitted, we fall - // back on the max gas and will attempt to send it anyway. + // back on the default gas and will attempt to send it anyway. // See `can_recover_from_simulation_failure` for more info. Err(e) if can_recover_from_simulation_failure(&e) => { warn!( diff --git a/relayer/src/config/error.rs b/relayer/src/config/error.rs index 94287b285e..a24078e666 100644 --- a/relayer/src/config/error.rs +++ b/relayer/src/config/error.rs @@ -13,6 +13,5 @@ define_error! { Encode [ TraceError ] |_| { "invalid configuration" }, - } } diff --git a/relayer/src/config/types.rs b/relayer/src/config/types.rs index ac2db093d1..7b6a7bf5ef 100644 --- a/relayer/src/config/types.rs +++ b/relayer/src/config/types.rs @@ -3,165 +3,320 @@ //! Implements defaults, as well as serializing and //! deserializing with upper-bound verification. -use core::fmt; +pub use max_msg_num::MaxMsgNum; -use serde::de::Unexpected; -use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer}; +pub mod max_msg_num { + flex_error::define_error! { + Error { + TooSmall + { value: usize } + |e| { + format_args!("`max_msg_num` must be greater than or equal to {}, found {}", + MaxMsgNum::MIN_BOUND, e.value) + }, -#[derive(Debug, Clone, Copy)] -pub struct MaxMsgNum(pub usize); + TooBig + { value: usize } + |e| { + format_args!("`max_msg_num` must be less than or equal to {}, found {}", + MaxMsgNum::MAX_BOUND, e.value) + }, + } + } -impl MaxMsgNum { - const DEFAULT: usize = 30; - const MAX_BOUND: usize = 100; -} + #[derive(Debug, Clone, Copy)] + pub struct MaxMsgNum(usize); + + impl MaxMsgNum { + const DEFAULT: usize = 30; + const MIN_BOUND: usize = 1; + const MAX_BOUND: usize = 100; + + pub fn new(value: usize) -> Result { + if value < Self::MIN_BOUND { + return Err(Error::too_small(value)); + } -impl Default for MaxMsgNum { - fn default() -> Self { - Self(Self::DEFAULT) + if value > Self::MAX_BOUND { + return Err(Error::too_big(value)); + } + + Ok(Self(value)) + } + + pub fn to_usize(self) -> usize { + self.0 + } + } + + impl Default for MaxMsgNum { + fn default() -> Self { + Self(Self::DEFAULT) + } } -} -impl<'de> Deserialize<'de> for MaxMsgNum { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let u = usize::deserialize(deserializer)?; + use serde::de::Unexpected; + use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; - 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(), - )); + impl<'de> Deserialize<'de> for MaxMsgNum { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = usize::deserialize(deserializer)?; + + MaxMsgNum::new(value).map_err(|e| match e.detail() { + ErrorDetail::TooSmall(_) => D::Error::invalid_value( + Unexpected::Unsigned(value as u64), + &format!("a usize greater than or equal to {}", Self::MIN_BOUND).as_str(), + ), + ErrorDetail::TooBig(_) => D::Error::invalid_value( + Unexpected::Unsigned(value as u64), + &format!("a usize less than or equal to {}", Self::MAX_BOUND).as_str(), + ), + }) } + } - Ok(MaxMsgNum(u)) + impl Serialize for MaxMsgNum { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.serialize(serializer) + } } -} -impl Serialize for MaxMsgNum { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - self.0.serialize(serializer) + impl From for usize { + fn from(m: MaxMsgNum) -> Self { + m.0 + } } } -impl From for usize { - fn from(m: MaxMsgNum) -> Self { - m.0 +pub use max_tx_size::MaxTxSize; + +pub mod max_tx_size { + flex_error::define_error! { + Error { + TooBig + { value: usize } + |e| { + format_args!("`max_tx_size` must be less than or equal to {}, found {}", + MaxTxSize::MAX_BOUND, e.value) + }, + } } -} -#[derive(Debug, Clone, Copy)] -pub struct MaxTxSize(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 MaxTxSize { + const DEFAULT: usize = 2 * 1048576; // 2 MBytes + const MAX_BOUND: usize = 8 * 1048576; // 8 MBytes + + pub fn new(value: usize) -> Result { + if value > Self::MAX_BOUND { + return Err(Error::too_big(value)); + } + + Ok(Self(value)) + } -impl Default for MaxTxSize { - fn default() -> Self { - Self(Self::DEFAULT) + pub fn to_usize(self) -> usize { + self.0 + } } -} -impl<'de> Deserialize<'de> for MaxTxSize { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let u = usize::deserialize(deserializer)?; + impl Default for MaxTxSize { + fn default() -> Self { + Self(Self::DEFAULT) + } + } + + use serde::de::Unexpected; + use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; + + impl<'de> Deserialize<'de> for MaxTxSize { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = 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(), - )); + MaxTxSize::new(value).map_err(|e| match e.detail() { + ErrorDetail::TooBig(_) => D::Error::invalid_value( + Unexpected::Unsigned(value as u64), + &format!("a usize less than or equal to {}", Self::MAX_BOUND).as_str(), + ), + }) } + } - Ok(MaxTxSize(u)) + impl Serialize for MaxTxSize { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.serialize(serializer) + } } -} -impl Serialize for MaxTxSize { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - self.0.serialize(serializer) + impl From for usize { + fn from(m: MaxTxSize) -> Self { + m.0 + } } } -impl From for usize { - fn from(m: MaxTxSize) -> Self { - m.0 +pub use memo::Memo; + +pub mod memo { + flex_error::define_error! { + Error { + TooLong + { length: usize } + |e| { + format_args!("`memo` must been no longer than {} characters, found length {}", + Memo::MAX_LEN, e.length) + } + } } -} -/// A memo domain-type. -/// -/// Hermes uses this type to populate the `tx.memo` field for -/// each transaction it submits. -/// The memo can be configured on a per-chain basis. -/// -#[derive(Clone, Debug, Default)] -pub struct Memo(String); + /// A memo domain-type. + /// + /// Hermes uses this type to populate the `tx.memo` field for + /// each transaction it submits. + /// The memo can be configured on a per-chain basis. + /// + #[derive(Clone, Debug, Default)] + pub struct Memo(String); + + impl Memo { + const MAX_LEN: usize = 50; + + pub fn new(memo: String) -> Result { + if memo.len() > Self::MAX_LEN { + return Err(Error::too_long(memo.len())); + } + + Ok(Self(memo)) + } -impl Memo { - const MAX_LEN: usize = 50; + pub fn apply_suffix(&mut self, suffix: &str) { + // Add a separator if the memo + // is pre-populated with some content already. + if !self.0.is_empty() { + self.0.push_str(" | "); + } - pub fn new(memo: &str) -> Self { - Self(memo.to_string()) + self.0.push_str(suffix); + } + + pub fn as_str(&self) -> &str { + &self.0 + } } - pub fn apply_suffix(&mut self, suffix: &str) { - // Add a separator if the memo - // is pre-populated with some content already. - if !self.0.is_empty() { - self.0.push_str(" | "); + use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; + + impl<'de> Deserialize<'de> for Memo { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = String::deserialize(deserializer)?; + + Memo::new(value).map_err(|e| match e.detail() { + ErrorDetail::TooLong(sub) => D::Error::invalid_length( + sub.length, + &format!("a string length of at most {}", Self::MAX_LEN).as_str(), + ), + }) } + } - self.0.push_str(suffix); + impl Serialize for Memo { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.serialize(serializer) + } } - pub fn as_str(&self) -> &str { - &self.0 + use core::fmt; + + impl fmt::Display for Memo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_str()) + } } } -impl<'de> Deserialize<'de> for Memo { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let m = String::deserialize(deserializer)?; +#[cfg(test)] +#[allow(dead_code)] // the fields of the structs defined below are never accessed +mod tests { + use super::*; + + use serde::Deserialize; + use test_log::test; - if m.len() > Self::MAX_LEN { - return Err(D::Error::invalid_length( - m.len(), - &format!("a string length of at most {}", Self::MAX_LEN).as_str(), - )); + #[test] + fn parse_invalid_max_msg_num_min() { + #[derive(Debug, Deserialize)] + struct DummyConfig { + max_msg_num: MaxMsgNum, } - Ok(Memo(m)) + let err = toml::from_str::("max_msg_num = 0") + .unwrap_err() + .to_string(); + + assert!(err.contains("expected a usize greater than or equal to")); } -} -impl Serialize for Memo { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - self.0.serialize(serializer) + #[test] + fn parse_invalid_max_msg_num_max() { + #[derive(Debug, Deserialize)] + struct DummyConfig { + max_msg_num: MaxMsgNum, + } + + let err = toml::from_str::("max_msg_num = 1024") + .unwrap_err() + .to_string(); + + assert!(err.contains("expected a usize less than or equal to")); } -} -impl fmt::Display for Memo { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.as_str()) + #[test] + fn parse_invalid_max_tx_size() { + #[derive(Debug, Deserialize)] + struct DummyConfig { + max_tx_size: MaxTxSize, + } + + let err = toml::from_str::("max_tx_size = 9999999999") + .unwrap_err() + .to_string(); + + assert!(err.contains("expected a usize less than or equal to")); + } + + #[test] + fn parse_invalid_memo() { + #[derive(Debug, Deserialize)] + struct DummyConfig { + memo: Memo, + } + + let err = toml::from_str::( + r#"memo = "foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz""#, + ) + .unwrap_err() + .to_string(); + + assert!(err.contains("a string length of at most")); } } diff --git a/relayer/src/error.rs b/relayer/src/error.rs index f349c43573..61e1e53b0a 100644 --- a/relayer/src/error.rs +++ b/relayer/src/error.rs @@ -521,19 +521,20 @@ impl Error { impl GrpcStatusSubdetail { /// Check whether this gRPC error matches - /// - status: InvalidArgument /// - message: verification failed: ... failed packet acknowledgement verification for client: client state height < proof height ... pub fn is_client_state_height_too_low(&self) -> bool { - if self.status.code() != tonic::Code::InvalidArgument { - return false; - } + // Gaia v6.0.1 (SDK 0.44.5) returns code`InvalidArgument`, whereas gaia v6.0.4 + // (SDK 0.44.6, and potentially others) returns code `Unknown`. + // Workaround by matching strictly on the status message. + // if self.status.code() != tonic::Code::InvalidArgument + // return false; + // } let msg = self.status.message(); msg.contains("verification failed") && msg.contains("client state height < proof height") } /// Check whether this gRPC error matches - /// - status: InvalidArgument /// - message: "account sequence mismatch, expected 166791, got 166793: incorrect account sequence: invalid request" /// /// # Note: diff --git a/tools/integration-test/src/tests/manual/simulation.rs b/tools/integration-test/src/tests/manual/simulation.rs index e30c4418ee..cac7b3bedb 100644 --- a/tools/integration-test/src/tests/manual/simulation.rs +++ b/tools/integration-test/src/tests/manual/simulation.rs @@ -29,7 +29,7 @@ pub struct SimulationTest; impl TestOverrides for SimulationTest { fn modify_relayer_config(&self, config: &mut Config) { for mut chain in config.chains.iter_mut() { - chain.max_msg_num = MaxMsgNum(MAX_MSGS); + chain.max_msg_num = MaxMsgNum::new(MAX_MSGS).unwrap(); } } } diff --git a/tools/integration-test/src/tests/memo.rs b/tools/integration-test/src/tests/memo.rs index 5a60254b38..b77f501e50 100644 --- a/tools/integration-test/src/tests/memo.rs +++ b/tools/integration-test/src/tests/memo.rs @@ -7,7 +7,7 @@ use ibc_test_framework::util::random::{random_string, random_u64_range}; #[test] fn test_memo() -> Result<(), Error> { - let memo = Memo::new(&random_string()); + let memo = Memo::new(random_string()).unwrap(); let test = MemoTest { memo }; run_binary_channel_test(&test) }