From fc6936060c69d82a74003c1163a92c33973cb4f1 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 5 Jan 2023 19:14:23 -0800 Subject: [PATCH 1/4] Exclude ChannelEnd from MsgChannelOpenInit and MsgChannelOpenTry --- .../20-refactor-channel-msgs.md | 2 + crates/ibc/src/core/ics04_channel/handler.rs | 14 ++--- .../ics04_channel/handler/chan_open_ack.rs | 8 +-- .../ics04_channel/handler/chan_open_init.rs | 18 +++--- .../ics04_channel/handler/chan_open_try.rs | 55 +++++++++---------- .../core/ics04_channel/msgs/chan_open_init.rs | 50 +++++++++++++---- .../core/ics04_channel/msgs/chan_open_try.rs | 47 ++++++++++++---- crates/ibc/src/core/ics26_routing/handler.rs | 2 +- 8 files changed, 127 insertions(+), 69 deletions(-) create mode 100644 .changelog/unreleased/breaking-changes/20-refactor-channel-msgs.md diff --git a/.changelog/unreleased/breaking-changes/20-refactor-channel-msgs.md b/.changelog/unreleased/breaking-changes/20-refactor-channel-msgs.md new file mode 100644 index 000000000..d72f933a7 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/20-refactor-channel-msgs.md @@ -0,0 +1,2 @@ +- Exclude `ChannelEnd` from `MsgChannelOpenInit` and `MsgChannelOpenTry` and refactor their fields to match the spec + ([#20](https://github.com/cosmos/ibc-rs/issues/20)) \ No newline at end of file diff --git a/crates/ibc/src/core/ics04_channel/handler.rs b/crates/ibc/src/core/ics04_channel/handler.rs index 96469cb17..56ca3fd79 100644 --- a/crates/ibc/src/core/ics04_channel/handler.rs +++ b/crates/ibc/src/core/ics04_channel/handler.rs @@ -116,12 +116,12 @@ where match msg { ChannelMsg::ChannelOpenInit(msg) => { let (extras, version) = cb.on_chan_open_init( - msg.chan_end_on_a.ordering, - &msg.chan_end_on_a.connection_hops, + msg.ordering_on_a, + &msg.connection_hops, &msg.port_id_on_a, &result.channel_id, - msg.chan_end_on_a.counterparty(), - &msg.chan_end_on_a.version, + &Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()), + &msg.version_on_a, )?; result.channel_end.version = version; @@ -129,11 +129,11 @@ where } ChannelMsg::ChannelOpenTry(msg) => { let (extras, version) = cb.on_chan_open_try( - msg.chan_end_on_b.ordering, - &msg.chan_end_on_b.connection_hops, + msg.ordering_on_b, + &msg.connection_hops, &msg.port_id_on_b, &result.channel_id, - msg.chan_end_on_b.counterparty(), + &Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), &msg.version_on_a, )?; result.channel_end.version = version; diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs index 0c500a7a7..979bb9ba6 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs @@ -198,24 +198,24 @@ mod tests { let chan_end = ChannelEnd::new( State::Init, - *msg_chan_try.chan_end_on_b.ordering(), + msg_chan_try.ordering_on_b, Counterparty::new( msg_chan_ack.port_id_on_a.clone(), Some(msg_chan_ack.chan_id_on_a.clone()), ), connection_vec0.clone(), - msg_chan_try.chan_end_on_b.version().clone(), + msg_chan_try.version_on_a, ); let failed_chan_end = ChannelEnd::new( State::Open, - *msg_chan_try.chan_end_on_b.ordering(), + msg_chan_try.ordering_on_b, Counterparty::new( msg_chan_ack.port_id_on_a.clone(), Some(msg_chan_ack.chan_id_on_a.clone()), ), connection_vec0, - msg_chan_try.chan_end_on_b.version().clone(), + msg_chan_try.version_on_b, ); let tests: Vec = vec![ diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs index b4a929d00..4f0f4691f 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs @@ -1,6 +1,6 @@ //! Protocol logic specific to ICS4 messages of type `MsgChannelOpenInit`. -use crate::core::ics04_channel::channel::{ChannelEnd, State}; +use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State}; use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::error::ChannelError; use crate::core::ics04_channel::handler::{ChannelIdState, ChannelResult}; @@ -16,32 +16,32 @@ pub(crate) fn process( ) -> HandlerResult { let mut output = HandlerOutput::builder(); - if msg.chan_end_on_a.connection_hops().len() != 1 { + if msg.connection_hops.len() != 1 { return Err(ChannelError::InvalidConnectionHopsLength { expected: 1, - actual: msg.chan_end_on_a.connection_hops().len(), + actual: msg.connection_hops.len(), }); } // An IBC connection running on the local (host) chain should exist. - let conn_end_on_a = ctx_a.connection_end(&msg.chan_end_on_a.connection_hops()[0])?; + let conn_end_on_a = ctx_a.connection_end(&msg.connection_hops[0])?; let conn_version = match conn_end_on_a.versions() { [version] => version, _ => return Err(ChannelError::InvalidVersionLengthConnection), }; - let channel_feature = msg.chan_end_on_a.ordering().to_string(); + let channel_feature = msg.ordering_on_a.to_string(); if !conn_version.is_supported_feature(channel_feature) { return Err(ChannelError::ChannelFeatureNotSuportedByConnection); } let chan_end_on_a = ChannelEnd::new( State::Init, - *msg.chan_end_on_a.ordering(), - msg.chan_end_on_a.counterparty().clone(), - msg.chan_end_on_a.connection_hops().clone(), - msg.chan_end_on_a.version().clone(), + msg.ordering_on_a, + Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()), + msg.connection_hops.clone(), + msg.version_on_a.clone(), ); let chan_id_on_a = ChannelId::new(ctx_a.channel_counter()?); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index 847be25af..710dc302c 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -19,17 +19,17 @@ pub(crate) fn process( let mut output = HandlerOutput::builder(); // An IBC connection running on the local (host) chain should exist. - if msg.chan_end_on_b.connection_hops().len() != 1 { + if msg.connection_hops.len() != 1 { return Err(ChannelError::InvalidConnectionHopsLength { expected: 1, - actual: msg.chan_end_on_b.connection_hops().len(), + actual: msg.connection_hops.len(), }); } - let conn_end_on_b = ctx_b.connection_end(&msg.chan_end_on_b.connection_hops()[0])?; + let conn_end_on_b = ctx_b.connection_end(&msg.connection_hops[0])?; if !conn_end_on_b.state_matches(&ConnectionState::Open) { return Err(ChannelError::ConnectionNotOpen { - connection_id: msg.chan_end_on_b.connection_hops()[0].clone(), + connection_id: msg.connection_hops[0].clone(), }); } @@ -38,7 +38,7 @@ pub(crate) fn process( _ => return Err(ChannelError::InvalidVersionLengthConnection), }; - let channel_feature = msg.chan_end_on_b.ordering().to_string(); + let channel_feature = msg.ordering_on_b.to_string(); if !conn_version.is_supported_feature(channel_feature) { return Err(ChannelError::ChannelFeatureNotSuportedByConnection); } @@ -50,15 +50,14 @@ pub(crate) fn process( let consensus_state_of_a_on_b = ctx_b.client_consensus_state(&client_id_on_b, &msg.proof_height_on_a)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); - let port_id_on_a = &msg.chan_end_on_b.counterparty().port_id; + let port_id_on_a = &&msg.port_id_on_a; let chan_id_on_a = msg - .chan_end_on_b - .counterparty() - .channel_id() + .chan_id_on_a + .clone() .ok_or(ChannelError::InvalidCounterpartyChannelId)?; let conn_id_on_a = conn_end_on_b.counterparty().connection_id().ok_or( ChannelError::UndefinedConnectionCounterparty { - connection_id: msg.chan_end_on_b.connection_hops()[0].clone(), + connection_id: msg.connection_hops[0].clone(), }, )?; @@ -71,7 +70,7 @@ pub(crate) fn process( let expected_chan_end_on_a = ChannelEnd::new( State::Init, - *msg.chan_end_on_b.ordering(), + msg.ordering_on_b, Counterparty::new(msg.port_id_on_b.clone(), None), vec![conn_id_on_a.clone()], msg.version_on_a.clone(), @@ -86,7 +85,7 @@ pub(crate) fn process( &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), port_id_on_a, - chan_id_on_a, + &chan_id_on_a, &expected_chan_end_on_a, ) .map_err(ChannelError::VerifyChannelFailed)?; @@ -94,9 +93,9 @@ pub(crate) fn process( let chan_end_on_b = ChannelEnd::new( State::TryOpen, - *msg.chan_end_on_b.ordering(), - msg.chan_end_on_b.counterparty().clone(), - msg.chan_end_on_b.connection_hops().clone(), + msg.ordering_on_b, + Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), + msg.connection_hops.clone(), // Note: This will be rewritten by the module callback Version::empty(), ); @@ -132,7 +131,7 @@ mod tests { use crate::core::ics03_connection::error as ics03_error; use crate::core::ics03_connection::msgs::test_util::get_dummy_raw_counterparty; use crate::core::ics03_connection::version::get_compatible_versions; - use crate::core::ics04_channel::channel::{ChannelEnd, State}; + use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State}; use crate::core::ics04_channel::error; use crate::core::ics04_channel::msgs::chan_open_try::test_util::get_dummy_raw_msg_chan_open_try; use crate::core::ics04_channel::msgs::chan_open_try::MsgChannelOpenTry; @@ -176,16 +175,16 @@ mod tests { let chan_id = ChannelId::new(24); let hops = vec![conn_id.clone()]; - msg.chan_end_on_b.connection_hops = hops; + msg.connection_hops = hops; // A preloaded channel end that resides in the context. This is constructed so as to be // consistent with the incoming ChanOpenTry message `msg`. let correct_chan_end = ChannelEnd::new( State::Init, - *msg.chan_end_on_b.ordering(), - msg.chan_end_on_b.counterparty().clone(), - msg.chan_end_on_b.connection_hops().clone(), - msg.chan_end_on_b.version().clone(), + msg.ordering_on_b, + Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), + msg.connection_hops.clone(), + msg.version_on_b.clone(), ); let tests: Vec = vec![ @@ -195,7 +194,7 @@ mod tests { msg: ChannelMsg::ChannelOpenTry(msg.clone()), want_pass: false, match_error: { - let connection_id = msg.chan_end_on_b.connection_hops()[0].clone(); + let connection_id = msg.connection_hops[0].clone(); Box::new(move |e| match e { error::ChannelError::Connection(e) => { assert_eq!( @@ -320,18 +319,18 @@ mod tests { // Note: we make the counterparty's channel_id `None`. let mut msg = MsgChannelOpenTry::try_from(get_dummy_raw_msg_chan_open_try(proof_height)).unwrap(); - msg.chan_end_on_b.remote.channel_id = None; + msg.chan_id_on_a = None; let chan_id = ChannelId::new(24); let hops = vec![conn_id.clone()]; - msg.chan_end_on_b.connection_hops = hops; + msg.connection_hops = hops; let chan_end = ChannelEnd::new( State::Init, - *msg.chan_end_on_b.ordering(), - msg.chan_end_on_b.counterparty().clone(), - msg.chan_end_on_b.connection_hops().clone(), - msg.chan_end_on_b.version().clone(), + msg.ordering_on_b, + Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_a.clone()), + msg.connection_hops.clone(), + msg.version_on_b.clone(), ); let context = MockContext::default() .with_client(&client_id, Height::new(0, proof_height).unwrap()) diff --git a/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs index 570bbda0e..069dc5134 100644 --- a/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs @@ -1,6 +1,9 @@ use crate::core::ics04_channel::channel::ChannelEnd; +use crate::core::ics04_channel::channel::Counterparty; +use crate::core::ics04_channel::channel::{Order, State}; use crate::core::ics04_channel::error::ChannelError; -use crate::core::ics24_host::identifier::PortId; +use crate::core::ics04_channel::Version; +use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId}; use crate::prelude::*; use crate::signer::Signer; use crate::tx_msg::Msg; @@ -17,15 +20,31 @@ pub const TYPE_URL: &str = "/ibc.core.channel.v1.MsgChannelOpenInit"; #[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgChannelOpenInit { pub port_id_on_a: PortId, - pub chan_end_on_a: ChannelEnd, + pub ordering_on_a: Order, + pub version_on_a: Version, + pub port_id_on_b: PortId, + pub chan_id_on_b: Option, + pub connection_hops: Vec, pub signer: Signer, } impl MsgChannelOpenInit { - pub fn new(port_id_on_a: PortId, chan_end_on_a: ChannelEnd, signer: Signer) -> Self { + pub fn new( + port_id_on_a: PortId, + ordering_on_a: Order, + port_id_on_b: PortId, + version_on_a: Version, + chan_id_on_b: Option, + connection_hops: Vec, + signer: Signer, + ) -> Self { Self { port_id_on_a, - chan_end_on_a, + ordering_on_a, + version_on_a, + port_id_on_b, + chan_id_on_b, + connection_hops, signer, } } @@ -45,22 +64,33 @@ impl TryFrom for MsgChannelOpenInit { type Error = ChannelError; fn try_from(raw_msg: RawMsgChannelOpenInit) -> Result { + let chan_end_on_a: ChannelEnd = raw_msg + .channel + .ok_or(ChannelError::MissingChannel)? + .try_into()?; Ok(MsgChannelOpenInit { port_id_on_a: raw_msg.port_id.parse().map_err(ChannelError::Identifier)?, - chan_end_on_a: raw_msg - .channel - .ok_or(ChannelError::MissingChannel)? - .try_into()?, + ordering_on_a: chan_end_on_a.ordering, + port_id_on_b: chan_end_on_a.remote.port_id, + chan_id_on_b: chan_end_on_a.remote.channel_id, + connection_hops: chan_end_on_a.connection_hops, + version_on_a: chan_end_on_a.version, signer: raw_msg.signer.parse().map_err(ChannelError::Signer)?, }) } } - impl From for RawMsgChannelOpenInit { fn from(domain_msg: MsgChannelOpenInit) -> Self { + let chan_end_on_a = ChannelEnd::new( + State::Init, + domain_msg.ordering_on_a, + Counterparty::new(domain_msg.port_id_on_b, domain_msg.chan_id_on_b), + domain_msg.connection_hops, + domain_msg.version_on_a, + ); RawMsgChannelOpenInit { port_id: domain_msg.port_id_on_a.to_string(), - channel: Some(domain_msg.chan_end_on_a.into()), + channel: Some(chan_end_on_a.into()), signer: domain_msg.signer.to_string(), } } diff --git a/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs index cadbeed55..3e76b4e01 100644 --- a/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs @@ -1,9 +1,11 @@ use crate::core::ics04_channel::channel::ChannelEnd; +use crate::core::ics04_channel::channel::Counterparty; +use crate::core::ics04_channel::channel::{Order, State}; use crate::core::ics04_channel::error::ChannelError; use crate::core::ics04_channel::Version; use crate::core::ics23_commitment::commitment::CommitmentProofBytes; use crate::core::ics24_host::error::ValidationError; -use crate::core::ics24_host::identifier::PortId; +use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId}; use crate::signer::Signer; use crate::tx_msg::Msg; use crate::{prelude::*, Height}; @@ -20,8 +22,12 @@ pub const TYPE_URL: &str = "/ibc.core.channel.v1.MsgChannelOpenTry"; #[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgChannelOpenTry { pub port_id_on_b: PortId, - pub chan_end_on_b: ChannelEnd, + pub ordering_on_b: Order, + pub version_on_b: Version, + pub port_id_on_a: PortId, + pub chan_id_on_a: Option, pub version_on_a: Version, + pub connection_hops: Vec, pub proof_chan_end_on_a: CommitmentProofBytes, pub proof_height_on_a: Height, pub signer: Signer, @@ -32,10 +38,15 @@ pub struct MsgChannelOpenTry { } impl MsgChannelOpenTry { + #[allow(clippy::too_many_arguments)] pub fn new( port_id_on_b: PortId, - chan_end_on_b: ChannelEnd, + ordering_on_b: Order, + version_on_b: Version, + port_id_on_a: PortId, + chan_id_on_a: Option, version_on_a: Version, + connection_hops: Vec, proof_chan_end_on_a: CommitmentProofBytes, proof_height_on_a: Height, signer: Signer, @@ -43,8 +54,12 @@ impl MsgChannelOpenTry { #[allow(deprecated)] Self { port_id_on_b, - chan_end_on_b, + ordering_on_b, + version_on_b, + port_id_on_a, + chan_id_on_a, version_on_a, + connection_hops, proof_chan_end_on_a, proof_height_on_a, signer, @@ -67,15 +82,20 @@ impl TryFrom for MsgChannelOpenTry { type Error = ChannelError; fn try_from(raw_msg: RawMsgChannelOpenTry) -> Result { + let chan_end_on_b: ChannelEnd = raw_msg + .channel + .ok_or(ChannelError::MissingChannel)? + .try_into()?; #[allow(deprecated)] let msg = MsgChannelOpenTry { port_id_on_b: raw_msg.port_id.parse().map_err(ChannelError::Identifier)?, + ordering_on_b: chan_end_on_b.ordering, previous_channel_id: raw_msg.previous_channel_id, - chan_end_on_b: raw_msg - .channel - .ok_or(ChannelError::MissingChannel)? - .try_into()?, + version_on_b: chan_end_on_b.version.clone(), + port_id_on_a: chan_end_on_b.remote.port_id, + chan_id_on_a: chan_end_on_b.remote.channel_id, version_on_a: raw_msg.counterparty_version.into(), + connection_hops: chan_end_on_b.connection_hops, proof_chan_end_on_a: raw_msg .proof_init .try_into() @@ -87,7 +107,7 @@ impl TryFrom for MsgChannelOpenTry { signer: raw_msg.signer.parse().map_err(ChannelError::Signer)?, }; - match msg.chan_end_on_b.counterparty().channel_id() { + match msg.chan_id_on_a.clone() { None => Err(ValidationError::InvalidCounterpartyChannelId), Some(_c) => Ok(()), } @@ -99,11 +119,18 @@ impl TryFrom for MsgChannelOpenTry { impl From for RawMsgChannelOpenTry { fn from(domain_msg: MsgChannelOpenTry) -> Self { + let chan_end_on_b = ChannelEnd::new( + State::Init, + domain_msg.ordering_on_b, + Counterparty::new(domain_msg.port_id_on_a, domain_msg.chan_id_on_a), + domain_msg.connection_hops, + domain_msg.version_on_b.clone(), + ); #[allow(deprecated)] RawMsgChannelOpenTry { port_id: domain_msg.port_id_on_b.to_string(), previous_channel_id: domain_msg.previous_channel_id, - channel: Some(domain_msg.chan_end_on_b.into()), + channel: Some(chan_end_on_b.into()), counterparty_version: domain_msg.version_on_a.to_string(), proof_init: domain_msg.proof_chan_end_on_a.clone().into(), proof_height: Some(domain_msg.proof_height_on_a.into()), diff --git a/crates/ibc/src/core/ics26_routing/handler.rs b/crates/ibc/src/core/ics26_routing/handler.rs index 4f4b33f43..dafb460c5 100644 --- a/crates/ibc/src/core/ics26_routing/handler.rs +++ b/crates/ibc/src/core/ics26_routing/handler.rs @@ -317,7 +317,7 @@ mod tests { // The handler will fail to process this b/c the associated connection does not exist let mut incorrect_msg_chan_init = msg_chan_init.clone(); - incorrect_msg_chan_init.chan_end_on_a.connection_hops = vec![ConnectionId::new(590)]; + incorrect_msg_chan_init.connection_hops = vec![ConnectionId::new(590)]; let msg_chan_try = MsgChannelOpenTry::try_from(get_dummy_raw_msg_chan_open_try(client_height)).unwrap(); From c8298f2319c3fe032fb6b18bf171c93ced50f9a6 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 6 Jan 2023 09:55:57 -0800 Subject: [PATCH 2/4] Apply naming convention on connection_hops --- crates/ibc/src/core/ics04_channel/handler.rs | 4 ++-- .../ics04_channel/handler/chan_open_init.rs | 8 +++---- .../ics04_channel/handler/chan_open_try.rs | 22 +++++++++---------- .../core/ics04_channel/msgs/chan_open_init.rs | 10 ++++----- .../core/ics04_channel/msgs/chan_open_try.rs | 10 ++++----- crates/ibc/src/core/ics26_routing/handler.rs | 2 +- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/handler.rs b/crates/ibc/src/core/ics04_channel/handler.rs index 56ca3fd79..be94568a0 100644 --- a/crates/ibc/src/core/ics04_channel/handler.rs +++ b/crates/ibc/src/core/ics04_channel/handler.rs @@ -117,7 +117,7 @@ where ChannelMsg::ChannelOpenInit(msg) => { let (extras, version) = cb.on_chan_open_init( msg.ordering_on_a, - &msg.connection_hops, + &msg.connection_hops_on_a, &msg.port_id_on_a, &result.channel_id, &Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()), @@ -130,7 +130,7 @@ where ChannelMsg::ChannelOpenTry(msg) => { let (extras, version) = cb.on_chan_open_try( msg.ordering_on_b, - &msg.connection_hops, + &msg.connection_hops_on_b, &msg.port_id_on_b, &result.channel_id, &Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs index 4f0f4691f..77f2e57a5 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs @@ -16,15 +16,15 @@ pub(crate) fn process( ) -> HandlerResult { let mut output = HandlerOutput::builder(); - if msg.connection_hops.len() != 1 { + if msg.connection_hops_on_a.len() != 1 { return Err(ChannelError::InvalidConnectionHopsLength { expected: 1, - actual: msg.connection_hops.len(), + actual: msg.connection_hops_on_a.len(), }); } // An IBC connection running on the local (host) chain should exist. - let conn_end_on_a = ctx_a.connection_end(&msg.connection_hops[0])?; + let conn_end_on_a = ctx_a.connection_end(&msg.connection_hops_on_a[0])?; let conn_version = match conn_end_on_a.versions() { [version] => version, @@ -40,7 +40,7 @@ pub(crate) fn process( State::Init, msg.ordering_on_a, Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()), - msg.connection_hops.clone(), + msg.connection_hops_on_a.clone(), msg.version_on_a.clone(), ); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index 710dc302c..1c75dd06f 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -19,17 +19,17 @@ pub(crate) fn process( let mut output = HandlerOutput::builder(); // An IBC connection running on the local (host) chain should exist. - if msg.connection_hops.len() != 1 { + if msg.connection_hops_on_b.len() != 1 { return Err(ChannelError::InvalidConnectionHopsLength { expected: 1, - actual: msg.connection_hops.len(), + actual: msg.connection_hops_on_b.len(), }); } - let conn_end_on_b = ctx_b.connection_end(&msg.connection_hops[0])?; + let conn_end_on_b = ctx_b.connection_end(&msg.connection_hops_on_b[0])?; if !conn_end_on_b.state_matches(&ConnectionState::Open) { return Err(ChannelError::ConnectionNotOpen { - connection_id: msg.connection_hops[0].clone(), + connection_id: msg.connection_hops_on_b[0].clone(), }); } @@ -57,7 +57,7 @@ pub(crate) fn process( .ok_or(ChannelError::InvalidCounterpartyChannelId)?; let conn_id_on_a = conn_end_on_b.counterparty().connection_id().ok_or( ChannelError::UndefinedConnectionCounterparty { - connection_id: msg.connection_hops[0].clone(), + connection_id: msg.connection_hops_on_b[0].clone(), }, )?; @@ -95,7 +95,7 @@ pub(crate) fn process( State::TryOpen, msg.ordering_on_b, Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), - msg.connection_hops.clone(), + msg.connection_hops_on_b.clone(), // Note: This will be rewritten by the module callback Version::empty(), ); @@ -175,7 +175,7 @@ mod tests { let chan_id = ChannelId::new(24); let hops = vec![conn_id.clone()]; - msg.connection_hops = hops; + msg.connection_hops_on_b = hops; // A preloaded channel end that resides in the context. This is constructed so as to be // consistent with the incoming ChanOpenTry message `msg`. @@ -183,7 +183,7 @@ mod tests { State::Init, msg.ordering_on_b, Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), - msg.connection_hops.clone(), + msg.connection_hops_on_b.clone(), msg.version_on_b.clone(), ); @@ -194,7 +194,7 @@ mod tests { msg: ChannelMsg::ChannelOpenTry(msg.clone()), want_pass: false, match_error: { - let connection_id = msg.connection_hops[0].clone(); + let connection_id = msg.connection_hops_on_b[0].clone(); Box::new(move |e| match e { error::ChannelError::Connection(e) => { assert_eq!( @@ -323,13 +323,13 @@ mod tests { let chan_id = ChannelId::new(24); let hops = vec![conn_id.clone()]; - msg.connection_hops = hops; + msg.connection_hops_on_b = hops; let chan_end = ChannelEnd::new( State::Init, msg.ordering_on_b, Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_a.clone()), - msg.connection_hops.clone(), + msg.connection_hops_on_b.clone(), msg.version_on_b.clone(), ); let context = MockContext::default() diff --git a/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs index 069dc5134..3dd6dd2b7 100644 --- a/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs @@ -22,9 +22,9 @@ pub struct MsgChannelOpenInit { pub port_id_on_a: PortId, pub ordering_on_a: Order, pub version_on_a: Version, + pub connection_hops_on_a: Vec, pub port_id_on_b: PortId, pub chan_id_on_b: Option, - pub connection_hops: Vec, pub signer: Signer, } @@ -32,19 +32,19 @@ impl MsgChannelOpenInit { pub fn new( port_id_on_a: PortId, ordering_on_a: Order, + connection_hops_on_a: Vec, port_id_on_b: PortId, version_on_a: Version, chan_id_on_b: Option, - connection_hops: Vec, signer: Signer, ) -> Self { Self { port_id_on_a, ordering_on_a, version_on_a, + connection_hops_on_a, port_id_on_b, chan_id_on_b, - connection_hops, signer, } } @@ -71,9 +71,9 @@ impl TryFrom for MsgChannelOpenInit { Ok(MsgChannelOpenInit { port_id_on_a: raw_msg.port_id.parse().map_err(ChannelError::Identifier)?, ordering_on_a: chan_end_on_a.ordering, + connection_hops_on_a: chan_end_on_a.connection_hops, port_id_on_b: chan_end_on_a.remote.port_id, chan_id_on_b: chan_end_on_a.remote.channel_id, - connection_hops: chan_end_on_a.connection_hops, version_on_a: chan_end_on_a.version, signer: raw_msg.signer.parse().map_err(ChannelError::Signer)?, }) @@ -85,7 +85,7 @@ impl From for RawMsgChannelOpenInit { State::Init, domain_msg.ordering_on_a, Counterparty::new(domain_msg.port_id_on_b, domain_msg.chan_id_on_b), - domain_msg.connection_hops, + domain_msg.connection_hops_on_a, domain_msg.version_on_a, ); RawMsgChannelOpenInit { diff --git a/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs index 3e76b4e01..35e4cb4ab 100644 --- a/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs @@ -24,10 +24,10 @@ pub struct MsgChannelOpenTry { pub port_id_on_b: PortId, pub ordering_on_b: Order, pub version_on_b: Version, + pub connection_hops_on_b: Vec, pub port_id_on_a: PortId, pub chan_id_on_a: Option, pub version_on_a: Version, - pub connection_hops: Vec, pub proof_chan_end_on_a: CommitmentProofBytes, pub proof_height_on_a: Height, pub signer: Signer, @@ -43,10 +43,10 @@ impl MsgChannelOpenTry { port_id_on_b: PortId, ordering_on_b: Order, version_on_b: Version, + connection_hops_on_b: Vec, port_id_on_a: PortId, chan_id_on_a: Option, version_on_a: Version, - connection_hops: Vec, proof_chan_end_on_a: CommitmentProofBytes, proof_height_on_a: Height, signer: Signer, @@ -56,10 +56,10 @@ impl MsgChannelOpenTry { port_id_on_b, ordering_on_b, version_on_b, + connection_hops_on_b, port_id_on_a, chan_id_on_a, version_on_a, - connection_hops, proof_chan_end_on_a, proof_height_on_a, signer, @@ -92,10 +92,10 @@ impl TryFrom for MsgChannelOpenTry { ordering_on_b: chan_end_on_b.ordering, previous_channel_id: raw_msg.previous_channel_id, version_on_b: chan_end_on_b.version.clone(), + connection_hops_on_b: chan_end_on_b.connection_hops, port_id_on_a: chan_end_on_b.remote.port_id, chan_id_on_a: chan_end_on_b.remote.channel_id, version_on_a: raw_msg.counterparty_version.into(), - connection_hops: chan_end_on_b.connection_hops, proof_chan_end_on_a: raw_msg .proof_init .try_into() @@ -123,7 +123,7 @@ impl From for RawMsgChannelOpenTry { State::Init, domain_msg.ordering_on_b, Counterparty::new(domain_msg.port_id_on_a, domain_msg.chan_id_on_a), - domain_msg.connection_hops, + domain_msg.connection_hops_on_b, domain_msg.version_on_b.clone(), ); #[allow(deprecated)] diff --git a/crates/ibc/src/core/ics26_routing/handler.rs b/crates/ibc/src/core/ics26_routing/handler.rs index dafb460c5..3918d38d4 100644 --- a/crates/ibc/src/core/ics26_routing/handler.rs +++ b/crates/ibc/src/core/ics26_routing/handler.rs @@ -317,7 +317,7 @@ mod tests { // The handler will fail to process this b/c the associated connection does not exist let mut incorrect_msg_chan_init = msg_chan_init.clone(); - incorrect_msg_chan_init.connection_hops = vec![ConnectionId::new(590)]; + incorrect_msg_chan_init.connection_hops_on_a = vec![ConnectionId::new(590)]; let msg_chan_try = MsgChannelOpenTry::try_from(get_dummy_raw_msg_chan_open_try(client_height)).unwrap(); From 5fe009311c319d2156b7aa3960ffd2fa223f54ae Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 6 Jan 2023 14:45:51 -0800 Subject: [PATCH 3/4] Revise naming of some fields --- crates/ibc/src/core/ics04_channel/channel.rs | 15 +++--- crates/ibc/src/core/ics04_channel/handler.rs | 12 ++--- .../ics04_channel/handler/chan_open_ack.rs | 8 +-- .../ics04_channel/handler/chan_open_init.rs | 8 +-- .../ics04_channel/handler/chan_open_try.rs | 29 +++++------ .../core/ics04_channel/msgs/chan_open_init.rs | 31 +++++------ .../core/ics04_channel/msgs/chan_open_try.rs | 51 +++++++++---------- 7 files changed, 73 insertions(+), 81 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/channel.rs b/crates/ibc/src/core/ics04_channel/channel.rs index b00807c4f..f22eba69a 100644 --- a/crates/ibc/src/core/ics04_channel/channel.rs +++ b/crates/ibc/src/core/ics04_channel/channel.rs @@ -501,34 +501,35 @@ impl Display for State { #[cfg(test)] pub mod test_util { - use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId}; + use crate::core::ics24_host::identifier::{ConnectionId, PortId}; use crate::prelude::*; use ibc_proto::ibc::core::channel::v1::Channel as RawChannel; use ibc_proto::ibc::core::channel::v1::Counterparty as RawCounterparty; /// Returns a dummy `RawCounterparty`, for testing only! /// Can be optionally parametrized with a specific channel identifier. - pub fn get_dummy_raw_counterparty() -> RawCounterparty { + pub fn get_dummy_raw_counterparty(channel_id: String) -> RawCounterparty { RawCounterparty { port_id: PortId::default().to_string(), - channel_id: ChannelId::default().to_string(), + channel_id, } } /// Returns a dummy `RawChannel`, for testing only! - pub fn get_dummy_raw_channel_end() -> RawChannel { + pub fn get_dummy_raw_channel_end(channel_id: String) -> RawChannel { RawChannel { state: 1, ordering: 2, - counterparty: Some(get_dummy_raw_counterparty()), + counterparty: Some(get_dummy_raw_counterparty(channel_id)), connection_hops: vec![ConnectionId::default().to_string()], - version: "ics20".to_string(), // The version is not validated. + version: "".to_string(), // The version is not validated. } } } #[cfg(test)] mod tests { + use crate::core::ics24_host::identifier::ChannelId; use crate::prelude::*; use core::str::FromStr; @@ -541,7 +542,7 @@ mod tests { #[test] fn channel_end_try_from_raw() { - let raw_channel_end = get_dummy_raw_channel_end(); + let raw_channel_end = get_dummy_raw_channel_end(ChannelId::default().to_string()); let empty_raw_channel_end = RawChannel { counterparty: None, diff --git a/crates/ibc/src/core/ics04_channel/handler.rs b/crates/ibc/src/core/ics04_channel/handler.rs index be94568a0..47dc202e1 100644 --- a/crates/ibc/src/core/ics04_channel/handler.rs +++ b/crates/ibc/src/core/ics04_channel/handler.rs @@ -116,12 +116,12 @@ where match msg { ChannelMsg::ChannelOpenInit(msg) => { let (extras, version) = cb.on_chan_open_init( - msg.ordering_on_a, + msg.ordering, &msg.connection_hops_on_a, &msg.port_id_on_a, &result.channel_id, - &Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()), - &msg.version_on_a, + &Counterparty::new(msg.port_id_on_b.clone(), None), + &msg.version_proposal, )?; result.channel_end.version = version; @@ -129,12 +129,12 @@ where } ChannelMsg::ChannelOpenTry(msg) => { let (extras, version) = cb.on_chan_open_try( - msg.ordering_on_b, + msg.ordering, &msg.connection_hops_on_b, &msg.port_id_on_b, &result.channel_id, - &Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), - &msg.version_on_a, + &Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), + &msg.version_supported_on_a, )?; result.channel_end.version = version; diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs index 979bb9ba6..f539601e1 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs @@ -198,24 +198,24 @@ mod tests { let chan_end = ChannelEnd::new( State::Init, - msg_chan_try.ordering_on_b, + msg_chan_try.ordering, Counterparty::new( msg_chan_ack.port_id_on_a.clone(), Some(msg_chan_ack.chan_id_on_a.clone()), ), connection_vec0.clone(), - msg_chan_try.version_on_a, + msg_chan_try.version_supported_on_a.clone(), ); let failed_chan_end = ChannelEnd::new( State::Open, - msg_chan_try.ordering_on_b, + msg_chan_try.ordering, Counterparty::new( msg_chan_ack.port_id_on_a.clone(), Some(msg_chan_ack.chan_id_on_a.clone()), ), connection_vec0, - msg_chan_try.version_on_b, + msg_chan_try.version_supported_on_a, ); let tests: Vec = vec![ diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs index 77f2e57a5..79d1e66ba 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_init.rs @@ -31,17 +31,17 @@ pub(crate) fn process( _ => return Err(ChannelError::InvalidVersionLengthConnection), }; - let channel_feature = msg.ordering_on_a.to_string(); + let channel_feature = msg.ordering.to_string(); if !conn_version.is_supported_feature(channel_feature) { return Err(ChannelError::ChannelFeatureNotSuportedByConnection); } let chan_end_on_a = ChannelEnd::new( State::Init, - msg.ordering_on_a, - Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_b.clone()), + msg.ordering, + Counterparty::new(msg.port_id_on_b.clone(), None), msg.connection_hops_on_a.clone(), - msg.version_on_a.clone(), + msg.version_proposal.clone(), ); let chan_id_on_a = ChannelId::new(ctx_a.channel_counter()?); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index 1c75dd06f..67f647c97 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -38,7 +38,7 @@ pub(crate) fn process( _ => return Err(ChannelError::InvalidVersionLengthConnection), }; - let channel_feature = msg.ordering_on_b.to_string(); + let channel_feature = msg.ordering.to_string(); if !conn_version.is_supported_feature(channel_feature) { return Err(ChannelError::ChannelFeatureNotSuportedByConnection); } @@ -51,10 +51,7 @@ pub(crate) fn process( ctx_b.client_consensus_state(&client_id_on_b, &msg.proof_height_on_a)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); let port_id_on_a = &&msg.port_id_on_a; - let chan_id_on_a = msg - .chan_id_on_a - .clone() - .ok_or(ChannelError::InvalidCounterpartyChannelId)?; + let chan_id_on_a = msg.chan_id_on_a.clone(); let conn_id_on_a = conn_end_on_b.counterparty().connection_id().ok_or( ChannelError::UndefinedConnectionCounterparty { connection_id: msg.connection_hops_on_b[0].clone(), @@ -70,10 +67,10 @@ pub(crate) fn process( let expected_chan_end_on_a = ChannelEnd::new( State::Init, - msg.ordering_on_b, + msg.ordering, Counterparty::new(msg.port_id_on_b.clone(), None), vec![conn_id_on_a.clone()], - msg.version_on_a.clone(), + msg.version_supported_on_a.clone(), ); // Verify the proof for the channel state against the expected channel end. @@ -93,8 +90,8 @@ pub(crate) fn process( let chan_end_on_b = ChannelEnd::new( State::TryOpen, - msg.ordering_on_b, - Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), + msg.ordering, + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), msg.connection_hops_on_b.clone(), // Note: This will be rewritten by the module callback Version::empty(), @@ -136,6 +133,7 @@ mod tests { use crate::core::ics04_channel::msgs::chan_open_try::test_util::get_dummy_raw_msg_chan_open_try; use crate::core::ics04_channel::msgs::chan_open_try::MsgChannelOpenTry; use crate::core::ics04_channel::msgs::ChannelMsg; + use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId}; use crate::mock::client_state::client_type as mock_client_type; use crate::mock::context::MockContext; @@ -181,10 +179,10 @@ mod tests { // consistent with the incoming ChanOpenTry message `msg`. let correct_chan_end = ChannelEnd::new( State::Init, - msg.ordering_on_b, - Counterparty::new(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone()), + msg.ordering, + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), msg.connection_hops_on_b.clone(), - msg.version_on_b.clone(), + Version::empty(), ); let tests: Vec = vec![ @@ -319,7 +317,6 @@ mod tests { // Note: we make the counterparty's channel_id `None`. let mut msg = MsgChannelOpenTry::try_from(get_dummy_raw_msg_chan_open_try(proof_height)).unwrap(); - msg.chan_id_on_a = None; let chan_id = ChannelId::new(24); let hops = vec![conn_id.clone()]; @@ -327,10 +324,10 @@ mod tests { let chan_end = ChannelEnd::new( State::Init, - msg.ordering_on_b, - Counterparty::new(msg.port_id_on_b.clone(), msg.chan_id_on_a.clone()), + msg.ordering, + Counterparty::new(msg.port_id_on_a.clone(), None), msg.connection_hops_on_b.clone(), - msg.version_on_b.clone(), + Version::empty(), ); let context = MockContext::default() .with_client(&client_id, Height::new(0, proof_height).unwrap()) diff --git a/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs b/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs index 3dd6dd2b7..1dfde5482 100644 --- a/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs +++ b/crates/ibc/src/core/ics04_channel/msgs/chan_open_init.rs @@ -3,7 +3,7 @@ use crate::core::ics04_channel::channel::Counterparty; use crate::core::ics04_channel::channel::{Order, State}; use crate::core::ics04_channel::error::ChannelError; use crate::core::ics04_channel::Version; -use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId}; +use crate::core::ics24_host::identifier::{ConnectionId, PortId}; use crate::prelude::*; use crate::signer::Signer; use crate::tx_msg::Msg; @@ -20,32 +20,30 @@ pub const TYPE_URL: &str = "/ibc.core.channel.v1.MsgChannelOpenInit"; #[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgChannelOpenInit { pub port_id_on_a: PortId, - pub ordering_on_a: Order, - pub version_on_a: Version, pub connection_hops_on_a: Vec, pub port_id_on_b: PortId, - pub chan_id_on_b: Option, + pub ordering: Order, pub signer: Signer, + /// Allow a relayer to specify a particular version by providing a non-empty version string + pub version_proposal: Version, } impl MsgChannelOpenInit { pub fn new( port_id_on_a: PortId, - ordering_on_a: Order, connection_hops_on_a: Vec, port_id_on_b: PortId, - version_on_a: Version, - chan_id_on_b: Option, + ordering: Order, signer: Signer, + version_proposal: Version, ) -> Self { Self { port_id_on_a, - ordering_on_a, - version_on_a, connection_hops_on_a, port_id_on_b, - chan_id_on_b, + ordering, signer, + version_proposal, } } } @@ -70,12 +68,11 @@ impl TryFrom for MsgChannelOpenInit { .try_into()?; Ok(MsgChannelOpenInit { port_id_on_a: raw_msg.port_id.parse().map_err(ChannelError::Identifier)?, - ordering_on_a: chan_end_on_a.ordering, connection_hops_on_a: chan_end_on_a.connection_hops, port_id_on_b: chan_end_on_a.remote.port_id, - chan_id_on_b: chan_end_on_a.remote.channel_id, - version_on_a: chan_end_on_a.version, + ordering: chan_end_on_a.ordering, signer: raw_msg.signer.parse().map_err(ChannelError::Signer)?, + version_proposal: chan_end_on_a.version, }) } } @@ -83,10 +80,10 @@ impl From for RawMsgChannelOpenInit { fn from(domain_msg: MsgChannelOpenInit) -> Self { let chan_end_on_a = ChannelEnd::new( State::Init, - domain_msg.ordering_on_a, - Counterparty::new(domain_msg.port_id_on_b, domain_msg.chan_id_on_b), + domain_msg.ordering, + Counterparty::new(domain_msg.port_id_on_b, None), domain_msg.connection_hops_on_a, - domain_msg.version_on_a, + domain_msg.version_proposal, ); RawMsgChannelOpenInit { port_id: domain_msg.port_id_on_a.to_string(), @@ -109,7 +106,7 @@ pub mod test_util { pub fn get_dummy_raw_msg_chan_open_init() -> RawMsgChannelOpenInit { RawMsgChannelOpenInit { port_id: PortId::default().to_string(), - channel: Some(get_dummy_raw_channel_end()), + channel: Some(get_dummy_raw_channel_end("".to_string())), signer: get_dummy_bech32_account(), } } diff --git a/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs index 35e4cb4ab..ee0b650e1 100644 --- a/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/msgs/chan_open_try.rs @@ -4,7 +4,6 @@ use crate::core::ics04_channel::channel::{Order, State}; use crate::core::ics04_channel::error::ChannelError; use crate::core::ics04_channel::Version; use crate::core::ics23_commitment::commitment::CommitmentProofBytes; -use crate::core::ics24_host::error::ValidationError; use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId}; use crate::signer::Signer; use crate::tx_msg::Msg; @@ -22,48 +21,49 @@ pub const TYPE_URL: &str = "/ibc.core.channel.v1.MsgChannelOpenTry"; #[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgChannelOpenTry { pub port_id_on_b: PortId, - pub ordering_on_b: Order, - pub version_on_b: Version, pub connection_hops_on_b: Vec, pub port_id_on_a: PortId, - pub chan_id_on_a: Option, - pub version_on_a: Version, + pub chan_id_on_a: ChannelId, + pub version_supported_on_a: Version, pub proof_chan_end_on_a: CommitmentProofBytes, pub proof_height_on_a: Height, + pub ordering: Order, pub signer: Signer, #[deprecated(since = "0.22.0")] /// Only kept here for proper conversion to/from the raw type pub previous_channel_id: String, + #[deprecated(since = "0.22.0")] + /// Only kept here for proper conversion to/from the raw type + pub version_proposal: Version, } impl MsgChannelOpenTry { #[allow(clippy::too_many_arguments)] pub fn new( port_id_on_b: PortId, - ordering_on_b: Order, - version_on_b: Version, connection_hops_on_b: Vec, port_id_on_a: PortId, - chan_id_on_a: Option, - version_on_a: Version, + chan_id_on_a: ChannelId, + version_supported_on_a: Version, proof_chan_end_on_a: CommitmentProofBytes, proof_height_on_a: Height, + ordering: Order, signer: Signer, ) -> Self { #[allow(deprecated)] Self { port_id_on_b, - ordering_on_b, - version_on_b, connection_hops_on_b, port_id_on_a, chan_id_on_a, - version_on_a, + version_supported_on_a, proof_chan_end_on_a, proof_height_on_a, + ordering, signer, previous_channel_id: "".to_string(), + version_proposal: Version::empty(), } } } @@ -89,13 +89,15 @@ impl TryFrom for MsgChannelOpenTry { #[allow(deprecated)] let msg = MsgChannelOpenTry { port_id_on_b: raw_msg.port_id.parse().map_err(ChannelError::Identifier)?, - ordering_on_b: chan_end_on_b.ordering, + ordering: chan_end_on_b.ordering, previous_channel_id: raw_msg.previous_channel_id, - version_on_b: chan_end_on_b.version.clone(), connection_hops_on_b: chan_end_on_b.connection_hops, port_id_on_a: chan_end_on_b.remote.port_id, - chan_id_on_a: chan_end_on_b.remote.channel_id, - version_on_a: raw_msg.counterparty_version.into(), + chan_id_on_a: chan_end_on_b + .remote + .channel_id + .ok_or(ChannelError::InvalidCounterpartyChannelId)?, + version_supported_on_a: raw_msg.counterparty_version.into(), proof_chan_end_on_a: raw_msg .proof_init .try_into() @@ -105,14 +107,9 @@ impl TryFrom for MsgChannelOpenTry { .and_then(|raw_height| raw_height.try_into().ok()) .ok_or(ChannelError::MissingHeight)?, signer: raw_msg.signer.parse().map_err(ChannelError::Signer)?, + version_proposal: chan_end_on_b.version, }; - match msg.chan_id_on_a.clone() { - None => Err(ValidationError::InvalidCounterpartyChannelId), - Some(_c) => Ok(()), - } - .map_err(|_| ChannelError::InvalidCounterpartyChannelId)?; - Ok(msg) } } @@ -121,17 +118,17 @@ impl From for RawMsgChannelOpenTry { fn from(domain_msg: MsgChannelOpenTry) -> Self { let chan_end_on_b = ChannelEnd::new( State::Init, - domain_msg.ordering_on_b, - Counterparty::new(domain_msg.port_id_on_a, domain_msg.chan_id_on_a), + domain_msg.ordering, + Counterparty::new(domain_msg.port_id_on_a, Some(domain_msg.chan_id_on_a)), domain_msg.connection_hops_on_b, - domain_msg.version_on_b.clone(), + Version::empty(), // Excessive field to satisfy the type conversion ); #[allow(deprecated)] RawMsgChannelOpenTry { port_id: domain_msg.port_id_on_b.to_string(), previous_channel_id: domain_msg.previous_channel_id, channel: Some(chan_end_on_b.into()), - counterparty_version: domain_msg.version_on_a.to_string(), + counterparty_version: domain_msg.version_supported_on_a.to_string(), proof_init: domain_msg.proof_chan_end_on_a.clone().into(), proof_height: Some(domain_msg.proof_height_on_a.into()), signer: domain_msg.signer.to_string(), @@ -155,7 +152,7 @@ pub mod test_util { RawMsgChannelOpenTry { port_id: PortId::default().to_string(), previous_channel_id: ChannelId::default().to_string(), - channel: Some(get_dummy_raw_channel_end()), + channel: Some(get_dummy_raw_channel_end(ChannelId::default().to_string())), counterparty_version: "".to_string(), proof_init: get_dummy_proof(), proof_height: Some(Height { From dc3ff7391c33de3ca29d3c030610b69903ea515e Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 9 Jan 2023 13:43:39 -0500 Subject: [PATCH 4/4] changelog fix --- .../unreleased/breaking-changes/338-make-process-pub-crate.md | 2 ++ .../unreleased/improvements/324-make-msg-structs-pub-crate.md | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 .changelog/unreleased/breaking-changes/338-make-process-pub-crate.md delete mode 100644 .changelog/unreleased/improvements/324-make-msg-structs-pub-crate.md diff --git a/.changelog/unreleased/breaking-changes/338-make-process-pub-crate.md b/.changelog/unreleased/breaking-changes/338-make-process-pub-crate.md new file mode 100644 index 000000000..84250ae37 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/338-make-process-pub-crate.md @@ -0,0 +1,2 @@ +- Make internal `process()` `pub(crate)` + ([#338](https://github.com/cosmos/ibc-rs/issues/338)) diff --git a/.changelog/unreleased/improvements/324-make-msg-structs-pub-crate.md b/.changelog/unreleased/improvements/324-make-msg-structs-pub-crate.md deleted file mode 100644 index ff276ed61..000000000 --- a/.changelog/unreleased/improvements/324-make-msg-structs-pub-crate.md +++ /dev/null @@ -1,2 +0,0 @@ -- Make Msg* structs pub(crate) ([#324](https://github.com/cosmos/ibc- - rs/issues/324)) \ No newline at end of file