Skip to content

Commit

Permalink
Remove dynamically typed hacks formerly used to construct client upgr…
Browse files Browse the repository at this point in the history
…ades
  • Loading branch information
romac committed Sep 13, 2023
1 parent 51038d7 commit 2b7b2b0
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 212 deletions.
47 changes: 22 additions & 25 deletions crates/relayer-types/src/clients/ics07_tendermint/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ 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, UpgradeOptions as CoreUpgradeOptions,
ClientState as Ics2ClientState, UpgradableClientState,
};
use crate::core::ics02_client::client_type::ClientType;
use crate::core::ics02_client::error::Error as Ics02Error;
Expand Down Expand Up @@ -199,36 +199,15 @@ pub struct UpgradeOptions {
pub unbonding_period: Duration,
}

impl CoreUpgradeOptions for UpgradeOptions {}

impl Ics2ClientState for ClientState {
fn chain_id(&self) -> ChainId {
self.chain_id.clone()
}

fn client_type(&self) -> ClientType {
ClientType::Tendermint
}

fn latest_height(&self) -> Height {
self.latest_height
}

fn frozen_height(&self) -> Option<Height> {
self.frozen_height
}
impl UpgradableClientState for ClientState {
type UpgradeOptions = UpgradeOptions;

fn upgrade(
&mut self,
upgrade_height: Height,
upgrade_options: &dyn CoreUpgradeOptions,
upgrade_options: UpgradeOptions,
chain_id: ChainId,
) {
let upgrade_options = upgrade_options
.as_any()
.downcast_ref::<UpgradeOptions>()
.expect("UpgradeOptions not of type Tendermint");

// Reset custom fields to zero values
self.trusting_period = ZERO_DURATION;
self.trust_threshold = TrustThreshold::CLIENT_STATE_RESET;
Expand All @@ -242,6 +221,24 @@ impl Ics2ClientState for ClientState {
self.unbonding_period = upgrade_options.unbonding_period;
self.chain_id = chain_id;
}
}

impl Ics2ClientState for ClientState {
fn chain_id(&self) -> ChainId {
self.chain_id.clone()
}

fn client_type(&self) -> ClientType {
ClientType::Tendermint
}

fn latest_height(&self) -> Height {
self.latest_height
}

fn frozen_height(&self) -> Option<Height> {
self.frozen_height
}

fn expired(&self, elapsed: Duration) -> bool {
elapsed > self.trusting_period
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use ibc_proto::ibc::lightclients::localhost::v1::ClientState as RawClientState;
use ibc_proto::protobuf::Protobuf;

use crate::core::ics02_client::client_state::ClientState as Ics02ClientState;
use crate::core::ics02_client::client_state::UpgradeOptions;
use crate::core::ics02_client::client_type::ClientType;
use crate::core::ics02_client::error::Error as Ics02Error;
use crate::core::ics24_host::identifier::ChainId;
Expand Down Expand Up @@ -52,16 +51,6 @@ impl Ics02ClientState for ClientState {
None
}

fn upgrade(
&mut self,
upgrade_height: Height,
_upgrade_options: &dyn UpgradeOptions,
chain_id: ChainId,
) {
self.height = upgrade_height;
self.chain_id = chain_id;
}

fn expired(&self, _elapsed: Duration) -> bool {
false
}
Expand Down
85 changes: 7 additions & 78 deletions crates/relayer-types/src/core/ics02_client/client_state.rs
Original file line number Diff line number Diff line change
@@ -1,30 +1,13 @@
use core::fmt::Debug;
use std::marker::{Send, Sync};
use std::time::Duration;

use dyn_clone::DynClone;
use erased_serde::Serialize as ErasedSerialize;
use ibc_proto::google::protobuf::Any;
use ibc_proto::protobuf::Protobuf as ErasedProtobuf;

use crate::core::ics02_client::client_type::ClientType;
use crate::core::ics02_client::error::Error;
use crate::core::ics24_host::identifier::ChainId;
use crate::dynamic_typing::AsAny;

use crate::Height;

use super::consensus_state::ConsensusState;

pub trait ClientState:
AsAny
+ sealed::ErasedPartialEqClientState
+ DynClone
+ ErasedSerialize
+ ErasedProtobuf<Any, Error = Error>
+ core::fmt::Debug
+ Send
+ Sync
{
pub trait ClientState: Clone + PartialEq + 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;
Expand All @@ -46,72 +29,18 @@ pub trait ClientState:
/// 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,
/// and updates the given fields.
fn upgrade(
&mut self,
upgrade_height: Height,
upgrade_options: &dyn UpgradeOptions,
upgrade_options: Self::UpgradeOptions,
chain_id: ChainId,
);

/// Convert into a boxed trait object
fn into_box(self) -> Box<dyn ClientState>
where
Self: Sized,
{
Box::new(self)
}
}

// Implements `Clone` for `Box<dyn ClientState>`
dyn_clone::clone_trait_object!(ClientState);

// Implements `serde::Serialize` for all types that have ClientState as supertrait
erased_serde::serialize_trait_object!(ClientState);

impl PartialEq for dyn ClientState {
fn eq(&self, other: &Self) -> bool {
self.eq_client_state(other)
}
}

// see https://github.com/rust-lang/rust/issues/31740
impl PartialEq<&Self> for Box<dyn ClientState> {
fn eq(&self, other: &&Self) -> bool {
self.eq_client_state(other.as_ref())
}
}

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>,
}

mod sealed {
use super::*;

pub trait ErasedPartialEqClientState {
fn eq_client_state(&self, other: &dyn ClientState) -> bool;
}

impl<CS> ErasedPartialEqClientState for CS
where
CS: ClientState + PartialEq,
{
fn eq_client_state(&self, other: &dyn ClientState) -> bool {
other
.as_any()
.downcast_ref::<CS>()
.map_or(false, |h| self == h)
}
}
}
27 changes: 1 addition & 26 deletions crates/relayer-types/src/mock/client_state.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::collections::HashMap;
use std::time::Duration;

use serde::{Deserialize, Serialize};
Expand All @@ -7,9 +6,8 @@ use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::mock::ClientState as RawMockClientState;
use ibc_proto::protobuf::Protobuf;

use crate::core::ics02_client::client_state::{ClientState, UpgradeOptions};
use crate::core::ics02_client::client_state::ClientState;
use crate::core::ics02_client::client_type::ClientType;
use crate::core::ics02_client::consensus_state::ConsensusState;
use crate::core::ics02_client::error::Error;

use crate::core::ics24_host::identifier::ChainId;
Expand All @@ -21,20 +19,6 @@ use crate::Height;

pub const MOCK_CLIENT_STATE_TYPE_URL: &str = "/ibc.mock.ClientState";

/// A mock of an IBC client record as it is stored in a mock context.
/// For testing ICS02 handlers mostly, cf. `MockClientContext`.
#[derive(Clone, Debug)]
pub struct MockClientRecord {
/// The type of this client.
pub client_type: ClientType,

/// The client state (representing only the latest height at the moment).
pub client_state: Option<Box<dyn ClientState>>,

/// Mapping of heights to consensus states for this client.
pub consensus_states: HashMap<Height, Box<dyn ConsensusState>>,
}

/// A mock of a client state. For an example of a real structure that this mocks, you can see
/// `ClientState` of ics07_tendermint/client_state.rs.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
Expand Down Expand Up @@ -132,15 +116,6 @@ impl ClientState for MockClientState {
self.frozen_height
}

fn upgrade(
&mut self,
_upgrade_height: Height,
_upgrade_options: &dyn UpgradeOptions,
_chain_id: ChainId,
) {
unimplemented!()
}

fn expired(&self, _elapsed: Duration) -> bool {
false
}
Expand Down
72 changes: 2 additions & 70 deletions crates/relayer/src/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,12 @@ use ibc_proto::ibc::lightclients::tendermint::v1::ClientState as RawTmClientStat
use ibc_proto::ibc::mock::ClientState as RawMockClientState;
use ibc_proto::protobuf::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::clients::ics09_localhost::v1::client_state::{
ClientState as LocalhostClientState, LOCALHOST_CLIENT_STATE_TYPE_URL,
};
use ibc_relayer_types::core::ics02_client::client_state::{
downcast_client_state, ClientState, UpgradeOptions,
};
use ibc_relayer_types::core::ics02_client::client_state::ClientState;
use ibc_relayer_types::core::ics02_client::client_type::ClientType;
use ibc_relayer_types::core::ics02_client::error::Error;
use ibc_relayer_types::core::ics02_client::trust_threshold::TrustThreshold;
Expand All @@ -31,27 +28,6 @@ use ibc_relayer_types::mock::client_state::MockClientState;
use ibc_relayer_types::mock::client_state::MOCK_CLIENT_STATE_TYPE_URL;
use ibc_relayer_types::Height;

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "type")]
pub enum AnyUpgradeOptions {
Tendermint(TmUpgradeOptions),
Localhost(()),

#[cfg(test)]
Mock(()),
}

impl AnyUpgradeOptions {
fn as_tm_upgrade_options(&self) -> Option<&TmUpgradeOptions> {
match self {
AnyUpgradeOptions::Tendermint(tm) => Some(tm),
_ => None,
}
}
}

impl UpgradeOptions for AnyUpgradeOptions {}

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type")]
pub enum AnyClientState {
Expand Down Expand Up @@ -209,35 +185,6 @@ impl ClientState for AnyClientState {
self.frozen_height()
}

fn upgrade(
&mut self,
upgrade_height: Height,
upgrade_options: &dyn UpgradeOptions,
chain_id: ChainId,
) {
let upgrade_options = upgrade_options
.as_any()
.downcast_ref::<AnyUpgradeOptions>()
.expect("UpgradeOptions not of type AnyUpgradeOptions");

match self {
AnyClientState::Tendermint(tm_state) => tm_state.upgrade(
upgrade_height,
upgrade_options.as_tm_upgrade_options().unwrap(), // FIXME: This is very brittle
chain_id,
),

AnyClientState::Localhost(local_state) => {
local_state.upgrade(upgrade_height, upgrade_options, chain_id)
}

#[cfg(test)]
AnyClientState::Mock(mock_state) => {
mock_state.upgrade(upgrade_height, upgrade_options, chain_id)
}
}
}

fn expired(&self, elapsed_since_latest: Duration) -> bool {
match self {
AnyClientState::Tendermint(tm_state) => tm_state.expired(elapsed_since_latest),
Expand Down Expand Up @@ -268,21 +215,6 @@ impl From<MockClientState> for AnyClientState {
}
}

impl From<&dyn ClientState> for AnyClientState {
fn from(client_state: &dyn ClientState) -> Self {
#[cfg(test)]
if let Some(cs) = downcast_client_state::<MockClientState>(client_state) {
return AnyClientState::from(*cs);
}

if let Some(cs) = downcast_client_state::<TmClientState>(client_state) {
AnyClientState::from(cs.clone())
} else {
unreachable!()
}
}
}

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type")]
pub struct IdentifiedAnyClientState {
Expand Down
4 changes: 2 additions & 2 deletions crates/relayer/src/upgrade_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use ibc_proto::cosmos::upgrade::v1beta1::Plan;
use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::core::client::v1::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};

Expand Down Expand Up @@ -109,7 +109,7 @@ pub fn build_and_send_ibc_upgrade_proposal(

client_state.upgrade(
upgraded_client_latest_height,
&upgrade_options,
upgrade_options,
opts.upgraded_chain_id.clone(),
);

Expand Down

0 comments on commit 2b7b2b0

Please sign in to comment.