feat: allow empty CommitmentPrefix (#1274)

* feat: allow empty CommitmentPrefix * fix: use from instead of try_from * use empty commitmentprefix * fix: cargo msrv by requireing cosmwasm to stay at 2.0.4 * fix: specify cosmwasm-derive version * fix: remove now unnecessary EmptyPrefix error variant --------- Co-authored-by: Ranadeep Biswas <[email protected]>
cosmos · Jul 11, 2024 · 0202dec · 0202dec
1 parent df9653a
commit 0202dec
Show file tree

Hide file tree

Showing 14 changed files with 24 additions and 28 deletions.
diff --git a/.changelog/unreleased/features/1006-allow-empty-commitment-prefix.md b/.changelog/unreleased/features/1006-allow-empty-commitment-prefix.md
@@ -0,0 +1,2 @@
+- [ibc-core-commitment-types] Allow empty `CommitmentPrefix`
+  ([#1006](https://github.com/cosmos/ibc-rs/issues/1006)).
diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs
@@ -156,7 +156,7 @@ impl TryFrom<VerifyMembershipMsgRaw> for VerifyMembershipMsg {
             path,
             value: raw.value.into(),
             height,
-            prefix: CommitmentPrefix::try_from(prefix)?,
+            prefix: CommitmentPrefix::from(prefix),
             delay_block_period: raw.delay_block_period,
             delay_time_period: raw.delay_time_period,
         })
@@ -194,7 +194,7 @@ impl TryFrom<VerifyNonMembershipMsgRaw> for VerifyNonMembershipMsg {
             proof,
             path,
             height,
-            prefix: CommitmentPrefix::try_from(prefix)?,
+            prefix: CommitmentPrefix::from(prefix),
             delay_block_period: raw.delay_block_period,
             delay_time_period: raw.delay_time_period,
         })

diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs
@@ -6,6 +6,7 @@ use ibc_core_client::types::Height;
 use ibc_core_commitment_types::commitment::{
     CommitmentPrefix, CommitmentProofBytes, CommitmentRoot,
 };
+use ibc_core_commitment_types::error::CommitmentError;
 use ibc_core_commitment_types::merkle::{apply_prefix, MerkleProof};
 use ibc_core_commitment_types::proto::ics23::{HostFunctionsManager, HostFunctionsProvider};
 use ibc_core_commitment_types::specs::ProofSpecs;
@@ -175,8 +176,7 @@ pub fn verify_upgrade_client<H: HostFunctionsProvider>(
         });
     };
 
-    let upgrade_path_prefix = CommitmentPrefix::try_from(upgrade_path[0].clone().into_bytes())
-        .map_err(ClientError::InvalidCommitmentProof)?;
+    let upgrade_path_prefix = CommitmentPrefix::from(upgrade_path[0].clone().into_bytes());
 
     let last_height = latest_height.revision_height();
 
@@ -216,6 +216,12 @@ pub fn verify_membership<H: HostFunctionsProvider>(
     path: Path,
     value: Vec<u8>,
 ) -> Result<(), ClientError> {
+    if prefix.is_empty() {
+        return Err(ClientError::Ics23Verification(
+            CommitmentError::EmptyCommitmentPrefix,
+        ));
+    }
+
     let merkle_path = apply_prefix(prefix, vec![path.to_string()]);
     let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?;
 

diff --git a/ibc-core/ics02-client/types/src/error.rs b/ibc-core/ics02-client/types/src/error.rs
@@ -46,8 +46,6 @@ pub enum ClientError {
     FailedTrustThresholdConversion { numerator: u64, denominator: u64 },
     /// unknown client state type: `{client_state_type}`
     UnknownClientStateType { client_state_type: String },
-    /// empty prefix
-    EmptyPrefix,
     /// unknown client consensus state type: `{consensus_state_type}`
     UnknownConsensusStateType { consensus_state_type: String },
     /// unknown header type: `{header_type}`

diff --git a/ibc-core/ics03-connection/types/src/connection.rs b/ibc-core/ics03-connection/types/src/connection.rs
@@ -3,7 +3,6 @@
 use core::fmt::{Display, Error as FmtError, Formatter};
 use core::time::Duration;
 
-use ibc_core_client_types::error::ClientError;
 use ibc_core_commitment_types::commitment::CommitmentPrefix;
 use ibc_core_host_types::identifiers::{ClientId, ConnectionId};
 use ibc_primitives::prelude::*;
@@ -392,8 +391,7 @@ impl TryFrom<RawCounterparty> for Counterparty {
                 .prefix
                 .ok_or(ConnectionError::MissingCounterparty)?
                 .key_prefix
-                .try_into()
-                .map_err(|_| ConnectionError::Client(ClientError::EmptyPrefix))?,
+                .into(),
         ))
     }
 }

diff --git a/ibc-core/ics23-commitment/types/src/commitment.rs b/ibc-core/ics23-commitment/types/src/commitment.rs
@@ -143,7 +143,7 @@ impl<'a> TryFrom<&'a CommitmentProofBytes> for MerkleProof {
 )]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[derive(Clone, PartialEq, Eq, Hash)]
+#[derive(Clone, PartialEq, Eq, Hash, derive_more::From)]
 pub struct CommitmentPrefix {
     bytes: Vec<u8>,
 }
