Skip to content

Commit

Permalink
feat: allow custom HostFunctionsProvider in MerkleProof (#1158)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Farhad-Shabani authored Apr 8, 2024
1 parent 91bee67 commit f7a3c2b
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 72 deletions.
Original file line number Diff line number Diff line change
@@ -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))
49 changes: 29 additions & 20 deletions ibc-clients/ics07-tendermint/src/client_state/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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,
)
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)
}

Expand All @@ -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,
Expand All @@ -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)
}
20 changes: 13 additions & 7 deletions ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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 = {
Expand Down Expand Up @@ -55,7 +58,7 @@ where

let current_timestamp = ctx.host_timestamp()?;

verify_misbehaviour_header(
verify_misbehaviour_header::<H>(
header_1,
chain_id,
options,
Expand All @@ -64,7 +67,7 @@ where
current_timestamp,
verifier,
)?;
verify_misbehaviour_header(
verify_misbehaviour_header::<H>(
header_2,
chain_id,
options,
Expand All @@ -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
{
Expand Down
12 changes: 8 additions & 4 deletions ibc-clients/ics07-tendermint/src/client_state/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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
Expand Down
12 changes: 8 additions & 4 deletions ibc-clients/ics07-tendermint/src/client_state/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
14 changes: 9 additions & 5 deletions ibc-clients/ics07-tendermint/types/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
Expand All @@ -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,
});
}

Expand Down
8 changes: 5 additions & 3 deletions ibc-clients/ics07-tendermint/types/src/misbehaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down
Loading

0 comments on commit f7a3c2b

Please sign in to comment.