From 6aba54f852f111cdff7f1ccfba3a7f1e071140e8 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Tue, 12 Mar 2024 11:13:35 +0700 Subject: [PATCH] refactor(ics24-host): remove From<&str> to avoid panic on host --- .../ics03-connection/types/src/connection.rs | 9 +++++++-- .../types/src/identifiers/client_id.rs | 6 ------ .../testapp/ibc/clients/mock/misbehaviour.rs | 7 +++---- ibc-testkit/src/testapp/ibc/core/types.rs | 2 +- .../tests/core/ics02_client/update_client.rs | 10 +++++----- .../core/ics04_channel/acknowledgement.rs | 11 +++++++---- .../tests/core/ics04_channel/recv_packet.rs | 13 +++++++++---- .../tests/core/ics04_channel/send_packet.rs | 8 +++++--- .../tests/core/ics04_channel/timeout.rs | 19 +++++++++++++------ .../core/ics04_channel/timeout_on_close.rs | 12 ++++++++---- 10 files changed, 58 insertions(+), 39 deletions(-) diff --git a/ibc-core/ics03-connection/types/src/connection.rs b/ibc-core/ics03-connection/types/src/connection.rs index bc5172275..8225763a4 100644 --- a/ibc-core/ics03-connection/types/src/connection.rs +++ b/ibc-core/ics03-connection/types/src/connection.rs @@ -1,6 +1,7 @@ //! Defines the types that define a connection use core::fmt::{Display, Error as FmtError, Formatter}; +use core::str::FromStr; use core::time::Duration; use core::u64; @@ -210,8 +211,12 @@ impl TryFrom for ConnectionEnd { if state == State::Uninitialized { return ConnectionEnd::new( State::Uninitialized, - "07-tendermint-0".into(), - Counterparty::new("07-tendermint-0".into(), None, CommitmentPrefix::empty()), + ClientId::from_str("07-tendermint-0").expect("should not fail"), + Counterparty::new( + ClientId::from_str("07-tendermint-0").expect("should not fail"), + None, + CommitmentPrefix::empty(), + ), Vec::new(), ZERO_DURATION, ); diff --git a/ibc-core/ics24-host/types/src/identifiers/client_id.rs b/ibc-core/ics24-host/types/src/identifiers/client_id.rs index 25304adb3..ecbfe1cca 100644 --- a/ibc-core/ics24-host/types/src/identifiers/client_id.rs +++ b/ibc-core/ics24-host/types/src/identifiers/client_id.rs @@ -76,12 +76,6 @@ impl FromStr for ClientId { } } -impl From<&str> for ClientId { - fn from(s: &str) -> Self { - Self::from_str(s).expect("Invalid client id") - } -} - /// Equality check against string literal (satisfies &ClientId == &str). /// ``` /// use core::str::FromStr; diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/misbehaviour.rs b/ibc-testkit/src/testapp/ibc/clients/mock/misbehaviour.rs index 04a5a10a5..969514aa9 100644 --- a/ibc-testkit/src/testapp/ibc/clients/mock/misbehaviour.rs +++ b/ibc-testkit/src/testapp/ibc/clients/mock/misbehaviour.rs @@ -1,11 +1,10 @@ +use crate::testapp::ibc::clients::mock::header::MockHeader; +use crate::testapp::ibc::clients::mock::proto::Misbehaviour as RawMisbehaviour; use ibc::core::client::types::error::ClientError; use ibc::core::host::types::identifiers::ClientId; use ibc::core::primitives::prelude::*; use ibc::primitives::proto::{Any, Protobuf}; -use crate::testapp::ibc::clients::mock::header::MockHeader; -use crate::testapp::ibc::clients::mock::proto::Misbehaviour as RawMisbehaviour; - pub const MOCK_MISBEHAVIOUR_TYPE_URL: &str = "/ibc.mock.Misbehavior"; #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -23,7 +22,7 @@ impl TryFrom for Misbehaviour { fn try_from(raw: RawMisbehaviour) -> Result { Ok(Self { - client_id: "07-tendermint-0".into(), + client_id: ClientId::new("07-tendermint", 0).expect("no error"), header1: raw .header1 .ok_or(ClientError::MissingRawMisbehaviour)? diff --git a/ibc-testkit/src/testapp/ibc/core/types.rs b/ibc-testkit/src/testapp/ibc/core/types.rs index 535f39613..7793931ad 100644 --- a/ibc-testkit/src/testapp/ibc/core/types.rs +++ b/ibc-testkit/src/testapp/ibc/core/types.rs @@ -125,7 +125,7 @@ pub struct MockContext { pub struct MockClientConfig { #[builder(default = ChainId::new("mockZ-1").expect("no error"))] client_chain_id: ChainId, - #[builder(default = "07-tendermint-0".into())] + #[builder(default = ClientId::new("07-tendermint", 0).expect("no error"))] client_id: ClientId, #[builder(default = mock_client_type())] client_type: ClientType, diff --git a/ibc-testkit/tests/core/ics02_client/update_client.rs b/ibc-testkit/tests/core/ics02_client/update_client.rs index 5957c9cdf..78eb53006 100644 --- a/ibc-testkit/tests/core/ics02_client/update_client.rs +++ b/ibc-testkit/tests/core/ics02_client/update_client.rs @@ -43,7 +43,7 @@ struct Fixture { #[fixture] fn fixture() -> Fixture { - let client_id = ClientId::from("07-tendermint-0"); + let client_id = ClientId::new("07-tendermint", 0).expect("no error"); let ctx = MockContext::default().with_client_config( MockClientConfig::builder() @@ -82,7 +82,7 @@ fn test_update_client_ok(fixture: Fixture) { mut router, } = fixture; - let client_id = ClientId::from("07-tendermint-0"); + let client_id = ClientId::new("07-tendermint", 0).expect("no error"); let signer = dummy_account_id(); let timestamp = Timestamp::now(); @@ -114,7 +114,7 @@ fn test_update_client_ok(fixture: Fixture) { // client's height and ensures that `ConsensusState` is stored at the correct // path (header height). fn test_update_client_with_prev_header() { - let client_id = ClientId::from("07-tendermint-0"); + let client_id = ClientId::new("07-tendermint", 0).expect("no error"); let chain_id_b = ChainId::new("mockgaiaA-0").unwrap(); let latest_height = Height::new(0, 42).unwrap(); let height_1 = Height::new(0, 43).unwrap(); @@ -780,7 +780,7 @@ fn test_update_client_events(fixture: Fixture) { mut router, } = fixture; - let client_id = ClientId::from("07-tendermint-0"); + let client_id = ClientId::new("07-tendermint", 0).expect("no error"); let signer = dummy_account_id(); let timestamp = Timestamp::now(); @@ -841,7 +841,7 @@ fn test_misbehaviour_client_ok(fixture: Fixture) { mut router, } = fixture; - let client_id = ClientId::from("07-tendermint-0"); + let client_id = ClientId::new("07-tendermint", 0).expect("no error"); let msg_envelope = msg_update_client(&client_id); let res = validate(&ctx, &router, msg_envelope.clone()); diff --git a/ibc-testkit/tests/core/ics04_channel/acknowledgement.rs b/ibc-testkit/tests/core/ics04_channel/acknowledgement.rs index f7ff3f88a..37bcd6643 100644 --- a/ibc-testkit/tests/core/ics04_channel/acknowledgement.rs +++ b/ibc-testkit/tests/core/ics04_channel/acknowledgement.rs @@ -12,7 +12,7 @@ use ibc::core::connection::types::{ use ibc::core::entrypoint::{execute, validate}; use ibc::core::handler::types::events::{IbcEvent, MessageEvent}; use ibc::core::handler::types::msgs::MsgEnvelope; -use ibc::core::host::types::identifiers::{ChannelId, ConnectionId, PortId}; +use ibc::core::host::types::identifiers::{ChannelId, ClientId, ConnectionId, PortId}; use ibc::core::host::ExecutionContext; use ibc::core::primitives::*; use ibc_testkit::fixtures::core::channel::dummy_raw_msg_acknowledgement; @@ -34,6 +34,8 @@ struct Fixture { #[fixture] fn fixture() -> Fixture { + let default_client_id = ClientId::new("07-tendermint", 0).expect("no error"); + let client_height = Height::new(0, 2).unwrap(); let ctx = MockContext::default().with_client_config( MockClientConfig::builder() @@ -70,9 +72,9 @@ fn fixture() -> Fixture { let conn_end_on_a = ConnectionEnd::new( ConnectionState::Open, - "07-tendermint-0".into(), + default_client_id.clone(), ConnectionCounterparty::new( - "07-tendermint-0".into(), + default_client_id.clone(), Some(ConnectionId::zero()), CommitmentPrefix::empty(), ), @@ -146,6 +148,7 @@ fn ack_success_no_packet_commitment(fixture: Fixture) { #[rstest] fn ack_success_happy_path(fixture: Fixture) { + let default_client_id = ClientId::new("07-tendermint", 0).expect("no error"); let Fixture { ctx, router, @@ -176,7 +179,7 @@ fn ack_success_happy_path(fixture: Fixture) { ); ctx.get_client_execution_context() .store_update_meta( - "07-tendermint-0".into(), + default_client_id, client_height, Timestamp::from_nanoseconds(1000).unwrap(), Height::new(0, 4).unwrap(), diff --git a/ibc-testkit/tests/core/ics04_channel/recv_packet.rs b/ibc-testkit/tests/core/ics04_channel/recv_packet.rs index b6cedfacc..47b508d60 100644 --- a/ibc-testkit/tests/core/ics04_channel/recv_packet.rs +++ b/ibc-testkit/tests/core/ics04_channel/recv_packet.rs @@ -12,7 +12,7 @@ use ibc::core::connection::types::{ use ibc::core::entrypoint::{execute, validate}; use ibc::core::handler::types::events::{IbcEvent, MessageEvent}; use ibc::core::handler::types::msgs::MsgEnvelope; -use ibc::core::host::types::identifiers::{ChannelId, ConnectionId, PortId}; +use ibc::core::host::types::identifiers::{ChannelId, ClientId, ConnectionId, PortId}; use ibc::core::host::ExecutionContext; use ibc::core::primitives::*; use ibc_testkit::fixtures::core::channel::{dummy_msg_recv_packet, dummy_raw_msg_recv_packet}; @@ -31,10 +31,13 @@ pub struct Fixture { pub msg: MsgRecvPacket, pub conn_end_on_b: ConnectionEnd, pub chan_end_on_b: ChannelEnd, + pub default_client_id: ClientId, } #[fixture] fn fixture() -> Fixture { + let default_client_id = ClientId::new("07-tendermint", 0).expect("no error"); + let context = MockContext::default(); let router = MockRouter::new_with_transfer(); @@ -59,9 +62,9 @@ fn fixture() -> Fixture { let conn_end_on_b = ConnectionEnd::new( ConnectionState::Open, - "07-tendermint-0".into(), + default_client_id.clone(), ConnectionCounterparty::new( - "07-tendermint-0".into(), + default_client_id.clone(), Some(ConnectionId::zero()), CommitmentPrefix::empty(), ), @@ -78,6 +81,7 @@ fn fixture() -> Fixture { msg, conn_end_on_b, chan_end_on_b, + default_client_id, } } @@ -110,6 +114,7 @@ fn recv_packet_validate_happy_path(fixture: Fixture) { chan_end_on_b, client_height, host_height, + default_client_id, .. } = fixture; @@ -142,7 +147,7 @@ fn recv_packet_validate_happy_path(fixture: Fixture) { context .get_client_execution_context() .store_update_meta( - "07-tendermint-0".into(), + default_client_id, client_height, Timestamp::from_nanoseconds(1000).unwrap(), Height::new(0, 5).unwrap(), diff --git a/ibc-testkit/tests/core/ics04_channel/send_packet.rs b/ibc-testkit/tests/core/ics04_channel/send_packet.rs index 66194da47..53b492f94 100644 --- a/ibc-testkit/tests/core/ics04_channel/send_packet.rs +++ b/ibc-testkit/tests/core/ics04_channel/send_packet.rs @@ -12,7 +12,7 @@ use ibc::core::connection::types::{ ConnectionEnd, Counterparty as ConnectionCounterparty, State as ConnectionState, }; use ibc::core::handler::types::events::{IbcEvent, MessageEvent}; -use ibc::core::host::types::identifiers::{ChannelId, ConnectionId, PortId}; +use ibc::core::host::types::identifiers::{ChannelId, ClientId, ConnectionId, PortId}; use ibc::core::primitives::*; use ibc_testkit::fixtures::core::channel::dummy_raw_packet; use ibc_testkit::testapp::ibc::core::types::{MockClientConfig, MockContext}; @@ -20,6 +20,8 @@ use test_log::test; #[test] fn send_packet_processing() { + let default_client_id = ClientId::new("07-tendermint", 0).expect("no error"); + struct Test { name: String, ctx: MockContext, @@ -40,9 +42,9 @@ fn send_packet_processing() { let conn_end_on_a = ConnectionEnd::new( ConnectionState::Open, - "07-tendermint-0".into(), + default_client_id.clone(), ConnectionCounterparty::new( - "07-tendermint-0".into(), + default_client_id.clone(), Some(ConnectionId::zero()), CommitmentPrefix::empty(), ), diff --git a/ibc-testkit/tests/core/ics04_channel/timeout.rs b/ibc-testkit/tests/core/ics04_channel/timeout.rs index 7c3e7c281..117e70214 100644 --- a/ibc-testkit/tests/core/ics04_channel/timeout.rs +++ b/ibc-testkit/tests/core/ics04_channel/timeout.rs @@ -12,7 +12,7 @@ use ibc::core::connection::types::{ use ibc::core::entrypoint::{execute, validate}; use ibc::core::handler::types::events::{IbcEvent, MessageEvent}; use ibc::core::handler::types::msgs::MsgEnvelope; -use ibc::core::host::types::identifiers::{ChannelId, ConnectionId, PortId}; +use ibc::core::host::types::identifiers::{ChannelId, ClientId, ConnectionId, PortId}; use ibc::core::host::ExecutionContext; use ibc::core::primitives::*; use ibc_testkit::fixtures::core::channel::dummy_raw_msg_timeout; @@ -29,10 +29,13 @@ struct Fixture { conn_end_on_a: ConnectionEnd, chan_end_on_a_ordered: ChannelEnd, chan_end_on_a_unordered: ChannelEnd, + default_client_id: ClientId, } #[fixture] fn fixture() -> Fixture { + let default_client_id = ClientId::new("07-tendermint", 0).expect("no error"); + let client_height = Height::new(0, 2).unwrap(); let ctx = MockContext::default().with_client_config( MockClientConfig::builder() @@ -77,9 +80,9 @@ fn fixture() -> Fixture { let conn_end_on_a = ConnectionEnd::new( ConnectionState::Open, - "07-tendermint-0".into(), + default_client_id.clone(), ConnectionCounterparty::new( - "07-tendermint-0".into(), + default_client_id.clone(), Some(ConnectionId::zero()), CommitmentPrefix::empty(), ), @@ -97,6 +100,7 @@ fn fixture() -> Fixture { conn_end_on_a, chan_end_on_a_ordered, chan_end_on_a_unordered, + default_client_id, } } @@ -170,6 +174,7 @@ fn timeout_fail_proof_timeout_not_reached(fixture: Fixture) { chan_end_on_a_unordered, conn_end_on_a, client_height, + default_client_id, .. } = fixture; @@ -205,7 +210,7 @@ fn timeout_fail_proof_timeout_not_reached(fixture: Fixture) { ); ctx.store_update_meta( - "07-tendermint-0".into(), + default_client_id, client_height, Timestamp::from_nanoseconds(5).unwrap(), Height::new(0, 4).unwrap(), @@ -261,6 +266,7 @@ fn timeout_unordered_channel_validate(fixture: Fixture) { conn_end_on_a, packet_commitment, client_height, + default_client_id, .. } = fixture; @@ -287,7 +293,7 @@ fn timeout_unordered_channel_validate(fixture: Fixture) { ctx.get_client_execution_context() .store_update_meta( - "07-tendermint-0".into(), + default_client_id, client_height, Timestamp::from_nanoseconds(1000).unwrap(), Height::new(0, 5).unwrap(), @@ -311,6 +317,7 @@ fn timeout_ordered_channel_validate(fixture: Fixture) { conn_end_on_a, packet_commitment, client_height, + default_client_id, .. } = fixture; @@ -332,7 +339,7 @@ fn timeout_ordered_channel_validate(fixture: Fixture) { ); ctx.store_update_meta( - "07-tendermint-0".into(), + default_client_id, client_height, Timestamp::from_nanoseconds(1000).unwrap(), Height::new(0, 4).unwrap(), diff --git a/ibc-testkit/tests/core/ics04_channel/timeout_on_close.rs b/ibc-testkit/tests/core/ics04_channel/timeout_on_close.rs index 266fab3d2..17489a81a 100644 --- a/ibc-testkit/tests/core/ics04_channel/timeout_on_close.rs +++ b/ibc-testkit/tests/core/ics04_channel/timeout_on_close.rs @@ -11,7 +11,7 @@ use ibc::core::connection::types::{ }; use ibc::core::entrypoint::validate; use ibc::core::handler::types::msgs::MsgEnvelope; -use ibc::core::host::types::identifiers::{ChannelId, ConnectionId, PortId}; +use ibc::core::host::types::identifiers::{ChannelId, ClientId, ConnectionId, PortId}; use ibc::core::host::ExecutionContext; use ibc::core::primitives::*; use ibc_testkit::fixtures::core::channel::dummy_raw_msg_timeout_on_close; @@ -30,6 +30,8 @@ pub struct Fixture { #[fixture] fn fixture() -> Fixture { + let default_client_id = ClientId::new("07-tendermint", 0).expect("no error"); + let client_height = Height::new(0, 2).unwrap(); let context = MockContext::default().with_client_config( MockClientConfig::builder() @@ -64,9 +66,9 @@ fn fixture() -> Fixture { let conn_end_on_a = ConnectionEnd::new( ConnectionState::Open, - "07-tendermint-0".into(), + default_client_id.clone(), ConnectionCounterparty::new( - "07-tendermint-0".into(), + default_client_id.clone(), Some(ConnectionId::zero()), CommitmentPrefix::empty(), ), @@ -131,6 +133,8 @@ fn timeout_on_close_success_no_packet_commitment(fixture: Fixture) { #[rstest] fn timeout_on_close_success_happy_path(fixture: Fixture) { + let default_client_id = ClientId::new("07-tendermint", 0).expect("no error"); + let Fixture { context, router, @@ -153,7 +157,7 @@ fn timeout_on_close_success_happy_path(fixture: Fixture) { context .get_client_execution_context() .store_update_meta( - "07-tendermint-0".into(), + default_client_id, Height::new(0, 2).unwrap(), Timestamp::from_nanoseconds(5000).unwrap(), Height::new(0, 5).unwrap(),