Split MsgUpdateClient back into MsgUpdateClient and MsgSubmitMisbehaviour

.changelog/unreleased/breaking-changes/628-split-msgupdateclient.md

@@ -0,0 +1,2 @@
+- Split `MsgUpdateClient` back into `MsgUpdateClient` and `MsgSubmitMisbehaviour`
+  ([#628](https://github.com/cosmos/ibc-rs/issues/628))

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

@@ -1,7 +1,6 @@
 mod misbehaviour;
 mod update_client;
 
-use crate::core::ics02_client::msgs::update_client::UpdateKind;
 use crate::prelude::*;
 
 use core::cmp::max;
@@ -26,7 +25,9 @@ use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConse
 use crate::clients::ics07_tendermint::error::Error;
 use crate::clients::ics07_tendermint::header::Header as TmHeader;
 use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour;
-use crate::core::ics02_client::client_state::{ClientState as Ics2ClientState, UpdatedState};
+use crate::core::ics02_client::client_state::{
+    ClientState as Ics2ClientState, UpdateKind, UpdatedState,
+};
 use crate::core::ics02_client::client_type::ClientType;
 use crate::core::ics02_client::consensus_state::ConsensusState;
 use crate::core::ics02_client::error::ClientError;
@@ -306,13 +307,9 @@ impl Ics2ClientState for ClientState {
         &self,
         ctx: &mut dyn ExecutionContext,
         client_id: &ClientId,
-        client_message: Any,
-        _update_kind: &UpdateKind,
+        header: Any,
     ) -> Result<Vec<Height>, ClientError> {
-        // we only expect headers to make it here; if a TmMisbehaviour makes it to here,
-        // then it is not a valid misbehaviour (check_for_misbehaviour returned false),
-        // and so transaction should fail.
-        let header = TmHeader::try_from(client_message)?;
+        let header = TmHeader::try_from(header)?;
         let header_height = header.height();
 
         let maybe_existing_consensus_state = {

crates/ibc/src/core/context.rs

@@ -22,6 +22,7 @@ use self::acknowledgement::{acknowledgement_packet_execute, acknowledgement_pack
 use self::recv_packet::{recv_packet_execute, recv_packet_validate};
 use self::timeout::{timeout_packet_execute, timeout_packet_validate, TimeoutMsgType};
 
+use super::ics02_client::msgs::MsgUpdateOrMisbehaviour;
 use super::{
     ics02_client::error::ClientError,
     ics03_connection::error::ConnectionError,
@@ -173,7 +174,12 @@ pub trait ValidationContext: Router {
         match msg {
             MsgEnvelope::Client(msg) => match msg {
                 ClientMsg::CreateClient(msg) => create_client::validate(self, msg),
-                ClientMsg::UpdateClient(msg) => update_client::validate(self, msg),
+                ClientMsg::UpdateClient(msg) => {
+                    update_client::validate(self, MsgUpdateOrMisbehaviour::UpdateClient(msg))
+                }
+                ClientMsg::Misbehaviour(msg) => {
+                    update_client::validate(self, MsgUpdateOrMisbehaviour::Misbehaviour(msg))
+                }
                 ClientMsg::UpgradeClient(msg) => upgrade_client::validate(self, msg),
             }
             .map_err(RouterError::ContextError),
@@ -381,7 +387,12 @@ pub trait ExecutionContext: ValidationContext {
         match msg {
             MsgEnvelope::Client(msg) => match msg {
                 ClientMsg::CreateClient(msg) => create_client::execute(self, msg),
-                ClientMsg::UpdateClient(msg) => update_client::execute(self, msg),
+                ClientMsg::UpdateClient(msg) => {
+                    update_client::execute(self, MsgUpdateOrMisbehaviour::UpdateClient(msg))
+                }
+                ClientMsg::Misbehaviour(msg) => {
+                    update_client::execute(self, MsgUpdateOrMisbehaviour::Misbehaviour(msg))
+                }
                 ClientMsg::UpgradeClient(msg) => upgrade_client::execute(self, msg),
             }
             .map_err(RouterError::ContextError),

crates/ibc/src/core/handler.rs

@@ -50,7 +50,6 @@ mod tests {
         msgs::transfer::test_util::get_dummy_msg_transfer, msgs::transfer::MsgTransfer,
         packet::PacketData, PrefixedCoin, MODULE_ID_STR,
     };
-    use crate::core::ics02_client::msgs::update_client::UpdateKind;
     use crate::core::ics02_client::msgs::{
         create_client::MsgCreateClient, update_client::MsgUpdateClient,
         upgrade_client::MsgUpgradeClient, ClientMsg,
@@ -270,10 +269,9 @@ mod tests {
                 name: "Client update successful".to_string(),
                 msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient {
                     client_id: client_id.clone(),
-                    client_message: MockHeader::new(update_client_height)
+                    header: MockHeader::new(update_client_height)
                         .with_timestamp(Timestamp::now())
                         .into(),
-                    update_kind: UpdateKind::UpdateClient,
                     signer: default_signer.clone(),
                 }))
                 .into(),
@@ -284,8 +282,7 @@ mod tests {
                 name: "Client update fails due to stale header".to_string(),
                 msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient {
                     client_id: client_id.clone(),
-                    client_message: MockHeader::new(update_client_height).into(),
-                    update_kind: UpdateKind::UpdateClient,
+                    header: MockHeader::new(update_client_height).into(),
                     signer: default_signer.clone(),
                 }))
                 .into(),
@@ -360,10 +357,9 @@ mod tests {
                 name: "Client update successful #2".to_string(),
                 msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient {
                     client_id: client_id.clone(),
-                    client_message: MockHeader::new(update_client_height_after_send)
+                    header: MockHeader::new(update_client_height_after_send)
                         .with_timestamp(Timestamp::now())
                         .into(),
-                    update_kind: UpdateKind::UpdateClient,
                     signer: default_signer.clone(),
                 }))
                 .into(),
@@ -406,8 +402,7 @@ mod tests {
                 name: "Client update successful".to_string(),
                 msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient {
                     client_id: client_id.clone(),
-                    client_message: MockHeader::new(update_client_height_after_second_send).into(),
-                    update_kind: UpdateKind::UpdateClient,
+                    header: MockHeader::new(update_client_height_after_second_send).into(),
                     signer: default_signer.clone(),
                 }))
                 .into(),

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

@@ -19,7 +19,6 @@ use crate::prelude::*;
 use crate::Height;
 
 use super::consensus_state::ConsensusState;
-use super::msgs::update_client::UpdateKind;
 
 use crate::core::{ExecutionContext, ValidationContext};
 
@@ -97,14 +96,15 @@ pub trait ClientState:
     /// successful update, a list of consensus heights is returned. It assumes
     /// the client_message has already been verified.
     ///
+    /// Note that `header` is the field associated with `UpdateKind::UpdateClient`.
+    ///
     /// Post-condition: on success, the return value MUST contain at least one
     /// height.
     fn update_state(
         &self,
         ctx: &mut dyn ExecutionContext,
         client_id: &ClientId,
-        client_message: Any,
-        update_kind: &UpdateKind,
+        header: Any,
     ) -> Result<Vec<Height>, ClientError>;
 
     /// update_state_on_misbehaviour should perform appropriate state changes on
@@ -189,6 +189,19 @@ pub fn downcast_client_state<CS: ClientState>(h: &dyn ClientState) -> Option<&CS
     h.as_any().downcast_ref::<CS>()
 }
 
+/// `UpdateKind` represents the 2 ways that a client can be updated
+/// in IBC: either through a `MsgUpdateClient`, or a `MsgSubmitMisbehaviour`.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum UpdateKind {
+    /// this is the typical scenario where a new header is submitted to the client
+    /// to update the client. Note that light clients are free to define the type
+    /// of the object used to update them (e.g. could be a list of headers).
+    UpdateClient,
+    /// this is the scenario where misbehaviour is submitted to the client
+    /// (e.g 2 headers with the same height in Tendermint)
+    SubmitMisbehaviour,
+}
+
 pub struct UpdatedState {
     pub client_state: Box<dyn ClientState>,
     pub consensus_state: Box<dyn ConsensusState>,