fix: support chain identifiers without {chain_name}-{revision_number} pattern

.changelog/unreleased/bug-fixes/940-optional-revision-number-in-chain-id.md

@@ -0,0 +1,2 @@
+- Support `ChainId`s without revision numbers
+  ([\#940](https://github.com/cosmos/ibc-rs/issues/940)).

crates/ibc/src/core/ics24_host/identifier.rs

@@ -72,12 +72,22 @@ impl ChainId {
         })
     }
 
+    pub fn new_without_revision_number(name: &str) -> Result<Self, IdentifierError> {
+        let prefix = name.trim();
+        validate_identifier_chars(prefix)?;
+        validate_identifier_length(prefix, 1, 43)?;
+        Ok(Self {
+            id: prefix.to_owned(),
+            revision_number: 0,
+        })
+    }
+
     /// Get a reference to the underlying string.
     pub fn as_str(&self) -> &str {
         &self.id
     }
 
-    pub fn split_chain_id(&self) -> (&str, u64) {
+    pub fn split_chain_id(&self) -> (&str, Option<u64>) {
         parse_chain_id_string(self.as_str())
             .expect("never fails because a valid chain identifier is parsed")
     }
@@ -92,6 +102,11 @@ impl ChainId {
         self.revision_number
     }
 
+    /// Check if the revision number is set
+    pub fn has_revision_number(&self) -> bool {
+        self.split_chain_id().1.is_some()
+    }
+
     /// Swaps `ChainId`s revision number with the new specified revision number
     /// ```
     /// use ibc::core::ics24_host::identifier::ChainId;
@@ -105,6 +120,21 @@ impl ChainId {
         self.revision_number = revision_number;
     }
 
+    /// Unsets the revision number of the `ChainId`
+    /// ```
+    /// use ibc::core::ics24_host::identifier::ChainId;
+    /// let mut chain_id = ChainId::new("chainA", 1).unwrap();
+    /// chain_id.unset_revision_number();
+    /// assert!(!chain_id.has_revision_number());
+    /// assert_eq!(chain_id.revision_number(), 0);
+    /// assert_eq!(chain_id.as_str(), "chainA");
+    /// ```
+    pub fn unset_revision_number(&mut self) {
+        let chain_name = self.chain_name();
+        self.id = chain_name.to_string();
+        self.revision_number = 0;
+    }
+
     /// A convenient method to check if the `ChainId` forms a valid identifier
     /// with the desired min/max length. However, ICS-24 does not specify a
     /// certain min or max lengths for chain identifiers.
@@ -122,7 +152,7 @@ impl FromStr for ChainId {
         let (_, revision_number) = parse_chain_id_string(id)?;
         Ok(Self {
             id: id.to_string(),
-            revision_number,
+            revision_number: revision_number.unwrap_or(0),
         })
     }
 }
@@ -134,36 +164,28 @@ impl Display for ChainId {
 }
 
 /// Parses a string intended to represent a `ChainId` and, if successful,
