imp: 2nd try, place serialize_path under the ClientStateCommon

ibc-clients/cw-context/src/handlers.rs

@@ -103,13 +103,20 @@ where
             SudoMsg::VerifyUpgradeAndUpdateState(msg) => {
                 let msg = VerifyUpgradeAndUpdateStateMsg::try_from(msg)?;
 
-                client_state.verify_upgrade_client(
-                    self,
+                let client_cons_state_path = ClientConsensusStatePath::new(
                     client_id.clone(),
+                    client_state.latest_height().revision_number(),
+                    client_state.latest_height().revision_height(),
+                );
+
+                let consensus_state = self.consensus_state(&client_cons_state_path)?;
+
+                client_state.verify_upgrade_client(
                     msg.upgrade_client_state.clone(),
                     msg.upgrade_consensus_state.clone(),
                     msg.proof_upgrade_client,
                     msg.proof_upgrade_consensus_state,
+                    consensus_state.root(),
                 )?;
 
                 client_state.update_state_on_upgrade(

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

@@ -1,7 +1,7 @@
 use ibc_client_tendermint_types::{client_type as tm_client_type, ClientState as ClientStateType};
 use ibc_core_client::context::client_state::ClientStateCommon;
 use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::types::error::ClientError;
+use ibc_core_client::types::error::{ClientError, UpgradeClientError};
 use ibc_core_client::types::Height;
 use ibc_core_commitment_types::commitment::{
     CommitmentPrefix, CommitmentProofBytes, CommitmentRoot,
@@ -10,9 +10,10 @@ use ibc_core_commitment_types::merkle::{MerklePath, 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::PathBytes;
+use ibc_core_host::types::path::{Path, PathBytes, UpgradeClientPath};
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
+use ibc_primitives::ToVec;
 
 use super::ClientState;
 use crate::consensus_state::ConsensusState as TmConsensusState;
@@ -34,6 +35,38 @@ impl ClientStateCommon for ClientState {
         validate_proof_height(self.inner(), proof_height)
     }
 
+    fn verify_upgrade_client(
+        &self,
+        upgraded_client_state: Any,
+        upgraded_consensus_state: Any,
+        proof_upgrade_client: CommitmentProofBytes,
+        proof_upgrade_consensus_state: CommitmentProofBytes,
+        root: &CommitmentRoot,
+    ) -> Result<(), ClientError> {
+        let last_height = self.latest_height().revision_height();
+
+        let upgrade_client_path_bytes =
+            self.serialize_path(UpgradeClientPath::UpgradedClientState(last_height))?;
+
+        let upgrade_consensus_path_bytes =
+            self.serialize_path(UpgradeClientPath::UpgradedClientConsensusState(last_height))?;
+
+        verify_upgrade_client::<HostFunctionsManager>(
+            self.inner(),
+            upgraded_client_state,
+            upgraded_consensus_state,
+            proof_upgrade_client,
+            proof_upgrade_consensus_state,
+            upgrade_client_path_bytes,
+            upgrade_consensus_path_bytes,
+            root,
+        )
+    }
+
+    fn serialize_path(&self, path: impl Into<Path>) -> Result<PathBytes, ClientError> {
+        Ok(path.into().to_string().into_bytes().into())
+    }
+
     fn verify_membership(
         &self,
         prefix: &CommitmentPrefix,
@@ -109,6 +142,82 @@ pub fn validate_proof_height(
     Ok(())
 }
 
+/// Perform client-specific verifications and check all data in the new
+/// client state to be the same across all valid Tendermint clients for the
+/// new chain.
+///
+/// You can learn more about how to upgrade IBC-connected SDK chains in
+/// [this](https://ibc.cosmos.network/main/ibc/upgrades/quick-guide.html)
+/// guide.
+///
+/// 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.
+#[allow(clippy::too_many_arguments)]
+pub fn verify_upgrade_client<H: HostFunctionsProvider>(
+    client_state: &ClientStateType,
+    upgraded_client_state: Any,
+    upgraded_consensus_state: Any,
+    proof_upgrade_client: CommitmentProofBytes,
+    proof_upgrade_consensus_state: CommitmentProofBytes,
+    upgrade_client_path_bytes: PathBytes,
+    upgrade_consensus_path_bytes: PathBytes,
+    root: &CommitmentRoot,
+) -> Result<(), ClientError> {
+    // Make sure that the client type is of Tendermint type `ClientState`
+    let upgraded_tm_client_state = ClientState::try_from(upgraded_client_state.clone())?;
+
+    // Make sure that the consensus type is of Tendermint type `ConsensusState`
+    TmConsensusState::try_from(upgraded_consensus_state.clone())?;
+
+    let latest_height = client_state.latest_height;
+    let upgraded_tm_client_state_height = upgraded_tm_client_state.latest_height();
+
+    // Make sure the latest height of the current client is not greater then
+    // the upgrade height This condition checks both the revision number and
+    // the height
+    if latest_height >= upgraded_tm_client_state_height {
+        Err(UpgradeClientError::LowUpgradeHeight {
+            upgraded_height: latest_height,
+            client_height: upgraded_tm_client_state_height,
+        })?
+    }
+
+    // Check to see if the upgrade path is set
+    let mut upgrade_path = client_state.upgrade_path.clone();
+
+    if upgrade_path.pop().is_none() {
+        return Err(ClientError::ClientSpecific {
+            description: "cannot upgrade client as no upgrade path has been set".to_string(),
+        });
+    };
+
+    let upgrade_path_prefix = CommitmentPrefix::try_from(upgrade_path[0].clone().into_bytes())
+        .map_err(ClientError::InvalidCommitmentProof)?;
+
+    // Verify the proof of the upgraded client state
+    verify_membership::<H>(
+        &client_state.proof_specs,
+        &upgrade_path_prefix,
+        &proof_upgrade_client,
+        root,
+        upgrade_client_path_bytes,
+        upgraded_client_state.to_vec(),
+    )?;
+
+    // Verify the proof of the upgraded consensus state
+    verify_membership::<H>(
+        &client_state.proof_specs,
+        &upgrade_path_prefix,
+        &proof_upgrade_consensus_state,
+        root,
+        upgrade_consensus_path_bytes,
+        upgraded_consensus_state.to_vec(),
+    )?;
+
+    Ok(())
+}
+
 /// Verify membership of the given value against the client's merkle proof.
 ///
 /// Note that this function is typically implemented as part of the

ibc-clients/ics07-tendermint/src/client_state/validation.rs

@@ -2,29 +2,21 @@ use ibc_client_tendermint_types::{
     ClientState as ClientStateType, ConsensusState as ConsensusStateType, Header as TmHeader,
     Misbehaviour as TmMisbehaviour, TENDERMINT_HEADER_TYPE_URL, TENDERMINT_MISBEHAVIOUR_TYPE_URL,
 };
-use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation};
-use ibc_core_client::context::consensus_state::ConsensusState;
-use ibc_core_client::context::{ClientValidationContext, Convertible, ExtClientValidationContext};
-use ibc_core_client::types::error::{ClientError, UpgradeClientError};
+use ibc_core_client::context::client_state::ClientStateValidation;
+use ibc_core_client::context::{Convertible, ExtClientValidationContext};
+use ibc_core_client::types::error::ClientError;
 use ibc_core_client::types::Status;
-use ibc_core_commitment_types::commitment::{CommitmentPrefix, CommitmentProofBytes};
-use ibc_core_commitment_types::proto::ics23::{HostFunctionsManager, HostFunctionsProvider};
 use ibc_core_host::types::identifiers::ClientId;
-use ibc_core_host::types::path::{ClientConsensusStatePath, UpgradeClientPath};
+use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
-use ibc_primitives::ToVec;
 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, verify_membership,
-    ClientState,
-};
+use super::{check_for_misbehaviour_on_misbehavior, check_for_misbehaviour_on_update, ClientState};
 use crate::client_state::{verify_header, verify_misbehaviour};
-use crate::consensus_state::ConsensusState as TmConsensusState;
 
 impl<V> ClientStateValidation<V> for ClientState
 where
@@ -79,26 +71,6 @@ where
         status(self.inner(), ctx, client_id)
     }
 
-    fn verify_upgrade_client(
-        &self,
-        ctx: &V,
-        client_id: ClientId,
-        upgraded_client_state: Any,
-        upgraded_consensus_state: Any,
-        proof_upgrade_client: CommitmentProofBytes,
-        proof_upgrade_consensus_state: CommitmentProofBytes,
-    ) -> Result<(), ClientError> {
-        verify_upgrade_client::<V, HostFunctionsManager>(
-            self.inner(),
-            ctx,
-            client_id,
-            upgraded_client_state,
-            upgraded_consensus_state,
-            proof_upgrade_client,
-            proof_upgrade_consensus_state,
-        )
-    }
-
     fn check_substitute(&self, _ctx: &V, substitute_client_state: Any) -> Result<(), ClientError> {
         check_substitute::<V>(self.inner(), substitute_client_state)
     }
@@ -257,99 +229,6 @@ where
     Ok(Status::Active)
 }
 
-/// Perform client-specific verifications and check all data in the new
-/// client state to be the same across all valid Tendermint clients for the
-/// new chain.
-///
-/// You can learn more about how to upgrade IBC-connected SDK chains in
-/// [this](https://ibc.cosmos.network/main/ibc/upgrades/quick-guide.html)
-/// guide.
-///
-/// 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<V, H>(
-    client_state: &ClientStateType,
-    ctx: &V,
-    client_id: ClientId,
-    upgraded_client_state: Any,
-    upgraded_consensus_state: Any,
-    proof_upgrade_client: CommitmentProofBytes,
-    proof_upgrade_consensus_state: CommitmentProofBytes,
-) -> Result<(), ClientError>
-where
-    V: ClientValidationContext,
-    H: HostFunctionsProvider,
-{
-    // Read the latest consensus state from the host chain store.
-    let old_client_cons_state_path = ClientConsensusStatePath::new(
-        client_id.clone(),
-        client_state.latest_height.revision_number(),
-        client_state.latest_height.revision_height(),
-    );
-    let old_consensus_state = ctx
-        .consensus_state(&old_client_cons_state_path)
-        .map_err(|_| ClientError::ConsensusStateNotFound {
-            client_id,
-            height: client_state.latest_height,
-        })?;
-
-    // Make sure that the client type is of Tendermint type `ClientState`
-    let upgraded_tm_client_state = ClientState::try_from(upgraded_client_state.clone())?;
-
-    // Make sure that the consensus type is of Tendermint type `ConsensusState`
-    TmConsensusState::try_from(upgraded_consensus_state.clone())?;
-
-    let latest_height = client_state.latest_height;
-    let upgraded_tm_client_state_height = upgraded_tm_client_state.latest_height();
-
-    // Make sure the latest height of the current client is not greater then
-    // the upgrade height This condition checks both the revision number and
-    // the height
-    if latest_height >= upgraded_tm_client_state_height {
-        Err(UpgradeClientError::LowUpgradeHeight {
-            upgraded_height: latest_height,
-            client_height: upgraded_tm_client_state_height,
-        })?
-    }
-
-    // Check to see if the upgrade path is set
-    let mut upgrade_path = client_state.upgrade_path.clone();
-
-    if upgrade_path.pop().is_none() {
-        return Err(ClientError::ClientSpecific {
-            description: "cannot upgrade client as no upgrade path has been set".to_string(),
-        });
-    };
-
-    let upgrade_path_prefix = CommitmentPrefix::try_from(upgrade_path[0].clone().into_bytes())
-        .map_err(ClientError::InvalidCommitmentProof)?;
-
-    let last_height = latest_height.revision_height();
-
-    // Verify the proof of the upgraded client state
-    verify_membership::<H>(
-        &client_state.proof_specs,
-        &upgrade_path_prefix,
-        &proof_upgrade_client,
-        old_consensus_state.root(),
-        ctx.serialize_path(UpgradeClientPath::UpgradedClientState(last_height))?,
-        upgraded_client_state.to_vec(),
-    )?;
-
-    // Verify the proof of the upgraded consensus state
-    verify_membership::<H>(
-        &client_state.proof_specs,
-        &upgrade_path_prefix,
-        &proof_upgrade_consensus_state,
-        old_consensus_state.root(),
-        ctx.serialize_path(UpgradeClientPath::UpgradedClientConsensusState(last_height))?,
-        upgraded_consensus_state.to_vec(),
-    )?;
-
-    Ok(())
-}
-
 /// Check that the subject and substitute client states match as part of
 /// the client recovery validation step.
 ///

ibc-core/ics02-client/context/src/client_state.rs

@@ -6,7 +6,7 @@ use ibc_core_commitment_types::commitment::{
     CommitmentPrefix, CommitmentProofBytes, CommitmentRoot,
 };
 use ibc_core_host_types::identifiers::{ClientId, ClientType};
-use ibc_core_host_types::path::PathBytes;
+use ibc_core_host_types::path::{Path, PathBytes};
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
 
@@ -33,6 +33,28 @@ pub trait ClientStateCommon: Convertible<Any> {
     /// Validate that the client is at a sufficient height
     fn validate_proof_height(&self, proof_height: Height) -> Result<(), ClientError>;
 
+    /// Verify the upgraded client and consensus states and validate proofs
+    /// against the given root.
+    ///
+    /// NOTE: proof heights are not included, as upgrade to a new revision is
+    /// expected to pass only on the last height committed by the current
+    /// revision. Clients are responsible for ensuring that the planned last
+    /// height of the current revision is somehow encoded in the proof
+    /// verification process. This is to ensure that no premature upgrades
+    /// occur, since upgrade plans committed to by the counterparty may be
+    /// cancelled or modified before the last planned height.
+    fn verify_upgrade_client(
+        &self,
+        upgraded_client_state: Any,
+        upgraded_consensus_state: Any,
+        proof_upgrade_client: CommitmentProofBytes,
+        proof_upgrade_consensus_state: CommitmentProofBytes,
+        root: &CommitmentRoot,
+    ) -> Result<(), ClientError>;
+
+    /// Determines how a path should be serialized into a `PathBytes` object.
+    fn serialize_path(&self, path: impl Into<Path>) -> Result<PathBytes, ClientError>;
+
     // Verify_membership is a generic proof verification method which verifies a
     // proof of the existence of a value at a given Path.
     fn verify_membership(
@@ -103,26 +125,6 @@ where
     /// Returns the status of the client. Only Active clients are allowed to process packets.
     fn status(&self, ctx: &V, client_id: &ClientId) -> Result<Status, ClientError>;
 
-    /// Verify the upgraded client and consensus states and validate proofs
-    /// against the given root.
-    ///
-    /// NOTE: proof heights are not included, as upgrade to a new revision is
-    /// expected to pass only on the last height committed by the current
-    /// revision. Clients are responsible for ensuring that the planned last
-    /// height of the current revision is somehow encoded in the proof
-    /// verification process. This is to ensure that no premature upgrades
-    /// occur, since upgrade plans committed to by the counterparty may be
-    /// cancelled or modified before the last planned height.
-    fn verify_upgrade_client(
-        &self,
-        ctx: &V,
-        client_id: ClientId,
-        upgraded_client_state: Any,
-        upgraded_consensus_state: Any,
-        proof_upgrade_client: CommitmentProofBytes,
-        proof_upgrade_consensus_state: CommitmentProofBytes,
-    ) -> Result<(), ClientError>;
-
     /// Verifies whether the calling (subject) client state matches the substitute
     /// client state for the purposes of client recovery.
     ///