From 8ef7036e78e4f73be0cf86828a4b172a19c03c92 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Mon, 27 Jan 2025 12:21:07 +0100 Subject: [PATCH 1/6] put tests under mod --- ibc-core/ics02-client/types/src/height.rs | 71 ++++++++++++----------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/ibc-core/ics02-client/types/src/height.rs b/ibc-core/ics02-client/types/src/height.rs index 1ae788add..4d7f8d525 100644 --- a/ibc-core/ics02-client/types/src/height.rs +++ b/ibc-core/ics02-client/types/src/height.rs @@ -179,37 +179,42 @@ impl FromStr for Height { } } -#[test] -fn test_valid_height() { - assert_eq!( - "1-1".parse::().unwrap(), - Height { - revision_number: 1, - revision_height: 1 - } - ); - assert_eq!( - "1-10".parse::().unwrap(), - Height { - revision_number: 1, - revision_height: 10 - } - ); -} - -#[test] -fn test_invalid_height() { - assert!("0-0".parse::().is_err()); - assert!("0-".parse::().is_err()); - assert!("-0".parse::().is_err()); - assert!("-".parse::().is_err()); - assert!("1-1-1".parse::().is_err()); - - let decoding_err = "1".parse::().unwrap_err(); - let decoding_err = decoding_err.to_string(); - assert!(decoding_err.contains("height `1` not properly formatted")); - - let decoding_err = "".parse::().unwrap_err(); - let decoding_err = decoding_err.to_string(); - assert!(decoding_err.contains("height `` not properly formatted")); +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_valid_height() { + assert_eq!( + "1-1".parse::().unwrap(), + Height { + revision_number: 1, + revision_height: 1 + } + ); + assert_eq!( + "1-10".parse::().unwrap(), + Height { + revision_number: 1, + revision_height: 10 + } + ); + } + + #[test] + fn test_invalid_height() { + assert!("0-0".parse::().is_err()); + assert!("0-".parse::().is_err()); + assert!("-0".parse::().is_err()); + assert!("-".parse::().is_err()); + assert!("1-1-1".parse::().is_err()); + + let decoding_err = "1".parse::().unwrap_err(); + let decoding_err = decoding_err.to_string(); + assert!(decoding_err.contains("height `1` not properly formatted")); + + let decoding_err = "".parse::().unwrap_err(); + let decoding_err = decoding_err.to_string(); + assert!(decoding_err.contains("height `` not properly formatted")); + } } From 25431cf597c3b68fb2aa9660c6a77724fe12a767 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Mon, 27 Jan 2025 12:21:48 +0100 Subject: [PATCH 2/6] optional rev number during Height deserialization --- ibc-core/ics02-client/types/src/height.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ibc-core/ics02-client/types/src/height.rs b/ibc-core/ics02-client/types/src/height.rs index 4d7f8d525..5ba77f593 100644 --- a/ibc-core/ics02-client/types/src/height.rs +++ b/ibc-core/ics02-client/types/src/height.rs @@ -31,6 +31,7 @@ use crate::error::ClientError; #[derive(Copy, Clone, PartialEq, Eq, Hash)] pub struct Height { /// Previously known as "epoch" + #[serde(default)] revision_number: u64, /// The height of a block From 555785817ce836a295e3628a14cdfab2c8101a14 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Mon, 27 Jan 2025 12:23:37 +0100 Subject: [PATCH 3/6] add test --- ibc-core/ics02-client/types/Cargo.toml | 3 ++- ibc-core/ics02-client/types/src/height.rs | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ibc-core/ics02-client/types/Cargo.toml b/ibc-core/ics02-client/types/Cargo.toml index 8e8451371..f774601db 100644 --- a/ibc-core/ics02-client/types/Cargo.toml +++ b/ibc-core/ics02-client/types/Cargo.toml @@ -42,7 +42,8 @@ parity-scale-codec = { workspace = true, optional = true } scale-info = { workspace = true, optional = true } [dev-dependencies] -rstest = { workspace = true } +rstest = { workspace = true } +serde-json = { workspace = true } [features] default = [ "std" ] diff --git a/ibc-core/ics02-client/types/src/height.rs b/ibc-core/ics02-client/types/src/height.rs index 5ba77f593..9ffda2376 100644 --- a/ibc-core/ics02-client/types/src/height.rs +++ b/ibc-core/ics02-client/types/src/height.rs @@ -218,4 +218,14 @@ mod tests { let decoding_err = decoding_err.to_string(); assert!(decoding_err.contains("height `` not properly formatted")); } + + #[test] + fn test_empty_rev_number_deserialization() { + // #1262: ibc-go uses `omitempty` in JSON serialization + let json_str = r#"{"revision_height": 10}"#; + let actual: Height = serde_json::from_str(json_str).unwrap(); + let expected = Height::new(0, 10).unwrap(); + + assert_eq!(actual, expected); + } } From 5a0b3f7ead307f7f4ee406f6c3d3bbd9d8875623 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Mon, 27 Jan 2025 12:43:59 +0100 Subject: [PATCH 4/6] use serde cfg_attr --- ibc-core/ics02-client/types/src/height.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc-core/ics02-client/types/src/height.rs b/ibc-core/ics02-client/types/src/height.rs index 9ffda2376..92ad6fd34 100644 --- a/ibc-core/ics02-client/types/src/height.rs +++ b/ibc-core/ics02-client/types/src/height.rs @@ -31,7 +31,7 @@ use crate::error::ClientError; #[derive(Copy, Clone, PartialEq, Eq, Hash)] pub struct Height { /// Previously known as "epoch" - #[serde(default)] + #[cfg_attr(feature = "serde", serde(default))] revision_number: u64, /// The height of a block From 58972b84bf9b9bccc231b623a604ef78ca8d9514 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Mon, 27 Jan 2025 18:19:04 +0100 Subject: [PATCH 5/6] changelog entry --- .../1262-deserialize-height-without-revision-number.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md diff --git a/.changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md b/.changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md new file mode 100644 index 000000000..be77623f4 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md @@ -0,0 +1,2 @@ +- Serde support for `Height` without `revision_number` + ([#1262](https://github.com/informalsystems/ibc-rs/issues/1262)). From 73698b0b645d5e7e80d312ec0b076250301e125a Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Mon, 27 Jan 2025 15:30:46 -0800 Subject: [PATCH 6/6] nit: changelog --- .../1262-deserialize-height-without-revision-number.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md b/.changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md index be77623f4..3cc3205f9 100644 --- a/.changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md +++ b/.changelog/unreleased/bug-fixes/1262-deserialize-height-without-revision-number.md @@ -1,2 +1,2 @@ -- Serde support for `Height` without `revision_number` - ([#1262](https://github.com/informalsystems/ibc-rs/issues/1262)). +- [ibc-core-client-types] Serde support for `Height` without `revision_number` + ([#1262](https://github.com/cosmos/ibc-rs/issues/1262)).