Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow custom HostFunctionsProvider in MerkleProof #1158

Merged
merged 6 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@
proof_upgrade_consensus_state: CommitmentProofBytes,
root: &CommitmentRoot,
) -> Result<(), ClientError> {
verify_upgrade_client(
verify_upgrade_client::<HostFunctionsManager>(

Check warning on line 46 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L46

Added line #L46 was not covered by tests
self.inner(),
upgraded_client_state,
upgraded_consensus_state,
Expand All @@ -59,7 +61,14 @@
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,
)

Check warning on line 71 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L64-L71

Added lines #L64 - L71 were not covered by tests
}

fn verify_non_membership(
Expand All @@ -69,7 +78,13 @@
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,
)

Check warning on line 87 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L81-L87

Added lines #L81 - L87 were not covered by tests
}
}

Expand Down Expand Up @@ -124,7 +139,7 @@
/// 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>(

Check warning on line 142 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L142

Added line #L142 was not covered by tests
client_state: &ClientStateType,
upgraded_client_state: Any,
upgraded_consensus_state: Any,
Expand Down Expand Up @@ -166,8 +181,8 @@
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,

Check warning on line 185 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L184-L185

Added lines #L184 - L185 were not covered by tests
&upgrade_path_prefix,
&proof_upgrade_client,
root,
Expand All @@ -176,8 +191,8 @@
)?;

// Verify the proof of the upgraded consensus state
verify_membership(
client_state,
verify_membership::<H>(
&client_state.proof_specs,

Check warning on line 195 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L194-L195

Added lines #L194 - L195 were not covered by tests
&upgrade_path_prefix,
&proof_upgrade_consensus_state,
root,
Expand All @@ -193,8 +208,8 @@
/// 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,

Check warning on line 212 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L211-L212

Added lines #L211 - L212 were not covered by tests
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
Expand All @@ -205,13 +220,7 @@
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)

Check warning on line 223 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L223

Added line #L223 was not covered by tests
.map_err(ClientError::Ics23Verification)
}

Expand All @@ -220,8 +229,8 @@
/// 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,

Check warning on line 233 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L232-L233

Added lines #L232 - L233 were not covered by tests
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
Expand All @@ -231,6 +240,6 @@
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)

Check warning on line 243 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L243

Added line #L243 was not covered by tests
.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 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 @@
/// `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 @@
}

/// 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 @@
});
}

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,

Check warning on line 145 in ibc-clients/ics07-tendermint/types/src/header.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/types/src/header.rs#L145

Added line #L145 was not covered by tests
});
}

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
Loading