From 2ca18f5866c8f19f72f44320af26b3278474147a Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Mon, 15 Apr 2024 12:11:50 -0700 Subject: [PATCH 1/9] imp: simplify introducing custom verifier object --- .../src/client_state/misbehaviour.rs | 6 +-- .../src/client_state/update_client.rs | 5 +-- .../src/client_state/validation.rs | 22 ++++++----- ibc-clients/ics07-tendermint/src/lib.rs | 8 ++-- ibc-clients/ics07-tendermint/src/verifier.rs | 37 ------------------- 5 files changed, 20 insertions(+), 58 deletions(-) delete mode 100644 ibc-clients/ics07-tendermint/src/verifier.rs diff --git a/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs b/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs index 26a96de19..6750a44eb 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs @@ -15,7 +15,6 @@ use tendermint_light_client_verifier::options::Options; use tendermint_light_client_verifier::Verifier; use crate::types::Header; -use crate::verifier::TmVerifier; /// Determines whether or not two conflicting headers at the same height would /// have convinced the light client. @@ -25,7 +24,7 @@ pub fn verify_misbehaviour( client_id: &ClientId, chain_id: &ChainId, options: &Options, - verifier: &impl TmVerifier, + verifier: &impl Verifier, ) -> Result<(), ClientError> where V: ExtClientValidationContext, @@ -87,7 +86,7 @@ pub fn verify_misbehaviour_header( trusted_timestamp: Time, trusted_next_validator_hash: Hash, current_timestamp: Timestamp, - verifier: &impl TmVerifier, + verifier: &impl Verifier, ) -> Result<(), ClientError> where H: MerkleHash + Sha256 + Default, @@ -134,7 +133,6 @@ where })?; verifier - .verifier() .verify_misbehaviour_header(untrusted_state, trusted_state, options, current_timestamp) .into_result()?; diff --git a/ibc-clients/ics07-tendermint/src/client_state/update_client.rs b/ibc-clients/ics07-tendermint/src/client_state/update_client.rs index 7b94f2d0e..e715643fb 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/update_client.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/update_client.rs @@ -12,15 +12,13 @@ use tendermint_light_client_verifier::options::Options; use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState}; use tendermint_light_client_verifier::Verifier; -use crate::verifier::TmVerifier; - pub fn verify_header( ctx: &V, header: &TmHeader, client_id: &ClientId, chain_id: &ChainId, options: &Options, - verifier: &impl TmVerifier, + verifier: &impl Verifier, ) -> Result<(), ClientError> where V: ExtClientValidationContext, @@ -92,7 +90,6 @@ where // main header verification, delegated to the tendermint-light-client crate. verifier - .verifier() .verify_update_header(untrusted_state, trusted_state, options, now) .into_result()?; } diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index 3fcad0dc1..e1bf834f3 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -13,10 +13,10 @@ use ibc_primitives::proto::Any; use tendermint::crypto::default::Sha256; use tendermint::crypto::Sha256 as Sha256Trait; use tendermint::merkle::MerkleHash; +use tendermint_light_client_verifier::{ProdVerifier, Verifier}; use super::{check_for_misbehaviour_on_misbehavior, check_for_misbehaviour_on_update, ClientState}; use crate::client_state::{verify_header, verify_misbehaviour}; -use crate::verifier::{DefaultVerifier, TmVerifier}; impl ClientStateValidation for ClientState where @@ -24,9 +24,13 @@ where V::ConsensusStateRef: Convertible, { /// The default verification logic exposed by ibc-rs simply delegates to a - /// standalone `verify_client_message` function. This is to make it as simple - /// as possible for those who merely need the `DefaultVerifier` behaviour, as - /// well as those who require custom verification logic. + /// standalone `verify_client_message` function. This is to make it as + /// simple as possible for those who merely need the default `ProdVerifier` + /// behaviour, as well as those who require custom verification logic. + /// + /// In situations when the Tendermint [`ProdVerifier`] doesn't provide the + /// desired outcome, users should define a custom verifier struct and then + /// implement the `tendermint_light_client_verifier::Verifier` trait for it. fn verify_client_message( &self, ctx: &V, @@ -38,7 +42,7 @@ where ctx, client_id, client_message, - &DefaultVerifier, + &ProdVerifier::default(), ) } @@ -63,9 +67,9 @@ where /// Verify the client message as part of the client state validation process. /// /// Note that this function is typically implemented as part of the -/// [`ClientStateValidation`] trait, but has been made a standalone function -/// in order to make the ClientState APIs more flexible. It mostly adheres to -/// the same signature as the `ClientStateValidation::verify_client_message` +/// [`ClientStateValidation`] trait, but has been made a standalone function in +/// order to make the ClientState APIs more flexible. It mostly adheres to the +/// same signature as the `ClientStateValidation::verify_client_message` /// function, except for an additional `verifier` parameter that allows users /// who require custom verification logic to easily pass in their own verifier /// implementation. @@ -74,7 +78,7 @@ pub fn verify_client_message( ctx: &V, client_id: &ClientId, client_message: Any, - verifier: &impl TmVerifier, + verifier: &impl Verifier, ) -> Result<(), ClientError> where V: ExtClientValidationContext, diff --git a/ibc-clients/ics07-tendermint/src/lib.rs b/ibc-clients/ics07-tendermint/src/lib.rs index 808679b6c..594aa0011 100644 --- a/ibc-clients/ics07-tendermint/src/lib.rs +++ b/ibc-clients/ics07-tendermint/src/lib.rs @@ -1,5 +1,5 @@ -//! ICS 07: Tendermint light client implementation along with re-exporting data -//! structures and implementations of IBC core client module. +//! ICS 07: Tendermint light client implementation along with re-exporting the +//! necessary types from `ibc-client-tendermint-types` crate. #![no_std] #![forbid(unsafe_code)] #![cfg_attr(not(test), deny(clippy::unwrap_used))] @@ -18,11 +18,11 @@ extern crate std; pub mod client_state; pub mod consensus_state; -pub mod verifier; pub const TENDERMINT_CLIENT_TYPE: &str = "07-tendermint"; -/// Re-export of Tendermint light client data structures from `ibc-client-tendermint` crate. +/// Re-exports Tendermint light client data structures from the +/// `ibc-client-tendermint-type` crate. pub mod types { #[doc(inline)] pub use ibc_client_tendermint_types::*; diff --git a/ibc-clients/ics07-tendermint/src/verifier.rs b/ibc-clients/ics07-tendermint/src/verifier.rs deleted file mode 100644 index 11bbaa756..000000000 --- a/ibc-clients/ics07-tendermint/src/verifier.rs +++ /dev/null @@ -1,37 +0,0 @@ -use tendermint_light_client_verifier::ProdVerifier; - -/// Specifies the Verifier interface that hosts must adhere to when customizing -/// Tendermint client verification behaviour. -/// -/// For users who require custom verification logic, i.e., in situations when -/// the Tendermint [`ProdVerifier`] doesn't provide the desired outcome, users -/// should define a custom verifier struct as a unit struct and then implement -/// `TmVerifier` for it. Note that the custom verifier does need to also -/// implement the `tendermint_light_client_verifier::Verifier` trait. -/// -/// In order to wire up the custom verifier, the `verify_client_message` method -/// on the `ClientStateValidation` trait must be implemented. The simplest way -/// to do this is to import and call the standalone `verify_client_message` -/// function located in the `ibc::clients::tendermint::client_state` module, -/// passing in your custom verifier type as its `verifier` parameter. The rest -/// of the methods in the `ClientStateValidation` trait can be implemented by -/// importing and calling their analogous standalone version from the -/// `tendermint::client_state` module, unless bespoke logic is desired for any -/// of those functions. -pub trait TmVerifier { - type Verifier: tendermint_light_client_verifier::Verifier; - - fn verifier(&self) -> Self::Verifier; -} - -/// The default verifier for IBC clients, the Tendermint light client -/// ProdVerifier, for those users who don't require custom verification logic. -pub struct DefaultVerifier; - -impl TmVerifier for DefaultVerifier { - type Verifier = ProdVerifier; - - fn verifier(&self) -> Self::Verifier { - ProdVerifier::default() - } -} From ad4fbddee741ab6e1d0202600730271977ac5dfa Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Mon, 15 Apr 2024 12:21:35 -0700 Subject: [PATCH 2/9] chore: add changelog --- .../unreleased/breaking-changes/1168-discard-TmVerifier.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md diff --git a/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md b/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md new file mode 100644 index 000000000..498eedc4b --- /dev/null +++ b/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md @@ -0,0 +1,4 @@ +- [ibc-client-tendermint] Make it simpler to use custom verifiers for Tendermint + clients by directly binding with `tendermint_light_client_verifier::Verifier` + and removing then unused `TmVerifier` trait. + ([\#1168](https://github.com/cosmos/ibc-rs/pull/1168)) From d39bfa7f378e36277310f7a2b254d88b5156ef92 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 16 Apr 2024 13:28:11 -0500 Subject: [PATCH 3/9] Update .changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md Co-authored-by: Rano | Ranadeep Signed-off-by: Sean Chen --- .../unreleased/breaking-changes/1168-discard-TmVerifier.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md b/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md index 498eedc4b..d62b1774f 100644 --- a/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md +++ b/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md @@ -1,4 +1,4 @@ -- [ibc-client-tendermint] Make it simpler to use custom verifiers for Tendermint +- [ibc-client-tendermint] Simplify custom verifiers usage for Tendermint clients by directly binding with `tendermint_light_client_verifier::Verifier` and removing then unused `TmVerifier` trait. ([\#1168](https://github.com/cosmos/ibc-rs/pull/1168)) From 03f2c380ffc0ada62021ef4a7d0a18f88b13449a Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 16 Apr 2024 13:28:22 -0500 Subject: [PATCH 4/9] Update ibc-clients/ics07-tendermint/src/client_state/validation.rs Co-authored-by: Rano | Ranadeep Signed-off-by: Sean Chen --- ibc-clients/ics07-tendermint/src/client_state/validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index e1bf834f3..7b2b55306 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -25,7 +25,7 @@ where { /// The default verification logic exposed by ibc-rs simply delegates to a /// standalone `verify_client_message` function. This is to make it as - /// simple as possible for those who merely need the default `ProdVerifier` + /// simple as possible for those who merely need the default [`ProdVerifier`] /// behaviour, as well as those who require custom verification logic. /// /// In situations when the Tendermint [`ProdVerifier`] doesn't provide the From aa73d13fc7a15377a232301d3dd92a25828665a7 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 16 Apr 2024 13:28:54 -0500 Subject: [PATCH 5/9] Update .changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md Co-authored-by: Rano | Ranadeep Signed-off-by: Sean Chen --- .../unreleased/breaking-changes/1168-discard-TmVerifier.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md b/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md index d62b1774f..3c84200a6 100644 --- a/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md +++ b/.changelog/unreleased/breaking-changes/1168-discard-TmVerifier.md @@ -1,4 +1,4 @@ - [ibc-client-tendermint] Simplify custom verifiers usage for Tendermint clients by directly binding with `tendermint_light_client_verifier::Verifier` - and removing then unused `TmVerifier` trait. + and removing the unused `TmVerifier` trait. ([\#1168](https://github.com/cosmos/ibc-rs/pull/1168)) From d76bd3701a1f283ac910d9db49d2ff4f2920d95d Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 16 Apr 2024 13:29:43 -0500 Subject: [PATCH 6/9] Update ibc-clients/ics07-tendermint/src/client_state/validation.rs Co-authored-by: Rano | Ranadeep Signed-off-by: Sean Chen --- ibc-clients/ics07-tendermint/src/client_state/validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index 7b2b55306..f36fb14b3 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -28,7 +28,7 @@ where /// simple as possible for those who merely need the default [`ProdVerifier`] /// behaviour, as well as those who require custom verification logic. /// - /// In situations when the Tendermint [`ProdVerifier`] doesn't provide the + /// In a situation where the Tendermint [`ProdVerifier`] doesn't provide the /// desired outcome, users should define a custom verifier struct and then /// implement the `tendermint_light_client_verifier::Verifier` trait for it. fn verify_client_message( From ea0fe694a6c65f861916f12793bcbb01f4fb0df6 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 16 Apr 2024 13:29:51 -0500 Subject: [PATCH 7/9] Update ibc-clients/ics07-tendermint/src/client_state/validation.rs Co-authored-by: Rano | Ranadeep Signed-off-by: Sean Chen --- ibc-clients/ics07-tendermint/src/client_state/validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index f36fb14b3..7223ffcb7 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -30,7 +30,7 @@ where /// /// In a situation where the Tendermint [`ProdVerifier`] doesn't provide the /// desired outcome, users should define a custom verifier struct and then - /// implement the `tendermint_light_client_verifier::Verifier` trait for it. + /// implement the [`Verifier`] trait for it. fn verify_client_message( &self, ctx: &V, From b8244a5dd50181f392a5ed70a88d0603da99ab8e Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 16 Apr 2024 14:51:08 -0700 Subject: [PATCH 8/9] docs: nudge toward implementing a newtype wrapper --- .../src/client_state/validation.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index 7223ffcb7..3068703c7 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -25,12 +25,23 @@ where { /// The default verification logic exposed by ibc-rs simply delegates to a /// standalone `verify_client_message` function. This is to make it as - /// simple as possible for those who merely need the default [`ProdVerifier`] - /// behaviour, as well as those who require custom verification logic. + /// simple as possible for those who merely need the default + /// [`ProdVerifier`] behaviour, as well as those who require custom + /// verification logic. /// /// In a situation where the Tendermint [`ProdVerifier`] doesn't provide the /// desired outcome, users should define a custom verifier struct and then /// implement the [`Verifier`] trait for it. + /// + /// In order to wire up the custom verifier, create a newtype `ClientState` + /// wrapper similar to [`ClientState`] and implement all client state traits + /// for it. For method implementation, the simplest way is to import and + /// call their analogous standalone versions under the + /// [`crate::client_state`] module, unless bespoke logic is desired for any + /// of those functions. Then, when it comes to implement the + /// `verify_client_message` method, use the [`verify_client_message`] + /// function and pass your custom verifier object as the `verifier` + /// parameter. fn verify_client_message( &self, ctx: &V, From 8fe44609a922ce1cee8e216ce45cb0fc9199d311 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 16 Apr 2024 14:54:16 -0700 Subject: [PATCH 9/9] nit --- ibc-clients/ics07-tendermint/src/client_state/validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index 3068703c7..73349d1ec 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -38,7 +38,7 @@ where /// for it. For method implementation, the simplest way is to import and /// call their analogous standalone versions under the /// [`crate::client_state`] module, unless bespoke logic is desired for any - /// of those functions. Then, when it comes to implement the + /// of those functions. Then, when it comes to implementing the /// `verify_client_message` method, use the [`verify_client_message`] /// function and pass your custom verifier object as the `verifier` /// parameter.