Split ICS 02 ClientState trait into ClientState and `UpgradableCl…

…ientState` traits (#3942)
informalsystems · Apr 11, 2024 · bb6ed41 · bb6ed41
1 parent 1edcb15
commit bb6ed41
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 59 deletions.
diff --git a/crates/relayer-types/src/clients/ics07_tendermint/client_state.rs b/crates/relayer-types/src/clients/ics07_tendermint/client_state.rs
@@ -12,7 +12,9 @@ use tendermint_light_client_verifier::options::Options;
 
 use crate::clients::ics07_tendermint::error::Error;
 use crate::clients::ics07_tendermint::header::Header as TmHeader;
-use crate::core::ics02_client::client_state::ClientState as Ics2ClientState;
+use crate::core::ics02_client::client_state::{
+    ClientState as Ics2ClientState, UpgradableClientState,
+};
 use crate::core::ics02_client::client_type::ClientType;
 use crate::core::ics02_client::error::Error as Ics02Error;
 use crate::core::ics02_client::trust_threshold::TrustThreshold;
@@ -192,8 +194,6 @@ pub struct UpgradeOptions {
 }
 
 impl Ics2ClientState for ClientState {
-    type UpgradeOptions = UpgradeOptions;
-
     fn chain_id(&self) -> ChainId {
         self.chain_id.clone()
     }
@@ -210,6 +210,14 @@ impl Ics2ClientState for ClientState {
         self.frozen_height
     }
 
+    fn expired(&self, elapsed: Duration) -> bool {
+        elapsed > self.trusting_period
+    }
+}
+
+impl UpgradableClientState for ClientState {
+    type UpgradeOptions = UpgradeOptions;
+
     fn upgrade(
         &mut self,
         upgrade_height: Height,
@@ -229,10 +237,6 @@ impl Ics2ClientState for ClientState {
         self.unbonding_period = upgrade_options.unbonding_period;
         self.chain_id = chain_id;
     }
-
-    fn expired(&self, elapsed: Duration) -> bool {
-        elapsed > self.trusting_period
-    }
 }
 
 impl Protobuf<RawTmClientState> for ClientState {}

diff --git a/crates/relayer-types/src/core/ics02_client/client_state.rs b/crates/relayer-types/src/core/ics02_client/client_state.rs
@@ -6,10 +6,7 @@ use crate::core::ics24_host::identifier::ChainId;
 
 use crate::Height;
 
-pub trait ClientState: Clone + Debug + Send + Sync // Any: From<Self>,
-{
-    type UpgradeOptions;
-
+pub trait ClientState: Clone + Debug + Send + Sync {
     /// Return the chain identifier which this client is serving (i.e., the client is verifying
     /// consensus states from this chain).
     fn chain_id(&self) -> ChainId;
@@ -31,6 +28,10 @@ pub trait ClientState: Clone + Debug + Send + Sync // Any: From<Self>,
     /// Check if the state is expired when `elapsed` time has passed since the latest consensus
     /// state timestamp
     fn expired(&self, elapsed: Duration) -> bool;
+}
+
+pub trait UpgradableClientState: ClientState {
+    type UpgradeOptions;
 
     /// Helper function to verify the upgrade client procedure.
     /// Resets all fields except the blockchain-specific ones,

diff --git a/crates/relayer/src/client_state.rs b/crates/relayer/src/client_state.rs
@@ -1,4 +1,4 @@
-use core::time::Duration;
+use std::time::Duration;
 
 use serde::{Deserialize, Serialize};
 
@@ -7,8 +7,7 @@ use ibc_proto::ibc::core::client::v1::IdentifiedClientState;
 use ibc_proto::ibc::lightclients::tendermint::v1::ClientState as RawTmClientState;
 use ibc_proto::Protobuf;
 use ibc_relayer_types::clients::ics07_tendermint::client_state::{
-    ClientState as TmClientState, UpgradeOptions as TmUpgradeOptions,
-    TENDERMINT_CLIENT_STATE_TYPE_URL,
+    ClientState as TmClientState, TENDERMINT_CLIENT_STATE_TYPE_URL,
 };
 use ibc_relayer_types::core::ics02_client::client_state::ClientState;
 use ibc_relayer_types::core::ics02_client::client_type::ClientType;
@@ -18,20 +17,6 @@ use ibc_relayer_types::core::ics24_host::error::ValidationError;
 use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ClientId};
 use ibc_relayer_types::Height;
 
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(tag = "type")]
-pub enum AnyUpgradeOptions {
-    Tendermint(TmUpgradeOptions),
-}
-
-impl AnyUpgradeOptions {
-    fn into_tm_upgrade_options(self) -> Option<TmUpgradeOptions> {
-        match self {
-            AnyUpgradeOptions::Tendermint(tm) => Some(tm),
-        }
-    }
-}
-
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 #[serde(tag = "type")]
 pub enum AnyClientState {
@@ -80,6 +65,12 @@ impl AnyClientState {
             Self::Tendermint(state) => state.client_type(),
         }
     }
+
+    pub fn expired(&self, elapsed: Duration) -> bool {
+        match self {
+            Self::Tendermint(state) => state.expired(elapsed),
+        }
+    }
 }
 
 impl Protobuf<Any> for AnyClientState {}
@@ -113,47 +104,24 @@ impl From<AnyClientState> for Any {
 }
 
 impl ClientState for AnyClientState {
-    type UpgradeOptions = AnyUpgradeOptions;
-
     fn chain_id(&self) -> ChainId {
-        match self {
-            AnyClientState::Tendermint(tm_state) => tm_state.chain_id(),
-        }
+        AnyClientState::chain_id(self)
     }
 
     fn client_type(&self) -> ClientType {
-        self.client_type()
+        AnyClientState::client_type(self)
     }
 
     fn latest_height(&self) -> Height {
-        self.latest_height()
+        AnyClientState::latest_height(self)
     }
 
     fn frozen_height(&self) -> Option<Height> {
-        self.frozen_height()
+        AnyClientState::frozen_height(self)
     }
 
-    fn upgrade(
-        &mut self,
-        upgrade_height: Height,
-        upgrade_options: AnyUpgradeOptions,
-        chain_id: ChainId,
-    ) {
-        match self {
-            AnyClientState::Tendermint(tm_state) => {
-                if let Some(upgrade_options) = upgrade_options.into_tm_upgrade_options() {
-                    tm_state.upgrade(upgrade_height, upgrade_options, chain_id);
-                }
-                // TODO: Handle case where upgrade options are not of the right type,
-                //       not a problem in practice for now but good to have.
-            }
-        }
-    }
-
-    fn expired(&self, elapsed_since_latest: Duration) -> bool {
-        match self {
-            AnyClientState::Tendermint(tm_state) => tm_state.expired(elapsed_since_latest),
-        }
+    fn expired(&self, elapsed: Duration) -> bool {
+        AnyClientState::expired(self, elapsed)
     }
 }
 

diff --git a/crates/relayer/src/link/operational_data.rs b/crates/relayer/src/link/operational_data.rs
@@ -2,9 +2,9 @@ use core::fmt::{Display, Error as FmtError, Formatter};
 use std::ops::Add;
 use std::time::{Duration, Instant};
 
-use ibc_proto::google::protobuf::Any;
 use tracing::{debug, info};
 
+use ibc_proto::google::protobuf::Any;
 use ibc_relayer_types::core::ics02_client::client_state::ClientState;
 use ibc_relayer_types::Height;
 

diff --git a/crates/relayer/src/upgrade_chain.rs b/crates/relayer/src/upgrade_chain.rs
@@ -14,7 +14,7 @@ use ibc_proto::cosmos::upgrade::v1beta1::Plan;
 use ibc_proto::google::protobuf::Any;
 use ibc_proto::ibc::core::client::v1::{MsgIbcSoftwareUpgrade, UpgradeProposal};
 use ibc_relayer_types::clients::ics07_tendermint::client_state::UpgradeOptions;
-use ibc_relayer_types::core::ics02_client::client_state::ClientState;
+use ibc_relayer_types::core::ics02_client::client_state::UpgradableClientState;
 use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ClientId};
 use ibc_relayer_types::{downcast, Height};
 use tracing::warn;