Skip to content

Commit

Permalink
feat: allow custom HostFunctionsProvider in MerkleProof (cosmos#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 and mina86 committed Oct 4, 2024
1 parent 1b34531 commit da0a87b
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 71 deletions.
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 @@ -8,14 +8,16 @@ use ibc_core_host::types::identifiers::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_light_client_verifier::Verifier;

use super::TmValidationContext;
use crate::context::{ConsensusStateConverter, TmVerifier};

/// 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>(
client_state: &ClientStateType,
ctx: &V,
client_id: &ClientId,
Expand All @@ -25,8 +27,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 @@ -54,14 +57,14 @@ where

let current_timestamp = ctx.host_timestamp()?;

verify_misbehaviour_header(
verify_misbehaviour_header::<H>(
client_state,
header_1,
&trusted_consensus_state_1,
current_timestamp,
verifier,
)?;
verify_misbehaviour_header(
verify_misbehaviour_header::<H>(
client_state,
header_2,
&trusted_consensus_state_2,
Expand All @@ -70,15 +73,18 @@ where
)
}

pub fn verify_misbehaviour_header(
pub fn verify_misbehaviour_header<H>(
client_state: &ClientStateType,
header: &TmHeader,
trusted_consensus_state: &ConsensusStateType,
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_consensus_state)?;
header.check_trusted_next_validator_set::<H>(trusted_consensus_state)?;

// ensure trusted consensus state is within trusting period
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ use ibc_core_client::types::error::ClientError;
use ibc_core_host::types::identifiers::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::types::{TrustedBlockState, UntrustedBlockState};
use tendermint_light_client_verifier::Verifier;

use crate::context::{
ConsensusStateConverter, TmVerifier, ValidationContext as TmValidationContext,
};

pub fn verify_header<V>(
pub fn verify_header<V, H>(
client_state: &ClientStateType,
ctx: &V,
client_id: &ClientId,
Expand All @@ -23,9 +25,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 @@ -44,7 +47,7 @@ where
.consensus_state(&trusted_client_cons_state_path)?
.try_into()?;

header.check_trusted_next_validator_set(&trusted_consensus_state)?;
header.check_trusted_next_validator_set::<H>(&trusted_consensus_state)?;

TrustedBlockState {
chain_id: &client_state.chain_id.to_string().try_into().map_err(|e| {
Expand Down
11 changes: 7 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,8 @@ 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::Sha256;
use tendermint::merkle::MerkleHash;

use super::{
check_for_misbehaviour_misbehavior, check_for_misbehaviour_update_client, ClientState,
Expand All @@ -33,7 +35,7 @@ where
client_id: &ClientId,
client_message: Any,
) -> Result<(), ClientError> {
verify_client_message(
verify_client_message::<V, tendermint::crypto::default::Sha256>(
self.inner(),
ctx,
client_id,
Expand Down Expand Up @@ -65,7 +67,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 @@ -75,15 +77,16 @@ pub fn verify_client_message<V>(
where
V: TmValidationContext,
V::ConsensusStateRef: ConsensusStateConverter,
H: MerkleHash + Sha256 + Default,
{
match client_message.type_url.as_str() {
TENDERMINT_HEADER_TYPE_URL => {
let header = TmHeader::try_from(client_message)?;
verify_header(client_state, ctx, client_id, &header, verifier)
verify_header::<V, H>(client_state, ctx, client_id, &header, verifier)
}
TENDERMINT_MISBEHAVIOUR_TYPE_URL => {
let misbehaviour = TmMisbehaviour::try_from(client_message)?;
verify_misbehaviour(client_state, ctx, client_id, &misbehaviour, verifier)
verify_misbehaviour::<V, H>(client_state, ctx, client_id, &misbehaviour, verifier)
}
_ => Err(ClientError::InvalidUpdateClientMessage),
}
Expand Down
15 changes: 10 additions & 5 deletions ibc-clients/ics07-tendermint/types/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use pretty::{PrettySignedHeader, PrettyValidatorSet};
use prost::Message;
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_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState};

Expand Down Expand Up @@ -101,11 +103,13 @@ impl Header {
// `header.trusted_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_consensus_state: &TmConsensusState,
) -> Result<(), ClientError> {
if self.trusted_next_validator_set.hash() == trusted_consensus_state.next_validators_hash {
if self.trusted_next_validator_set.hash_with::<H>()
== trusted_consensus_state.next_validators_hash
{
Ok(())
} else {
Err(ClientError::HeaderVerificationFailure {
Expand All @@ -117,7 +121,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 +139,11 @@ 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 @@ -7,6 +7,8 @@ use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::lightclients::tendermint::v1::Misbehaviour as RawMisbehaviour;
use ibc_proto::Protobuf;
use prost::Message;
use tendermint::crypto::Sha256;
use tendermint::merkle::MerkleHash;

use crate::error::Error;
use crate::header::Header;
Expand Down Expand Up @@ -43,9 +45,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 da0a87b

Please sign in to comment.