refactor: simplify identifier length validation

.changelog/unreleased/breaking-changes/960-simplify-length-validation.md

@@ -0,0 +1,3 @@
+- Remove IdentifierError::Empty variant.  It was a special case of
+  InvalidLength error which is now used.
+  ([\#960](https://github.com/cosmos/ibc-rs/pull/960/))

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

@@ -526,8 +526,6 @@ pub enum IdentifierError {
     UnformattedRevisionNumber { chain_id: String },
     /// revision number overflowed
     RevisionNumberOverflow,
-    /// identifier cannot be empty
-    Empty,
 }
 
 #[cfg(feature = "std")]

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

@@ -9,11 +9,6 @@ const VALID_SPECIAL_CHARS: &str = "._+-#[]<>";
 /// [`ICS-24`](https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements#paths-identifiers-separators)]
 /// spec.
 pub fn validate_identifier_chars(id: &str) -> Result<(), Error> {
-    // Check identifier is not empty
-    if id.is_empty() {
-        return Err(Error::Empty);
-    }
-
     // Check identifier does not contain path separators
     if id.contains(PATH_SEPARATOR) {
         return Err(Error::ContainSeparator { id: id.into() });
@@ -38,19 +33,19 @@ pub fn validate_identifier_chars(id: &str) -> Result<(), Error> {
 /// [`ICS-24`](https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements#paths-identifiers-separators)]
 /// spec.
 pub fn validate_identifier_length(id: &str, min: u64, max: u64) -> Result<(), Error> {
-    assert!(max >= min);
-
-    // Check identifier length is between given min/max
-    if (id.len() as u64) < min || id.len() as u64 > max {
-        return Err(Error::InvalidLength {
+    // Make sure min is at least one so we reject empty identifiers.
+    let min = min.max(1);
+    let length = id.len() as u64;
+    if (min..=max).contains(&length) {
+        Ok(())
+    } else {
+        Err(Error::InvalidLength {
             id: id.into(),
-            length: id.len() as u64,
+            length,
             min,
             max,
-        });
+        })
     }
-
-    Ok(())
 }
 
 /// Checks if a prefix forms a valid identifier with the given min/max identifier's length.
@@ -61,42 +56,16 @@ pub fn validate_prefix_length(
     min_id_length: u64,
     max_id_length: u64,
 ) -> Result<(), Error> {
-    assert!(max_id_length >= min_id_length);
-
-    if prefix.is_empty() {
-        return Err(Error::InvalidPrefix {
-            prefix: prefix.into(),
-        });
-    }
-
-    // Statically checks if the prefix forms a valid identifier length when constructed with `u64::MAX`
-    // len(prefix + '-' + u64::MAX) <= max_id_length (minimum prefix length is 1)
-    if max_id_length < 22 {
-        return Err(Error::InvalidLength {
-            id: prefix.into(),
-            length: prefix.len() as u64,
-            min: 0,
-            max: 0,
-        });
-    }
-
-    // Checks if the prefix forms a valid identifier length when constructed with `u64::MIN`
-    // len('-' + u64::MIN) = 2
-    validate_identifier_length(
-        prefix,
-        min_id_length.saturating_sub(2),
-        max_id_length.saturating_sub(2),
-    )?;
-
-    // Checks if the prefix forms a valid identifier length when constructed with `u64::MAX`
-    // len('-' + u64::MAX) = 21
-    validate_identifier_length(
-        prefix,
-        min_id_length.saturating_sub(21),
-        max_id_length.saturating_sub(21),
-    )?;
-
-    Ok(())
+    // Prefix must be at least `min_id_length - 2` characters long since the
+    // shortest identifier we can construct is `{prefix}-0` which extends prefix
+    // by two characters.
+    let min = min_id_length.saturating_sub(2);
+    // Prefix must be at most `max_id_length - 21` characters long since the
+    // longest identifier we can construct is `{prefix}-{u64::MAX}` which
+    // extends prefix by 21 characters.
+    let max = max_id_length.saturating_sub(21);
+
+    validate_identifier_length(prefix, min, max)
 }
 
 /// Default validator function for the Client types.
@@ -219,13 +188,6 @@ mod tests {
         assert!(id.is_err())
     }
 
-    #[test]
-    fn parse_invalid_id_empty() {
-        // invalid id empty
-        let id = validate_identifier_chars("");
-        assert!(id.is_err())
-    }
-
     #[test]
     fn parse_invalid_id_path_separator() {
         // invalid id with path separator