@@ -160,17 +160,9 @@ impl CommitmentPrefix {
     pub fn empty() -> Self {
         Self { bytes: Vec::new() }
     }
-}
-
-impl TryFrom<Vec<u8>> for CommitmentPrefix {
-    type Error = CommitmentError;
 
-    fn try_from(bytes: Vec<u8>) -> Result<Self, Self::Error> {
-        if bytes.is_empty() {
-            Err(Self::Error::EmptyCommitmentPrefix)
-        } else {
-            Ok(Self { bytes })
-        }
+    pub fn is_empty(&self) -> bool {
+        self.bytes.is_empty()
     }
 }
 

diff --git a/ibc-testkit/src/testapp/ibc/core/core_ctx.rs b/ibc-testkit/src/testapp/ibc/core/core_ctx.rs
@@ -135,7 +135,7 @@ where
     fn commitment_prefix(&self) -> CommitmentPrefix {
         // this is prefix of ibc store
         // using a dummy prefix as in our mock context
-        CommitmentPrefix::try_from(b"mock".to_vec()).expect("Never fails")
+        CommitmentPrefix::from(b"mock".to_vec())
     }
 
     fn connection_counter(&self) -> Result<u64, ContextError> {

diff --git a/tests-integration/tests/core/ics03_connection/conn_open_ack.rs b/tests-integration/tests/core/ics03_connection/conn_open_ack.rs
@@ -48,7 +48,7 @@ fn conn_open_ack_fixture(ctx: Ctx) -> Fixture<MsgConnectionOpenAck> {
         Counterparty::new(
             client_id.clone(),
             Some(msg.conn_id_on_b.clone()),
-            CommitmentPrefix::try_from(b"ibc".to_vec()).unwrap(),
+            CommitmentPrefix::from(b"ibc".to_vec()),
         ),
         vec![msg.version.clone()],
         ZERO_DURATION,

diff --git a/tests-integration/tests/core/ics03_connection/conn_open_confirm.rs b/tests-integration/tests/core/ics03_connection/conn_open_confirm.rs
@@ -31,7 +31,7 @@ fn conn_open_confirm_fixture(ctx: Ctx) -> Fixture<MsgConnectionOpenConfirm> {
     let counterparty = Counterparty::new(
         client_id.clone(),
         Some(msg.conn_id_on_b.clone()),
-        CommitmentPrefix::try_from(b"ibc".to_vec()).unwrap(),
+        CommitmentPrefix::from(b"ibc".to_vec()),
     );
 
     let ctx_default = MockContext::default();

diff --git a/tests-integration/tests/core/ics04_channel/acknowledgement.rs b/tests-integration/tests/core/ics04_channel/acknowledgement.rs
@@ -75,7 +75,7 @@ fn fixture() -> Fixture {
         ConnectionCounterparty::new(
             default_client_id,
             Some(ConnectionId::zero()),
-            CommitmentPrefix::try_from(vec![0]).expect("no error"),
+            CommitmentPrefix::empty(),
         ),
         ConnectionVersion::compatibles(),
         ZERO_DURATION,

diff --git a/tests-integration/tests/core/ics04_channel/recv_packet.rs b/tests-integration/tests/core/ics04_channel/recv_packet.rs
@@ -65,7 +65,7 @@ fn fixture() -> Fixture {
         ConnectionCounterparty::new(
             client_id.clone(),
             Some(ConnectionId::zero()),
-            CommitmentPrefix::try_from(vec![0]).expect("no error"),
+            CommitmentPrefix::empty(),
         ),
         ConnectionVersion::compatibles(),
         ZERO_DURATION,

diff --git a/tests-integration/tests/core/ics04_channel/send_packet.rs b/tests-integration/tests/core/ics04_channel/send_packet.rs
@@ -47,7 +47,7 @@ fn send_packet_processing() {
         ConnectionCounterparty::new(
             default_client_id,
             Some(ConnectionId::zero()),
-            CommitmentPrefix::try_from(vec![0]).expect("no error"),
+            CommitmentPrefix::empty(),
         ),
         ConnectionVersion::compatibles(),
         ZERO_DURATION,

diff --git a/tests-integration/tests/core/ics04_channel/timeout.rs b/tests-integration/tests/core/ics04_channel/timeout.rs
@@ -86,7 +86,7 @@ fn fixture() -> Fixture {
         ConnectionCounterparty::new(
             client_id.clone(),
             Some(ConnectionId::zero()),
-            CommitmentPrefix::try_from(vec![0]).expect("no error"),
+            CommitmentPrefix::empty(),
         ),
         ConnectionVersion::compatibles(),
         ZERO_DURATION,

diff --git a/tests-integration/tests/core/ics04_channel/timeout_on_close.rs b/tests-integration/tests/core/ics04_channel/timeout_on_close.rs
@@ -69,7 +69,7 @@ fn fixture() -> Fixture {
         ConnectionCounterparty::new(
             default_client_id,
             Some(ConnectionId::zero()),
-            CommitmentPrefix::try_from(vec![0]).expect("no error"),
+            CommitmentPrefix::empty(),
         ),
         ConnectionVersion::compatibles(),
         ZERO_DURATION,