-/// returns a tuple containing the chain name and revision number.
-fn parse_chain_id_string(chain_id_str: &str) -> Result<(&str, u64), IdentifierError> {
-    let (name, rev_number_str) = match chain_id_str.rsplit_once('-') {
-        Some((name, rev_number_str)) => (name, rev_number_str),
-        None => {
-            return Err(IdentifierError::InvalidCharacter {
-                id: chain_id_str.to_string(),
-            })
-        }
-    };
+/// returns a tuple containing the chain name and (optional) revision number.
+fn parse_chain_id_string(chain_id_str: &str) -> Result<(&str, Option<u64>), IdentifierError> {
+    let (chain_name, revision_number) = chain_id_str
+        .rsplit_once('-')
+        .filter(|(_, rev_number_str)| {
+            // Validates the revision number not to start with leading zeros, like "01".
+            // Zero is the only allowed revision number with leading zero.
+            rev_number_str.as_bytes().first() != Some(&b'0') || rev_number_str.len() == 1
+        })
+        .and_then(|(chain_name, rev_number_str)| {
+            // Parses the revision number string into a `u64` and checks its validity.
+            rev_number_str
+                .parse()
+                .ok()
+                .map(|revision_number| (chain_name, Some(revision_number)))
+        })
+        .unwrap_or((chain_id_str, None));
 
     // Validates the chain name for allowed characters according to ICS-24.
-    validate_identifier_chars(name)?;
-
-    // Validates the revision number not to start with leading zeros, like "01".
-    if rev_number_str.as_bytes().first() == Some(&b'0') && rev_number_str.len() > 1 {
-        return Err(IdentifierError::InvalidCharacter {
-            id: chain_id_str.to_string(),
-        });
-    }
+    validate_identifier_chars(chain_name)?;
 
-    // Parses the revision number string into a `u64` and checks its validity.
-    let revision_number =
-        rev_number_str
-            .parse()
-            .map_err(|_| IdentifierError::InvalidCharacter {
-                id: chain_id_str.to_string(),
-            })?;
-
-    Ok((name, revision_number))
+    Ok((chain_name, revision_number))
 }
 
 #[cfg_attr(
@@ -518,27 +540,81 @@ impl std::error::Error for IdentifierError {}
 
 #[cfg(test)]
 mod tests {
+    use rstest::rstest;
 
     use super::*;
 
-    #[test]
-    fn test_valid_chain_id() {
-        assert!(ChainId::from_str("chainA-0").is_ok());
-        assert!(ChainId::from_str("chainA-1").is_ok());
-        assert!(ChainId::from_str("chainA--1").is_ok());
-        assert!(ChainId::from_str("chainA-1-2").is_ok());
+    #[rstest]
+    #[case("chainA-0", "chainA", 0)]
+    #[case("chainA-1", "chainA", 1)]
+    #[case("chainA--1", "chainA-", 1)]
+    #[case("chainA-1-2", "chainA-1", 2)]
+    #[case("111-2", "111", 2)]
+    #[case("----1", "---", 1)]
+    #[case("._+-1", "._+", 1)]
+    fn test_valid_chain_id_with_rev(
+        #[case] raw_chain_id: &str,
+        #[case] chain_name: &str,
+        #[case] revision_number: u64,
+    ) {
+        assert_eq!(
+            ChainId::from_str(raw_chain_id).unwrap(),
+            ChainId::new(chain_name, revision_number).unwrap()
+        );
+    }
+
+    #[rstest]
+    #[case("chainA")]
+    #[case("chainA.2")]
+    #[case("123")]
+    #[case("._+")]
+    #[case("chainA-")]
+    #[case("chainA-a")]
+    #[case("chainA-01")]
+    #[case("chainA-1-")]
+    fn test_valid_chain_id_without_rev(#[case] chain_name: &str) {
+        assert_eq!(
+            ChainId::from_str(chain_name).unwrap(),
+            ChainId::new_without_revision_number(chain_name).unwrap()
+        );
+    }
+
+    #[rstest]
+    #[case("-1")]
+    #[case(" ----1")]
+    #[case(" ")]
+    #[case(" chainA")]
+    #[case("chain A")]
+    #[case(" chainA.2")]
+    #[case(" chainA.2-1")]
+    #[case(" 1")]
+    #[case(" -")]
+    #[case("   -1")]
+    #[case("/chainA-1")]
+    fn test_invalid_chain_id(#[case] chain_id_str: &str) {
+        assert!(ChainId::from_str(chain_id_str).is_err());
     }
 
     #[test]
-    fn test_invalid_chain_id() {
-        assert!(ChainId::from_str("1").is_err());
-        assert!(ChainId::from_str("-1").is_err());
-        assert!(ChainId::from_str("   -1").is_err());
-        assert!(ChainId::from_str("chainA").is_err());
-        assert!(ChainId::from_str("chainA-").is_err());
-        assert!(ChainId::from_str("chainA-a").is_err());
-        assert!(ChainId::from_str("chainA-01").is_err());
-        assert!(ChainId::from_str("/chainA-1").is_err());
-        assert!(ChainId::from_str("chainA-1-").is_err());
+    fn test_opt_revision_number() {
+        let mut chain_id = ChainId::from_str("chainA-1").unwrap();
+        assert!(chain_id.has_revision_number());
+        assert_eq!(chain_id.revision_number(), 1);
+        assert_eq!(chain_id.as_str(), "chainA-1");
+
+        chain_id.unset_revision_number();
+        assert!(!chain_id.has_revision_number());
+        assert_eq!(chain_id.revision_number(), 0);
+        assert_eq!(chain_id.as_str(), "chainA");
+
+        chain_id.set_revision_number(2);
+        assert!(chain_id.has_revision_number());
+        assert_eq!(chain_id.revision_number(), 2);
+        assert_eq!(chain_id.as_str(), "chainA-2");
+
+        chain_id.set_revision_number(0);
+        assert!(chain_id.has_revision_number());
+        assert_eq!(chain_id.revision_number(), 0);
+        assert_eq!(chain_id.as_str(), "chainA-0");
     }
 }