Skip to content

Commit

Permalink
fix: use Binary for checksum and merkle path so base64 deser works (#…
Browse files Browse the repository at this point in the history
…1283)

* fix: de/serialize checksum with hex

* imp: remove unnecessary cosmwasm feature

* chore: add changelog

* fix: use Binary as checksum type for InstantiateMsg

* chore: update changelog

* fix: revert checksum type back to Binary and use base64 deser

* chore: update changelog

* fix: use Binary for MeklePath + some helper methods

* add regression test

* update tests

---------

Co-authored-by: Ranadeep Biswas <mail@rnbguy.at>
  • Loading branch information
Farhad-Shabani and rnbguy committed Jul 19, 2024
1 parent f7500f2 commit 3208b8a
Show file tree
Hide file tree
Showing 15 changed files with 98 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -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))
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 1 addition & 0 deletions ci/no-std-check/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion ibc-clients/cw-context/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ 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 }
cosmwasm-std = { workspace = true }
cw-storage-plus = { workspace = true }

[dev-dependencies]
serde-json = { workspace = true }

[features]
default = [ "std" ]
std = [
Expand Down
19 changes: 6 additions & 13 deletions ibc-clients/cw-context/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -40,7 +40,7 @@ where
deps_mut: Option<DepsMut<'a>>,
env: Env,
client_id: ClientId,
checksum: Option<Checksum>,
checksum: Option<Binary>,
migration_prefix: MigrationPrefix,
client_type: std::marker::PhantomData<C>,
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -272,9 +272,9 @@ where
}

/// Returns the checksum of the current contract.
pub fn obtain_checksum(&self) -> Result<Checksum, ClientError> {
pub fn obtain_checksum(&self) -> Result<Binary, ClientError> {
match &self.checksum {
Some(checksum) => Ok(*checksum),
Some(checksum) => Ok(checksum.clone()),
None => {
let client_state_value = self.retrieve(ClientStatePath::leaf())?;

Expand All @@ -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())
}
}
}
Expand Down
48 changes: 43 additions & 5 deletions ibc-clients/cw-context/src/types/msgs.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
//! 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};
use ibc_core::commitment_types::merkle::MerklePath;
use ibc_core::host::types::path::PathBytes;
use ibc_core::primitives::proto::Any;
use prost::Message;
Expand All @@ -20,7 +19,7 @@ use super::error::ContractError;
pub struct InstantiateMsg {
pub client_state: Binary,
pub consensus_state: Binary,
pub checksum: Checksum,
pub checksum: Binary,
}

// ------------------------------------------------------------
Expand Down Expand Up @@ -115,6 +114,11 @@ impl TryFrom<VerifyUpgradeAndUpdateStateMsgRaw> for VerifyUpgradeAndUpdateStateM
}
}

#[cw_serde]
pub struct MerklePath {
pub key_path: Vec<Binary>,
}

#[cw_serde]
pub struct VerifyMembershipMsgRaw {
pub proof: Binary,
Expand All @@ -140,7 +144,7 @@ impl TryFrom<VerifyMembershipMsgRaw> for VerifyMembershipMsg {

fn try_from(mut raw: VerifyMembershipMsgRaw) -> Result<Self, Self::Error> {
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)?;

Expand Down Expand Up @@ -179,7 +183,7 @@ impl TryFrom<VerifyNonMembershipMsgRaw> for VerifyNonMembershipMsg {

fn try_from(mut raw: VerifyNonMembershipMsgRaw) -> Result<Self, Self::Error> {
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()?;

Expand Down Expand Up @@ -264,3 +268,37 @@ impl TryFrom<CheckForMisbehaviourMsgRaw> for CheckForMisbehaviourMsg {
Ok(Self { client_message })
}
}

#[cfg(test)]
mod test {
use super::{InstantiateMsg, SudoMsg};

#[test]
fn verify_membership_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"
}
}"#;
assert!(matches!(
serde_json::from_str::<SudoMsg>(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::<InstantiateMsg>(instantiate_msg).unwrap();
}
}
13 changes: 5 additions & 8 deletions ibc-clients/ics08-wasm/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ description = """

[dependencies]
# external dependencies
base64 = { workspace = true, features = [ "alloc" ] }
displaydoc = { workspace = true }
serde = { workspace = true, optional = true }
cosmwasm-schema = { workspace = true, optional = true }
base64 = { workspace = true, features = [ "alloc" ] }
displaydoc = { workspace = true }
schemars = { workspace = true, optional = true }
serde = { workspace = true, optional = true }

# ibc dependencies
ibc-core-client = { workspace = true }
Expand Down Expand Up @@ -54,10 +54,7 @@ schema = [
"ibc-core-host-types/schema",
"ibc-primitives/schema",
"ibc-proto/json-schema",
"schemars",
"serde",
"std",
]
cosmwasm = [
"cosmwasm-schema",
"schema",
]
18 changes: 8 additions & 10 deletions ibc-clients/ics08-wasm/types/src/client_state.rs
Original file line number Diff line number Diff line change
@@ -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;
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 = "Base64", default))]
#[cfg_attr(feature = "schema", schemars(with = "String"))]
#[cfg_attr(feature = "serde", serde(with = "Base64", default))]
pub checksum: Bytes,
pub latest_height: Height,
}
Expand Down
14 changes: 6 additions & 8 deletions ibc-clients/ics08-wasm/types/src/consensus_state.rs
Original file line number Diff line number Diff line change
@@ -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,
}

Expand Down
2 changes: 1 addition & 1 deletion ibc-clients/ics08-wasm/types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions ibc-core/ics23-commitment/types/src/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 6 additions & 2 deletions ibc-core/ics24-host/types/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ pub const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState";
pub struct PathBytes(Vec<u8>);

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()
}
Expand All @@ -60,10 +64,10 @@ impl PathBytes {
}

/// Flattens a list of path bytes into a single path.
pub fn flatten(paths: Vec<PathBytes>) -> Self {
pub fn flatten<T: AsRef<[u8]>>(paths: Vec<T>) -> 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)
}
Expand Down
3 changes: 0 additions & 3 deletions ibc-data-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
1 change: 1 addition & 0 deletions ibc-testkit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ rstest = { workspace = true }
[features]
default = [ "std" ]
std = [
"hex/std",
"serde/std",
"serde-json/std",
"ibc/std",
Expand Down
11 changes: 7 additions & 4 deletions tests-integration/tests/cosmwasm/helper.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand Down

0 comments on commit 3208b8a

Please sign in to comment.