feat: implement verify_upgrade_client method (#121)

* feat: implement verify_upgrade_client

* chore: update basecoin rev

* docs: add comment for client upgradability

* nit

* chore: update ibc revision to include query_height or client upgrade RPCs

* chore: update basecoin revision

Cargo.toml

@@ -65,15 +65,15 @@ thiserror   = "1.0.38"
 tracing     = { version = "0.1.40", default-features = false }
 
 # ibc depedenencies
-ibc-core              = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false, features = ["borsh","schema"] }
-ibc-core-client       = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false }
-ibc-core-host-cosmos  = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false }
-ibc-client-tendermint = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false }
-ibc-client-wasm-types = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false }
-ibc-app-transfer      = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false }
-ibc-primitives        = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false }
-ibc-query             = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false, features = ["schema"] }
-ibc-testkit           = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d6cbee7201", default-features = false }
+ibc-core              = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false, features = ["borsh","schema"] }
+ibc-core-client       = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false }
+ibc-core-host-cosmos  = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false }
+ibc-client-tendermint = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false }
+ibc-client-wasm-types = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false }
+ibc-app-transfer      = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false }
+ibc-primitives        = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false }
+ibc-query             = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false, features = ["schema"] }
+ibc-testkit           = { git = "https://github.com/cosmos/ibc-rs.git", rev = "d4c36554ba", default-features = false }
 
 # NOTE: `ibc-proto` is solely required by `sov-ibc-proto`. When needing Protobuf
 # Rust types in the project, importing from their respective `ibc` type crates is

clients/sov-celestia-cw/src/tests.rs

@@ -71,8 +71,6 @@ pub fn dummy_client_message(trusted_height: Height, target_height: Height) -> Ve
 
     let val = light_block.next_validators.hash();
 
-    dbg!(val);
-
     let tm_header = Header {
         signed_header: light_block.signed_header,
         validator_set: light_block.validators,

clients/sov-celestia/src/client_state/common.rs

@@ -4,18 +4,19 @@ use borsh::de::BorshDeserialize;
 use borsh::BorshSerialize;
 use ibc_core::client::context::client_state::ClientStateCommon;
 use ibc_core::client::context::consensus_state::ConsensusState as ConsensusStateTrait;
-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,
 };
 use ibc_core::commitment_types::error::CommitmentError;
 use ibc_core::host::types::identifiers::ClientType;
-use ibc_core::host::types::path::Path;
+use ibc_core::host::types::path::{Path, UpgradeClientPath};
 use ibc_core::primitives::prelude::*;
 use ibc_core::primitives::proto::Any;
+use ibc_core::primitives::ToVec;
 use jmt::proof::SparseMerkleProof;
-use sov_celestia_client_types::client_state::sov_client_type;
+use sov_celestia_client_types::client_state::{sov_client_type, SovTmClientState};
 
 use super::ClientState;
 use crate::consensus_state::ConsensusState;
