From d5b41bc2ce0753fd8895c5fb4764a22ef5f8a274 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 16 Feb 2023 19:12:47 -0800 Subject: [PATCH 1/2] Refactor connection handler unit tests --- .../440-refactor-conn-unit-tests.md | 2 + crates/ibc/src/core/handler.rs | 27 +- .../ibc/src/core/ics03_connection/handler.rs | 32 ++ .../ics03_connection/handler/conn_open_ack.rs | 306 ++++++++---------- .../handler/conn_open_confirm.rs | 187 +++++------ .../handler/conn_open_init.rs | 262 +++++++-------- .../ics03_connection/handler/conn_open_try.rs | 268 +++++++-------- .../ics03_connection/msgs/conn_open_ack.rs | 14 + .../msgs/conn_open_confirm.rs | 9 + .../ics03_connection/msgs/conn_open_init.rs | 46 ++- .../ics03_connection/msgs/conn_open_try.rs | 8 + .../ics04_channel/handler/chan_open_ack.rs | 10 +- .../ics04_channel/handler/chan_open_init.rs | 4 +- 13 files changed, 557 insertions(+), 618 deletions(-) create mode 100644 .changelog/unreleased/improvements/440-refactor-conn-unit-tests.md diff --git a/.changelog/unreleased/improvements/440-refactor-conn-unit-tests.md b/.changelog/unreleased/improvements/440-refactor-conn-unit-tests.md new file mode 100644 index 000000000..96867ca29 --- /dev/null +++ b/.changelog/unreleased/improvements/440-refactor-conn-unit-tests.md @@ -0,0 +1,2 @@ +- Refactor connection handler unit tests to adapt with new Validation/Execution API +([#440](https://github.com/cosmos/ibc-rs/issues/440)). \ No newline at end of file diff --git a/crates/ibc/src/core/handler.rs b/crates/ibc/src/core/handler.rs index 325263698..42bda3b9e 100644 --- a/crates/ibc/src/core/handler.rs +++ b/crates/ibc/src/core/handler.rs @@ -50,10 +50,8 @@ mod tests { ConnectionEnd, Counterparty as ConnCounterparty, State as ConnState, }; use crate::core::ics03_connection::msgs::{ - conn_open_ack::{test_util::get_dummy_raw_msg_conn_open_ack, MsgConnectionOpenAck}, - conn_open_init::{test_util::get_dummy_raw_msg_conn_open_init, MsgConnectionOpenInit}, - conn_open_try::{test_util::get_dummy_raw_msg_conn_open_try, MsgConnectionOpenTry}, - ConnectionMsg, + conn_open_ack::MsgConnectionOpenAck, conn_open_init::MsgConnectionOpenInit, + conn_open_try::MsgConnectionOpenTry, ConnectionMsg, }; use crate::core::ics03_connection::version::Version as ConnVersion; use crate::core::ics04_channel::channel::ChannelEnd; @@ -162,26 +160,15 @@ mod tests { // // Connection handshake messages. // - let msg_conn_init = - MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init(None)).unwrap(); + let msg_conn_init = MsgConnectionOpenInit::new_dummy(); - let correct_msg_conn_try = MsgConnectionOpenTry::try_from(get_dummy_raw_msg_conn_open_try( - client_height, - client_height, - )) - .unwrap(); + let correct_msg_conn_try = MsgConnectionOpenTry::new_dummy(client_height, client_height); // The handler will fail to process this msg because the client height is too advanced. - let incorrect_msg_conn_try = MsgConnectionOpenTry::try_from( - get_dummy_raw_msg_conn_open_try(client_height + 1, client_height + 1), - ) - .unwrap(); + let incorrect_msg_conn_try = + MsgConnectionOpenTry::new_dummy(client_height + 1, client_height + 1); - let msg_conn_ack = MsgConnectionOpenAck::try_from(get_dummy_raw_msg_conn_open_ack( - client_height, - client_height, - )) - .unwrap(); + let msg_conn_ack = MsgConnectionOpenAck::new_dummy(client_height, client_height); // // Channel handshake messages. diff --git a/crates/ibc/src/core/ics03_connection/handler.rs b/crates/ibc/src/core/ics03_connection/handler.rs index 30d233a07..75cb70e15 100644 --- a/crates/ibc/src/core/ics03_connection/handler.rs +++ b/crates/ibc/src/core/ics03_connection/handler.rs @@ -54,3 +54,35 @@ where ConnectionMsg::OpenConfirm(msg) => conn_open_confirm::process(ctx, msg), } } + +#[cfg(test)] +pub mod test_util { + use core::fmt::Debug; + + use crate::{core::ContextError, mock::context::MockContext, prelude::String}; + use alloc::format; + + pub enum Expect { + Success, + Failure(Option), + } + + #[derive(Clone, Debug)] + pub struct Fixture { + pub ctx: MockContext, + pub msg: M, + } + + pub fn generate_error_msg( + expect: &Expect, + process: &str, + res: &Result<(), ContextError>, + fxt: &Fixture, + ) -> String { + let msg = match expect { + Expect::Success => "step failed!", + Expect::Failure(_) => "step passed but was supposed to fail!", + }; + format!("{process} {msg} /n {res:?} /n {fxt:?}") + } +} 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 f333a2f61..a293bd5d4 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 @@ -342,58 +342,43 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use crate::prelude::*; + use super::*; use core::str::FromStr; use test_log::test; + use crate::core::ics02_client::height::Height; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; - use crate::core::ics03_connection::error; - use crate::core::ics03_connection::handler::{dispatch, ConnectionResult}; - use crate::core::ics03_connection::msgs::conn_open_ack::test_util::get_dummy_raw_msg_conn_open_ack; + use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_ack::MsgConnectionOpenAck; - use crate::core::ics03_connection::msgs::ConnectionMsg; use crate::core::ics23_commitment::commitment::CommitmentPrefix; use crate::core::ics24_host::identifier::{ChainId, ClientId}; + use crate::core::ValidationContext; + use crate::events::IbcEvent; use crate::mock::context::MockContext; use crate::mock::host::HostType; use crate::timestamp::ZERO_DURATION; - use crate::core::ics26_routing::msgs::MsgEnvelope; - - use crate::core::ValidationContext; - - #[test] - fn conn_open_ack_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ConnectionMsg, - want_pass: bool, - match_error: Box, - } + enum Ctx { + New, + NewWithConnection, + NewWithConnectionEndOpen, + DefaultWithConnection, + } - let msg_ack = - MsgConnectionOpenAck::try_from(get_dummy_raw_msg_conn_open_ack(10, 10)).unwrap(); - let conn_id = msg_ack.conn_id_on_a.clone(); - let counterparty_conn_id = msg_ack.conn_id_on_b.clone(); + fn conn_open_ack_fixture(ctx: Ctx) -> Fixture { + let msg = MsgConnectionOpenAck::new_dummy(10, 10); // Client parameters -- identifier and correct height (matching the proof height) let client_id = ClientId::from_str("mock_clientid").unwrap(); - let proof_height = msg_ack.proofs_height_on_b; - let consensus_state_height = msg_ack.consensus_height_of_a_on_b; + let proof_height = msg.proofs_height_on_b; + let conn_id = msg.conn_id_on_a.clone(); // Parametrize the host chain to have a height at least as recent as the // the height of the proofs in the Ack msg. let latest_height = proof_height.increment(); let max_history_size = 5; - let default_context = MockContext::new( - ChainId::new("mockgaia".to_string(), latest_height.revision_number()), - HostType::Mock, - max_history_size, - latest_height, - ); // A connection end that will exercise the successful path. let default_conn_end = ConnectionEnd::new( @@ -401,10 +386,10 @@ mod tests { client_id.clone(), Counterparty::new( client_id.clone(), - Some(msg_ack.conn_id_on_b.clone()), + Some(msg.conn_id_on_b.clone()), CommitmentPrefix::try_from(b"ibc".to_vec()).unwrap(), ), - vec![msg_ack.version.clone()], + vec![msg.version.clone()], ZERO_DURATION, ); @@ -412,149 +397,128 @@ mod tests { let mut conn_end_open = default_conn_end.clone(); conn_end_open.set_state(State::Open); // incorrect field - let tests: Vec = vec![ - Test { - name: "Successful processing of an Ack message".to_string(), - ctx: default_context - .clone() - .with_client(&client_id, proof_height) - .with_connection(conn_id.clone(), default_conn_end.clone()), - msg: ConnectionMsg::OpenAck(msg_ack.clone()), - want_pass: true, - match_error: Box::new(|_| panic!("should not have error")), - }, - Test { - name: "Processing fails because the connection does not exist in the context" - .to_string(), - ctx: default_context.clone(), - msg: ConnectionMsg::OpenAck(msg_ack.clone()), - want_pass: false, - match_error: { - let right_connection_id = conn_id.clone(); - Box::new(move |e| match e { - error::ConnectionError::ConnectionNotFound { connection_id } => { - assert_eq!(connection_id, right_connection_id) - } - _ => { - panic!("Expected ConnectionNotFound error"); - } - }) - }, - }, - Test { - name: "Processing fails due to InvalidConsensusHeight".to_string(), - ctx: MockContext::default() - .with_client(&client_id, proof_height) - .with_connection(conn_id.clone(), default_conn_end), - msg: ConnectionMsg::OpenAck(msg_ack.clone()), - want_pass: false, - match_error: { - Box::new(move |e| match e { - error::ConnectionError::InvalidConsensusHeight { - target_height, - current_height: _, - } => { - assert_eq!(consensus_state_height, target_height); - } - _ => { - panic!("Expected InvalidConsensusHeight error"); - } - }) - }, - }, - Test { - name: "Processing fails due to connections mismatch (incorrect 'open' state)" - .to_string(), - ctx: default_context - .with_client(&client_id, proof_height) - .with_connection(conn_id.clone(), conn_end_open), - msg: ConnectionMsg::OpenAck(msg_ack), - want_pass: false, - match_error: { - let right_connection_id = conn_id; - Box::new(move |e| match e { - error::ConnectionError::ConnectionMismatch { connection_id } => { - assert_eq!(connection_id, right_connection_id); - } - _ => { - panic!("Expected ConnectionMismatch error"); - } - }) - }, - }, - ]; - - for test in tests { - { - let res = ValidationContext::validate( - &test.ctx, - MsgEnvelope::Connection(test.msg.clone()), + let ctx_default = MockContext::default(); + let ctx_new = MockContext::new( + ChainId::new("mockgaia".to_string(), latest_height.revision_number()), + HostType::Mock, + max_history_size, + latest_height, + ); + let ctx = match ctx { + Ctx::New => ctx_new, + Ctx::NewWithConnection => ctx_new + .with_client(&client_id, proof_height) + .with_connection(conn_id, default_conn_end), + Ctx::DefaultWithConnection => ctx_default + .with_client(&client_id, proof_height) + .with_connection(conn_id, default_conn_end), + Ctx::NewWithConnectionEndOpen => ctx_new + .with_client(&client_id, proof_height) + .with_connection(conn_id, conn_end_open), + }; + + Fixture { ctx, msg } + } + + fn conn_open_ack_validate(fxt: &Fixture, expect: Expect) { + let res = validate(&fxt.ctx, fxt.msg.clone()); + let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + match expect { + Expect::Failure(err) => { + assert!(res.is_err(), "{err_msg}"); + assert_eq!( + core::mem::discriminant(res.as_ref().unwrap_err()), + core::mem::discriminant(&err.unwrap()) ); + } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}"); + return; + } + }; + let right_connection_id = fxt.msg.conn_id_on_a.clone(); + let cons_state_height = fxt.msg.consensus_height_of_a_on_b; + match res.unwrap_err() { + ContextError::ConnectionError(ConnectionError::ConnectionNotFound { + connection_id, + }) => { + assert_eq!(connection_id, right_connection_id) + } + ContextError::ConnectionError(ConnectionError::InvalidConsensusHeight { + target_height, + current_height: _, + }) => { + assert_eq!(cons_state_height, target_height); + } + ContextError::ConnectionError(ConnectionError::ConnectionMismatch { + connection_id, + }) => { + assert_eq!(connection_id, right_connection_id) + } + _ => unreachable!(), + } + } - match res { - Ok(_) => { - assert!( - test.want_pass, - "conn_open_ack: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ) - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_ack: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } - } + fn conn_open_ack_execute(fxt: &mut Fixture, expect: Expect) { + let res = execute(&mut fxt.ctx, fxt.msg.clone()); + let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + match expect { + Expect::Failure(_) => { + assert!(res.is_err(), "{err_msg}"); } - let res = dispatch(&test.ctx, test.msg.clone()); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "conn_open_ack: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ); - - assert!(!proto_output.events.is_empty()); // Some events must exist. - - // The object in the output is a ConnectionEnd, should have OPEN state. - let res: ConnectionResult = proto_output.result; - assert_eq!(res.connection_end.state().clone(), State::Open); - - // assert that counterparty connection id is correct - assert_eq!( - res.connection_end.counterparty().connection_id, - Some(counterparty_conn_id.clone()) - ); - - for e in proto_output.events.iter() { - assert!(matches!(e, &IbcEvent::OpenAckConnection(_))); - } - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_ack: failed for test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - - // Verify that the error kind matches - (test.match_error)(e); - } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}"); + assert_eq!(fxt.ctx.events.len(), 1); + + let event = fxt.ctx.events.first().unwrap(); + assert!(matches!(event, &IbcEvent::OpenAckConnection(_))); + + let conn_open_try_event = match event { + IbcEvent::OpenAckConnection(e) => e, + _ => unreachable!(), + }; + let conn_end = ::connection_end( + &fxt.ctx, + conn_open_try_event.connection_id(), + ) + .unwrap(); + assert_eq!(conn_end.state().clone(), State::Open); } } } + + #[test] + fn conn_open_ack_healthy() { + let mut fxt = conn_open_ack_fixture(Ctx::NewWithConnection); + conn_open_ack_validate(&fxt, Expect::Success); + conn_open_ack_execute(&mut fxt, Expect::Success); + } + + #[test] + fn conn_open_ack_no_connection() { + let fxt = conn_open_ack_fixture(Ctx::New); + let expected_err = ContextError::ConnectionError(ConnectionError::ConnectionNotFound { + connection_id: fxt.msg.conn_id_on_a.clone(), + }); + conn_open_ack_validate(&fxt, Expect::Failure(Some(expected_err))); + } + + #[test] + fn conn_open_ack_invalid_consensus_height() { + let fxt = conn_open_ack_fixture(Ctx::DefaultWithConnection); + let expected_err = ContextError::ConnectionError(ConnectionError::InvalidConsensusHeight { + target_height: fxt.msg.consensus_height_of_a_on_b, + current_height: Height::new(0, 10).unwrap(), + }); + conn_open_ack_validate(&fxt, Expect::Failure(Some(expected_err))); + } + + #[test] + fn conn_open_ack_connection_mismatch() { + let fxt = conn_open_ack_fixture(Ctx::NewWithConnectionEndOpen); + let expected_err = ContextError::ConnectionError(ConnectionError::ConnectionMismatch { + connection_id: fxt.msg.conn_id_on_a.clone(), + }); + conn_open_ack_validate(&fxt, Expect::Failure(Some(expected_err))); + } } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs index 486b0efbd..c5a9ad317 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs @@ -255,17 +255,14 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use crate::prelude::*; + use super::*; use core::str::FromStr; use test_log::test; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; - use crate::core::ics03_connection::context::ConnectionReader; - use crate::core::ics03_connection::handler::{dispatch, ConnectionResult}; - use crate::core::ics03_connection::msgs::conn_open_confirm::test_util::get_dummy_raw_msg_conn_open_confirm; + use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_confirm::MsgConnectionOpenConfirm; - use crate::core::ics03_connection::msgs::ConnectionMsg; use crate::core::ics23_commitment::commitment::CommitmentPrefix; use crate::core::ics24_host::identifier::ClientId; use crate::events::IbcEvent; @@ -273,132 +270,106 @@ mod tests { use crate::timestamp::ZERO_DURATION; use crate::Height; - use crate::core::ics26_routing::msgs::MsgEnvelope; - use crate::core::ValidationContext; - #[test] - fn conn_open_confirm_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ConnectionMsg, - want_pass: bool, - } + enum Ctx { + Default, + CorrectConnection, + IncorrectConnection, + } + fn conn_open_confirm_fixture(ctx: Ctx) -> Fixture { let client_id = ClientId::from_str("mock_clientid").unwrap(); - let msg_confirm = - MsgConnectionOpenConfirm::try_from(get_dummy_raw_msg_conn_open_confirm()).unwrap(); + let msg = MsgConnectionOpenConfirm::new_dummy(); let counterparty = Counterparty::new( client_id.clone(), - Some(msg_confirm.conn_id_on_b.clone()), + Some(msg.conn_id_on_b.clone()), CommitmentPrefix::try_from(b"ibc".to_vec()).unwrap(), ); - let context = MockContext::default(); + let ctx_default = MockContext::default(); let incorrect_conn_end_state = ConnectionEnd::new( State::Init, client_id.clone(), counterparty, - ConnectionReader::get_compatible_versions(&context), + ValidationContext::get_compatible_versions(&ctx_default), ZERO_DURATION, ); let mut correct_conn_end = incorrect_conn_end_state.clone(); correct_conn_end.set_state(State::TryOpen); - let tests: Vec = vec![ - Test { - name: "Processing fails due to missing connection in context".to_string(), - ctx: context.clone(), - msg: ConnectionMsg::OpenConfirm(msg_confirm.clone()), - want_pass: false, - }, - Test { - name: "Processing fails due to connections mismatch (incorrect state)".to_string(), - ctx: context - .clone() - .with_client(&client_id, Height::new(0, 10).unwrap()) - .with_connection(msg_confirm.conn_id_on_b.clone(), incorrect_conn_end_state), - msg: ConnectionMsg::OpenConfirm(msg_confirm.clone()), - want_pass: false, - }, - Test { - name: "Processing successful".to_string(), - ctx: context - .with_client(&client_id, Height::new(0, 10).unwrap()) - .with_connection(msg_confirm.conn_id_on_b.clone(), correct_conn_end), - msg: ConnectionMsg::OpenConfirm(msg_confirm), - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - { - let res = ValidationContext::validate( - &test.ctx, - MsgEnvelope::Connection(test.msg.clone()), - ); - - match res { - Ok(_) => { - assert!( - test.want_pass, - "conn_open_confirm: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ) - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_confirm: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } - } + let ctx = match ctx { + Ctx::Default => ctx_default, + Ctx::IncorrectConnection => ctx_default + .with_client(&client_id, Height::new(0, 10).unwrap()) + .with_connection(msg.conn_id_on_b.clone(), incorrect_conn_end_state), + Ctx::CorrectConnection => ctx_default + .with_client(&client_id, Height::new(0, 10).unwrap()) + .with_connection(msg.conn_id_on_b.clone(), correct_conn_end), + }; + + Fixture { ctx, msg } + } + + fn conn_open_confirm_validate(fxt: &Fixture, expect: Expect) { + let res = validate(&fxt.ctx, &fxt.msg); + let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + match expect { + Expect::Failure(_) => { + assert!(res.is_err(), "{err_msg}"); + } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}"); } + }; + } - let res = dispatch(&test.ctx, test.msg.clone()); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "conn_open_confirm: test passed but was supposed to fail for: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ); - - assert!(!proto_output.events.is_empty()); // Some events must exist. - - // The object in the output is a ConnectionEnd, should have OPEN state. - let res: ConnectionResult = proto_output.result; - assert_eq!(res.connection_end.state().clone(), State::Open); - - for e in proto_output.events.iter() { - assert!(matches!(e, &IbcEvent::OpenConfirmConnection(_))); - } - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_confirm: failed for test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } + fn conn_open_confirm_execute(fxt: &mut Fixture, expect: Expect) { + let res = execute(&mut fxt.ctx, &fxt.msg); + let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + match expect { + Expect::Failure(_) => { + assert!(res.is_err(), "{err_msg}"); + } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}"); + assert_eq!(fxt.ctx.events.len(), 1); + + let event = fxt.ctx.events.first().unwrap(); + assert!(matches!(event, &IbcEvent::OpenConfirmConnection(_))); + + let conn_open_try_event = match event { + IbcEvent::OpenConfirmConnection(e) => e, + _ => unreachable!(), + }; + let conn_end = ::connection_end( + &fxt.ctx, + conn_open_try_event.connection_id(), + ) + .unwrap(); + assert_eq!(conn_end.state().clone(), State::Open); } } } + + #[test] + fn conn_open_confirm_healthy() { + let mut fxt = conn_open_confirm_fixture(Ctx::CorrectConnection); + conn_open_confirm_validate(&fxt, Expect::Success); + conn_open_confirm_execute(&mut fxt, Expect::Success); + } + + #[test] + fn conn_open_confirm_no_connection() { + let fxt = conn_open_confirm_fixture(Ctx::Default); + conn_open_confirm_validate(&fxt, Expect::Failure(None)); + } + + #[test] + fn conn_open_confirm_connection_mismatch() { + let fxt = conn_open_confirm_fixture(Ctx::IncorrectConnection); + conn_open_confirm_validate(&fxt, Expect::Failure(None)); + } } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs index 0810db3ec..835d37b53 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs @@ -1,6 +1,7 @@ //! Protocol logic specific to ICS3 messages of type `MsgConnectionOpenInit`. use crate::prelude::*; +use crate::core::context::ContextError; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; use crate::core::ics03_connection::context::ConnectionReader; use crate::core::ics03_connection::error::ConnectionError; @@ -8,14 +9,10 @@ use crate::core::ics03_connection::events::OpenInit; use crate::core::ics03_connection::handler::ConnectionResult; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; use crate::core::ics24_host::identifier::ConnectionId; -use crate::events::IbcEvent; -use crate::handler::{HandlerOutput, HandlerResult}; - -use crate::core::context::ContextError; - use crate::core::ics24_host::path::{ClientConnectionPath, ConnectionPath}; - use crate::core::{ExecutionContext, ValidationContext}; +use crate::events::IbcEvent; +use crate::handler::{HandlerOutput, HandlerResult}; use super::ConnectionIdState; @@ -150,165 +147,130 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use crate::prelude::*; - - use test_log::test; + use super::*; use crate::core::ics03_connection::connection::State; - use crate::core::ics03_connection::context::ConnectionReader; - use crate::core::ics03_connection::handler::{dispatch, ConnectionResult}; - use crate::core::ics03_connection::msgs::conn_open_init::test_util::get_dummy_raw_msg_conn_open_init; + use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; - use crate::core::ics03_connection::msgs::ConnectionMsg; use crate::core::ics03_connection::version::Version; use crate::events::IbcEvent; use crate::mock::context::MockContext; use crate::Height; + use test_log::test; - use crate::core::ics26_routing::msgs::MsgEnvelope; + enum Ctx { + Default, + WithClient, + } + + enum Msg { + Default, + NoVersion, + BadVersion, + WithCounterpartyConnId, + } - use crate::core::ValidationContext; + fn conn_open_init_fixture( + ctx_variant: Ctx, + msg_variant: Msg, + ) -> Fixture { + let msg_default = MsgConnectionOpenInit::new_dummy(); + let msg = match msg_variant { + Msg::Default => msg_default, + Msg::NoVersion => msg_default.with_version(None), + Msg::BadVersion => msg_default.with_version(Some("random identifier 424242")), + Msg::WithCounterpartyConnId => msg_default.with_counterparty_conn_id(2), + }; - use ibc_proto::ibc::core::connection::v1::Version as RawVersion; + let ctx_default = MockContext::default(); + let ctx = match ctx_variant { + Ctx::WithClient => { + ctx_default.with_client(&msg.client_id_on_a, Height::new(0, 10).unwrap()) + } + _ => ctx_default, + }; - #[test] - fn conn_open_init_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ConnectionMsg, - expected_versions: Vec, - want_pass: bool, + Fixture { ctx, msg } + } + + fn conn_open_init_validate(fxt: &Fixture, expect: Expect) { + let res = validate(&fxt.ctx, fxt.msg.clone()); + let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + match expect { + Expect::Failure(_) => { + assert!(res.is_err(), "{err_msg}") + } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}") + } } + } - let msg_conn_init_default = - MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init(None)).unwrap(); - let msg_conn_init_no_version = MsgConnectionOpenInit { - version: None, - ..msg_conn_init_default.clone() - }; - let msg_conn_init_bad_version = MsgConnectionOpenInit { - version: Version::try_from(RawVersion { - identifier: "random identifier 424242".to_string(), - features: vec![], - }) - .unwrap() - .into(), - ..msg_conn_init_default.clone() - }; - let msg_conn_init_with_couterparty_conn_id_some = - MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init(Some(0))).unwrap(); - let default_context = MockContext::default(); - let good_context = default_context.clone().with_client( - &msg_conn_init_default.client_id_on_a, - Height::new(0, 10).unwrap(), - ); - - let tests: Vec = vec![ - Test { - name: "Processing fails because no client exists in the context".to_string(), - ctx: default_context, - msg: ConnectionMsg::OpenInit(msg_conn_init_default.clone()), - expected_versions: vec![msg_conn_init_default.version.clone().unwrap()], - want_pass: false, - }, - Test { - name: "Incompatible version in MsgConnectionOpenInit msg".to_string(), - ctx: good_context.clone(), - msg: ConnectionMsg::OpenInit(msg_conn_init_bad_version), - expected_versions: vec![], - want_pass: false, - }, - Test { - name: "No version in MsgConnectionOpenInit msg".to_string(), - ctx: good_context.clone(), - msg: ConnectionMsg::OpenInit(msg_conn_init_no_version), - expected_versions: ConnectionReader::get_compatible_versions(&good_context), - want_pass: true, - }, - Test { - name: "Counterparty connection id is some in MsgConnectionOpenInit msg".to_string(), - ctx: good_context.clone(), - msg: ConnectionMsg::OpenInit(msg_conn_init_with_couterparty_conn_id_some), - expected_versions: ConnectionReader::get_compatible_versions(&good_context), - want_pass: true, - }, - Test { - name: "Good parameters".to_string(), - ctx: good_context, - msg: ConnectionMsg::OpenInit(msg_conn_init_default.clone()), - expected_versions: vec![msg_conn_init_default.version.unwrap()], - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - { - let res = ValidationContext::validate( - &test.ctx, - MsgEnvelope::Connection(test.msg.clone()), - ); - - match res { - Ok(_) => { - assert!( - test.want_pass, - "conn_open_init: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ) - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_init: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } - } + fn conn_open_init_execute( + fxt: &mut Fixture, + expect: Expect, + expected_version: Vec, + ) { + let res = execute(&mut fxt.ctx, fxt.msg.clone()); + let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + match expect { + Expect::Failure(_) => { + assert!(res.is_err(), "{err_msg}") } - let res = dispatch(&test.ctx, test.msg.clone()); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!(!proto_output.events.is_empty()); // Some events must exist. - - // The object in the output is a ConnectionEnd, should have init state. - let res: ConnectionResult = proto_output.result; - assert_eq!(res.connection_end.state().clone(), State::Init); - - for e in proto_output.events.iter() { - assert!(matches!(e, &IbcEvent::OpenInitConnection(_))); - } - - assert_eq!(res.connection_end.versions(), test.expected_versions); - - // This needs to be last - assert!( - test.want_pass, - "conn_open_init: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ); - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_init: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}"); + assert_eq!(fxt.ctx.events.len(), 1); + + let event = fxt.ctx.events.first().unwrap(); + assert!(matches!(event, &IbcEvent::OpenInitConnection(_))); + + let conn_open_init_event = match event { + IbcEvent::OpenInitConnection(e) => e, + _ => unreachable!(), + }; + let conn_end = ::connection_end( + &fxt.ctx, + conn_open_init_event.connection_id(), + ) + .unwrap(); + assert_eq!(conn_end.state().clone(), State::Init); + assert_eq!(conn_end.versions(), expected_version); } } } + + #[test] + fn conn_open_init_healthy() { + let mut fxt = conn_open_init_fixture(Ctx::WithClient, Msg::Default); + conn_open_init_validate(&fxt, Expect::Success); + let expected_version = vec![fxt.msg.version.clone().unwrap()]; + conn_open_init_execute(&mut fxt, Expect::Success, expected_version); + } + + #[test] + fn conn_open_init_no_context() { + let fxt = conn_open_init_fixture(Ctx::Default, Msg::Default); + conn_open_init_validate(&fxt, Expect::Failure(None)); + } + + #[test] + fn conn_open_init_no_version() { + let mut fxt = conn_open_init_fixture(Ctx::WithClient, Msg::NoVersion); + conn_open_init_validate(&fxt, Expect::Success); + let expected_version = ValidationContext::get_compatible_versions(&fxt.ctx.clone()); + conn_open_init_execute(&mut fxt, Expect::Success, expected_version); + } + #[test] + fn conn_open_init_incompatible_version() { + let fxt = conn_open_init_fixture(Ctx::WithClient, Msg::BadVersion); + conn_open_init_validate(&fxt, Expect::Failure(None)); + } + + #[test] + fn conn_open_init_with_counterparty_conn_id() { + let mut fxt = conn_open_init_fixture(Ctx::WithClient, Msg::WithCounterpartyConnId); + conn_open_init_validate(&fxt, Expect::Success); + let expected_version = vec![fxt.msg.version.clone().unwrap()]; + conn_open_init_execute(&mut fxt, Expect::Success, expected_version); + } } 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 f111c19c9..c83b5187c 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 @@ -330,178 +330,144 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use crate::prelude::*; + use super::*; use test_log::test; use crate::core::ics03_connection::connection::State; - use crate::core::ics03_connection::handler::{dispatch, ConnectionResult}; - use crate::core::ics03_connection::msgs::conn_open_try::test_util::get_dummy_raw_msg_conn_open_try; + use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_try::MsgConnectionOpenTry; - use crate::core::ics03_connection::msgs::ConnectionMsg; use crate::core::ics24_host::identifier::ChainId; + use crate::core::ValidationContext; use crate::events::IbcEvent; use crate::mock::context::MockContext; use crate::mock::host::HostType; use crate::Height; - use crate::core::ics26_routing::msgs::MsgEnvelope; - - use crate::core::ValidationContext; + enum Ctx { + Default, + WithClient, + } - #[test] - fn conn_open_try_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ConnectionMsg, - want_pass: bool, - } + enum Msg { + Default, + HeightAdvanced, + HeightOld, + ProofHeightMissing, + } - let host_chain_height = Height::new(0, 35).unwrap(); + fn conn_open_try_fixture(ctx_variant: Ctx, msg_variant: Msg) -> Fixture { let max_history_size = 5; - let context = MockContext::new( - ChainId::new("mockgaia".to_string(), 0), - HostType::Mock, - max_history_size, - host_chain_height, - ); - let client_consensus_state_height = 10; - - let msg_conn_try = MsgConnectionOpenTry::try_from(get_dummy_raw_msg_conn_open_try( - client_consensus_state_height, - host_chain_height.revision_height(), - )) - .unwrap(); - - // The proof targets a height that does not exist (i.e., too advanced) on destination chain. - let msg_height_advanced = MsgConnectionOpenTry::try_from(get_dummy_raw_msg_conn_open_try( - client_consensus_state_height, - host_chain_height.increment().revision_height(), - )) - .unwrap(); + let client_cons_state_height = 10; + let host_chain_height = Height::new(0, 35).unwrap(); let pruned_height = host_chain_height .sub(max_history_size as u64 + 1) .unwrap() .revision_height(); - // The consensus proof targets a missing height (pruned) on destination chain. - let msg_height_old = MsgConnectionOpenTry::try_from(get_dummy_raw_msg_conn_open_try( - client_consensus_state_height, - pruned_height, - )) - .unwrap(); - - // The proofs in this message are created at a height which the client on destination chain does not have. - let msg_proof_height_missing = - MsgConnectionOpenTry::try_from(get_dummy_raw_msg_conn_open_try( - client_consensus_state_height - 1, + + let msg = match msg_variant { + Msg::Default => MsgConnectionOpenTry::new_dummy( + client_cons_state_height, + host_chain_height.revision_height(), + ), + Msg::HeightAdvanced => MsgConnectionOpenTry::new_dummy( + client_cons_state_height, + host_chain_height.increment().revision_height(), + ), + Msg::HeightOld => { + MsgConnectionOpenTry::new_dummy(client_cons_state_height, pruned_height) + } + Msg::ProofHeightMissing => MsgConnectionOpenTry::new_dummy( + client_cons_state_height - 1, host_chain_height.revision_height(), - )) - .unwrap(); - - let tests: Vec = vec![ - Test { - name: "Processing fails because the height is too advanced".to_string(), - ctx: context.clone(), - msg: ConnectionMsg::OpenTry(msg_height_advanced), - want_pass: false, - }, - Test { - name: "Processing fails because the height is too old".to_string(), - ctx: context.clone(), - msg: ConnectionMsg::OpenTry(msg_height_old), - want_pass: false, - }, - Test { - name: "Processing fails because no client exists".to_string(), - ctx: context.clone(), - msg: ConnectionMsg::OpenTry(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::OpenTry(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::OpenTry(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::OpenTry(msg_conn_try), - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - { - let res = ValidationContext::validate( - &test.ctx, - MsgEnvelope::Connection(test.msg.clone()), - ); - - match res { - Ok(_) => { - assert!( - test.want_pass, - "conn_open_try: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ) - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_try: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } - } + ), + }; + + let ctx_new = MockContext::new( + ChainId::new("mockgaia".to_string(), 0), + HostType::Mock, + max_history_size, + host_chain_height, + ); + let ctx = match ctx_variant { + Ctx::Default => MockContext::default(), + Ctx::WithClient => ctx_new.with_client( + &msg.client_id_on_b, + Height::new(0, client_cons_state_height).unwrap(), + ), + }; + Fixture { ctx, msg } + } + + fn conn_open_try_validate(fxt: &Fixture, expect: Expect) { + let res = validate(&fxt.ctx, fxt.msg.clone()); + let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + match expect { + Expect::Failure(_) => { + assert!(res.is_err(), "{err_msg}") } - let res = dispatch(&test.ctx, test.msg.clone()); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "conn_open_try: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ); - - assert!(!proto_output.events.is_empty()); // Some events must exist. - - // The object in the output is a ConnectionEnd, should have TryOpen state. - let res: ConnectionResult = proto_output.result; - assert_eq!(res.connection_end.state().clone(), State::TryOpen); - - for e in proto_output.events.iter() { - assert!(matches!(e, &IbcEvent::OpenTryConnection(_))); - } - } - Err(e) => { - assert!( - !test.want_pass, - "conn_open_try: failed for test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}"); } } } + + fn conn_open_try_execute(fxt: &mut Fixture, expect: Expect) { + let res = execute(&mut fxt.ctx, fxt.msg.clone()); + let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + match expect { + Expect::Failure(_) => { + assert!(res.is_err(), "{err_msg}") + } + Expect::Success => { + assert!(res.is_ok(), "{err_msg}"); + assert_eq!(fxt.ctx.events.len(), 1); + + let event = fxt.ctx.events.first().unwrap(); + assert!(matches!(event, &IbcEvent::OpenTryConnection(_))); + + let conn_open_try_event = match event { + IbcEvent::OpenTryConnection(e) => e, + _ => unreachable!(), + }; + let conn_end = ::connection_end( + &fxt.ctx, + conn_open_try_event.connection_id(), + ) + .unwrap(); + assert_eq!(conn_end.state().clone(), State::TryOpen); + } + } + } + + #[test] + fn conn_open_try_healthy() { + let mut fxt = conn_open_try_fixture(Ctx::WithClient, Msg::Default); + conn_open_try_validate(&fxt, Expect::Success); + conn_open_try_execute(&mut fxt, Expect::Success); + } + + #[test] + fn conn_open_try_height_advanced() { + let fxt = conn_open_try_fixture(Ctx::WithClient, Msg::HeightAdvanced); + conn_open_try_validate(&fxt, Expect::Failure(None)); + } + + #[test] + fn conn_open_try_height_old() { + let fxt = conn_open_try_fixture(Ctx::WithClient, Msg::HeightOld); + conn_open_try_validate(&fxt, Expect::Failure(None)); + } + + #[test] + fn conn_open_try_proof_height_missing() { + let fxt = conn_open_try_fixture(Ctx::WithClient, Msg::ProofHeightMissing); + conn_open_try_validate(&fxt, Expect::Failure(None)); + } + + #[test] + fn conn_open_try_no_client() { + let fxt = conn_open_try_fixture(Ctx::Default, Msg::Default); + conn_open_try_validate(&fxt, Expect::Failure(None)); + } } diff --git a/crates/ibc/src/core/ics03_connection/msgs/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/msgs/conn_open_ack.rs index 1cb18068e..bfb15b747 100644 --- a/crates/ibc/src/core/ics03_connection/msgs/conn_open_ack.rs +++ b/crates/ibc/src/core/ics03_connection/msgs/conn_open_ack.rs @@ -123,6 +123,20 @@ pub mod test_util { use crate::core::ics24_host::identifier::ConnectionId; use crate::test_utils::{get_dummy_bech32_account, get_dummy_proof}; + use super::MsgConnectionOpenAck; + + /// Testing-specific helper methods. + impl MsgConnectionOpenAck { + /// Returns a new `MsgConnectionOpenAck` with dummy values. + pub fn new_dummy(proof_height: u64, consensus_height: u64) -> Self { + MsgConnectionOpenAck::try_from(get_dummy_raw_msg_conn_open_ack( + proof_height, + consensus_height, + )) + .unwrap() + } + } + pub fn get_dummy_raw_msg_conn_open_ack( proof_height: u64, consensus_height: u64, diff --git a/crates/ibc/src/core/ics03_connection/msgs/conn_open_confirm.rs b/crates/ibc/src/core/ics03_connection/msgs/conn_open_confirm.rs index 53bf32914..5960f4e91 100644 --- a/crates/ibc/src/core/ics03_connection/msgs/conn_open_confirm.rs +++ b/crates/ibc/src/core/ics03_connection/msgs/conn_open_confirm.rs @@ -70,12 +70,21 @@ impl From for RawMsgConnectionOpenConfirm { #[cfg(test)] pub mod test_util { + use super::MsgConnectionOpenConfirm; use crate::prelude::*; use ibc_proto::ibc::core::client::v1::Height; use ibc_proto::ibc::core::connection::v1::MsgConnectionOpenConfirm as RawMsgConnectionOpenConfirm; use crate::test_utils::{get_dummy_bech32_account, get_dummy_proof}; + /// Testing-specific helper methods. + impl MsgConnectionOpenConfirm { + /// Returns a new `MsgConnectionOpenConfirm` with dummy values. + pub fn new_dummy() -> Self { + MsgConnectionOpenConfirm::try_from(get_dummy_raw_msg_conn_open_confirm()).unwrap() + } + } + pub fn get_dummy_raw_msg_conn_open_confirm() -> RawMsgConnectionOpenConfirm { RawMsgConnectionOpenConfirm { connection_id: "srcconnection".to_string(), diff --git a/crates/ibc/src/core/ics03_connection/msgs/conn_open_init.rs b/crates/ibc/src/core/ics03_connection/msgs/conn_open_init.rs index 2ad287df1..56270111e 100644 --- a/crates/ibc/src/core/ics03_connection/msgs/conn_open_init.rs +++ b/crates/ibc/src/core/ics03_connection/msgs/conn_open_init.rs @@ -70,8 +70,11 @@ impl From for RawMsgConnectionOpenInit { #[cfg(test)] pub mod test_util { + use crate::core::ics03_connection::connection::Counterparty; use crate::prelude::*; - use ibc_proto::ibc::core::connection::v1::MsgConnectionOpenInit as RawMsgConnectionOpenInit; + use ibc_proto::ibc::core::connection::v1::{ + MsgConnectionOpenInit as RawMsgConnectionOpenInit, Version as RawVersion, + }; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; use crate::core::ics03_connection::msgs::test_util::get_dummy_raw_counterparty; @@ -81,6 +84,11 @@ pub mod test_util { /// Extends the implementation with additional helper methods. impl MsgConnectionOpenInit { + /// Returns a new `MsgConnectionOpenInit` with dummy values. + pub fn new_dummy() -> Self { + MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init()).unwrap() + } + /// Setter for `client_id`. Amenable to chaining, since it consumes the input message. pub fn with_client_id(self, client_id: ClientId) -> Self { MsgConnectionOpenInit { @@ -88,16 +96,38 @@ pub mod test_util { ..self } } + + /// Setter for `counterparty`. Amenable to chaining, since it consumes the input message.\ + pub fn with_counterparty_conn_id(self, counterparty_conn_id: u64) -> Self { + let counterparty = + Counterparty::try_from(get_dummy_raw_counterparty(Some(counterparty_conn_id))) + .unwrap(); + MsgConnectionOpenInit { + counterparty, + ..self + } + } + + pub fn with_version(self, identifier: Option<&str>) -> Self { + let version = match identifier { + Some(v) => Version::try_from(RawVersion { + identifier: v.to_string(), + features: vec![], + }) + .unwrap() + .into(), + None => None, + }; + MsgConnectionOpenInit { version, ..self } + } } /// Returns a dummy message, for testing only. /// Other unit tests may import this if they depend on a MsgConnectionOpenInit. - pub fn get_dummy_raw_msg_conn_open_init( - counterparty_conn_id: Option, - ) -> RawMsgConnectionOpenInit { + pub fn get_dummy_raw_msg_conn_open_init() -> RawMsgConnectionOpenInit { RawMsgConnectionOpenInit { client_id: ClientId::default().to_string(), - counterparty: Some(get_dummy_raw_counterparty(counterparty_conn_id)), + counterparty: Some(get_dummy_raw_counterparty(None)), version: Some(Version::default().into()), delay_period: 0, signer: get_dummy_bech32_account(), @@ -127,7 +157,7 @@ mod tests { want_pass: bool, } - let default_init_msg = get_dummy_raw_msg_conn_open_init(None); + let default_init_msg = get_dummy_raw_msg_conn_open_init(); let tests: Vec = vec![ Test { @@ -176,7 +206,7 @@ mod tests { #[test] fn to_and_from() { - let raw = get_dummy_raw_msg_conn_open_init(None); + let raw = get_dummy_raw_msg_conn_open_init(); let msg = MsgConnectionOpenInit::try_from(raw.clone()).unwrap(); let raw_back = RawMsgConnectionOpenInit::from(msg.clone()); let msg_back = MsgConnectionOpenInit::try_from(raw_back.clone()).unwrap(); @@ -185,7 +215,7 @@ mod tests { // Check if handler sets counterparty connection id to `None` // in case relayer passes `MsgConnectionOpenInit` message with it set to `Some(_)`. - let raw_with_counterpary_conn_id_some = get_dummy_raw_msg_conn_open_init(None); + let raw_with_counterpary_conn_id_some = get_dummy_raw_msg_conn_open_init(); let msg_with_counterpary_conn_id_some = MsgConnectionOpenInit::try_from(raw_with_counterpary_conn_id_some).unwrap(); let raw_with_counterpary_conn_id_some_back = diff --git a/crates/ibc/src/core/ics03_connection/msgs/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/msgs/conn_open_try.rs index ed3db170a..21f484f31 100644 --- a/crates/ibc/src/core/ics03_connection/msgs/conn_open_try.rs +++ b/crates/ibc/src/core/ics03_connection/msgs/conn_open_try.rs @@ -154,6 +154,14 @@ pub mod test_util { /// Testing-specific helper methods. impl MsgConnectionOpenTry { + /// Returns a new `MsgConnectionOpenTry` with dummy values. + pub fn new_dummy(proof_height: u64, consensus_height: u64) -> Self { + MsgConnectionOpenTry::try_from(get_dummy_raw_msg_conn_open_try( + proof_height, + consensus_height, + )) + .unwrap() + } /// Setter for `client_id`. pub fn with_client_id(self, client_id: ClientId) -> MsgConnectionOpenTry { MsgConnectionOpenTry { 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 e3ed159d6..19f7f35a3 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 @@ -213,9 +213,7 @@ mod tests { use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics03_connection::connection::Counterparty as ConnectionCounterparty; use crate::core::ics03_connection::connection::State as ConnectionState; - use crate::core::ics03_connection::msgs::conn_open_init::test_util::get_dummy_raw_msg_conn_open_init; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; - use crate::core::ics03_connection::msgs::conn_open_try::test_util::get_dummy_raw_msg_conn_open_try; use crate::core::ics03_connection::msgs::conn_open_try::MsgConnectionOpenTry; use crate::core::ics03_connection::version::get_compatible_versions; use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State}; @@ -246,8 +244,7 @@ mod tests { let context = MockContext::default(); - let msg_conn_init = - MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init(None)).unwrap(); + let msg_conn_init = MsgConnectionOpenInit::new_dummy(); let conn_end = ConnectionEnd::new( ConnectionState::Open, @@ -276,11 +273,10 @@ mod tests { }, ); - let msg_conn_try = MsgConnectionOpenTry::try_from(get_dummy_raw_msg_conn_open_try( + let msg_conn_try = MsgConnectionOpenTry::new_dummy( client_consensus_state_height, host_chain_height.revision_height(), - )) - .unwrap(); + ); let msg_chan_ack = MsgChannelOpenAck::try_from(get_dummy_raw_msg_chan_open_ack(proof_height)).unwrap(); 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 c5969dad3..817c2153a 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 @@ -98,7 +98,6 @@ mod tests { use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics03_connection::connection::State as ConnectionState; - use crate::core::ics03_connection::msgs::conn_open_init::test_util::get_dummy_raw_msg_conn_open_init; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; use crate::core::ics03_connection::version::get_compatible_versions; use crate::core::ics04_channel::channel::State; @@ -126,8 +125,7 @@ mod tests { let context = MockContext::default(); - let msg_conn_init = - MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init(None)).unwrap(); + let msg_conn_init = MsgConnectionOpenInit::new_dummy(); let init_conn_end = ConnectionEnd::new( ConnectionState::Init, From f4d373db6d25d8d569c2eda3cee659b667d2d96e Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 17 Feb 2023 07:10:48 -0800 Subject: [PATCH 2/2] Move generate_error_msg under Fixture struct --- .../ibc/src/core/ics03_connection/handler.rs | 27 +++++++++++-------- .../ics03_connection/handler/conn_open_ack.rs | 6 ++--- .../handler/conn_open_confirm.rs | 6 ++--- .../handler/conn_open_init.rs | 6 ++--- .../ics03_connection/handler/conn_open_try.rs | 6 ++--- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/crates/ibc/src/core/ics03_connection/handler.rs b/crates/ibc/src/core/ics03_connection/handler.rs index 75cb70e15..597b6ccd1 100644 --- a/crates/ibc/src/core/ics03_connection/handler.rs +++ b/crates/ibc/src/core/ics03_connection/handler.rs @@ -73,16 +73,21 @@ pub mod test_util { pub msg: M, } - pub fn generate_error_msg( - expect: &Expect, - process: &str, - res: &Result<(), ContextError>, - fxt: &Fixture, - ) -> String { - let msg = match expect { - Expect::Success => "step failed!", - Expect::Failure(_) => "step passed but was supposed to fail!", - }; - format!("{process} {msg} /n {res:?} /n {fxt:?}") + impl Fixture { + pub fn generate_error_msg( + &self, + expect: &Expect, + process: &str, + res: &Result<(), ContextError>, + ) -> String { + let base_error = match expect { + Expect::Success => "step failed!", + Expect::Failure(_) => "step passed but was supposed to fail!", + }; + format!( + "{process} {base_error} /n {res:?} /n {:?} /n {:?}", + &self.msg, &self.ctx + ) + } } } 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 a293bd5d4..cedb0d5aa 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 @@ -349,7 +349,7 @@ mod tests { use crate::core::ics02_client::height::Height; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; - use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; + use crate::core::ics03_connection::handler::test_util::{Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_ack::MsgConnectionOpenAck; use crate::core::ics23_commitment::commitment::CommitmentPrefix; use crate::core::ics24_host::identifier::{ChainId, ClientId}; @@ -422,7 +422,7 @@ mod tests { fn conn_open_ack_validate(fxt: &Fixture, expect: Expect) { let res = validate(&fxt.ctx, fxt.msg.clone()); - let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "validation", &res); match expect { Expect::Failure(err) => { assert!(res.is_err(), "{err_msg}"); @@ -461,7 +461,7 @@ mod tests { fn conn_open_ack_execute(fxt: &mut Fixture, expect: Expect) { let res = execute(&mut fxt.ctx, fxt.msg.clone()); - let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "execution", &res); match expect { Expect::Failure(_) => { assert!(res.is_err(), "{err_msg}"); diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs index c5a9ad317..b68d68c77 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs @@ -261,7 +261,7 @@ mod tests { use test_log::test; use crate::core::ics03_connection::connection::{ConnectionEnd, Counterparty, State}; - use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; + use crate::core::ics03_connection::handler::test_util::{Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_confirm::MsgConnectionOpenConfirm; use crate::core::ics23_commitment::commitment::CommitmentPrefix; use crate::core::ics24_host::identifier::ClientId; @@ -315,7 +315,7 @@ mod tests { fn conn_open_confirm_validate(fxt: &Fixture, expect: Expect) { let res = validate(&fxt.ctx, &fxt.msg); - let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "validation", &res); match expect { Expect::Failure(_) => { assert!(res.is_err(), "{err_msg}"); @@ -328,7 +328,7 @@ mod tests { fn conn_open_confirm_execute(fxt: &mut Fixture, expect: Expect) { let res = execute(&mut fxt.ctx, &fxt.msg); - let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "execution", &res); match expect { Expect::Failure(_) => { assert!(res.is_err(), "{err_msg}"); diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs index 835d37b53..cb112889e 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_init.rs @@ -150,7 +150,7 @@ mod tests { use super::*; use crate::core::ics03_connection::connection::State; - use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; + use crate::core::ics03_connection::handler::test_util::{Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_init::MsgConnectionOpenInit; use crate::core::ics03_connection::version::Version; use crate::events::IbcEvent; @@ -195,7 +195,7 @@ mod tests { fn conn_open_init_validate(fxt: &Fixture, expect: Expect) { let res = validate(&fxt.ctx, fxt.msg.clone()); - let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "validation", &res); match expect { Expect::Failure(_) => { assert!(res.is_err(), "{err_msg}") @@ -212,7 +212,7 @@ mod tests { expected_version: Vec, ) { let res = execute(&mut fxt.ctx, fxt.msg.clone()); - let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "execution", &res); match expect { Expect::Failure(_) => { assert!(res.is_err(), "{err_msg}") 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 c83b5187c..29c51f8ca 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 @@ -335,7 +335,7 @@ mod tests { use test_log::test; use crate::core::ics03_connection::connection::State; - use crate::core::ics03_connection::handler::test_util::{generate_error_msg, Expect, Fixture}; + use crate::core::ics03_connection::handler::test_util::{Expect, Fixture}; use crate::core::ics03_connection::msgs::conn_open_try::MsgConnectionOpenTry; use crate::core::ics24_host::identifier::ChainId; use crate::core::ValidationContext; @@ -401,7 +401,7 @@ mod tests { fn conn_open_try_validate(fxt: &Fixture, expect: Expect) { let res = validate(&fxt.ctx, fxt.msg.clone()); - let err_msg = generate_error_msg(&expect, "validation", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "validation", &res); match expect { Expect::Failure(_) => { assert!(res.is_err(), "{err_msg}") @@ -414,7 +414,7 @@ mod tests { fn conn_open_try_execute(fxt: &mut Fixture, expect: Expect) { let res = execute(&mut fxt.ctx, fxt.msg.clone()); - let err_msg = generate_error_msg(&expect, "execution", &res, fxt); + let err_msg = fxt.generate_error_msg(&expect, "execution", &res); match expect { Expect::Failure(_) => { assert!(res.is_err(), "{err_msg}")