fix: revert frozen_height to 0 for active clients in conversions (#…

…1064) * fix: revert proto encoded frozen_height to height = zero * fix: disallow proto frozen_height equal to None * fix: test_update_synthetic_tendermint_client_duplicate_ok * Update ibc-clients/ics07-tendermint/types/src/error.rs Co-authored-by: Rano | Ranadeep <[email protected]> Signed-off-by: Farhad Shabani <[email protected]> --------- Signed-off-by: Farhad Shabani <[email protected]> Co-authored-by: Rano | Ranadeep <[email protected]>
cosmos · Jan 29, 2024 · 4a0990f · 4a0990f
1 parent 06725c1
commit 4a0990f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 7 deletions.
diff --git a/ibc-clients/ics07-tendermint/types/src/client_state.rs b/ibc-clients/ics07-tendermint/types/src/client_state.rs
@@ -6,6 +6,7 @@ use core::str::FromStr;
 use core::time::Duration;
 
 use ibc_core_client_types::error::ClientError;
+use ibc_core_client_types::proto::v1::Height as RawHeight;
 use ibc_core_client_types::Height;
 use ibc_core_commitment_types::specs::ProofSpecs;
 use ibc_core_host_types::identifiers::ChainId;
@@ -283,10 +284,11 @@ impl TryFrom<RawTmClientState> for ClientState {
             .try_into()
             .map_err(|_| Error::MissingLatestHeight)?;
 
-        // In `RawClientState`, a `frozen_height` of `0` means "not frozen".
-        // See:
+        // NOTE: In `RawClientState`, a `frozen_height` of `0` means "not
+        // frozen". See:
         // https://github.com/cosmos/ibc-go/blob/8422d0c4c35ef970539466c5bdec1cd27369bab3/modules/light-clients/07-tendermint/types/client_state.go#L74
-        let frozen_height = raw.frozen_height.and_then(|h| Height::try_from(h).ok());
+        let frozen_height =
+            Height::try_from(raw.frozen_height.ok_or(Error::MissingFrozenHeight)?).ok();
 
         // We use set this deprecated field just so that we can properly convert
         // it back in its raw form
@@ -322,7 +324,17 @@ impl From<ClientState> for RawTmClientState {
             trusting_period: Some(value.trusting_period.into()),
             unbonding_period: Some(value.unbonding_period.into()),
             max_clock_drift: Some(value.max_clock_drift.into()),
-            frozen_height: value.frozen_height.map(|height| height.into()),
+            // NOTE: The protobuf encoded `frozen_height` of an active client
+            // must be set to `0` so that `ibc-go` driven chains can properly
+            // decode the `ClientState` value. In `RawClientState`, a
+            // `frozen_height` of `0` means "not frozen". See:
+            // https://github.com/cosmos/ibc-go/blob/8422d0c4c35ef970539466c5bdec1cd27369bab3/modules/light-clients/07-tendermint/types/client_state.go#L74
+            frozen_height: Some(value.frozen_height.map(|height| height.into()).unwrap_or(
+                RawHeight {
+                    revision_number: 0,
+                    revision_height: 0,
+                },
+            )),
             latest_height: Some(value.latest_height.into()),
             proof_specs: value.proof_specs.into(),
             upgrade_path: value.upgrade_path,

diff --git a/ibc-clients/ics07-tendermint/types/src/error.rs b/ibc-clients/ics07-tendermint/types/src/error.rs
@@ -61,8 +61,8 @@ pub enum Error {
     HeaderTimestampTooLow { actual: String, min: String },
     /// header revision height = `{height}` is invalid
     InvalidHeaderHeight { height: u64 },
-    /// Disallowed to create a new client with a frozen height
-    FrozenHeightNotAllowed,
+    /// frozen height is missing
+    MissingFrozenHeight,
     /// the header's trusted revision number (`{trusted_revision}`) and the update's revision number (`{header_revision}`) should be the same
     MismatchHeightRevisions {
         trusted_revision: u64,

diff --git a/ibc-testkit/tests/core/ics02_client/update_client.rs b/ibc-testkit/tests/core/ics02_client/update_client.rs
@@ -10,6 +10,7 @@ use ibc::clients::tendermint::types::{
 use ibc::core::client::context::client_state::{ClientStateCommon, ClientStateValidation};
 use ibc::core::client::context::ClientValidationContext;
 use ibc::core::client::types::msgs::{ClientMsg, MsgSubmitMisbehaviour, MsgUpdateClient};
+use ibc::core::client::types::proto::v1::Height as RawHeight;
 use ibc::core::client::types::Height;
 use ibc::core::commitment_types::specs::ProofSpecs;
 use ibc::core::entrypoint::{execute, validate};
@@ -588,7 +589,10 @@ fn test_update_synthetic_tendermint_client_duplicate_ok() {
                 ),
                 proof_specs: ProofSpecs::default().into(),
                 upgrade_path: Default::default(),
-                frozen_height: None,
+                frozen_height: Some(RawHeight {
+                    revision_number: 0,
+                    revision_height: 0,
+                }),
                 allow_update_after_expiry: false,
                 allow_update_after_misbehaviour: false,
             };