From 4428fa98316fb453cb006db574dc39fe5e3b1999 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Tue, 6 Feb 2024 22:03:21 -0600 Subject: [PATCH] chore: deprecate `MsgSubmitMisbehaviour` (#1079) * Decorate MsgSubmitMisbehaviour with `deprecated` attribute * Add #[allow(deprecated)] attributes in ibc-testkit * Add #![allow(deprecated)] attribute to ics02/types/msgs/mod.rs * Remove usages of MsgSubmitMisbehaviour from ibc-testkit/update_client.rs * Remove unused rstest fixture * Fix incorrect doc comment * Add changelog entry * chore: add missing unclog for PR835 --------- Co-authored-by: Farhad Shabani --- .../1077-deprecate-msgsubmitmisbehaviour.md | 3 + ...date-client-handler-accept-misbehaviour.md | 3 + .../types/src/msgs/misbehaviour.rs | 9 ++- ibc-core/ics02-client/types/src/msgs/mod.rs | 3 + ibc-core/ics25-handler/types/src/msgs.rs | 2 + .../tests/core/ics02_client/update_client.rs | 72 ++++--------------- 6 files changed, 32 insertions(+), 60 deletions(-) create mode 100644 .changelog/unreleased/improvements/1077-deprecate-msgsubmitmisbehaviour.md create mode 100644 .changelog/unreleased/improvements/835-msg-update-client-handler-accept-misbehaviour.md diff --git a/.changelog/unreleased/improvements/1077-deprecate-msgsubmitmisbehaviour.md b/.changelog/unreleased/improvements/1077-deprecate-msgsubmitmisbehaviour.md new file mode 100644 index 000000000..e92dc29d2 --- /dev/null +++ b/.changelog/unreleased/improvements/1077-deprecate-msgsubmitmisbehaviour.md @@ -0,0 +1,3 @@ +- [ibc-core] Deprecate `MsgSubmitMisbehaviour` in favor of `MsgUpdateClient` for + submitting misbehaviour reports + ([\#1077](https://github.com/cosmos/ibc-rs/issues/1077)) diff --git a/.changelog/unreleased/improvements/835-msg-update-client-handler-accept-misbehaviour.md b/.changelog/unreleased/improvements/835-msg-update-client-handler-accept-misbehaviour.md new file mode 100644 index 000000000..6e6b8488e --- /dev/null +++ b/.changelog/unreleased/improvements/835-msg-update-client-handler-accept-misbehaviour.md @@ -0,0 +1,3 @@ +- [ibc-core] Update `MsgUpdateClient` handler to accept misbehaviour reports via + its `client_message` field + ([\#835](https://github.com/cosmos/ibc-rs/issues/835)) diff --git a/ibc-core/ics02-client/types/src/msgs/misbehaviour.rs b/ibc-core/ics02-client/types/src/msgs/misbehaviour.rs index b47be6166..fc13aa91f 100644 --- a/ibc-core/ics02-client/types/src/msgs/misbehaviour.rs +++ b/ibc-core/ics02-client/types/src/msgs/misbehaviour.rs @@ -12,6 +12,13 @@ use crate::error::ClientError; pub const SUBMIT_MISBEHAVIOUR_TYPE_URL: &str = "/ibc.core.client.v1.MsgSubmitMisbehaviour"; /// A type of message that submits client misbehaviour proof. +/// +/// Deprecated since v0.51.0. Misbehaviour reports should be submitted via the `MsgUpdateClient` +/// type through its `client_message` field. +#[deprecated( + since = "0.51.0", + note = "Misbehaviour reports should be submitted via `MsgUpdateClient` through its `client_message` field" +)] #[cfg_attr( feature = "borsh", derive(borsh::BorshSerialize, borsh::BorshDeserialize) @@ -32,7 +39,6 @@ impl Protobuf for MsgSubmitMisbehaviour {} impl TryFrom for MsgSubmitMisbehaviour { type Error = ClientError; - #[allow(deprecated)] fn try_from(raw: RawMsgSubmitMisbehaviour) -> Result { let raw_misbehaviour = raw .misbehaviour @@ -51,7 +57,6 @@ impl TryFrom for MsgSubmitMisbehaviour { impl From for RawMsgSubmitMisbehaviour { fn from(ics_msg: MsgSubmitMisbehaviour) -> Self { - #[allow(deprecated)] RawMsgSubmitMisbehaviour { client_id: ics_msg.client_id.to_string(), misbehaviour: Some(ics_msg.misbehaviour), diff --git a/ibc-core/ics02-client/types/src/msgs/mod.rs b/ibc-core/ics02-client/types/src/msgs/mod.rs index 33330a77d..ab9a145ed 100644 --- a/ibc-core/ics02-client/types/src/msgs/mod.rs +++ b/ibc-core/ics02-client/types/src/msgs/mod.rs @@ -1,4 +1,7 @@ +#![allow(deprecated)] + //! Defines the client message types that are sent to the chain by the relayer. + use ibc_core_host_types::identifiers::ClientId; use ibc_primitives::prelude::*; use ibc_primitives::Signer; diff --git a/ibc-core/ics25-handler/types/src/msgs.rs b/ibc-core/ics25-handler/types/src/msgs.rs index 849bcaaf6..1272c0aff 100644 --- a/ibc-core/ics25-handler/types/src/msgs.rs +++ b/ibc-core/ics25-handler/types/src/msgs.rs @@ -6,6 +6,7 @@ use ibc_core_channel_types::msgs::{ CHAN_OPEN_INIT_TYPE_URL, CHAN_OPEN_TRY_TYPE_URL, RECV_PACKET_TYPE_URL, TIMEOUT_ON_CLOSE_TYPE_URL, TIMEOUT_TYPE_URL, }; +#[allow(deprecated)] use ibc_core_client_types::msgs::{ ClientMsg, MsgCreateClient, MsgSubmitMisbehaviour, MsgUpdateClient, MsgUpgradeClient, CREATE_CLIENT_TYPE_URL, SUBMIT_MISBEHAVIOUR_TYPE_URL, UPDATE_CLIENT_TYPE_URL, @@ -35,6 +36,7 @@ pub enum MsgEnvelope { Packet(PacketMsg), } +#[allow(deprecated)] impl TryFrom for MsgEnvelope { type Error = RouterError; diff --git a/ibc-testkit/tests/core/ics02_client/update_client.rs b/ibc-testkit/tests/core/ics02_client/update_client.rs index 2d4e7bbf3..fc60fcbab 100644 --- a/ibc-testkit/tests/core/ics02_client/update_client.rs +++ b/ibc-testkit/tests/core/ics02_client/update_client.rs @@ -9,7 +9,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::msgs::{ClientMsg, MsgUpdateClient}; use ibc::core::client::types::proto::v1::Height as RawHeight; use ibc::core::client::types::Height; use ibc::core::commitment_types::specs::ProofSpecs; @@ -57,11 +57,8 @@ fn fixture() -> Fixture { Fixture { ctx, router } } -/// rstest fixture that returns a `MsgEnvelope` with the `client_message` -/// field set to a `MockMisbehaviour` report. -#[fixture] -fn msg_update_client() -> MsgEnvelope { - let client_id = ClientId::default(); +/// Returns a `MsgEnvelope` with the `client_message` field set to a `MockMisbehaviour` report. +fn msg_update_client(client_id: &ClientId) -> MsgEnvelope { let timestamp = Timestamp::now(); let height = Height::new(0, 46).unwrap(); let msg = MsgUpdateClient { @@ -78,27 +75,6 @@ fn msg_update_client() -> MsgEnvelope { MsgEnvelope::from(ClientMsg::from(msg)) } -/// rstest fixture that returns a `MsgEnvelope` with the `misbehaviour` -/// field set to a `MockMisbehaviour` report. -#[fixture] -fn msg_submit_misbehaviour() -> MsgEnvelope { - let client_id = ClientId::default(); - let timestamp = Timestamp::now(); - let height = Height::new(0, 46).unwrap(); - let msg = MsgSubmitMisbehaviour { - client_id: client_id.clone(), - misbehaviour: MockMisbehaviour { - client_id: client_id.clone(), - header1: MockHeader::new(height).with_timestamp(timestamp), - header2: MockHeader::new(height).with_timestamp(timestamp), - } - .into(), - signer: dummy_account_id(), - }; - - MsgEnvelope::from(ClientMsg::from(msg)) -} - #[rstest] fn test_update_client_ok(fixture: Fixture) { let Fixture { @@ -782,15 +758,14 @@ fn ensure_misbehaviour(ctx: &MockContext, client_id: &ClientId, client_type: &Cl /// Misbehaviour evidence consists of identical headers - mock misbehaviour handler /// considers it a valid proof of misbehaviour. #[rstest] -#[case::msg_submit_misbehaviour(msg_submit_misbehaviour())] -#[case::msg_update_client(msg_update_client())] -fn test_misbehaviour_client_ok(fixture: Fixture, #[case] msg_envelope: MsgEnvelope) { +fn test_misbehaviour_client_ok(fixture: Fixture) { let Fixture { mut ctx, mut router, } = fixture; let client_id = ClientId::default(); + let msg_envelope = msg_update_client(&client_id); let res = validate(&ctx, &router, msg_envelope.clone()); assert!(res.is_ok()); @@ -806,18 +781,8 @@ fn test_submit_misbehaviour_nonexisting_client(fixture: Fixture) { let Fixture { router, .. } = fixture; let client_id = ClientId::from_str("mockclient1").unwrap(); - let height = Height::new(0, 46).unwrap(); - let msg = MsgSubmitMisbehaviour { - client_id: ClientId::from_str("nonexistingclient").unwrap(), - misbehaviour: MockMisbehaviour { - client_id: client_id.clone(), - header1: MockHeader::new(height), - header2: MockHeader::new(height), - } - .into(), - signer: dummy_account_id(), - }; - let msg_envelope = MsgEnvelope::from(ClientMsg::from(msg)); + + let msg_envelope = msg_update_client(&ClientId::from_str("nonexistingclient").unwrap()); let ctx = MockContext::default().with_client_config( MockClientConfig::builder() @@ -834,18 +799,8 @@ fn test_client_update_misbehaviour_nonexisting_client(fixture: Fixture) { let Fixture { router, .. } = fixture; let client_id = ClientId::from_str("mockclient1").unwrap(); - let height = Height::new(0, 46).unwrap(); - let msg = MsgUpdateClient { - client_id: ClientId::from_str("nonexistingclient").unwrap(), - client_message: MockMisbehaviour { - client_id: client_id.clone(), - header1: MockHeader::new(height), - header2: MockHeader::new(height), - } - .into(), - signer: dummy_account_id(), - }; - let msg_envelope = MsgEnvelope::from(ClientMsg::from(msg)); + + let msg_envelope = msg_update_client(&ClientId::from_str("nonexistingclient").unwrap()); let ctx = MockContext::default().with_client_config( MockClientConfig::builder() @@ -907,9 +862,9 @@ fn test_misbehaviour_synthetic_tendermint_equivocation() { tm_block.into() }; - let msg = MsgSubmitMisbehaviour { + let msg = MsgUpdateClient { client_id: client_id.clone(), - misbehaviour: TmMisbehaviour::new(client_id.clone(), header1, header2).into(), + client_message: TmMisbehaviour::new(client_id.clone(), header1, header2).into(), signer: dummy_account_id(), }; let msg_envelope = MsgEnvelope::from(ClientMsg::from(msg)); @@ -969,9 +924,10 @@ fn test_misbehaviour_synthetic_tendermint_bft_time() { tm_block }; - let msg = MsgSubmitMisbehaviour { + let msg = MsgUpdateClient { client_id: client_id.clone(), - misbehaviour: TmMisbehaviour::new(client_id.clone(), header1.into(), header2.into()).into(), + client_message: TmMisbehaviour::new(client_id.clone(), header1.into(), header2.into()) + .into(), signer: dummy_account_id(), };