@@ -58,13 +59,20 @@ impl ClientStateCommon for ClientState {
     /// guide
     fn verify_upgrade_client(
         &self,
-        _upgraded_client_state: Any,
-        _upgraded_consensus_state: Any,
-        _proof_upgrade_client: CommitmentProofBytes,
-        _proof_upgrade_consensus_state: CommitmentProofBytes,
-        _root: &CommitmentRoot,
+        upgraded_client_state: Any,
+        upgraded_consensus_state: Any,
+        proof_upgrade_client: CommitmentProofBytes,
+        proof_upgrade_consensus_state: CommitmentProofBytes,
+        root: &CommitmentRoot,
     ) -> Result<(), ClientError> {
-        Ok(())
+        verify_upgrade_client(
+            self.inner(),
+            upgraded_client_state,
+            upgraded_consensus_state,
+            proof_upgrade_client,
+            proof_upgrade_consensus_state,
+            root,
+        )
     }
 
     fn verify_membership(
@@ -89,6 +97,61 @@ impl ClientStateCommon for ClientState {
     }
 }
 
+/// Perform client-specific verifications and check all data in the new client
+/// state to be the same across all valid Sovereign clients for the new rollup.
+pub fn verify_upgrade_client(
+    client_state: &SovTmClientState,
+    upgraded_client_state: Any,
+    upgraded_consensus_state: Any,
+    proof_upgrade_client: CommitmentProofBytes,
+    proof_upgrade_consensus_state: CommitmentProofBytes,
+    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`
+    ConsensusState::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 than 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,
+        })?;
+    }
+
+    let upgrade_path_prefix = client_state.upgrade_path.clone().try_into()?;
+
+    let last_height = latest_height.revision_height();
+
+    // Verify the proof of the upgraded client state
+    verify_membership(
+        &upgrade_path_prefix,
+        &proof_upgrade_client,
+        root,
+        Path::UpgradeClient(UpgradeClientPath::UpgradedClientState(last_height)),
+        upgraded_client_state.to_vec(),
+    )?;
+
+    // Verify the proof of the upgraded consensus state
+    verify_membership(
+        &upgrade_path_prefix,
+        &proof_upgrade_consensus_state,
+        root,
+        Path::UpgradeClient(UpgradeClientPath::UpgradedClientConsensusState(last_height)),
+        upgraded_consensus_state.to_vec(),
+    )?;
+
+    Ok(())
+}
+
 pub fn verify_membership(
     prefix: &CommitmentPrefix,
     proof: &CommitmentProofBytes,
@@ -151,6 +214,14 @@ fn obtain_key_hash(prefix: &CommitmentPrefix, path: Path) -> Result<jmt::KeyHash
         Path::Commitment(p) => ("packet_commitment_map", p.try_to_vec()),
         Path::Ack(p) => ("packet_ack_map", p.try_to_vec()),
         Path::Receipt(p) => ("packet_receipt_map", p.try_to_vec()),
+        Path::UpgradeClient(p) => match p {
+            UpgradeClientPath::UpgradedClientState(_) => {
+                ("upgraded_client_state_map", p.try_to_vec())
+            }
+            UpgradeClientPath::UpgradedClientConsensusState(_) => {
+                ("upgraded_consensus_state_map", p.try_to_vec())
+            }
+        },
         _ => Err(ClientError::Other {
             description: "unsupported path".into(),
         })?,

clients/sov-celestia/src/client_state/execution.rs

@@ -69,6 +69,15 @@ where
             upgraded_consensus_state,
         )
     }
+
+    fn update_on_recovery(
+        &self,
+        ctx: &mut E,
+        subject_client_id: &ClientId,
+        substitute_client_state: Any,
+    ) -> Result<(), ClientError> {
+        unimplemented!()
+    }
 }
 
 pub fn initialise<E>(

clients/sov-celestia/src/client_state/validation.rs

@@ -49,6 +49,10 @@ where
     fn status(&self, ctx: &V, client_id: &ClientId) -> Result<Status, ClientError> {
         status(self.inner(), ctx, client_id)
     }
+
+    fn check_substitute(&self, ctx: &V, substitute_client_state: Any) -> Result<(), ClientError> {
+        unimplemented!()
+    }
 }
 
 /// Verify the client message as part of the validation process during the

clients/sov-celestia/types/src/client_state/definition.rs

@@ -1,8 +1,10 @@
 use core::cmp::max;
+use std::str::FromStr;
 
 use ibc_client_tendermint::types::{Header as TmHeader, TrustThreshold};
-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;
 use ibc_core::host::types::identifiers::ChainId;
 use ibc_core::primitives::proto::{Any, Protobuf};
 use ibc_core::primitives::ZERO_DURATION;
@@ -22,15 +24,15 @@ pub struct ClientState<Da> {
     pub rollup_id: ChainId,
     pub latest_height: Height,
     pub frozen_height: Option<Height>,
-    pub upgrade_path: Vec<String>,
+    pub upgrade_path: UpgradePath,
     pub da_params: Da,
 }
 
 impl<Da> ClientState<Da> {
     pub fn new(
         rollup_id: ChainId,
         latest_height: Height,
-        upgrade_path: Vec<String>,
+        upgrade_path: UpgradePath,
         da_params: Da,
     ) -> Self {
         Self {
@@ -126,7 +128,7 @@ impl TryFrom<RawClientState> for SovTmClientState {
         Ok(Self::new(
             rollup_id,
             latest_height,
-            upgrade_path,
+            upgrade_path.try_into()?,
             tendermint_params,
         ))
     }
@@ -138,7 +140,7 @@ impl From<SovTmClientState> for RawClientState {
             rollup_id: value.rollup_id.to_string(),
             latest_height: Some(value.latest_height.into()),
             frozen_height: value.frozen_height.map(|h| h.into()),
-            upgrade_path: value.upgrade_path,
+            upgrade_path: value.upgrade_path.0,
             tendermint_params: Some(value.da_params.into()),
         }
     }
@@ -176,3 +178,54 @@ impl From<SovTmClientState> for Any {
         }
     }
 }
+
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Debug, PartialEq)]
+pub struct UpgradePath(String);
+
+impl Default for UpgradePath {
+    fn default() -> Self {
+        Self("sov_ibc/Ibc/".to_string())
+    }
+}
+
+impl UpgradePath {
+    pub fn new(path: String) -> Self {
+        Self(path)
+    }
+
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+}
+
+impl TryFrom<String> for UpgradePath {
+    type Error = ClientError;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        Self::from_str(&value)
+    }
+}
+
+impl FromStr for UpgradePath {
+    type Err = ClientError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.is_empty() {
+            return Err(UpgradeClientError::Other {
+                reason: "empty upgrade path".into(),
+            })?;
+        }
+
+        Ok(Self(s.to_string()))
+    }
+}
+
+impl TryFrom<UpgradePath> for CommitmentPrefix {
+    type Error = ClientError;
+
+    fn try_from(value: UpgradePath) -> Result<Self, Self::Error> {
+        CommitmentPrefix::try_from(value.0.into_bytes())
+            .map_err(ClientError::InvalidCommitmentProof)
+    }
+}

clients/sov-celestia/types/src/client_state/mod.rs

@@ -42,7 +42,7 @@ pub mod test_util {
         #[builder(default)]
         pub frozen_height: Option<Height>,
         #[builder(default)]
-        pub upgrade_path: Vec<String>,
+        pub upgrade_path: UpgradePath,
         pub tendermint_params: TmClientParams,
     }
 

mocks/Cargo.toml

@@ -32,10 +32,10 @@ tracing       = "0.1.36"
 typed-builder = "0.18.0"
 
 # internal dependencies
-sov-ibc                     = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "272989d2" }
-sov-ibc-transfer            = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "272989d2" }
-sov-consensus-state-tracker = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "272989d2" }
-sov-celestia-client         = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "272989d2", features = ["test-util"] }
+sov-ibc                     = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "5967c70f" }
+sov-ibc-transfer            = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "5967c70f" }
+sov-consensus-state-tracker = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "5967c70f" }
+sov-celestia-client         = { git = "https://github.com/informalsystems/sovereign-ibc.git", rev = "5967c70f", features = ["test-util"] }
 
 # ibc dependencies
 ibc-core              = { workspace = true }
@@ -46,7 +46,7 @@ ibc-query             = { workspace = true }
 ibc-testkit           = { workspace = true }
 
 # cosmos dependencies
-basecoin             = { git = "https://github.com/informalsystems/basecoin-rs.git", rev = "91608f5" }
+basecoin             = { git = "https://github.com/informalsystems/basecoin-rs.git", rev = "9846258" }
 tendermint           = { workspace = true }
 tendermint-testgen   = { workspace = true }
 

mocks/src/tests/transfer.rs

@@ -89,8 +89,6 @@ async fn test_escrow_unescrow_on_sov() {
         .setup_cfg()
         .get_token_id_for_relayer(&gas_token.token_name);
 
-    std::dbg!(fake_token_id);
-
     cfg.sov_token_id = Some(fake_token_id);
     cfg.amount = 50;
 

modules/sov-ibc/src/clients/mod.rs

@@ -232,6 +232,22 @@ where
             ),
         }
     }
+
+    fn update_on_recovery(
+        &self,
+        ctx: &mut IbcContext<'a, S>,
+        subject_client_id: &ClientId,
+        substitute_client_state: Any,
+    ) -> Result<(), ClientError> {
+        match self {
+            AnyClientState::Tendermint(cs) => {
+                cs.update_on_recovery(ctx, subject_client_id, substitute_client_state)
+            }
+            AnyClientState::Sovereign(cs) => {
+                cs.update_on_recovery(ctx, subject_client_id, substitute_client_state)
+            }
+        }
+    }
 }
 
 impl<'a, S> ClientStateValidation<IbcContext<'a, S>> for AnyClientState
@@ -276,6 +292,17 @@ where
             AnyClientState::Sovereign(cs) => cs.status(ctx, client_id),
         }
     }
+
+    fn check_substitute(
+        &self,
+        ctx: &IbcContext<'a, S>,
+        substitute_client_state: Any,
+    ) -> Result<(), ClientError> {
+        match self {
+            AnyClientState::Tendermint(cs) => cs.check_substitute(ctx, substitute_client_state),
+            AnyClientState::Sovereign(cs) => cs.check_substitute(ctx, substitute_client_state),
+        }
+    }
 }
 
 #[derive(Clone, ConsensusState)]