fix: support chain identifiers without `{chain_name}-{revision_number…

…}` pattern (#941) * optional revision number for chain id * add additional methods * update tests * add changelog entry * new test for codecov * revert optional u64 * rename method * update tests * visualize actual testcases * revert method rename * update changelog entry * refactor ChainId::new * rm ChainId::has_revision_number method * refactor ChainId::set_revision_number to ChainId::increment_revision_number * rm old set unset revision_number test * add tests for ChainId::increment_revision_number * add length validation tests * refactor source code for ChainId::new * fix doc test * fix chain identifier length validation * update tests * fix typo * reword changelog entry * reword doc string * fix ChainId::validate_length * add ChainId::validate_length in tests * replace ID with identifier * use validate_prefix_length over validate_identifier_length * optimize validate_prefix_length * rename changelog entry * fix incorrect doc comment Co-authored-by: Farhad Shabani <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> * use u64::checked_add * update doc comment * update doc tests * simplify validate_prefix_length * tests for validate_prefix_length * fix tests * add changelog entry * import test_log::test * fix rstest test attribute * cargo fmt * fix typo Co-authored-by: Farhad Shabani <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> --------- Signed-off-by: Rano | Ranadeep <[email protected]> Co-authored-by: Farhad Shabani <[email protected]>
cosmos · Nov 6, 2023 · 1ec4b8c · 1ec4b8c
1 parent ccca3ff
commit 1ec4b8c
Show file tree

Hide file tree

Showing 10 changed files with 256 additions and 119 deletions.
diff --git a/...g/unreleased/bug-fixes/940-chain-identifiers-without-revision-number-pattern.md b/...g/unreleased/bug-fixes/940-chain-identifiers-without-revision-number-pattern.md
@@ -0,0 +1,2 @@
+- Support chain identifiers without the `{chain_name}-{revision_number}` pattern
+  of Tendermint chains. ([\#940](https://github.com/cosmos/ibc-rs/issues/940)).
diff --git a/.changelog/unreleased/bug-fixes/943-optimize-validate-prefix-length.md b/.changelog/unreleased/bug-fixes/943-optimize-validate-prefix-length.md
@@ -0,0 +1,2 @@
+- Remove redundant `String` creation in `validate_prefix_length`
+  ([\#943](https://github.com/cosmos/ibc-rs/issues/943)).
diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs
@@ -827,7 +827,7 @@ mod tests {
     fn client_state_new() {
         // Define a "default" set of parameters to reuse throughout these tests.
         let default_params: ClientStateParams = ClientStateParams {
-            id: ChainId::new("ibc", 0).unwrap(),
+            id: ChainId::new("ibc-0").unwrap(),
             trust_level: TrustThreshold::ONE_THIRD,
             trusting_period: Duration::new(64000, 0),
             unbonding_period: Duration::new(128000, 0),
@@ -870,17 +870,17 @@ mod tests {
                 want_pass: true,
             },
             Test {
-                name: "Valid long (50 chars) chain-id that satisfies revision_number length < `u16::MAX` length".to_string(),
+                name: "Valid long (50 chars) chain-id that satisfies revision_number length < `u64::MAX` length".to_string(),
                 params: ClientStateParams {
-                    id: ChainId::new(&"a".repeat(29), 0).unwrap(),
+                    id: ChainId::new(&format!("{}-{}", "a".repeat(29), 0)).unwrap(),
                     ..default_params.clone()
                 },
                 want_pass: true,
             },
             Test {
                 name: "Invalid too-long (51 chars) chain-id".to_string(),
                 params: ClientStateParams {
-                    id: ChainId::new(&"a".repeat(30), 0).unwrap(),
+                    id: ChainId::new(&format!("{}-{}", "a".repeat(30), 0)).unwrap(),
                     ..default_params.clone()
                 },
                 want_pass: false,
@@ -993,7 +993,7 @@ mod tests {
     fn client_state_verify_height() {
         // Define a "default" set of parameters to reuse throughout these tests.
         let default_params: ClientStateParams = ClientStateParams {
-            id: ChainId::new("ibc", 1).unwrap(),
+            id: ChainId::new("ibc-1").unwrap(),
             trust_level: TrustThreshold::ONE_THIRD,
             trusting_period: Duration::new(64000, 0),
             unbonding_period: Duration::new(128000, 0),
@@ -1173,7 +1173,7 @@ pub mod test_util {
     pub fn get_dummy_raw_tm_client_state(frozen_height: RawHeight) -> RawTmClientState {
         #[allow(deprecated)]
         RawTmClientState {
-            chain_id: ChainId::new("ibc", 0).expect("Never fails").to_string(),
+            chain_id: ChainId::new("ibc-0").expect("Never fails").to_string(),
             trust_level: Some(Fraction {
                 numerator: 1,
                 denominator: 3,

diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs
@@ -196,7 +196,7 @@ mod tests {
     /// contained in the store and has not expired.
     #[test]
     fn test_consensus_state_pruning() {
-        let chain_id = ChainId::new("mockgaiaA", 1).unwrap();
+        let chain_id = ChainId::new("mockgaiaA-1").unwrap();
 
         let client_height = Height::new(1, 1).unwrap();
 
@@ -295,10 +295,10 @@ mod tests {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();
         let client_height = Height::new(1, 20).unwrap();
         let update_height = Height::new(1, 21).unwrap();
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         let mut ctx = MockContext::new(
-            ChainId::new("mockgaiaA", 1).unwrap(),
+            ChainId::new("mockgaiaA-1").unwrap(),
             HostType::Mock,
             5,
             Height::new(1, 1).unwrap(),
@@ -343,10 +343,10 @@ mod tests {
     fn test_update_synthetic_tendermint_client_validator_change_ok() {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();
         let client_height = Height::new(1, 20).unwrap();
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         let mut ctx_a = MockContextConfig::builder()
-            .host_id(ChainId::new("mockgaiaA", 1).unwrap())
+            .host_id(ChainId::new("mockgaiaA-1").unwrap())
             .latest_height(Height::new(1, 1).unwrap())
             .build()
             .with_client_config(
@@ -435,10 +435,10 @@ mod tests {
     fn test_update_synthetic_tendermint_client_validator_change_fail() {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();
         let client_height = Height::new(1, 20).unwrap();
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         let ctx_a = MockContextConfig::builder()
-            .host_id(ChainId::new("mockgaiaA", 1).unwrap())
+            .host_id(ChainId::new("mockgaiaA-1").unwrap())
             .latest_height(Height::new(1, 1).unwrap())
             .build()
             .with_client_config(
@@ -515,10 +515,10 @@ mod tests {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();
         let client_height = Height::new(1, 20).unwrap();
         let update_height = Height::new(1, 21).unwrap();
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         let mut ctx = MockContext::new(
-            ChainId::new("mockgaiaA", 1).unwrap(),
+            ChainId::new("mockgaiaA-1").unwrap(),
             HostType::Mock,
             5,
             Height::new(1, 1).unwrap(),
@@ -565,8 +565,8 @@ mod tests {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();
         let client_height = Height::new(1, 20).unwrap();
 
-        let ctx_a_chain_id = ChainId::new("mockgaiaA", 1).unwrap();
-        let ctx_b_chain_id = ChainId::new("mockgaiaB", 1).unwrap();
+        let ctx_a_chain_id = ChainId::new("mockgaiaA-1").unwrap();
+        let ctx_b_chain_id = ChainId::new("mockgaiaB-1").unwrap();
         let start_height = Height::new(1, 11).unwrap();
 
         let mut ctx_a = MockContext::new(ctx_a_chain_id, HostType::Mock, 5, start_height)
@@ -695,7 +695,7 @@ mod tests {
         let chain_start_height = Height::new(1, 11).unwrap();
 
         let ctx = MockContext::new(
-            ChainId::new("mockgaiaA", 1).unwrap(),
+            ChainId::new("mockgaiaA-1").unwrap(),
             HostType::Mock,
             5,
             chain_start_height,
@@ -708,7 +708,7 @@ mod tests {
         );
 
         let ctx_b = MockContext::new(
-            ChainId::new("mockgaiaB", 1).unwrap(),
+            ChainId::new("mockgaiaB-1").unwrap(),
             HostType::SyntheticTendermint,
             5,
             client_height,
@@ -835,11 +835,11 @@ mod tests {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();
         let client_height = Height::new(1, 20).unwrap();
         let misbehaviour_height = Height::new(1, 21).unwrap();
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         // Create a mock context for chain-A with a synthetic tendermint light client for chain-B
         let mut ctx_a = MockContext::new(
-            ChainId::new("mockgaiaA", 1).unwrap(),
+            ChainId::new("mockgaiaA-1").unwrap(),
             HostType::Mock,
             5,
             Height::new(1, 1).unwrap(),
@@ -896,11 +896,11 @@ mod tests {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();
         let client_height = Height::new(1, 20).unwrap();
         let misbehaviour_height = Height::new(1, 21).unwrap();
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         // Create a mock context for chain-A with a synthetic tendermint light client for chain-B
         let mut ctx_a = MockContext::new(
-            ChainId::new("mockgaiaA", 1).unwrap(),
+            ChainId::new("mockgaiaA-1").unwrap(),
             HostType::Mock,
             5,
             Height::new(1, 1).unwrap(),
@@ -955,7 +955,7 @@ mod tests {
 
     #[test]
     fn test_expired_client() {
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         let update_height = Height::new(1, 21).unwrap();
         let client_height = update_height.sub(3).unwrap();
@@ -967,7 +967,7 @@ mod tests {
         let trusting_period = Duration::from_secs(64);
 
         let mut ctx = MockContextConfig::builder()
-            .host_id(ChainId::new("mockgaiaA", 1).unwrap())
+            .host_id(ChainId::new("mockgaiaA-1").unwrap())
             .latest_height(Height::new(1, 1).unwrap())
             .latest_timestamp(timestamp)
             .build()
@@ -995,7 +995,7 @@ mod tests {
 
     #[test]
     fn test_client_update_max_clock_drift() {
-        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB-1").unwrap();
 
         let client_height = Height::new(1, 20).unwrap();
 
@@ -1006,7 +1006,7 @@ mod tests {
         let max_clock_drift = Duration::from_secs(64);
 
         let ctx_a = MockContextConfig::builder()
-            .host_id(ChainId::new("mockgaiaA", 1).unwrap())
+            .host_id(ChainId::new("mockgaiaA-1").unwrap())
             .latest_height(Height::new(1, 1).unwrap())
             .latest_timestamp(timestamp)
             .build()

diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs
@@ -261,7 +261,7 @@ mod tests {
 
         let ctx_default = MockContext::default();
         let ctx_new = MockContext::new(
-            ChainId::new("mockgaia", latest_height.revision_number()).unwrap(),
+            ChainId::new(&format!("mockgaia-{}", latest_height.revision_number())).unwrap(),
             HostType::Mock,
             max_history_size,
             latest_height,

diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs
@@ -257,7 +257,7 @@ mod tests {
         };
 
         let ctx_new = MockContext::new(
-            ChainId::new("mockgaia", 0).unwrap(),
+            ChainId::new("mockgaia-0").unwrap(),
             HostType::Mock,
             max_history_size,
             host_chain_height,
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		- Support chain identifiers without the `{chain_name}-{revision_number}` pattern
		of Tendermint chains. ([\#940](https://github.com/cosmos/ibc-rs/issues/940)).
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		- Remove redundant `String` creation in `validate_prefix_length`
		([\#943](https://github.com/cosmos/ibc-rs/issues/943)).