From 720aa16614727621eeb7908d7c504c5759896835 Mon Sep 17 00:00:00 2001 From: Davirain Date: Tue, 6 Dec 2022 13:03:29 +0800 Subject: [PATCH 1/3] Remove ConnectionMsg variant connection{Try, Ack} Box Wrap --- crates/ibc/src/core/ics03_connection/handler.rs | 4 ++-- .../core/ics03_connection/handler/conn_open_ack.rs | 6 +++--- .../core/ics03_connection/handler/conn_open_try.rs | 12 ++++++------ crates/ibc/src/core/ics03_connection/msgs.rs | 5 ++--- crates/ibc/src/core/ics26_routing/handler.rs | 14 ++++++-------- crates/ibc/src/core/ics26_routing/msgs.rs | 4 ++-- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/crates/ibc/src/core/ics03_connection/handler.rs b/crates/ibc/src/core/ics03_connection/handler.rs index bcf627f62..75afe20fd 100644 --- a/crates/ibc/src/core/ics03_connection/handler.rs +++ b/crates/ibc/src/core/ics03_connection/handler.rs @@ -49,8 +49,8 @@ where { match msg { ConnectionMsg::ConnectionOpenInit(msg) => conn_open_init::process(ctx, msg), - ConnectionMsg::ConnectionOpenTry(msg) => conn_open_try::process(ctx, *msg), - ConnectionMsg::ConnectionOpenAck(msg) => conn_open_ack::process(ctx, *msg), + ConnectionMsg::ConnectionOpenTry(msg) => conn_open_try::process(ctx, msg), + ConnectionMsg::ConnectionOpenAck(msg) => conn_open_ack::process(ctx, msg), ConnectionMsg::ConnectionOpenConfirm(msg) => conn_open_confirm::process(ctx, msg), } } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs index 8004b3ba8..a121b9706 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs @@ -211,7 +211,7 @@ mod tests { .clone() .with_client(&client_id, proof_height) .with_connection(conn_id.clone(), default_conn_end), - msg: ConnectionMsg::ConnectionOpenAck(Box::new(msg_ack.clone())), + msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()), want_pass: true, match_error: Box::new(|_| panic!("should not have error")), }, @@ -219,7 +219,7 @@ mod tests { name: "Processing fails because the connection does not exist in the context" .to_string(), ctx: default_context.clone(), - msg: ConnectionMsg::ConnectionOpenAck(Box::new(msg_ack.clone())), + msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()), want_pass: false, match_error: { let right_connection_id = conn_id.clone(); @@ -239,7 +239,7 @@ mod tests { ctx: default_context .with_client(&client_id, proof_height) .with_connection(conn_id.clone(), conn_end_open), - msg: ConnectionMsg::ConnectionOpenAck(Box::new(msg_ack)), + msg: ConnectionMsg::ConnectionOpenAck(msg_ack), want_pass: false, match_error: { let right_connection_id = conn_id; diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs index 96dfe5956..ba7396b3b 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs @@ -201,37 +201,37 @@ mod tests { Test { name: "Processing fails because the height is too advanced".to_string(), ctx: context.clone(), - msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_height_advanced)), + msg: ConnectionMsg::ConnectionOpenTry(msg_height_advanced), want_pass: false, }, Test { name: "Processing fails because the height is too old".to_string(), ctx: context.clone(), - msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_height_old)), + msg: ConnectionMsg::ConnectionOpenTry(msg_height_old), want_pass: false, }, Test { name: "Processing fails because no client exists".to_string(), ctx: context.clone(), - msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_conn_try.clone())), + msg: ConnectionMsg::ConnectionOpenTry(msg_conn_try.clone()), want_pass: false, }, Test { name: "Processing fails because the client misses the consensus state targeted by the proof".to_string(), ctx: context.clone().with_client(&msg_proof_height_missing.client_id_on_b, Height::new(0, client_consensus_state_height).unwrap()), - msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_proof_height_missing)), + msg: ConnectionMsg::ConnectionOpenTry(msg_proof_height_missing), want_pass: false, }, Test { name: "Good parameters (no previous_connection_id)".to_string(), ctx: context.clone().with_client(&msg_conn_try.client_id_on_b, Height::new(0, client_consensus_state_height).unwrap()), - msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_conn_try.clone())), + msg: ConnectionMsg::ConnectionOpenTry(msg_conn_try.clone()), want_pass: true, }, Test { name: "Good parameters".to_string(), ctx: context.with_client(&msg_conn_try.client_id_on_b, Height::new(0, client_consensus_state_height).unwrap()), - msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_conn_try)), + msg: ConnectionMsg::ConnectionOpenTry(msg_conn_try), want_pass: true, }, ] diff --git a/crates/ibc/src/core/ics03_connection/msgs.rs b/crates/ibc/src/core/ics03_connection/msgs.rs index ab2cbec66..6f1b00162 100644 --- a/crates/ibc/src/core/ics03_connection/msgs.rs +++ b/crates/ibc/src/core/ics03_connection/msgs.rs @@ -16,7 +16,6 @@ use crate::core::ics03_connection::msgs::conn_open_ack::MsgConnectionOpenAck; use crate::core::ics03_connection::msgs::conn_open_confirm::MsgConnectionOpenConfirm; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; use crate::core::ics03_connection::msgs::conn_open_try::MsgConnectionOpenTry; -use alloc::boxed::Box; pub mod conn_open_ack; pub mod conn_open_confirm; @@ -27,8 +26,8 @@ pub mod conn_open_try; #[derive(Clone, Debug, PartialEq, Eq)] pub enum ConnectionMsg { ConnectionOpenInit(MsgConnectionOpenInit), - ConnectionOpenTry(Box), - ConnectionOpenAck(Box), + ConnectionOpenTry(MsgConnectionOpenTry), + ConnectionOpenAck(MsgConnectionOpenAck), ConnectionOpenConfirm(MsgConnectionOpenConfirm), } diff --git a/crates/ibc/src/core/ics26_routing/handler.rs b/crates/ibc/src/core/ics26_routing/handler.rs index 141ba46e1..7764f927c 100644 --- a/crates/ibc/src/core/ics26_routing/handler.rs +++ b/crates/ibc/src/core/ics26_routing/handler.rs @@ -436,28 +436,26 @@ mod tests { Test { name: "Connection open try fails due to InvalidConsensusHeight (too high)" .to_string(), - msg: MsgEnvelope::ConnectionMsg(ConnectionMsg::ConnectionOpenTry(Box::new( + msg: MsgEnvelope::ConnectionMsg(ConnectionMsg::ConnectionOpenTry( incorrect_msg_conn_try, - ))) + )) .into(), want_pass: false, state_check: None, }, Test { name: "Connection open try succeeds".to_string(), - msg: MsgEnvelope::ConnectionMsg(ConnectionMsg::ConnectionOpenTry(Box::new( + msg: MsgEnvelope::ConnectionMsg(ConnectionMsg::ConnectionOpenTry( correct_msg_conn_try.with_client_id(client_id.clone()), - ))) + )) .into(), want_pass: true, state_check: None, }, Test { name: "Connection open ack succeeds".to_string(), - msg: MsgEnvelope::ConnectionMsg(ConnectionMsg::ConnectionOpenAck(Box::new( - msg_conn_ack, - ))) - .into(), + msg: MsgEnvelope::ConnectionMsg(ConnectionMsg::ConnectionOpenAck(msg_conn_ack)) + .into(), want_pass: true, state_check: None, }, diff --git a/crates/ibc/src/core/ics26_routing/msgs.rs b/crates/ibc/src/core/ics26_routing/msgs.rs index 7d0142dcb..99ecd5929 100644 --- a/crates/ibc/src/core/ics26_routing/msgs.rs +++ b/crates/ibc/src/core/ics26_routing/msgs.rs @@ -57,14 +57,14 @@ impl TryFrom for MsgEnvelope { let domain_msg = conn_open_try::MsgConnectionOpenTry::decode_vec(&any_msg.value) .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::ConnectionMsg( - ConnectionMsg::ConnectionOpenTry(Box::new(domain_msg)), + ConnectionMsg::ConnectionOpenTry(domain_msg), )) } conn_open_ack::TYPE_URL => { let domain_msg = conn_open_ack::MsgConnectionOpenAck::decode_vec(&any_msg.value) .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::ConnectionMsg( - ConnectionMsg::ConnectionOpenAck(Box::new(domain_msg)), + ConnectionMsg::ConnectionOpenAck(domain_msg), )) } conn_open_confirm::TYPE_URL => { From 1be2cafa71eeae4931f9e9b0c6dccd5e3a494ba0 Mon Sep 17 00:00:00 2001 From: Davirain Date: Tue, 6 Dec 2022 13:07:08 +0800 Subject: [PATCH 2/3] Create 258-remove-connectionmsg-variant-box-wrap.md --- .../enhancement/258-remove-connectionmsg-variant-box-wrap.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/enhancement/258-remove-connectionmsg-variant-box-wrap.md diff --git a/.changelog/unreleased/enhancement/258-remove-connectionmsg-variant-box-wrap.md b/.changelog/unreleased/enhancement/258-remove-connectionmsg-variant-box-wrap.md new file mode 100644 index 000000000..514257235 --- /dev/null +++ b/.changelog/unreleased/enhancement/258-remove-connectionmsg-variant-box-wrap.md @@ -0,0 +1,2 @@ +- ConnectionMsg::ConnectionOpen{Try, Ack} should not wrap a Box + ([#258](https://github.com/cosmos/ibc-rs/issues/258)) \ No newline at end of file From fbcd63a9633ba612847978d39ea57a3918232fc2 Mon Sep 17 00:00:00 2001 From: Davirain Date: Wed, 7 Dec 2022 11:16:38 +0800 Subject: [PATCH 3/3] Update context.rs --- crates/ibc/src/core/context.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 204afe5e9..fda25efc9 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -108,12 +108,8 @@ pub trait ValidationContext { ConnectionMsg::ConnectionOpenInit(message) => { conn_open_init::validate(self, message) } - ConnectionMsg::ConnectionOpenTry(message) => { - conn_open_try::validate(self, *message) - } - ConnectionMsg::ConnectionOpenAck(message) => { - conn_open_ack::validate(self, *message) - } + ConnectionMsg::ConnectionOpenTry(message) => conn_open_try::validate(self, message), + ConnectionMsg::ConnectionOpenAck(message) => conn_open_ack::validate(self, message), ConnectionMsg::ConnectionOpenConfirm(ref message) => { conn_open_confirm::validate(self, message) } @@ -325,8 +321,8 @@ pub trait ExecutionContext: ValidationContext { ConnectionMsg::ConnectionOpenInit(message) => { conn_open_init::execute(self, message) } - ConnectionMsg::ConnectionOpenTry(message) => conn_open_try::execute(self, *message), - ConnectionMsg::ConnectionOpenAck(message) => conn_open_ack::execute(self, *message), + ConnectionMsg::ConnectionOpenTry(message) => conn_open_try::execute(self, message), + ConnectionMsg::ConnectionOpenAck(message) => conn_open_ack::execute(self, message), ConnectionMsg::ConnectionOpenConfirm(ref message) => { conn_open_confirm::execute(self, message) }