From 812cdcdc7acf7539e5a706f7680037f5b45308d5 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 16 Jul 2024 17:17:37 -0700 Subject: [PATCH 01/10] fix: de/serialize checksum with hex --- ibc-clients/ics08-wasm/types/Cargo.toml | 1 + .../ics08-wasm/types/src/client_state.rs | 4 +- .../ics08-wasm/types/src/serializer.rs | 41 +++++++++++++++++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/ibc-clients/ics08-wasm/types/Cargo.toml b/ibc-clients/ics08-wasm/types/Cargo.toml index 70b62e6a1..da15a113f 100644 --- a/ibc-clients/ics08-wasm/types/Cargo.toml +++ b/ibc-clients/ics08-wasm/types/Cargo.toml @@ -18,6 +18,7 @@ description = """ [dependencies] # external dependencies base64 = { workspace = true, features = [ "alloc" ] } +hex = { workspace = true } displaydoc = { workspace = true } serde = { workspace = true, optional = true } cosmwasm-schema = { workspace = true, optional = true } diff --git a/ibc-clients/ics08-wasm/types/src/client_state.rs b/ibc-clients/ics08-wasm/types/src/client_state.rs index 4dda117a0..e4ac6fb64 100644 --- a/ibc-clients/ics08-wasm/types/src/client_state.rs +++ b/ibc-clients/ics08-wasm/types/src/client_state.rs @@ -9,7 +9,7 @@ use ibc_proto::ibc::lightclients::wasm::v1::ClientState as RawClientState; use crate::error::Error; #[cfg(feature = "cosmwasm")] -use crate::serializer::Base64; +use crate::serializer::{Base64, Hex}; use crate::Bytes; pub const WASM_CLIENT_STATE_TYPE_URL: &str = "/ibc.lightclients.wasm.v1.ClientState"; @@ -22,7 +22,7 @@ pub struct ClientState { #[cfg_attr(feature = "cosmwasm", serde(with = "Base64", default))] pub data: Bytes, #[cfg_attr(feature = "cosmwasm", schemars(with = "String"))] - #[cfg_attr(feature = "cosmwasm", serde(with = "Base64", default))] + #[cfg_attr(feature = "cosmwasm", serde(with = "Hex", default))] pub checksum: Bytes, pub latest_height: Height, } diff --git a/ibc-clients/ics08-wasm/types/src/serializer.rs b/ibc-clients/ics08-wasm/types/src/serializer.rs index c44dc7325..39d658216 100644 --- a/ibc-clients/ics08-wasm/types/src/serializer.rs +++ b/ibc-clients/ics08-wasm/types/src/serializer.rs @@ -4,6 +4,7 @@ use ibc_primitives::prelude::*; use serde::de::Error; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +/// Defines a convenient base64 (de)serializer for `Bytes`. pub struct Base64; impl Base64 { @@ -22,25 +23,59 @@ impl Base64 { } } +/// Defines a convenient hex (de)serializer for `Bytes`. +pub struct Hex; + +impl Hex { + pub fn serialize(bytes: &[u8], serializer: S) -> Result { + let hex = hex::encode(bytes); + String::serialize(&hex, serializer) + } + + pub fn deserialize<'de, D: Deserializer<'de>>(deserializer: D) -> Result, D::Error> { + let hex = String::deserialize(deserializer)?; + let bytes = hex::decode(hex).map_err(Error::custom)?; + + Ok(bytes) + } +} + #[cfg(test)] mod tests { use rstest::rstest; use super::*; + use crate::Bytes; #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] - struct Foo(#[serde(with = "Base64")] crate::Bytes); + struct Foo(#[serde(with = "Base64")] Bytes); - // Ensures Base64 serialize and deserialize work as expected + // Ensures `Base64` serialize and deserialize work as expected #[rstest] #[case(b"", "")] #[case(&[118], "dg==")] #[case(&[0x1, 0x2, 0x3, 0x4, 0x5], "AQIDBAU=")] #[case(b"hello world", "aGVsbG8gd29ybGQ=")] - pub fn test_ser_and_deser(#[case] bytes: &[u8], #[case] hash: &str) { + pub fn test_base64_serializer(#[case] bytes: &[u8], #[case] hash: &str) { let foo = Foo(bytes.to_vec()); let json = format!("\"{hash}\""); assert_eq!(serde_json::to_string(&foo).unwrap(), json); assert_eq!(serde_json::from_str::(&json).unwrap(), foo); } + + #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] + struct Bar(#[serde(with = "Hex")] Bytes); + + // Ensures `Hex` serialize and deserialize work as expected + #[rstest] + #[case(b"", "")] + #[case(&[118], "76")] + #[case(&[0x1, 0x2, 0x3, 0x4, 0x5], "0102030405")] + #[case(b"hello world", "68656c6c6f20776f726c64")] + pub fn test_hex_serializer(#[case] bytes: &[u8], #[case] hash: &str) { + let bar = Bar(bytes.to_vec()); + let json = format!("\"{hash}\""); + assert_eq!(serde_json::to_string(&bar).unwrap(), json); + assert_eq!(serde_json::from_str::(&json).unwrap(), bar); + } } From 52719165c83fb4cc875f383a6bb867ef34dcec6d Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 16 Jul 2024 17:38:47 -0700 Subject: [PATCH 02/10] imp: remove unnecessary cosmwasm feature --- Cargo.toml | 2 +- ci/cw-check/Cargo.lock | 1 + ci/no-std-check/Cargo.lock | 1 + ibc-clients/cw-context/Cargo.toml | 2 +- ibc-clients/ics08-wasm/types/Cargo.toml | 16 +++++++--------- .../ics08-wasm/types/src/client_state.rs | 18 ++++++++---------- .../ics08-wasm/types/src/consensus_state.rs | 14 ++++++-------- ibc-clients/ics08-wasm/types/src/lib.rs | 2 +- ibc-data-types/Cargo.toml | 3 --- ibc-testkit/Cargo.toml | 1 + 10 files changed, 27 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f1b8dc3ec..c0aaa0252 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,7 +67,7 @@ sha2 = { version = "0.10.8", default-features = false } serde = { version = "1.0", default-features = false } serde-json = { package = "serde-json-wasm", version = "1.0.1", default-features = false } subtle-encoding = { version = "0.5", default-features = false } -hex = { version = "0.4.3" } +hex = { version = "0.4.3", default-features = false } # ibc dependencies ibc = { version = "0.53.0", path = "./ibc", default-features = false } diff --git a/ci/cw-check/Cargo.lock b/ci/cw-check/Cargo.lock index 04c40474e..c5eaeb7c2 100644 --- a/ci/cw-check/Cargo.lock +++ b/ci/cw-check/Cargo.lock @@ -823,6 +823,7 @@ version = "0.53.0" dependencies = [ "base64 0.22.1", "displaydoc", + "hex", "ibc-core-client", "ibc-core-host-types", "ibc-primitives", diff --git a/ci/no-std-check/Cargo.lock b/ci/no-std-check/Cargo.lock index 01ea501f8..baad9f1a6 100644 --- a/ci/no-std-check/Cargo.lock +++ b/ci/no-std-check/Cargo.lock @@ -1247,6 +1247,7 @@ version = "0.53.0" dependencies = [ "base64 0.22.1", "displaydoc", + "hex", "ibc-core-client", "ibc-core-host-types", "ibc-primitives", diff --git a/ibc-clients/cw-context/Cargo.toml b/ibc-clients/cw-context/Cargo.toml index 7cb2ef06f..9fbcd5a9e 100644 --- a/ibc-clients/cw-context/Cargo.toml +++ b/ibc-clients/cw-context/Cargo.toml @@ -24,7 +24,7 @@ serde = { workspace = true, features = [ "derive" ] } # ibc dependencies ibc-core = { workspace = true } -ibc-client-wasm-types = { workspace = true, features = [ "cosmwasm" ] } +ibc-client-wasm-types = { workspace = true, features = ["schema"] } # cosmwasm dependencies cosmwasm-schema = { workspace = true } diff --git a/ibc-clients/ics08-wasm/types/Cargo.toml b/ibc-clients/ics08-wasm/types/Cargo.toml index da15a113f..5902b3fc4 100644 --- a/ibc-clients/ics08-wasm/types/Cargo.toml +++ b/ibc-clients/ics08-wasm/types/Cargo.toml @@ -17,11 +17,11 @@ description = """ [dependencies] # external dependencies -base64 = { workspace = true, features = [ "alloc" ] } -hex = { workspace = true } -displaydoc = { workspace = true } -serde = { workspace = true, optional = true } -cosmwasm-schema = { workspace = true, optional = true } +base64 = { workspace = true, features = [ "alloc" ] } +hex = { workspace = true } +displaydoc = { workspace = true } +schemars = { workspace = true, optional = true } +serde = { workspace = true, optional = true } # ibc dependencies ibc-core-client = { workspace = true } @@ -41,6 +41,7 @@ std = [ "ibc-primitives/std", "ibc-proto/std", "base64/std", + "hex/std", "serde/std", ] serde = [ @@ -55,10 +56,7 @@ schema = [ "ibc-core-host-types/schema", "ibc-primitives/schema", "ibc-proto/json-schema", + "schemars", "serde", "std", ] -cosmwasm = [ - "cosmwasm-schema", - "schema", -] diff --git a/ibc-clients/ics08-wasm/types/src/client_state.rs b/ibc-clients/ics08-wasm/types/src/client_state.rs index e4ac6fb64..3a2355bea 100644 --- a/ibc-clients/ics08-wasm/types/src/client_state.rs +++ b/ibc-clients/ics08-wasm/types/src/client_state.rs @@ -1,28 +1,26 @@ //! Defines the client state type for the ICS-08 Wasm light client. -#[cfg(feature = "cosmwasm")] -use cosmwasm_schema::cw_serde; use ibc_core_client::types::Height; use ibc_primitives::prelude::*; use ibc_primitives::proto::{Any, Protobuf}; use ibc_proto::ibc::lightclients::wasm::v1::ClientState as RawClientState; use crate::error::Error; -#[cfg(feature = "cosmwasm")] +#[cfg(feature = "serde")] use crate::serializer::{Base64, Hex}; use crate::Bytes; pub const WASM_CLIENT_STATE_TYPE_URL: &str = "/ibc.lightclients.wasm.v1.ClientState"; -#[cfg_attr(feature = "cosmwasm", cw_serde)] -#[cfg_attr(not(feature = "cosmwasm"), derive(Clone, Debug, PartialEq))] -#[derive(Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct ClientState { - #[cfg_attr(feature = "cosmwasm", schemars(with = "String"))] - #[cfg_attr(feature = "cosmwasm", serde(with = "Base64", default))] + #[cfg_attr(feature = "schema", schemars(with = "String"))] + #[cfg_attr(feature = "serde", serde(with = "Base64", default))] pub data: Bytes, - #[cfg_attr(feature = "cosmwasm", schemars(with = "String"))] - #[cfg_attr(feature = "cosmwasm", serde(with = "Hex", default))] + #[cfg_attr(feature = "schema", schemars(with = "String"))] + #[cfg_attr(feature = "serde", serde(with = "Hex", default))] pub checksum: Bytes, pub latest_height: Height, } diff --git a/ibc-clients/ics08-wasm/types/src/consensus_state.rs b/ibc-clients/ics08-wasm/types/src/consensus_state.rs index 32e6476a9..597cc4e87 100644 --- a/ibc-clients/ics08-wasm/types/src/consensus_state.rs +++ b/ibc-clients/ics08-wasm/types/src/consensus_state.rs @@ -1,24 +1,22 @@ //! Defines the consensus state type for the ICS-08 Wasm light client. -#[cfg(feature = "cosmwasm")] -use cosmwasm_schema::cw_serde; use ibc_core_client::types::error::ClientError; use ibc_primitives::prelude::*; use ibc_primitives::proto::{Any, Protobuf}; use ibc_proto::ibc::lightclients::wasm::v1::ConsensusState as RawConsensusState; -#[cfg(feature = "cosmwasm")] +#[cfg(feature = "serde")] use crate::serializer::Base64; use crate::Bytes; pub const WASM_CONSENSUS_STATE_TYPE_URL: &str = "/ibc.lightclients.wasm.v1.ConsensusState"; -#[cfg_attr(feature = "cosmwasm", cw_serde)] -#[cfg_attr(not(feature = "cosmwasm"), derive(Clone, Debug, PartialEq))] -#[derive(Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct ConsensusState { - #[cfg_attr(feature = "cosmwasm", schemars(with = "String"))] - #[cfg_attr(feature = "cosmwasm", serde(with = "Base64", default))] + #[cfg_attr(feature = "schema", schemars(with = "String"))] + #[cfg_attr(feature = "serde", serde(with = "Base64", default))] pub data: Bytes, } diff --git a/ibc-clients/ics08-wasm/types/src/lib.rs b/ibc-clients/ics08-wasm/types/src/lib.rs index 98bd27915..169eb3b30 100644 --- a/ibc-clients/ics08-wasm/types/src/lib.rs +++ b/ibc-clients/ics08-wasm/types/src/lib.rs @@ -18,7 +18,7 @@ pub mod consensus_state; pub mod error; pub mod msgs; -#[cfg(feature = "cosmwasm")] +#[cfg(feature = "serde")] pub mod serializer; use core::str::FromStr; diff --git a/ibc-data-types/Cargo.toml b/ibc-data-types/Cargo.toml index da618570d..3c4338dc4 100644 --- a/ibc-data-types/Cargo.toml +++ b/ibc-data-types/Cargo.toml @@ -95,6 +95,3 @@ parity-scale-codec = [ "ibc-client-tendermint-types/parity-scale-codec", "ibc-primitives/parity-scale-codec", ] -cosmwasm = [ - "ibc-client-wasm-types/cosmwasm", -] diff --git a/ibc-testkit/Cargo.toml b/ibc-testkit/Cargo.toml index 05310e28b..54830887e 100644 --- a/ibc-testkit/Cargo.toml +++ b/ibc-testkit/Cargo.toml @@ -46,6 +46,7 @@ rstest = { workspace = true } [features] default = [ "std" ] std = [ + "hex/std", "serde/std", "serde-json/std", "ibc/std", From 00aa0617fbbf1430a47dbded29bd24adfc5ee372 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 16 Jul 2024 19:00:06 -0700 Subject: [PATCH 03/10] chore: add changelog --- .../unreleased/bug-fixes/1283-serialize-checksum-with-hex.md | 2 ++ ibc-clients/cw-context/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 .changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md diff --git a/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md b/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md new file mode 100644 index 000000000..9dd0f855f --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md @@ -0,0 +1,2 @@ +- [ibc-client-wasm-types] De/Serialize checksum fields with `hex` + ([#1283](https://github.com/cosmos/ibc-rs/pull/1283)). diff --git a/ibc-clients/cw-context/Cargo.toml b/ibc-clients/cw-context/Cargo.toml index 9fbcd5a9e..b478e7c22 100644 --- a/ibc-clients/cw-context/Cargo.toml +++ b/ibc-clients/cw-context/Cargo.toml @@ -24,7 +24,7 @@ serde = { workspace = true, features = [ "derive" ] } # ibc dependencies ibc-core = { workspace = true } -ibc-client-wasm-types = { workspace = true, features = ["schema"] } +ibc-client-wasm-types = { workspace = true, features = [ "schema" ] } # cosmwasm dependencies cosmwasm-schema = { workspace = true } From 0f0207dbcd8f591d64cf47e492786051fee4b1e7 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 17 Jul 2024 09:54:59 -0700 Subject: [PATCH 04/10] fix: use Binary as checksum type for InstantiateMsg --- ibc-clients/cw-context/src/handlers.rs | 2 +- ibc-clients/cw-context/src/types/msgs.rs | 13 +++++++++++-- tests-integration/tests/cosmwasm/helper.rs | 11 +++++++---- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/ibc-clients/cw-context/src/handlers.rs b/ibc-clients/cw-context/src/handlers.rs index eb92e0928..8f8d6e4f4 100644 --- a/ibc-clients/cw-context/src/handlers.rs +++ b/ibc-clients/cw-context/src/handlers.rs @@ -27,7 +27,7 @@ where let any_consensus_state = Any::decode(&mut msg.consensus_state.as_slice())?; - self.set_checksum(msg.checksum); + self.set_checksum(msg.checksum.to_array()?.into()); client_state.initialise(self, &self.client_id(), any_consensus_state)?; diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index 1a7da1a4c..9e92c6ee9 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -1,7 +1,7 @@ //! Defines the messages sent to the CosmWasm contract by the 08-wasm proxy //! light client. use cosmwasm_schema::{cw_serde, QueryResponses}; -use cosmwasm_std::{Binary, Checksum}; +use cosmwasm_std::Binary; use ibc_core::client::types::proto::v1::Height as RawHeight; use ibc_core::client::types::Height; use ibc_core::commitment_types::commitment::{CommitmentPrefix, CommitmentProofBytes}; @@ -20,7 +20,16 @@ use super::error::ContractError; pub struct InstantiateMsg { pub client_state: Binary, pub consensus_state: Binary, - pub checksum: Checksum, + /// The checksum of the contract. + /// + /// NOTE: The checksum included in any type of 08-wasm messages, such as + /// [`WasmClientState`](ibc_client_wasm_types::client_state::ClientState), + /// is hex-encoded bytes. The ibc-go 08-wasm light client initially + /// hex-decodes this to a valid checksum. In a subsequent step, the entire + /// payload, including the checksum, is base64-encoded by the VM before + /// being passed to a CosmWasm contract entry point. Therefore, we use the + /// `Binary` type here to properly deserialize a base64-encoded checksum. + pub checksum: Binary, } // ------------------------------------------------------------ diff --git a/tests-integration/tests/cosmwasm/helper.rs b/tests-integration/tests/cosmwasm/helper.rs index e32e4d3e4..b0ba53b0c 100644 --- a/tests-integration/tests/cosmwasm/helper.rs +++ b/tests-integration/tests/cosmwasm/helper.rs @@ -1,7 +1,7 @@ use std::str::FromStr; use cosmwasm_std::testing::{message_info, mock_dependencies, mock_env}; -use cosmwasm_std::{coins, Checksum, Env, MessageInfo, Timestamp as CwTimestamp}; +use cosmwasm_std::{coins, Binary, Checksum, Env, MessageInfo, Timestamp as CwTimestamp}; use ibc::clients::tendermint::types::ConsensusState; use ibc::core::primitives::Timestamp as IbcTimestamp; use tendermint::Hash; @@ -13,9 +13,12 @@ pub fn dummy_msg_info() -> MessageInfo { message_info(&creator, &coins(1000, "ibc")) } -pub fn dummy_checksum() -> Checksum { - Checksum::from_hex("2469f43c3ca20d476442bd3d98cbd97a180776ab37332aa7b02cae5a620acfc6") - .expect("Never fails") +pub fn dummy_checksum() -> Binary { + let hex_bytes = + Checksum::from_hex("2469f43c3ca20d476442bd3d98cbd97a180776ab37332aa7b02cae5a620acfc6") + .expect("Never fails"); + + hex_bytes.as_slice().into() } pub fn dummy_sov_consensus_state(timestamp: IbcTimestamp) -> ConsensusState { From e92c6f40d6d1b3cf42ea746b16baabe0f0b05fd3 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 17 Jul 2024 10:12:45 -0700 Subject: [PATCH 05/10] chore: update changelog --- .../unreleased/bug-fixes/1283-serialize-checksum-with-hex.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md b/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md index 9dd0f855f..f5dbd9cfd 100644 --- a/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md +++ b/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md @@ -1,2 +1,2 @@ - [ibc-client-wasm-types] De/Serialize checksum fields with `hex` - ([#1283](https://github.com/cosmos/ibc-rs/pull/1283)). + ([#1285](https://github.com/cosmos/ibc-rs/issues/1285)). From 39d830a667fa0799a05159adf163d3cb610ed59b Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 18 Jul 2024 09:44:47 -0700 Subject: [PATCH 06/10] fix: revert checksum type back to Binary and use base64 deser --- ci/cw-check/Cargo.lock | 1 - ibc-clients/cw-context/src/context/mod.rs | 19 +++------ ibc-clients/cw-context/src/handlers.rs | 2 +- ibc-clients/cw-context/src/types/msgs.rs | 9 ---- ibc-clients/ics08-wasm/types/Cargo.toml | 2 - .../ics08-wasm/types/src/client_state.rs | 4 +- .../ics08-wasm/types/src/serializer.rs | 41 ++----------------- 7 files changed, 12 insertions(+), 66 deletions(-) diff --git a/ci/cw-check/Cargo.lock b/ci/cw-check/Cargo.lock index c5eaeb7c2..04c40474e 100644 --- a/ci/cw-check/Cargo.lock +++ b/ci/cw-check/Cargo.lock @@ -823,7 +823,6 @@ version = "0.53.0" dependencies = [ "base64 0.22.1", "displaydoc", - "hex", "ibc-core-client", "ibc-core-host-types", "ibc-primitives", diff --git a/ibc-clients/cw-context/src/context/mod.rs b/ibc-clients/cw-context/src/context/mod.rs index 096115970..45d410265 100644 --- a/ibc-clients/cw-context/src/context/mod.rs +++ b/ibc-clients/cw-context/src/context/mod.rs @@ -3,7 +3,7 @@ pub mod custom_ctx; use std::str::FromStr; -use cosmwasm_std::{Checksum, Deps, DepsMut, Empty, Env, Order, Storage}; +use cosmwasm_std::{Binary, Deps, DepsMut, Empty, Env, Order, Storage}; use cw_storage_plus::{Bound, Map}; use ibc_client_wasm_types::client_state::ClientState as WasmClientState; use ibc_core::client::context::client_state::ClientStateCommon; @@ -40,7 +40,7 @@ where deps_mut: Option>, env: Env, client_id: ClientId, - checksum: Option, + checksum: Option, migration_prefix: MigrationPrefix, client_type: std::marker::PhantomData, } @@ -96,7 +96,7 @@ where } /// Sets the checksum of the context. - pub fn set_checksum(&mut self, checksum: Checksum) { + pub fn set_checksum(&mut self, checksum: Binary) { self.checksum = Some(checksum); } @@ -272,9 +272,9 @@ where } /// Returns the checksum of the current contract. - pub fn obtain_checksum(&self) -> Result { + pub fn obtain_checksum(&self) -> Result { match &self.checksum { - Some(checksum) => Ok(*checksum), + Some(checksum) => Ok(checksum.clone()), None => { let client_state_value = self.retrieve(ClientStatePath::leaf())?; @@ -285,14 +285,7 @@ where } })?; - let checksum = - Checksum::try_from(wasm_client_state.checksum.as_slice()).map_err(|e| { - ClientError::Other { - description: e.to_string(), - } - })?; - - Ok(checksum) + Ok(wasm_client_state.checksum.into()) } } } diff --git a/ibc-clients/cw-context/src/handlers.rs b/ibc-clients/cw-context/src/handlers.rs index 8f8d6e4f4..eb92e0928 100644 --- a/ibc-clients/cw-context/src/handlers.rs +++ b/ibc-clients/cw-context/src/handlers.rs @@ -27,7 +27,7 @@ where let any_consensus_state = Any::decode(&mut msg.consensus_state.as_slice())?; - self.set_checksum(msg.checksum.to_array()?.into()); + self.set_checksum(msg.checksum); client_state.initialise(self, &self.client_id(), any_consensus_state)?; diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index 9e92c6ee9..fbbe4740b 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -20,15 +20,6 @@ use super::error::ContractError; pub struct InstantiateMsg { pub client_state: Binary, pub consensus_state: Binary, - /// The checksum of the contract. - /// - /// NOTE: The checksum included in any type of 08-wasm messages, such as - /// [`WasmClientState`](ibc_client_wasm_types::client_state::ClientState), - /// is hex-encoded bytes. The ibc-go 08-wasm light client initially - /// hex-decodes this to a valid checksum. In a subsequent step, the entire - /// payload, including the checksum, is base64-encoded by the VM before - /// being passed to a CosmWasm contract entry point. Therefore, we use the - /// `Binary` type here to properly deserialize a base64-encoded checksum. pub checksum: Binary, } diff --git a/ibc-clients/ics08-wasm/types/Cargo.toml b/ibc-clients/ics08-wasm/types/Cargo.toml index 5902b3fc4..393c5815f 100644 --- a/ibc-clients/ics08-wasm/types/Cargo.toml +++ b/ibc-clients/ics08-wasm/types/Cargo.toml @@ -18,7 +18,6 @@ description = """ [dependencies] # external dependencies base64 = { workspace = true, features = [ "alloc" ] } -hex = { workspace = true } displaydoc = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, optional = true } @@ -41,7 +40,6 @@ std = [ "ibc-primitives/std", "ibc-proto/std", "base64/std", - "hex/std", "serde/std", ] serde = [ diff --git a/ibc-clients/ics08-wasm/types/src/client_state.rs b/ibc-clients/ics08-wasm/types/src/client_state.rs index 3a2355bea..e9570b320 100644 --- a/ibc-clients/ics08-wasm/types/src/client_state.rs +++ b/ibc-clients/ics08-wasm/types/src/client_state.rs @@ -7,7 +7,7 @@ use ibc_proto::ibc::lightclients::wasm::v1::ClientState as RawClientState; use crate::error::Error; #[cfg(feature = "serde")] -use crate::serializer::{Base64, Hex}; +use crate::serializer::Base64; use crate::Bytes; pub const WASM_CLIENT_STATE_TYPE_URL: &str = "/ibc.lightclients.wasm.v1.ClientState"; @@ -20,7 +20,7 @@ pub struct ClientState { #[cfg_attr(feature = "serde", serde(with = "Base64", default))] pub data: Bytes, #[cfg_attr(feature = "schema", schemars(with = "String"))] - #[cfg_attr(feature = "serde", serde(with = "Hex", default))] + #[cfg_attr(feature = "serde", serde(with = "Base64", default))] pub checksum: Bytes, pub latest_height: Height, } diff --git a/ibc-clients/ics08-wasm/types/src/serializer.rs b/ibc-clients/ics08-wasm/types/src/serializer.rs index 39d658216..c44dc7325 100644 --- a/ibc-clients/ics08-wasm/types/src/serializer.rs +++ b/ibc-clients/ics08-wasm/types/src/serializer.rs @@ -4,7 +4,6 @@ use ibc_primitives::prelude::*; use serde::de::Error; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -/// Defines a convenient base64 (de)serializer for `Bytes`. pub struct Base64; impl Base64 { @@ -23,59 +22,25 @@ impl Base64 { } } -/// Defines a convenient hex (de)serializer for `Bytes`. -pub struct Hex; - -impl Hex { - pub fn serialize(bytes: &[u8], serializer: S) -> Result { - let hex = hex::encode(bytes); - String::serialize(&hex, serializer) - } - - pub fn deserialize<'de, D: Deserializer<'de>>(deserializer: D) -> Result, D::Error> { - let hex = String::deserialize(deserializer)?; - let bytes = hex::decode(hex).map_err(Error::custom)?; - - Ok(bytes) - } -} - #[cfg(test)] mod tests { use rstest::rstest; use super::*; - use crate::Bytes; #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] - struct Foo(#[serde(with = "Base64")] Bytes); + struct Foo(#[serde(with = "Base64")] crate::Bytes); - // Ensures `Base64` serialize and deserialize work as expected + // Ensures Base64 serialize and deserialize work as expected #[rstest] #[case(b"", "")] #[case(&[118], "dg==")] #[case(&[0x1, 0x2, 0x3, 0x4, 0x5], "AQIDBAU=")] #[case(b"hello world", "aGVsbG8gd29ybGQ=")] - pub fn test_base64_serializer(#[case] bytes: &[u8], #[case] hash: &str) { + pub fn test_ser_and_deser(#[case] bytes: &[u8], #[case] hash: &str) { let foo = Foo(bytes.to_vec()); let json = format!("\"{hash}\""); assert_eq!(serde_json::to_string(&foo).unwrap(), json); assert_eq!(serde_json::from_str::(&json).unwrap(), foo); } - - #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] - struct Bar(#[serde(with = "Hex")] Bytes); - - // Ensures `Hex` serialize and deserialize work as expected - #[rstest] - #[case(b"", "")] - #[case(&[118], "76")] - #[case(&[0x1, 0x2, 0x3, 0x4, 0x5], "0102030405")] - #[case(b"hello world", "68656c6c6f20776f726c64")] - pub fn test_hex_serializer(#[case] bytes: &[u8], #[case] hash: &str) { - let bar = Bar(bytes.to_vec()); - let json = format!("\"{hash}\""); - assert_eq!(serde_json::to_string(&bar).unwrap(), json); - assert_eq!(serde_json::from_str::(&json).unwrap(), bar); - } } From 4453cbd5d085ef008ef8d379d6b990c705367e04 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 18 Jul 2024 15:12:42 -0700 Subject: [PATCH 07/10] chore: update changelog --- .../unreleased/bug-fixes/1283-serialize-checksum-with-hex.md | 2 -- .../improvements/1283-remove-cosmwasm-feature-flag.md | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 .changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md create mode 100644 .changelog/unreleased/improvements/1283-remove-cosmwasm-feature-flag.md diff --git a/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md b/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md deleted file mode 100644 index f5dbd9cfd..000000000 --- a/.changelog/unreleased/bug-fixes/1283-serialize-checksum-with-hex.md +++ /dev/null @@ -1,2 +0,0 @@ -- [ibc-client-wasm-types] De/Serialize checksum fields with `hex` - ([#1285](https://github.com/cosmos/ibc-rs/issues/1285)). diff --git a/.changelog/unreleased/improvements/1283-remove-cosmwasm-feature-flag.md b/.changelog/unreleased/improvements/1283-remove-cosmwasm-feature-flag.md new file mode 100644 index 000000000..17e4c9285 --- /dev/null +++ b/.changelog/unreleased/improvements/1283-remove-cosmwasm-feature-flag.md @@ -0,0 +1,3 @@ +- [ibc-client-wasm-type] Remove the `cosmwasm` feature for consistent feature + flags across ibc-rs, and use existing `serde` and `schema` features. + ([\#1283](https://github.com/cosmos/ibc-rs/pull/1283)) From cb83f438f33b6aa1e20c4395dff2dbf6f6020cf2 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 18 Jul 2024 16:56:39 -0700 Subject: [PATCH 08/10] fix: use Binary for MeklePath + some helper methods --- ibc-clients/cw-context/src/types/msgs.rs | 10 +++++++--- ibc-core/ics23-commitment/types/src/commitment.rs | 6 ++++++ ibc-core/ics24-host/types/src/path.rs | 8 ++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index fbbe4740b..25b55313b 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -5,7 +5,6 @@ use cosmwasm_std::Binary; use ibc_core::client::types::proto::v1::Height as RawHeight; use ibc_core::client::types::Height; use ibc_core::commitment_types::commitment::{CommitmentPrefix, CommitmentProofBytes}; -use ibc_core::commitment_types::merkle::MerklePath; use ibc_core::host::types::path::PathBytes; use ibc_core::primitives::proto::Any; use prost::Message; @@ -115,6 +114,11 @@ impl TryFrom for VerifyUpgradeAndUpdateStateM } } +#[cw_serde] +pub struct MerklePath { + pub key_path: Vec, +} + #[cw_serde] pub struct VerifyMembershipMsgRaw { pub proof: Binary, @@ -140,7 +144,7 @@ impl TryFrom for VerifyMembershipMsg { fn try_from(mut raw: VerifyMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof.to_vec())?; - let prefix = CommitmentPrefix::from(raw.path.key_path.remove(0).into_vec()); + let prefix = CommitmentPrefix::from_bytes(raw.path.key_path.remove(0)); let path = PathBytes::flatten(raw.path.key_path); let height = Height::try_from(raw.height)?; @@ -179,7 +183,7 @@ impl TryFrom for VerifyNonMembershipMsg { fn try_from(mut raw: VerifyNonMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof.to_vec())?; - let prefix = CommitmentPrefix::from(raw.path.key_path.remove(0).into_vec()); + let prefix = CommitmentPrefix::from_bytes(raw.path.key_path.remove(0)); let path = PathBytes::flatten(raw.path.key_path); let height = raw.height.try_into()?; diff --git a/ibc-core/ics23-commitment/types/src/commitment.rs b/ibc-core/ics23-commitment/types/src/commitment.rs index d9a911135..2555f89ac 100644 --- a/ibc-core/ics23-commitment/types/src/commitment.rs +++ b/ibc-core/ics23-commitment/types/src/commitment.rs @@ -149,6 +149,12 @@ pub struct CommitmentPrefix { } impl CommitmentPrefix { + pub fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { + Self { + bytes: bytes.as_ref().to_vec(), + } + } + pub fn as_bytes(&self) -> &[u8] { &self.bytes } diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index c9e384989..bd6b120de 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -51,6 +51,10 @@ pub const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState"; pub struct PathBytes(Vec); impl PathBytes { + pub fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { + Self(bytes.as_ref().to_vec()) + } + pub fn is_empty(&self) -> bool { self.0.is_empty() } @@ -60,10 +64,10 @@ impl PathBytes { } /// Flattens a list of path bytes into a single path. - pub fn flatten(paths: Vec) -> Self { + pub fn flatten>(paths: Vec) -> Self { let mut bytes = Vec::new(); paths.iter().for_each(|path| { - bytes.extend_from_slice(&path.0); + bytes.extend_from_slice(path.as_ref()); }); Self(bytes) } From 8f4661bda3357045b373ad23b4ee98d191c6a1f9 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Fri, 19 Jul 2024 08:44:56 +0200 Subject: [PATCH 09/10] add regression test --- ibc-clients/cw-context/Cargo.toml | 3 +++ ibc-clients/cw-context/src/types/msgs.rs | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/ibc-clients/cw-context/Cargo.toml b/ibc-clients/cw-context/Cargo.toml index b478e7c22..ead5e2601 100644 --- a/ibc-clients/cw-context/Cargo.toml +++ b/ibc-clients/cw-context/Cargo.toml @@ -31,6 +31,9 @@ cosmwasm-schema = { workspace = true } cosmwasm-std = { workspace = true } cw-storage-plus = { workspace = true } +[dev-dependencies] +serde-json = { workspace = true } + [features] default = [ "std" ] std = [ diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index 25b55313b..47f677a68 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -268,3 +268,24 @@ impl TryFrom for CheckForMisbehaviourMsg { Ok(Self { client_message }) } } + +#[cfg(test)] +mod test { + use super::SudoMsg; + + #[test] + fn deserialize_from_json() { + let sudo_msg = r#"{ + "verify_membership":{ + "height": + {"revision_height":57}, + "delay_time_period":0, + "delay_block_period":0, + "proof":"CuECCt4CChhjb25uZWN0aW9ucy9jb25uZWN0aW9uLTASWgoPMDctdGVuZGVybWludC0wEiMKATESDU9SREVSX09SREVSRUQSD09SREVSX1VOT1JERVJFRBgCIiAKCTA4LXdhc20tMBIMY29ubmVjdGlvbi0wGgUKA2liYxoLCAEYASABKgMAAkgiKQgBEiUCBHAg3HTYmBAMxlr6u0mv6wCpm3ur2WQc7A3Af6aV7Ye0Fe0gIisIARIEBAZwIBohIHXEkQ9RIH08ZZYBIP6THxOOJiRmjXWGn1G4RCWT3V6rIisIARIEBgxwIBohIEUjGWV7YLPEzdFVLAb0lv4VvP7A+l1TqFkjpx1kDKAPIikIARIlCBhwILWsAKEot+2MknVyn5zcS0qsqVhRj4AHpgDx7fNPbfhtICIpCAESJQxAcCCzyYMGE+CdCltudr1ddHvCJrqv3kl/i7YnMLx3XWJt/yAK/AEK+QEKA2liYxIg2nvqL76rejXXGlX6ng/UKrbw+72C8uKKgM2vP0JKj1QaCQgBGAEgASoBACIlCAESIQEGuZwNgRn/HtvL4WXQ8ZM327wIDmd8iOV6oq52fr8PDyInCAESAQEaIKplBAbqDXbjndQ9LqapHj/aockI/CGnymjl5izIEVY5IiUIARIhAdt4G8DCLINAaaJnhUMIzv74AV3zZiugAyyZ/lWYRv+cIiUIARIhAf+sohoEV+uWeKThAPEbqCUivWT4H8KNT7Giw9//LsyvIicIARIBARogNHO4HC5KxPCwBdQGgBCscVtEKw+YSn2pnf654Y3Oxik=", + "path":{"key_path":["aWJj","Y29ubmVjdGlvbnMvY29ubmVjdGlvbi0w"]}, + "value":"Cg8wNy10ZW5kZXJtaW50LTASIwoBMRINT1JERVJfT1JERVJFRBIPT1JERVJfVU5PUkRFUkVEGAIiIAoJMDgtd2FzbS0wEgxjb25uZWN0aW9uLTAaBQoDaWJj" + } + }"#; + serde_json::from_str::(sudo_msg).unwrap(); + } +} From b4a198b661c9ef1771f713bdd0a02cd8e561a6db Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Fri, 19 Jul 2024 18:17:33 +0200 Subject: [PATCH 10/10] update tests --- ibc-clients/cw-context/src/types/msgs.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index 47f677a68..232bd0595 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -271,10 +271,10 @@ impl TryFrom for CheckForMisbehaviourMsg { #[cfg(test)] mod test { - use super::SudoMsg; + use super::{InstantiateMsg, SudoMsg}; #[test] - fn deserialize_from_json() { + fn verify_membership_from_json() { let sudo_msg = r#"{ "verify_membership":{ "height": @@ -286,6 +286,19 @@ mod test { "value":"Cg8wNy10ZW5kZXJtaW50LTASIwoBMRINT1JERVJfT1JERVJFRBIPT1JERVJfVU5PUkRFUkVEGAIiIAoJMDgtd2FzbS0wEgxjb25uZWN0aW9uLTAaBQoDaWJj" } }"#; - serde_json::from_str::(sudo_msg).unwrap(); + assert!(matches!( + serde_json::from_str::(sudo_msg).unwrap(), + SudoMsg::VerifyMembership(_) + )); + } + + #[test] + fn instantiate_msg_from_json() { + let instantiate_msg = r#"{ + "client_state":"Y2xpZW50X3N0YXRlCg==", + "consensus_state":"Y29uc2Vuc3VzX3N0YXRlCg==", + "checksum":"Y2hlY2tzdW0K" + }"#; + serde_json::from_str::(instantiate_msg).unwrap(); } }