Implement verify_upgrade_and_update_state for Tendermint Client (#349)

* Implement verify_upgrade_and_update_state

* Construct new_client_state and new_consensus_state

* Split off verification and execution steps

* Revise some namings and comments

* Comment out some upgrade tests

* Update mock tests related to ugrade_client

* Add changelog entry

* Refactor verify_upgrade_client to use Path and Encode error type

* Remove pub before UPGRADE const

* Rewrite upgrade_client unit tests

* Add unbonding period validation step

* Set root of new consensus state with sentinel value

* Revise some comments

* Refactor upgrade method to zero_custom_fields

* Mend fields of new client state

* Disable upgrade client handler

* Fix issue with cargo test

* Flip upgrade_client feature flag

* Remove unnecessary unbonding period check

---------

Signed-off-by: Farhad Shabani <[email protected]>

Author: Farhad-Shabani

.changelog/unreleased/breaking-changes/19-implement-verify-upgrade-client.md

@@ -0,0 +1,2 @@
+- Implement `verify_upgrade_and_update_state` method for Tendermint clients
+  ([#19](https://github.com/cosmos/ibc-rs/issues/19)).

crates/ibc/Cargo.toml

@@ -45,6 +45,9 @@ serde = ["dep:serde", "dep:serde_derive", "serde_json", "erased-serde"]
 # This feature guards the unfinished implementation of ADR 5.
 val_exec_ctx = []
 
+# This feature guards the unfinished implementation of the `UpgradeClient` handler.
+upgrade_client = []
+
 # This feature grants access to development-time mocking libraries, such as `MockContext` or `MockHeader`.
 # Depends on the `testgen` suite for generating Tendermint light blocks.
 mocks = ["tendermint-testgen", "tendermint/clock", "cfg-if", "parking_lot"]

crates/ibc/src/clients/ics07_tendermint/client_state.rs

@@ -5,8 +5,10 @@ use core::time::Duration;
 
 use ibc_proto::google::protobuf::Any;
 use ibc_proto::ibc::core::client::v1::Height as RawHeight;
-use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof;
-use ibc_proto::ibc::lightclients::tendermint::v1::ClientState as RawTmClientState;
+use ibc_proto::ibc::core::commitment::v1::{MerklePath, MerkleProof as RawMerkleProof};
+use ibc_proto::ibc::lightclients::tendermint::v1::{
+    ClientState as RawTmClientState, ConsensusState as RawTmConsensusState,
+};
 use ibc_proto::protobuf::Protobuf;
 use prost::Message;
 use tendermint::chain::id::MAX_LENGTH as MaxChainIdLen;
@@ -15,13 +17,12 @@ use tendermint_light_client_verifier::options::Options;
 use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState};
 use tendermint_light_client_verifier::{ProdVerifier, Verifier};
 
+use crate::clients::ics07_tendermint::client_state::ClientState as TmClientState;
 use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState;
 use crate::clients::ics07_tendermint::error::{Error, IntoResult};
 use crate::clients::ics07_tendermint::header::{Header as TmHeader, Header};
 use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour;
-use crate::core::ics02_client::client_state::{
-    ClientState as Ics2ClientState, UpdatedState, UpgradeOptions as CoreUpgradeOptions,
-};
+use crate::core::ics02_client::client_state::{ClientState as Ics2ClientState, UpdatedState};
 use crate::core::ics02_client::client_type::ClientType;
 use crate::core::ics02_client::consensus_state::ConsensusState;
 use crate::core::ics02_client::context::ClientReader;
@@ -38,8 +39,8 @@ use crate::core::ics23_commitment::merkle::{apply_prefix, MerkleProof};
 use crate::core::ics23_commitment::specs::ProofSpecs;
 use crate::core::ics24_host::identifier::{ChainId, ChannelId, ClientId, ConnectionId, PortId};
 use crate::core::ics24_host::path::{
-    AcksPath, ChannelEndsPath, ClientConsensusStatePath, ClientStatePath, CommitmentsPath,
-    ConnectionsPath, ReceiptsPath, SeqRecvsPath,
+    AcksPath, ChannelEndsPath, ClientConsensusStatePath, ClientStatePath, ClientUpgradePath,
+    CommitmentsPath, ConnectionsPath, ReceiptsPath, SeqRecvsPath,
 };
 use crate::core::ics24_host::Path;
 use crate::timestamp::{Timestamp, ZERO_DURATION};
@@ -351,14 +352,6 @@ impl ClientState {
     }
 }
 
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct UpgradeOptions {
-    pub unbonding_period: Duration,
-}
-
-impl CoreUpgradeOptions for UpgradeOptions {}
-
 impl Ics2ClientState for ClientState {
     fn chain_id(&self) -> ChainId {
         self.chain_id.clone()
@@ -376,29 +369,14 @@ impl Ics2ClientState for ClientState {
         self.frozen_height
     }
 
-    fn upgrade(
-        &mut self,
-        upgrade_height: Height,
-        upgrade_options: &dyn CoreUpgradeOptions,
-        chain_id: ChainId,
-    ) {
-        let upgrade_options = upgrade_options
-            .as_any()
-            .downcast_ref::<UpgradeOptions>()
-            .expect("UpgradeOptions not of type Tendermint");
-
+    fn zero_custom_fields(&mut self) {
         // Reset custom fields to zero values
         self.trusting_period = ZERO_DURATION;
         self.trust_level = TrustThreshold::ZERO;
         self.allow_update.after_expiry = false;
         self.allow_update.after_misbehaviour = false;
         self.frozen_height = None;
         self.max_clock_drift = ZERO_DURATION;
-
-        // Upgrade the client state
-        self.latest_height = upgrade_height;
-        self.unbonding_period = upgrade_options.unbonding_period;
-        self.chain_id = chain_id;
     }
 
     fn expired(&self, elapsed: Duration) -> bool {
@@ -876,13 +854,154 @@ impl Ics2ClientState for ClientState {
         })
     }
 
-    fn verify_upgrade_and_update_state(
+    /// 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
+    fn verify_upgrade_client(
+        &self,
+        upgraded_client_state: Any,
+        upgraded_consensus_state: Any,
+        proof_upgrade_client: RawMerkleProof,
+        proof_upgrade_consensus_state: RawMerkleProof,
+        root: &CommitmentRoot,
+    ) -> Result<(), ClientError> {
+        // Make sure that the client type is of Tendermint type `ClientState`
+        let mut upgraded_tm_client_state = TmClientState::try_from(upgraded_client_state)?;
+
+        // Make sure that the consensus type is of Tendermint type `ConsensusState`
+        let upgraded_tm_cons_state = TmConsensusState::try_from(upgraded_consensus_state)?;
+
+        // Note: verification of proofs that unmarshalled correctly has been done
+        // while decoding the proto message into a `MsgEnvelope` domain type
+        let merkle_proof_upgrade_client = MerkleProof::from(proof_upgrade_client);
+        let merkle_proof_upgrade_cons_state = MerkleProof::from(proof_upgrade_consensus_state);
+
+        // 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 self.latest_height() >= upgraded_tm_client_state.latest_height() {
+            return Err(ClientError::LowUpgradeHeight {
+                upgraded_height: self.latest_height(),
+                client_height: upgraded_tm_client_state.latest_height(),
+            });
+        }
+
+        // Check to see if the upgrade path is set
+        let mut upgrade_path = self.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 last_height = self.latest_height().revision_height();
+
+        // Construct the merkle path for the client state
+        let mut client_upgrade_path = upgrade_path.clone();
+        client_upgrade_path.push(ClientUpgradePath::UpgradedClientState(last_height).to_string());
+
+        let client_upgrade_merkle_path = MerklePath {
+            key_path: client_upgrade_path,
+        };
+
+        upgraded_tm_client_state.zero_custom_fields();
+        let client_state_value =
+            Protobuf::<RawTmClientState>::encode_vec(&upgraded_tm_client_state)
+                .map_err(ClientError::Encode)?;
+
+        // Verify the proof of the upgraded client state
+        merkle_proof_upgrade_client
+            .verify_membership(
+                &self.proof_specs,
+                root.clone().into(),
+                client_upgrade_merkle_path,
+                client_state_value,
+                0,
+            )
+            .map_err(ClientError::Ics23Verification)?;
+
+        // Construct the merkle path for the consensus state
+        let mut cons_upgrade_path = upgrade_path;
+        cons_upgrade_path
+            .push(ClientUpgradePath::UpgradedClientConsensusState(last_height).to_string());
+        let cons_upgrade_merkle_path = MerklePath {
+            key_path: cons_upgrade_path,
+        };
+
+        let cons_state_value = Protobuf::<RawTmConsensusState>::encode_vec(&upgraded_tm_cons_state)
+            .map_err(ClientError::Encode)?;
+
+        // Verify the proof of the upgraded consensus state
+        merkle_proof_upgrade_cons_state
+            .verify_membership(
+                &self.proof_specs,
+                root.clone().into(),
+                cons_upgrade_merkle_path,
+                cons_state_value,
+                0,
+            )
+            .map_err(ClientError::Ics23Verification)?;
+
+        Ok(())
+    }
+
+    // Commit the new client state and consensus state to the store
+    fn update_state_with_upgrade_client(
         &self,
-        _consensus_state: Any,
-        _proof_upgrade_client: RawMerkleProof,
-        _proof_upgrade_consensus_state: RawMerkleProof,
+        upgraded_client_state: Any,
+        upgraded_consensus_state: Any,
     ) -> Result<UpdatedState, ClientError> {
-        unimplemented!()
+        let upgraded_tm_client_state = TmClientState::try_from(upgraded_client_state)?;
+        let upgraded_tm_cons_state = TmConsensusState::try_from(upgraded_consensus_state)?;
+
+        // Frozen height is set to None fo the new client state
+        let new_frozen_height = None;
+
+        // Construct new client state and consensus state relayer chosen client
+        // parameters are ignored. All chain-chosen parameters come from
+        // committed client, all client-chosen parameters come from current
+        // client.
+        let new_client_state = TmClientState::new(
+            upgraded_tm_client_state.chain_id,
+            self.trust_level,
+            self.trusting_period,
+            upgraded_tm_client_state.unbonding_period,
+            self.max_clock_drift,
+            upgraded_tm_client_state.latest_height,
+            upgraded_tm_client_state.proof_specs,
+            upgraded_tm_client_state.upgrade_path,
+            self.allow_update,
+            new_frozen_height,
+        )?;
+
+        // The new consensus state is merely used as a trusted kernel against
+        // which headers on the new chain can be verified. The root is just a
+        // stand-in sentinel value as it cannot be known in advance, thus no
+        // proof verification will pass. The timestamp and the
+        // NextValidatorsHash of the consensus state is the blocktime and
+        // NextValidatorsHash of the last block committed by the old chain. This
+        // will allow the first block of the new chain to be verified against
+        // the last validators of the old chain so long as it is submitted
+        // within the TrustingPeriod of this client.
+        // NOTE: We do not set processed time for this consensus state since
+        // this consensus state should not be used for packet verification as
+        // the root is empty. The next consensus state submitted using update
+        // will be usable for packet-verification.
+        let sentinel_root = "sentinel_root".as_bytes().to_vec();
+        let new_consensus_state = TmConsensusState::new(
+            sentinel_root.into(),
+            upgraded_tm_cons_state.timestamp,
+            upgraded_tm_cons_state.next_validators_hash,
+        );
+
+        Ok(UpdatedState {
+            client_state: new_client_state.into_box(),
+            consensus_state: new_consensus_state.into_box(),
+        })
     }
 
     fn verify_client_consensus_state(

crates/ibc/src/core/ics02_client/client_state.rs

@@ -63,12 +63,7 @@ pub trait ClientState:
     /// Helper function to verify the upgrade client procedure.
     /// Resets all fields except the blockchain-specific ones,
     /// and updates the given fields.
-    fn upgrade(
-        &mut self,
-        upgrade_height: Height,
-        upgrade_options: &dyn UpgradeOptions,
-        chain_id: ChainId,
-    );
+    fn zero_custom_fields(&mut self);
 
     /// Convert into a boxed trait object
     fn into_box(self) -> Box<dyn ClientState>
@@ -112,11 +107,30 @@ pub trait ClientState:
         misbehaviour: Any,
     ) -> Result<Box<dyn ClientState>, ContextError>;
 
-    fn verify_upgrade_and_update_state(
+    /// 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,
-        consensus_state: Any,
+        upgraded_client_state: Any,
+        upgraded_consensus_state: Any,
         proof_upgrade_client: MerkleProof,
         proof_upgrade_consensus_state: MerkleProof,
+        root: &CommitmentRoot,
+    ) -> Result<(), ClientError>;
+
+    // Update the client state and consensus state in the store with the upgraded ones.
+    fn update_state_with_upgrade_client(
+        &self,
+        upgraded_client_state: Any,
+        upgraded_consensus_state: Any,
     ) -> Result<UpdatedState, ClientError>;
 
     /// Verification functions as specified in:
@@ -274,8 +288,6 @@ pub fn downcast_client_state<CS: ClientState>(h: &dyn ClientState) -> Option<&CS
     h.as_any().downcast_ref::<CS>()
 }
 
-pub trait UpgradeOptions: AsAny {}
-
 pub struct UpdatedState {
     pub client_state: Box<dyn ClientState>,
     pub consensus_state: Box<dyn ConsensusState>,

crates/ibc/src/core/ics02_client/error.rs

@@ -49,6 +49,8 @@ pub enum ClientError {
     MissingRawConsensusState,
     /// invalid client id in the update client message: `{0}`
     InvalidMsgUpdateClientId(ValidationError),
+    /// Encode error: `{0}`
+    Encode(TendermintProtoError),
     /// decode error: `{0}`
     Decode(prost::DecodeError),
     /// invalid client identifier error: `{0}`