From da0861cae55df4efc9192aaadbbb5839fbf05dac Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 10 Nov 2023 18:05:08 +0100 Subject: [PATCH 1/5] remove ContainsSeparator --- crates/ibc/src/core/ics24_host/identifier.rs | 2 -- crates/ibc/src/core/ics24_host/identifier/validate.rs | 7 ------- 2 files changed, 9 deletions(-) diff --git a/crates/ibc/src/core/ics24_host/identifier.rs b/crates/ibc/src/core/ics24_host/identifier.rs index a822d5715..7f87ceeb9 100644 --- a/crates/ibc/src/core/ics24_host/identifier.rs +++ b/crates/ibc/src/core/ics24_host/identifier.rs @@ -516,8 +516,6 @@ impl PartialEq for ChannelId { #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[derive(Debug, Display)] pub enum IdentifierError { - /// identifier `{id}` cannot contain separator '/' - ContainSeparator { id: String }, /// identifier `{id}` has invalid length `{length}` must be between `{min}`-`{max}` characters InvalidLength { id: String, diff --git a/crates/ibc/src/core/ics24_host/identifier/validate.rs b/crates/ibc/src/core/ics24_host/identifier/validate.rs index 7e0f2acf3..42debd4d1 100644 --- a/crates/ibc/src/core/ics24_host/identifier/validate.rs +++ b/crates/ibc/src/core/ics24_host/identifier/validate.rs @@ -1,8 +1,6 @@ use super::IdentifierError as Error; use crate::prelude::*; -/// Path separator (ie. forward slash '/') -const PATH_SEPARATOR: char = '/'; const VALID_SPECIAL_CHARS: &str = "._+-#[]<>"; /// Checks if the identifier only contains valid characters as specified in the @@ -14,11 +12,6 @@ pub fn validate_identifier_chars(id: &str) -> Result<(), Error> { return Err(Error::Empty); } - // Check identifier does not contain path separators - if id.contains(PATH_SEPARATOR) { - return Err(Error::ContainSeparator { id: id.into() }); - } - // Check that the identifier comprises only valid characters: // - Alphanumeric // - `.`, `_`, `+`, `-`, `#` From defa1f50768f4f50144068b990d8f497acb064ca Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 10 Nov 2023 18:06:20 +0100 Subject: [PATCH 2/5] remove length --- crates/ibc/src/core/ics24_host/identifier.rs | 9 ++------- crates/ibc/src/core/ics24_host/identifier/validate.rs | 2 -- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/crates/ibc/src/core/ics24_host/identifier.rs b/crates/ibc/src/core/ics24_host/identifier.rs index 7f87ceeb9..175c7ebc0 100644 --- a/crates/ibc/src/core/ics24_host/identifier.rs +++ b/crates/ibc/src/core/ics24_host/identifier.rs @@ -516,13 +516,8 @@ impl PartialEq for ChannelId { #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[derive(Debug, Display)] pub enum IdentifierError { - /// identifier `{id}` has invalid length `{length}` must be between `{min}`-`{max}` characters - InvalidLength { - id: String, - length: u64, - min: u64, - max: u64, - }, + /// identifier `{id}` has invalid length; must be between `{min}` and `{max}` characters + InvalidLength { id: String, min: u64, max: u64 }, /// identifier `{id}` must only contain alphanumeric characters or `.`, `_`, `+`, `-`, `#`, - `[`, `]`, `<`, `>` InvalidCharacter { id: String }, /// identifier prefix `{prefix}` is invalid diff --git a/crates/ibc/src/core/ics24_host/identifier/validate.rs b/crates/ibc/src/core/ics24_host/identifier/validate.rs index 42debd4d1..9a26171b4 100644 --- a/crates/ibc/src/core/ics24_host/identifier/validate.rs +++ b/crates/ibc/src/core/ics24_host/identifier/validate.rs @@ -37,7 +37,6 @@ pub fn validate_identifier_length(id: &str, min: u64, max: u64) -> Result<(), Er if (id.len() as u64) < min || id.len() as u64 > max { return Err(Error::InvalidLength { id: id.into(), - length: id.len() as u64, min, max, }); @@ -67,7 +66,6 @@ pub fn validate_prefix_length( if max_id_length < 22 { return Err(Error::InvalidLength { id: prefix.into(), - length: prefix.len() as u64, min: 0, max: 0, }); From bd128befd56ac0118f49b885d54f192b820780fc Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 10 Nov 2023 18:07:54 +0100 Subject: [PATCH 3/5] remove Empty --- crates/ibc/src/core/ics24_host/identifier.rs | 2 -- .../ibc/src/core/ics24_host/identifier/validate.rs | 12 ------------ 2 files changed, 14 deletions(-) diff --git a/crates/ibc/src/core/ics24_host/identifier.rs b/crates/ibc/src/core/ics24_host/identifier.rs index 175c7ebc0..bbf6002cd 100644 --- a/crates/ibc/src/core/ics24_host/identifier.rs +++ b/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")] diff --git a/crates/ibc/src/core/ics24_host/identifier/validate.rs b/crates/ibc/src/core/ics24_host/identifier/validate.rs index 9a26171b4..2702b7610 100644 --- a/crates/ibc/src/core/ics24_host/identifier/validate.rs +++ b/crates/ibc/src/core/ics24_host/identifier/validate.rs @@ -7,11 +7,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 that the identifier comprises only valid characters: // - Alphanumeric // - `.`, `_`, `+`, `-`, `#` @@ -210,13 +205,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 From 2234b34093cba5972b8be9db55659cc7ebd00a41 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 10 Nov 2023 18:09:18 +0100 Subject: [PATCH 4/5] changelog --- .../unreleased/breaking-changes/942-identifier-error.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changelog/unreleased/breaking-changes/942-identifier-error.md diff --git a/.changelog/unreleased/breaking-changes/942-identifier-error.md b/.changelog/unreleased/breaking-changes/942-identifier-error.md new file mode 100644 index 000000000..8a65ece3a --- /dev/null +++ b/.changelog/unreleased/breaking-changes/942-identifier-error.md @@ -0,0 +1,6 @@ +- Remove IdentifierError::Empty and ContainSeparator variants as they + are special cases of other existing errors. + ([\#942](https://github.com/cosmos/ibc-rs/pull/942)) +- Remove IdentifierError::InvalidLength::length field since it can be + deduced from the id. + ([\#942](https://github.com/cosmos/ibc-rs/pull/942)) From 251f9ef4c1ef45a6605d5938fade441e2e1da082 Mon Sep 17 00:00:00 2001 From: Rano | Ranadeep Date: Mon, 27 Nov 2023 11:36:31 -0500 Subject: [PATCH 5/5] update changelog entry Signed-off-by: Rano | Ranadeep --- .../unreleased/breaking-changes/978-identifier-error.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.changelog/unreleased/breaking-changes/978-identifier-error.md b/.changelog/unreleased/breaking-changes/978-identifier-error.md index b7084f19f..cd03b558e 100644 --- a/.changelog/unreleased/breaking-changes/978-identifier-error.md +++ b/.changelog/unreleased/breaking-changes/978-identifier-error.md @@ -1,6 +1,2 @@ -- Remove IdentifierError::Empty and ContainSeparator variants as they - are special cases of other existing errors. - ([\#978](https://github.com/cosmos/ibc-rs/issues/978)) -- Remove IdentifierError::InvalidLength::length field since it can be - deduced from the id. +- Optimize `IdentifierError` variants and make them mutually exclusive. ([\#978](https://github.com/cosmos/ibc-rs/issues/978))