feat: allow custom HostFunctionsProvider in MerkleProof (#1158)

* feat: allow HostFunctionsProvider in MerkleProof * nit: consistent generic name H * feat: use hash_with<H> for hashing validator sets * fix: make verify_upgrade_client func generic over HostFunctionsProvider * chore: add changelog
informalsystems · Apr 8, 2024 · f7a3c2b · f7a3c2b
1 parent 91bee67
commit f7a3c2b
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 72 deletions.
diff --git a/.changelog/v0.51.0/feature/1147-allow-custom-HostFunctionsProvider.md b/.changelog/v0.51.0/feature/1147-allow-custom-HostFunctionsProvider.md
@@ -0,0 +1,4 @@
+- [ibc] Enable the use of custom hashing functions by making `MerkleProof`
+  validation methods generic over `HostFunctionsProvider` and using
+  `hash_with<H>()` instead of `hash()` wherever validators' hash is computed.
+  ([\#1147](https://github.com/cosmos/ibc-rs/issues/1147))
diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs
@@ -7,6 +7,8 @@ use ibc_core_commitment_types::commitment::{
     CommitmentPrefix, CommitmentProofBytes, CommitmentRoot,
 };
 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;
 use ibc_core_host::types::identifiers::ClientType;
 use ibc_core_host::types::path::{Path, UpgradeClientPath};
 use ibc_primitives::prelude::*;
@@ -41,7 +43,7 @@ impl ClientStateCommon for ClientState {
         proof_upgrade_consensus_state: CommitmentProofBytes,
         root: &CommitmentRoot,
     ) -> Result<(), ClientError> {
-        verify_upgrade_client(
+        verify_upgrade_client::<HostFunctionsManager>(
             self.inner(),
             upgraded_client_state,
             upgraded_consensus_state,
@@ -59,7 +61,14 @@ impl ClientStateCommon for ClientState {
         path: Path,
         value: Vec<u8>,
     ) -> Result<(), ClientError> {
-        verify_membership(self.inner(), prefix, proof, root, path, value)
+        verify_membership::<HostFunctionsManager>(
+            &self.inner().proof_specs,
+            prefix,
+            proof,
+            root,
+            path,
+            value,
+        )
     }
 
     fn verify_non_membership(
@@ -69,7 +78,13 @@ impl ClientStateCommon for ClientState {
         root: &CommitmentRoot,
         path: Path,
     ) -> Result<(), ClientError> {
-        verify_non_membership(self.inner(), prefix, proof, root, path)
+        verify_non_membership::<HostFunctionsManager>(
+            &self.inner().proof_specs,
+            prefix,
+            proof,
+            root,
+            path,
+        )
     }
 }
 
@@ -124,7 +139,7 @@ pub fn validate_proof_height(
 /// Note that this function is typically implemented as part of the
 /// [`ClientStateCommon`] trait, but has been made a standalone function
 /// in order to make the ClientState APIs more flexible.
-pub fn verify_upgrade_client(
+pub fn verify_upgrade_client<H: HostFunctionsProvider>(
     client_state: &ClientStateType,
     upgraded_client_state: Any,
     upgraded_consensus_state: Any,
@@ -166,8 +181,8 @@ pub fn verify_upgrade_client(
     let last_height = latest_height.revision_height();
 
     // Verify the proof of the upgraded client state
-    verify_membership(
-        client_state,
+    verify_membership::<H>(
+        &client_state.proof_specs,
         &upgrade_path_prefix,
         &proof_upgrade_client,
         root,
@@ -176,8 +191,8 @@ pub fn verify_upgrade_client(
     )?;
 
     // Verify the proof of the upgraded consensus state
-    verify_membership(
-        client_state,
+    verify_membership::<H>(
+        &client_state.proof_specs,
         &upgrade_path_prefix,
         &proof_upgrade_consensus_state,
         root,
@@ -193,8 +208,8 @@ pub fn verify_upgrade_client(
 /// Note that this function is typically implemented as part of the
 /// [`ClientStateCommon`] trait, but has been made a standalone function
 /// in order to make the ClientState APIs more flexible.
-pub fn verify_membership(
-    client_state: &ClientStateType,
+pub fn verify_membership<H: HostFunctionsProvider>(
+    proof_specs: &ProofSpecs,
     prefix: &CommitmentPrefix,
     proof: &CommitmentProofBytes,
     root: &CommitmentRoot,
@@ -205,13 +220,7 @@ pub fn verify_membership(
     let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?;
 
     merkle_proof
-        .verify_membership(
-            &client_state.proof_specs,
-            root.clone().into(),
-            merkle_path,
-            value,
-            0,
-        )
+        .verify_membership::<H>(proof_specs, root.clone().into(), merkle_path, value, 0)
         .map_err(ClientError::Ics23Verification)
 }
 
@@ -220,8 +229,8 @@ pub fn verify_membership(
 /// Note that this function is typically implemented as part of the
 /// [`ClientStateCommon`] trait, but has been made a standalone function
 /// in order to make the ClientState APIs more flexible.
-pub fn verify_non_membership(
-    client_state: &ClientStateType,
+pub fn verify_non_membership<H: HostFunctionsProvider>(
+    proof_specs: &ProofSpecs,
     prefix: &CommitmentPrefix,
     proof: &CommitmentProofBytes,
     root: &CommitmentRoot,
@@ -231,6 +240,6 @@ pub fn verify_non_membership(
     let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?;
 
     merkle_proof
-        .verify_non_membership(&client_state.proof_specs, root.clone().into(), merkle_path)
+        .verify_non_membership::<H>(proof_specs, root.clone().into(), merkle_path)
         .map_err(ClientError::Ics23Verification)
 }
diff --git a/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs b/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs
@@ -5,6 +5,8 @@ use ibc_core_host::types::identifiers::{ChainId, ClientId};
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
 use ibc_primitives::Timestamp;
+use tendermint::crypto::Sha256;
+use tendermint::merkle::MerkleHash;
 use tendermint::{Hash, Time};
 use tendermint_light_client_verifier::options::Options;
 use tendermint_light_client_verifier::Verifier;
@@ -15,7 +17,7 @@ use crate::types::Header;
 
 /// Determines whether or not two conflicting headers at the same height would
 /// have convinced the light client.
-pub fn verify_misbehaviour<V>(
+pub fn verify_misbehaviour<V, H>(
     ctx: &V,
     misbehaviour: &TmMisbehaviour,
     client_id: &ClientId,
@@ -26,8 +28,9 @@ pub fn verify_misbehaviour<V>(
 where
     V: TmValidationContext,
     V::ConsensusStateRef: ConsensusStateConverter,
+    H: MerkleHash + Sha256 + Default,
 {
-    misbehaviour.validate_basic()?;
+    misbehaviour.validate_basic::<H>()?;
 
     let header_1 = misbehaviour.header1();
     let trusted_consensus_state_1 = {
@@ -55,7 +58,7 @@ where
 
     let current_timestamp = ctx.host_timestamp()?;
 
-    verify_misbehaviour_header(
+    verify_misbehaviour_header::<H>(
         header_1,
         chain_id,
         options,
@@ -64,7 +67,7 @@ where
         current_timestamp,
         verifier,
     )?;
-    verify_misbehaviour_header(
+    verify_misbehaviour_header::<H>(
         header_2,
         chain_id,
         options,
@@ -75,17 +78,20 @@ where
     )
 }
 
-pub fn verify_misbehaviour_header(
+pub fn verify_misbehaviour_header<H>(
     header: &TmHeader,
     chain_id: &ChainId,
     options: &Options,
     trusted_timestamp: Time,
     trusted_next_validator_hash: Hash,
     current_timestamp: Timestamp,
     verifier: &impl TmVerifier,
-) -> Result<(), ClientError> {
+) -> Result<(), ClientError>
+where
+    H: MerkleHash + Sha256 + Default,
+{
     // ensure correctness of the trusted next validator set provided by the relayer
-    header.check_trusted_next_validator_set(&trusted_next_validator_hash)?;
+    header.check_trusted_next_validator_set::<H>(&trusted_next_validator_hash)?;
 
     // ensure trusted consensus state is within trusting period
     {

diff --git a/ibc-clients/ics07-tendermint/src/client_state/update_client.rs b/ibc-clients/ics07-tendermint/src/client_state/update_client.rs
@@ -5,6 +5,8 @@ use ibc_core_client::types::Height;
 use ibc_core_host::types::identifiers::{ChainId, ClientId};
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
+use tendermint::crypto::Sha256;
+use tendermint::merkle::MerkleHash;
 use tendermint_light_client_verifier::options::Options;
 use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState};
 use tendermint_light_client_verifier::Verifier;
@@ -13,7 +15,7 @@ use crate::context::{
     ConsensusStateConverter, TmVerifier, ValidationContext as TmValidationContext,
 };
 
-pub fn verify_header<V>(
+pub fn verify_header<V, H>(
     ctx: &V,
     header: &TmHeader,
     client_id: &ClientId,
@@ -24,9 +26,10 @@ pub fn verify_header<V>(
 where
     V: TmValidationContext,
     V::ConsensusStateRef: ConsensusStateConverter,
+    H: MerkleHash + Sha256 + Default,
 {
     // Checks that the header fields are valid.
-    header.validate_basic()?;
+    header.validate_basic::<H>()?;
 
     // The tendermint-light-client crate though works on heights that are assumed
     // to have the same revision number. We ensure this here.
@@ -45,8 +48,9 @@ where
                 .consensus_state(&trusted_client_cons_state_path)?
                 .try_into()?;
 
-            header
-                .check_trusted_next_validator_set(&trusted_consensus_state.next_validators_hash)?;
+            header.check_trusted_next_validator_set::<H>(
+                &trusted_consensus_state.next_validators_hash,
+            )?;
 
             TrustedBlockState {
                 chain_id: &chain_id

diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs
@@ -9,6 +9,9 @@ use ibc_core_host::types::identifiers::ClientId;
 use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
+use tendermint::crypto::default::Sha256;
+use tendermint::crypto::Sha256 as Sha256Trait;
+use tendermint::merkle::MerkleHash;
 
 use super::{check_for_misbehaviour_on_misbehavior, check_for_misbehaviour_on_update, ClientState};
 use crate::client_state::{verify_header, verify_misbehaviour};
@@ -31,7 +34,7 @@ where
         client_id: &ClientId,
         client_message: Any,
     ) -> Result<(), ClientError> {
-        verify_client_message(
+        verify_client_message::<V, Sha256>(
             self.inner(),
             ctx,
             client_id,
@@ -67,7 +70,7 @@ where
 /// function, except for an additional `verifier` parameter that allows users
 /// who require custom verification logic to easily pass in their own verifier
 /// implementation.
-pub fn verify_client_message<V>(
+pub fn verify_client_message<V, H>(
     client_state: &ClientStateType,
     ctx: &V,
     client_id: &ClientId,
@@ -77,11 +80,12 @@ pub fn verify_client_message<V>(
 where
     V: TmValidationContext,
     V::ConsensusStateRef: ConsensusStateConverter,
+    H: MerkleHash + Sha256Trait + Default,
 {
     match client_message.type_url.as_str() {
         TENDERMINT_HEADER_TYPE_URL => {
             let header = TmHeader::try_from(client_message)?;
-            verify_header(
+            verify_header::<V, H>(
                 ctx,
                 &header,
                 client_id,
@@ -92,7 +96,7 @@ where
         }
         TENDERMINT_MISBEHAVIOUR_TYPE_URL => {
             let misbehaviour = TmMisbehaviour::try_from(client_message)?;
-            verify_misbehaviour(
+            verify_misbehaviour::<V, H>(
                 ctx,
                 &misbehaviour,
                 client_id,

diff --git a/ibc-clients/ics07-tendermint/types/src/header.rs b/ibc-clients/ics07-tendermint/types/src/header.rs
@@ -14,6 +14,8 @@ use ibc_proto::Protobuf;
 use pretty::{PrettySignedHeader, PrettyValidatorSet};
 use tendermint::block::signed_header::SignedHeader;
 use tendermint::chain::Id as TmChainId;
+use tendermint::crypto::Sha256;
+use tendermint::merkle::MerkleHash;
 use tendermint::validator::Set as ValidatorSet;
 use tendermint::{Hash, Time};
 use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState};
@@ -101,11 +103,11 @@ impl Header {
     /// `header.trusted_next_validator_set` was given to us by the relayer.
     /// Thus, we need to ensure that the relayer gave us the right set, i.e. by
     /// ensuring that it matches the hash we have stored on chain.
-    pub fn check_trusted_next_validator_set(
+    pub fn check_trusted_next_validator_set<H: MerkleHash + Sha256 + Default>(
         &self,
         trusted_next_validator_hash: &Hash,
     ) -> Result<(), ClientError> {
-        if &self.trusted_next_validator_set.hash() == trusted_next_validator_hash {
+        if &self.trusted_next_validator_set.hash_with::<H>() == trusted_next_validator_hash {
             Ok(())
         } else {
             Err(ClientError::HeaderVerificationFailure {
@@ -117,7 +119,7 @@ impl Header {
     }
 
     /// Checks if the fields of a given header are consistent with the trusted fields of this header.
-    pub fn validate_basic(&self) -> Result<(), Error> {
+    pub fn validate_basic<H: MerkleHash + Sha256 + Default>(&self) -> Result<(), Error> {
         if self.height().revision_number() != self.trusted_height.revision_number() {
             return Err(Error::MismatchHeightRevisions {
                 trusted_revision: self.trusted_height.revision_number(),
@@ -135,10 +137,12 @@ impl Header {
             });
         }
 
-        if self.validator_set.hash() != self.signed_header.header.validators_hash {
+        let validators_hash = self.validator_set.hash_with::<H>();
+
+        if validators_hash != self.signed_header.header.validators_hash {
             return Err(Error::MismatchValidatorsHashes {
                 signed_header_validators_hash: self.signed_header.header.validators_hash,
-                validators_hash: self.validator_set.hash(),
+                validators_hash,
             });
         }
 

diff --git a/ibc-clients/ics07-tendermint/types/src/misbehaviour.rs b/ibc-clients/ics07-tendermint/types/src/misbehaviour.rs
@@ -6,6 +6,8 @@ use ibc_primitives::prelude::*;
 use ibc_proto::google::protobuf::Any;
 use ibc_proto::ibc::lightclients::tendermint::v1::Misbehaviour as RawMisbehaviour;
 use ibc_proto::Protobuf;
+use tendermint::crypto::Sha256;
+use tendermint::merkle::MerkleHash;
 
 use crate::error::Error;
 use crate::header::Header;
@@ -42,9 +44,9 @@ impl Misbehaviour {
         &self.header2
     }
 
-    pub fn validate_basic(&self) -> Result<(), Error> {
-        self.header1.validate_basic()?;
-        self.header2.validate_basic()?;
+    pub fn validate_basic<H: MerkleHash + Sha256 + Default>(&self) -> Result<(), Error> {
+        self.header1.validate_basic::<H>()?;
+        self.header2.validate_basic::<H>()?;
 
         if self.header1.signed_header.header.chain_id != self.header2.signed_header.header.chain_id
         {