diff --git a/.changelog/unreleased/features/1006-allow-empty-commitment-prefix.md b/.changelog/unreleased/features/1006-allow-empty-commitment-prefix.md new file mode 100644 index 000000000..08ed07818 --- /dev/null +++ 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 index e5647ee4c..2f15258aa 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -156,7 +156,7 @@ impl TryFrom 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 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 index b7a7daf5b..c7b9a9525 100644 --- 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( }); }; - 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( path: Path, value: Vec, ) -> 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 index decfbfe1e..14e86274e 100644 --- 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 index 227632161..dbfa87b01 100644 --- 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 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 index bc0707d9c..d9a911135 100644 --- 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, } @@ -160,17 +160,9 @@ impl CommitmentPrefix { pub fn empty() -> Self { Self { bytes: Vec::new() } } -} - -impl TryFrom> for CommitmentPrefix { - type Error = CommitmentError; - fn try_from(bytes: Vec) -> Result { - 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 index d650cccd1..a475f90d7 100644 --- 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 { diff --git a/tests-integration/tests/core/ics03_connection/conn_open_ack.rs b/tests-integration/tests/core/ics03_connection/conn_open_ack.rs index 8860e3227..651ab378f 100644 --- 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 { 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 index dc21dadef..12c24a97d 100644 --- 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 { 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 index 1afd22fe1..d75108a5a 100644 --- 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 index df946f1e3..121134aa1 100644 --- 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 index 3a2bace44..ed560f8fc 100644 --- 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 index 240ddd4a8..2849ce424 100644 --- 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 index a454261fd..c3b06c218 100644 --- 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,