From 71ce6e2b4d0d036adda98cb9eb95e9837fe45ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Laferri=C3=A8re?= Date: Fri, 17 Feb 2023 15:32:08 -0500 Subject: [PATCH] Finish adapting unit tests to new API (#430) * remove redundant msg in test * adapt timeout test * clippy * timeout no-op case * ack validation tests * ack tests * recv_packet validation * recv_packet execute test * chan_open_init_validation * chan_open_init tests done * `chan_open_try` validate tests * chan_open_try tests * chan_open_ack tests * chan_open_confirm tests * chan_close_init tests * chan_close_confirm tests * create_client tests * update_client tests * misbehaviour tests * upgrade client tests * Remove Readers and Keepers from MockContext * remove write_ack tests they are now covered in `recv_packet` tests * adapt send_packet * changelog * fix validation msgs * fmt --- .../430-adapt-unit-tests-to-val-exec-ctx.md | 2 + .../ibc/src/applications/transfer/context.rs | 29 +- crates/ibc/src/applications/transfer/error.rs | 8 +- .../transfer/relay/send_transfer.rs | 8 +- .../ibc/src/core/context/acknowledgement.rs | 172 ++++ .../src/core/context/chan_close_confirm.rs | 82 ++ .../ibc/src/core/context/chan_close_init.rs | 87 ++ crates/ibc/src/core/context/chan_open_ack.rs | 119 +++ .../ibc/src/core/context/chan_open_confirm.rs | 123 +++ crates/ibc/src/core/context/chan_open_init.rs | 81 ++ crates/ibc/src/core/context/chan_open_try.rs | 109 +++ crates/ibc/src/core/context/recv_packet.rs | 118 +++ .../ics02_client/handler/create_client.rs | 161 +--- .../core/ics02_client/handler/misbehaviour.rs | 133 +-- .../ics02_client/handler/update_client.rs | 231 ++--- .../ics02_client/handler/upgrade_client.rs | 219 ++--- crates/ibc/src/core/ics04_channel/context.rs | 40 +- .../ics04_channel/handler/acknowledgement.rs | 167 ++-- .../handler/chan_close_confirm.rs | 12 +- .../ics04_channel/handler/chan_close_init.rs | 12 +- .../ics04_channel/handler/chan_open_ack.rs | 323 ++++--- .../handler/chan_open_confirm.rs | 199 +++-- .../ics04_channel/handler/chan_open_init.rs | 138 ++- .../ics04_channel/handler/chan_open_try.rs | 256 ++---- .../core/ics04_channel/handler/recv_packet.rs | 223 ++--- .../core/ics04_channel/handler/send_packet.rs | 20 +- .../src/core/ics04_channel/handler/timeout.rs | 336 +++---- .../handler/write_acknowledgement.rs | 120 --- crates/ibc/src/mock/context.rs | 838 +----------------- crates/ibc/src/mock/ics18_relayer/context.rs | 4 +- crates/ibc/src/test_utils.rs | 27 +- 31 files changed, 2030 insertions(+), 2367 deletions(-) create mode 100644 .changelog/unreleased/improvement/430-adapt-unit-tests-to-val-exec-ctx.md diff --git a/.changelog/unreleased/improvement/430-adapt-unit-tests-to-val-exec-ctx.md b/.changelog/unreleased/improvement/430-adapt-unit-tests-to-val-exec-ctx.md new file mode 100644 index 0000000000..75ba689c93 --- /dev/null +++ b/.changelog/unreleased/improvement/430-adapt-unit-tests-to-val-exec-ctx.md @@ -0,0 +1,2 @@ +- Make all unit tests test the ValidationContext/ExecutionContext API + ([#430](https://github.com/cosmos/ibc-rs/issues/430)) \ No newline at end of file diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index 55ab689680..2916443d73 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -9,20 +9,21 @@ use crate::applications::transfer::relay::refund_packet_token; use crate::applications::transfer::{PrefixedCoin, PrefixedDenom, VERSION}; use crate::core::ics04_channel::channel::{Counterparty, Order}; use crate::core::ics04_channel::commitment::PacketCommitment; -use crate::core::ics04_channel::context::{ChannelKeeper, SendPacketReader}; -use crate::core::ics04_channel::error::PacketError; +use crate::core::ics04_channel::context::SendPacketReader; use crate::core::ics04_channel::handler::send_packet::SendPacketResult; use crate::core::ics04_channel::handler::ModuleExtras; use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement; use crate::core::ics04_channel::packet::{Packet, Sequence}; use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId}; +use crate::core::ics24_host::path::{CommitmentPath, SeqSendPath}; use crate::core::ics26_routing::context::ModuleOutputBuilder; +use crate::core::{ContextError, ExecutionContext}; use crate::prelude::*; use crate::signer::Signer; pub trait TokenTransferKeeper: BankKeeper { - fn store_send_packet_result(&mut self, result: SendPacketResult) -> Result<(), PacketError> { + fn store_send_packet_result(&mut self, result: SendPacketResult) -> Result<(), ContextError> { self.store_next_sequence_send( result.port_id.clone(), result.channel_id.clone(), @@ -44,14 +45,14 @@ pub trait TokenTransferKeeper: BankKeeper { channel_id: ChannelId, sequence: Sequence, commitment: PacketCommitment, - ) -> Result<(), PacketError>; + ) -> Result<(), ContextError>; fn store_next_sequence_send( &mut self, port_id: PortId, channel_id: ChannelId, seq: Sequence, - ) -> Result<(), PacketError>; + ) -> Result<(), ContextError>; } pub trait TokenTransferReader: SendPacketReader { @@ -82,7 +83,7 @@ pub trait TokenTransferReader: SendPacketReader { impl TokenTransferKeeper for T where - T: ChannelKeeper + BankKeeper, + T: ExecutionContext + BankKeeper, { fn store_packet_commitment( &mut self, @@ -90,8 +91,14 @@ where channel_id: ChannelId, sequence: Sequence, commitment: PacketCommitment, - ) -> Result<(), PacketError> { - ChannelKeeper::store_packet_commitment(self, port_id, channel_id, sequence, commitment) + ) -> Result<(), ContextError> { + let commitment_path = CommitmentPath { + port_id, + channel_id, + sequence, + }; + + self.store_packet_commitment(&commitment_path, commitment) } fn store_next_sequence_send( @@ -99,8 +106,10 @@ where port_id: PortId, channel_id: ChannelId, seq: Sequence, - ) -> Result<(), PacketError> { - ChannelKeeper::store_next_sequence_send(self, port_id, channel_id, seq) + ) -> Result<(), ContextError> { + let seq_send_path = SeqSendPath(port_id, channel_id); + + self.store_next_sequence_send(&seq_send_path, seq) } } diff --git a/crates/ibc/src/applications/transfer/error.rs b/crates/ibc/src/applications/transfer/error.rs index d2f89830db..ec477677bc 100644 --- a/crates/ibc/src/applications/transfer/error.rs +++ b/crates/ibc/src/applications/transfer/error.rs @@ -5,17 +5,17 @@ use ibc_proto::protobuf::Error as TendermintProtoError; use uint::FromDecStrErr; use crate::core::ics04_channel::channel::Order; -use crate::core::ics04_channel::error as channel_error; use crate::core::ics04_channel::Version; use crate::core::ics24_host::error::ValidationError; use crate::core::ics24_host::identifier::{ChannelId, PortId}; +use crate::core::ContextError; use crate::prelude::*; use crate::signer::SignerError; #[derive(Display, Debug)] pub enum TokenTransferError { - /// packet error: `{0}` - PacketError(channel_error::PacketError), + /// context error: `{0}` + ContextError(ContextError), /// destination channel not found in the counterparty of port_id `{port_id}` and channel_id `{channel_id}` DestinationChannelNotFound { port_id: PortId, @@ -101,7 +101,7 @@ pub enum TokenTransferError { impl std::error::Error for TokenTransferError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match &self { - Self::PacketError(e) => Some(e), + Self::ContextError(e) => Some(e), Self::InvalidPortId { validation_error: e, .. diff --git a/crates/ibc/src/applications/transfer/relay/send_transfer.rs b/crates/ibc/src/applications/transfer/relay/send_transfer.rs index b243f264ae..9a5e1c8327 100644 --- a/crates/ibc/src/applications/transfer/relay/send_transfer.rs +++ b/crates/ibc/src/applications/transfer/relay/send_transfer.rs @@ -29,7 +29,7 @@ where let chan_end_path_on_a = ChannelEndPath::new(&msg.port_on_a, &msg.chan_on_a); let chan_end_on_a = ctx .channel_end(&chan_end_path_on_a) - .map_err(TokenTransferError::PacketError)?; + .map_err(TokenTransferError::ContextError)?; let port_on_b = chan_end_on_a.counterparty().port_id().clone(); let chan_on_b = chan_end_on_a @@ -45,7 +45,7 @@ where let seq_send_path_on_a = SeqSendPath::new(&msg.port_on_a, &msg.chan_on_a); let sequence = ctx .get_next_sequence_send(&seq_send_path_on_a) - .map_err(TokenTransferError::PacketError)?; + .map_err(TokenTransferError::ContextError)?; let token = msg .token @@ -94,10 +94,10 @@ where result, log, events, - } = send_packet(ctx, packet).map_err(TokenTransferError::PacketError)?; + } = send_packet(ctx, packet).map_err(TokenTransferError::ContextError)?; ctx.store_send_packet_result(result) - .map_err(TokenTransferError::PacketError)?; + .map_err(TokenTransferError::ContextError)?; output.merge_output( HandlerOutput::builder() diff --git a/crates/ibc/src/core/context/acknowledgement.rs b/crates/ibc/src/core/context/acknowledgement.rs index 600389613c..95dece455c 100644 --- a/crates/ibc/src/core/context/acknowledgement.rs +++ b/crates/ibc/src/core/context/acknowledgement.rs @@ -110,3 +110,175 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use rstest::*; + + use crate::core::ics03_connection::connection::Counterparty as ConnectionCounterparty; + use crate::core::ics03_connection::connection::State as ConnectionState; + use crate::core::ics04_channel::channel::Counterparty; + use crate::core::ics04_channel::channel::State; + use crate::core::ics04_channel::Version; + use crate::core::ics24_host::identifier::ChannelId; + use crate::core::ics24_host::identifier::PortId; + use crate::{ + applications::transfer::MODULE_ID_STR, + core::{ + ics03_connection::{connection::ConnectionEnd, version::get_compatible_versions}, + ics04_channel::{ + channel::ChannelEnd, commitment::PacketCommitment, + msgs::acknowledgement::test_util::get_dummy_raw_msg_acknowledgement, + }, + ics24_host::identifier::{ClientId, ConnectionId}, + }, + mock::context::MockContext, + test_utils::DummyTransferModule, + timestamp::ZERO_DURATION, + Height, + }; + + struct Fixture { + ctx: MockContext, + module_id: ModuleId, + msg: MsgAcknowledgement, + packet_commitment: PacketCommitment, + conn_end_on_a: ConnectionEnd, + chan_end_on_a_ordered: ChannelEnd, + chan_end_on_a_unordered: ChannelEnd, + } + + #[fixture] + fn fixture() -> Fixture { + let client_height = Height::new(0, 2).unwrap(); + let mut ctx = MockContext::default().with_client(&ClientId::default(), client_height); + + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + let module = DummyTransferModule::new(ctx.ibc_store_share()); + ctx.add_route(module_id.clone(), module).unwrap(); + + let msg = MsgAcknowledgement::try_from(get_dummy_raw_msg_acknowledgement( + client_height.revision_height(), + )) + .unwrap(); + + let packet = msg.packet.clone(); + + let packet_commitment = ctx.packet_commitment( + &msg.packet.data, + &msg.packet.timeout_height_on_b, + &msg.packet.timeout_timestamp_on_b, + ); + + let chan_end_on_a_unordered = ChannelEnd::new( + State::Open, + Order::Unordered, + Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b.clone())), + vec![ConnectionId::default()], + Version::new("ics20-1".to_string()), + ); + + let chan_end_on_a_ordered = ChannelEnd::new( + State::Open, + Order::Ordered, + Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b)), + vec![ConnectionId::default()], + Version::new("ics20-1".to_string()), + ); + + let conn_end_on_a = ConnectionEnd::new( + ConnectionState::Open, + ClientId::default(), + ConnectionCounterparty::new( + ClientId::default(), + Some(ConnectionId::default()), + Default::default(), + ), + get_compatible_versions(), + ZERO_DURATION, + ); + + Fixture { + ctx, + module_id, + msg, + packet_commitment, + conn_end_on_a, + chan_end_on_a_unordered, + chan_end_on_a_ordered, + } + } + + #[rstest] + fn ack_unordered_chan_execute(fixture: Fixture) { + let Fixture { + ctx, + module_id, + msg, + packet_commitment, + conn_end_on_a, + chan_end_on_a_unordered, + .. + } = fixture; + let mut ctx = ctx + .with_channel( + PortId::default(), + ChannelId::default(), + chan_end_on_a_unordered, + ) + .with_connection(ConnectionId::default(), conn_end_on_a) + .with_packet_commitment( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + packet_commitment, + ); + + let res = acknowledgement_packet_execute(&mut ctx, module_id, msg); + + assert!(res.is_ok()); + + assert_eq!(ctx.events.len(), 1); + assert!(matches!( + ctx.events.first().unwrap(), + &IbcEvent::AcknowledgePacket(_) + )); + } + + #[rstest] + fn ack_ordered_chan_execute(fixture: Fixture) { + let Fixture { + ctx, + module_id, + msg, + packet_commitment, + conn_end_on_a, + chan_end_on_a_ordered, + .. + } = fixture; + let mut ctx = ctx + .with_channel( + PortId::default(), + ChannelId::default(), + chan_end_on_a_ordered, + ) + .with_connection(ConnectionId::default(), conn_end_on_a) + .with_packet_commitment( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + packet_commitment, + ); + + let res = acknowledgement_packet_execute(&mut ctx, module_id, msg); + + assert!(res.is_ok()); + + assert_eq!(ctx.events.len(), 1); + assert!(matches!( + ctx.events.first().unwrap(), + &IbcEvent::AcknowledgePacket(_) + )); + } +} diff --git a/crates/ibc/src/core/context/chan_close_confirm.rs b/crates/ibc/src/core/context/chan_close_confirm.rs index e81a3d37f1..9c9fb702e0 100644 --- a/crates/ibc/src/core/context/chan_close_confirm.rs +++ b/crates/ibc/src/core/context/chan_close_confirm.rs @@ -92,3 +92,85 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use crate::applications::transfer::MODULE_ID_STR; + use crate::core::context::chan_close_confirm::chan_close_confirm_execute; + use crate::core::ics04_channel::msgs::chan_close_confirm::test_util::get_dummy_raw_msg_chan_close_confirm; + use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm; + use crate::core::ics26_routing::context::ModuleId; + use crate::core::ValidationContext; + use crate::events::IbcEvent; + use crate::prelude::*; + + 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::test_util::get_dummy_raw_counterparty; + use crate::core::ics03_connection::version::get_compatible_versions; + use crate::core::ics04_channel::channel::{ + ChannelEnd, Counterparty, Order, State as ChannelState, + }; + use crate::core::ics04_channel::Version; + use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; + + use crate::mock::client_state::client_type as mock_client_type; + use crate::mock::context::MockContext; + use crate::test_utils::DummyTransferModule; + use crate::timestamp::ZERO_DURATION; + + #[test] + fn chan_close_confirm_event_height() { + let client_id = ClientId::new(mock_client_type(), 24).unwrap(); + let conn_id = ConnectionId::new(2); + let default_context = MockContext::default(); + let client_consensus_state_height = default_context.host_height().unwrap(); + + let conn_end = ConnectionEnd::new( + ConnectionState::Open, + client_id.clone(), + ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), + get_compatible_versions(), + ZERO_DURATION, + ); + + let msg_chan_close_confirm = MsgChannelCloseConfirm::try_from( + get_dummy_raw_msg_chan_close_confirm(client_consensus_state_height.revision_height()), + ) + .unwrap(); + + let chan_end = ChannelEnd::new( + ChannelState::Open, + Order::default(), + Counterparty::new( + msg_chan_close_confirm.port_id_on_b.clone(), + Some(msg_chan_close_confirm.chan_id_on_b.clone()), + ), + vec![conn_id.clone()], + Version::default(), + ); + + let mut context = default_context + .with_client(&client_id, client_consensus_state_height) + .with_connection(conn_id, conn_end) + .with_channel( + msg_chan_close_confirm.port_id_on_b.clone(), + msg_chan_close_confirm.chan_id_on_b.clone(), + chan_end, + ); + + let module = DummyTransferModule::new(context.ibc_store_share()); + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + context.add_route(module_id.clone(), module).unwrap(); + + let res = chan_close_confirm_execute(&mut context, module_id, msg_chan_close_confirm); + assert!(res.is_ok(), "Execution success: happy path"); + + assert_eq!(context.events.len(), 1); + assert!(matches!( + context.events.first().unwrap(), + &IbcEvent::CloseConfirmChannel(_) + )); + } +} diff --git a/crates/ibc/src/core/context/chan_close_init.rs b/crates/ibc/src/core/context/chan_close_init.rs index 46115940e4..49b005c415 100644 --- a/crates/ibc/src/core/context/chan_close_init.rs +++ b/crates/ibc/src/core/context/chan_close_init.rs @@ -92,3 +92,90 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use crate::applications::transfer::MODULE_ID_STR; + use crate::core::ics04_channel::msgs::chan_close_init::test_util::get_dummy_raw_msg_chan_close_init; + use crate::core::ics04_channel::msgs::chan_close_init::MsgChannelCloseInit; + use crate::core::ics26_routing::context::ModuleId; + use crate::core::ValidationContext; + use crate::events::IbcEvent; + use crate::prelude::*; + + 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::test_util::get_dummy_raw_counterparty; + use crate::core::ics03_connection::version::get_compatible_versions; + use crate::core::ics04_channel::channel::{ + ChannelEnd, Counterparty, Order, State as ChannelState, + }; + use crate::core::ics04_channel::Version; + use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; + + use crate::mock::client_state::client_type as mock_client_type; + use crate::mock::context::MockContext; + use crate::test_utils::DummyTransferModule; + use crate::timestamp::ZERO_DURATION; + + use super::chan_close_init_execute; + #[test] + fn chan_close_init_event_height() { + let client_id = ClientId::new(mock_client_type(), 24).unwrap(); + let conn_id = ConnectionId::new(2); + + let conn_end = ConnectionEnd::new( + ConnectionState::Open, + client_id.clone(), + ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), + get_compatible_versions(), + ZERO_DURATION, + ); + + let msg_chan_close_init = + MsgChannelCloseInit::try_from(get_dummy_raw_msg_chan_close_init()).unwrap(); + + let chan_end = ChannelEnd::new( + ChannelState::Open, + Order::default(), + Counterparty::new( + msg_chan_close_init.port_id_on_a.clone(), + Some(msg_chan_close_init.chan_id_on_a.clone()), + ), + vec![conn_id.clone()], + Version::default(), + ); + + let mut context = { + let mut default_context = MockContext::default(); + let client_consensus_state_height = default_context.host_height().unwrap(); + + let module = DummyTransferModule::new(default_context.ibc_store_share()); + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + default_context.add_route(module_id, module).unwrap(); + + default_context + .with_client(&client_id, client_consensus_state_height) + .with_connection(conn_id, conn_end) + .with_channel( + msg_chan_close_init.port_id_on_a.clone(), + msg_chan_close_init.chan_id_on_a.clone(), + chan_end, + ) + }; + + let res = chan_close_init_execute( + &mut context, + MODULE_ID_STR.parse().unwrap(), + msg_chan_close_init, + ); + assert!(res.is_ok(), "Execution happy path"); + + assert_eq!(context.events.len(), 1); + assert!(matches!( + context.events.first().unwrap(), + &IbcEvent::CloseInitChannel(_) + )); + } +} diff --git a/crates/ibc/src/core/context/chan_open_ack.rs b/crates/ibc/src/core/context/chan_open_ack.rs index ff6be3a84c..db3279d27a 100644 --- a/crates/ibc/src/core/context/chan_open_ack.rs +++ b/crates/ibc/src/core/context/chan_open_ack.rs @@ -89,3 +89,122 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use crate::{ + core::context::chan_open_ack::chan_open_ack_execute, events::IbcEvent, prelude::*, Height, + }; + use rstest::*; + + use crate::{ + applications::transfer::MODULE_ID_STR, + core::{ + ics03_connection::{ + connection::ConnectionEnd, msgs::test_util::get_dummy_raw_counterparty, + version::get_compatible_versions, + }, + ics04_channel::{ + channel::{ChannelEnd, Counterparty, Order, State}, + msgs::chan_open_ack::{ + test_util::get_dummy_raw_msg_chan_open_ack, MsgChannelOpenAck, + }, + }, + ics24_host::identifier::{ClientId, ConnectionId}, + ics26_routing::context::ModuleId, + }, + mock::context::MockContext, + test_utils::DummyTransferModule, + timestamp::ZERO_DURATION, + }; + + use crate::core::ics03_connection::connection::Counterparty as ConnectionCounterparty; + use crate::core::ics03_connection::connection::State as ConnectionState; + use crate::mock::client_state::client_type as mock_client_type; + + pub struct Fixture { + pub context: MockContext, + pub module_id: ModuleId, + pub msg: MsgChannelOpenAck, + pub client_id_on_a: ClientId, + pub conn_id_on_a: ConnectionId, + pub conn_end_on_a: ConnectionEnd, + pub chan_end_on_a: ChannelEnd, + pub proof_height: u64, + } + + #[fixture] + fn fixture() -> Fixture { + let proof_height = 10; + let mut context = MockContext::default(); + let module = DummyTransferModule::new(context.ibc_store_share()); + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + context.add_route(module_id.clone(), module).unwrap(); + + let client_id_on_a = ClientId::new(mock_client_type(), 45).unwrap(); + let conn_id_on_a = ConnectionId::new(2); + let conn_end_on_a = ConnectionEnd::new( + ConnectionState::Open, + client_id_on_a.clone(), + ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), + get_compatible_versions(), + ZERO_DURATION, + ); + + let msg = + MsgChannelOpenAck::try_from(get_dummy_raw_msg_chan_open_ack(proof_height)).unwrap(); + + let chan_end_on_a = ChannelEnd::new( + State::Init, + Order::Unordered, + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b.clone())), + vec![conn_id_on_a.clone()], + msg.version_on_b.clone(), + ); + + Fixture { + context, + module_id, + msg, + client_id_on_a, + conn_id_on_a, + conn_end_on_a, + chan_end_on_a, + proof_height, + } + } + + #[rstest] + fn chan_open_ack_execute_happy_path(fixture: Fixture) { + let Fixture { + context, + module_id, + msg, + client_id_on_a, + conn_id_on_a, + conn_end_on_a, + chan_end_on_a, + proof_height, + .. + } = fixture; + + let mut context = context + .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_a, conn_end_on_a) + .with_channel( + msg.port_id_on_a.clone(), + msg.chan_id_on_a.clone(), + chan_end_on_a, + ); + + let res = chan_open_ack_execute(&mut context, module_id, msg); + + assert!(res.is_ok(), "Execution happy path"); + + assert_eq!(context.events.len(), 1); + assert!(matches!( + context.events.first().unwrap(), + &IbcEvent::OpenAckChannel(_) + )); + } +} diff --git a/crates/ibc/src/core/context/chan_open_confirm.rs b/crates/ibc/src/core/context/chan_open_confirm.rs index 90b991389a..d84b0a35f0 100644 --- a/crates/ibc/src/core/context/chan_open_confirm.rs +++ b/crates/ibc/src/core/context/chan_open_confirm.rs @@ -92,3 +92,126 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use crate::{ + core::{context::chan_open_confirm::chan_open_confirm_execute, ics04_channel::Version}, + events::IbcEvent, + prelude::*, + Height, + }; + use rstest::*; + + use crate::{ + applications::transfer::MODULE_ID_STR, + core::{ + ics03_connection::{ + connection::ConnectionEnd, msgs::test_util::get_dummy_raw_counterparty, + version::get_compatible_versions, + }, + ics04_channel::{ + channel::{ChannelEnd, Counterparty, Order, State}, + msgs::chan_open_confirm::{ + test_util::get_dummy_raw_msg_chan_open_confirm, MsgChannelOpenConfirm, + }, + }, + ics24_host::identifier::{ChannelId, ClientId, ConnectionId}, + ics26_routing::context::ModuleId, + }, + mock::context::MockContext, + test_utils::DummyTransferModule, + timestamp::ZERO_DURATION, + }; + + use crate::core::ics03_connection::connection::Counterparty as ConnectionCounterparty; + use crate::core::ics03_connection::connection::State as ConnectionState; + use crate::mock::client_state::client_type as mock_client_type; + + pub struct Fixture { + pub context: MockContext, + pub module_id: ModuleId, + pub msg: MsgChannelOpenConfirm, + pub client_id_on_b: ClientId, + pub conn_id_on_b: ConnectionId, + pub conn_end_on_b: ConnectionEnd, + pub chan_end_on_b: ChannelEnd, + pub proof_height: u64, + } + + #[fixture] + fn fixture() -> Fixture { + let proof_height = 10; + let mut context = MockContext::default(); + let module = DummyTransferModule::new(context.ibc_store_share()); + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + context.add_route(module_id.clone(), module).unwrap(); + + let client_id_on_b = ClientId::new(mock_client_type(), 45).unwrap(); + let conn_id_on_b = ConnectionId::new(2); + let conn_end_on_b = ConnectionEnd::new( + ConnectionState::Open, + client_id_on_b.clone(), + ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), + get_compatible_versions(), + ZERO_DURATION, + ); + + let msg = + MsgChannelOpenConfirm::try_from(get_dummy_raw_msg_chan_open_confirm(proof_height)) + .unwrap(); + + let chan_end_on_b = ChannelEnd::new( + State::TryOpen, + Order::Unordered, + Counterparty::new(msg.port_id_on_b.clone(), Some(ChannelId::default())), + vec![conn_id_on_b.clone()], + Version::default(), + ); + + Fixture { + context, + module_id, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + chan_end_on_b, + proof_height, + } + } + + #[rstest] + fn chan_open_confirm_execute_happy_path(fixture: Fixture) { + let Fixture { + context, + module_id, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + chan_end_on_b, + proof_height, + .. + } = fixture; + + let mut context = context + .with_client(&client_id_on_b, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_b, conn_end_on_b) + .with_channel( + msg.port_id_on_b.clone(), + ChannelId::default(), + chan_end_on_b, + ); + + let res = chan_open_confirm_execute(&mut context, module_id, msg); + + assert!(res.is_ok(), "Execution happy path"); + + assert_eq!(context.events.len(), 1); + assert!(matches!( + context.events.first().unwrap(), + &IbcEvent::OpenConfirmChannel(_) + )); + } +} diff --git a/crates/ibc/src/core/context/chan_open_init.rs b/crates/ibc/src/core/context/chan_open_init.rs index df6d81bd57..9b7b200ae4 100644 --- a/crates/ibc/src/core/context/chan_open_init.rs +++ b/crates/ibc/src/core/context/chan_open_init.rs @@ -112,3 +112,84 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use crate::applications::transfer::MODULE_ID_STR; + use crate::core::ics03_connection::connection::State as ConnectionState; + use crate::core::ics24_host::identifier::ConnectionId; + use crate::test_utils::DummyTransferModule; + use crate::{ + core::{ + ics03_connection::{ + connection::ConnectionEnd, + msgs::conn_open_init::{ + test_util::get_dummy_raw_msg_conn_open_init, MsgConnectionOpenInit, + }, + version::get_compatible_versions, + }, + ics04_channel::msgs::chan_open_init::test_util::get_dummy_raw_msg_chan_open_init, + }, + mock::context::MockContext, + }; + + use super::*; + use rstest::*; + + pub struct Fixture { + pub context: MockContext, + pub module_id: ModuleId, + pub msg: MsgChannelOpenInit, + pub conn_end_on_a: ConnectionEnd, + } + + #[fixture] + fn fixture() -> Fixture { + let msg = MsgChannelOpenInit::try_from(get_dummy_raw_msg_chan_open_init(None)).unwrap(); + + let mut context = MockContext::default(); + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + let module = DummyTransferModule::new(context.ibc_store_share()); + context.add_route(module_id.clone(), module).unwrap(); + + let msg_conn_init = + MsgConnectionOpenInit::try_from(get_dummy_raw_msg_conn_open_init()).unwrap(); + + let conn_end_on_a = ConnectionEnd::new( + ConnectionState::Init, + msg_conn_init.client_id_on_a.clone(), + msg_conn_init.counterparty.clone(), + get_compatible_versions(), + msg_conn_init.delay_period, + ); + + Fixture { + context, + module_id, + msg, + conn_end_on_a, + } + } + + #[rstest] + fn chan_open_init_execute_events(fixture: Fixture) { + let Fixture { + context, + module_id, + msg, + conn_end_on_a, + } = fixture; + + let mut context = context.with_connection(ConnectionId::default(), conn_end_on_a); + + let res = chan_open_init_execute(&mut context, module_id, msg); + + assert!(res.is_ok(), "Execution succeeds; good parameters"); + + assert_eq!(context.events.len(), 1); + assert!(matches!( + context.events.first().unwrap(), + &IbcEvent::OpenInitChannel(_) + )); + } +} diff --git a/crates/ibc/src/core/context/chan_open_try.rs b/crates/ibc/src/core/context/chan_open_try.rs index c26b684d76..e0b215ebb7 100644 --- a/crates/ibc/src/core/context/chan_open_try.rs +++ b/crates/ibc/src/core/context/chan_open_try.rs @@ -114,3 +114,112 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use crate::{ + applications::transfer::MODULE_ID_STR, + core::{context::chan_open_try::chan_open_try_execute, ics26_routing::context::ModuleId}, + events::IbcEvent, + prelude::*, + test_utils::DummyTransferModule, + Height, + }; + use rstest::*; + + use crate::{ + core::{ + ics03_connection::{ + connection::ConnectionEnd, msgs::test_util::get_dummy_raw_counterparty, + version::get_compatible_versions, + }, + ics04_channel::msgs::chan_open_try::{ + test_util::get_dummy_raw_msg_chan_open_try, MsgChannelOpenTry, + }, + ics24_host::identifier::{ClientId, ConnectionId}, + }, + mock::context::MockContext, + timestamp::ZERO_DURATION, + }; + + use crate::core::ics03_connection::connection::Counterparty as ConnectionCounterparty; + use crate::core::ics03_connection::connection::State as ConnectionState; + use crate::mock::client_state::client_type as mock_client_type; + + pub struct Fixture { + pub context: MockContext, + pub module_id: ModuleId, + pub msg: MsgChannelOpenTry, + pub client_id_on_b: ClientId, + pub conn_id_on_b: ConnectionId, + pub conn_end_on_b: ConnectionEnd, + pub proof_height: u64, + } + + #[fixture] + fn fixture() -> Fixture { + let proof_height = 10; + let conn_id_on_b = ConnectionId::new(2); + let client_id_on_b = ClientId::new(mock_client_type(), 45).unwrap(); + + // This is the connection underlying the channel we're trying to open. + let conn_end_on_b = ConnectionEnd::new( + ConnectionState::Open, + client_id_on_b.clone(), + ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), + get_compatible_versions(), + ZERO_DURATION, + ); + + // We're going to test message processing against this message. + // 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(); + + let hops = vec![conn_id_on_b.clone()]; + msg.connection_hops_on_b = hops; + + let mut context = MockContext::default(); + let module = DummyTransferModule::new(context.ibc_store_share()); + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + context.add_route(module_id.clone(), module).unwrap(); + + Fixture { + context, + module_id, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + proof_height, + } + } + + #[rstest] + fn chan_open_try_execute_events(fixture: Fixture) { + let Fixture { + context, + module_id, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + proof_height, + .. + } = fixture; + + let mut context = context + .with_client(&client_id_on_b, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_b, conn_end_on_b); + + let res = chan_open_try_execute(&mut context, module_id, msg); + + assert!(res.is_ok(), "Execution success: happy path"); + + assert_eq!(context.events.len(), 1); + assert!(matches!( + context.events.first().unwrap(), + &IbcEvent::OpenTryChannel(_) + )); + } +} diff --git a/crates/ibc/src/core/context/recv_packet.rs b/crates/ibc/src/core/context/recv_packet.rs index 3d929ee7b7..469ebbbb53 100644 --- a/crates/ibc/src/core/context/recv_packet.rs +++ b/crates/ibc/src/core/context/recv_packet.rs @@ -134,3 +134,121 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use crate::{ + applications::transfer::MODULE_ID_STR, + core::{ + context::recv_packet::recv_packet_execute, + ics03_connection::version::get_compatible_versions, + ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}, + ics26_routing::context::ModuleId, + }, + events::IbcEvent, + prelude::*, + test_utils::DummyTransferModule, + timestamp::ZERO_DURATION, + }; + use rstest::*; + + use crate::core::ics03_connection::connection::Counterparty as ConnectionCounterparty; + use crate::core::ics03_connection::connection::State as ConnectionState; + use crate::{ + core::{ + ics03_connection::connection::ConnectionEnd, + ics04_channel::{ + channel::{ChannelEnd, Counterparty, Order, State}, + msgs::recv_packet::{test_util::get_dummy_raw_msg_recv_packet, MsgRecvPacket}, + Version, + }, + }, + mock::{context::MockContext, ics18_relayer::context::RelayerContext}, + Height, + }; + + pub struct Fixture { + pub context: MockContext, + pub module_id: ModuleId, + pub client_height: Height, + pub host_height: Height, + pub msg: MsgRecvPacket, + pub conn_end_on_b: ConnectionEnd, + pub chan_end_on_b: ChannelEnd, + } + + #[fixture] + fn fixture() -> Fixture { + let mut context = MockContext::default(); + + let module_id: ModuleId = MODULE_ID_STR.parse().unwrap(); + let module = DummyTransferModule::new(context.ibc_store_share()); + context.add_route(module_id.clone(), module).unwrap(); + + let host_height = context.query_latest_height().unwrap().increment(); + + let client_height = host_height.increment(); + + let msg = MsgRecvPacket::try_from(get_dummy_raw_msg_recv_packet( + client_height.revision_height(), + )) + .unwrap(); + + let packet = msg.packet.clone(); + + let chan_end_on_b = ChannelEnd::new( + State::Open, + Order::default(), + Counterparty::new(packet.port_on_a, Some(packet.chan_on_a)), + vec![ConnectionId::default()], + Version::new("ics20-1".to_string()), + ); + + let conn_end_on_b = ConnectionEnd::new( + ConnectionState::Open, + ClientId::default(), + ConnectionCounterparty::new( + ClientId::default(), + Some(ConnectionId::default()), + Default::default(), + ), + get_compatible_versions(), + ZERO_DURATION, + ); + + Fixture { + context, + module_id, + client_height, + host_height, + msg, + conn_end_on_b, + chan_end_on_b, + } + } + + #[rstest] + fn recv_packet_execute_test(fixture: Fixture) { + let Fixture { + context, + module_id, + msg, + conn_end_on_b, + chan_end_on_b, + client_height, + .. + } = fixture; + let mut ctx = context + .with_client(&ClientId::default(), client_height) + .with_connection(ConnectionId::default(), conn_end_on_b) + .with_channel(PortId::default(), ChannelId::default(), chan_end_on_b); + + let res = recv_packet_execute(&mut ctx, module_id, msg); + + assert!(res.is_ok()); + + assert_eq!(ctx.events.len(), 2); + assert!(matches!(&ctx.events[0], &IbcEvent::ReceivePacket(_))); + assert!(matches!(&ctx.events[1], &IbcEvent::WriteAcknowledgement(_))); + } +} diff --git a/crates/ibc/src/core/ics02_client/handler/create_client.rs b/crates/ibc/src/core/ics02_client/handler/create_client.rs index d170e77b72..2dab2a7098 100644 --- a/crates/ibc/src/core/ics02_client/handler/create_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/create_client.rs @@ -180,6 +180,8 @@ pub(crate) fn process( #[cfg(test)] mod tests { use crate::clients::ics07_tendermint::client_type as tm_client_type; + use crate::core::ics02_client::handler::create_client::{execute, validate}; + use crate::core::ValidationContext; use crate::prelude::*; use core::time::Duration; @@ -190,13 +192,10 @@ mod tests { }; use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::clients::ics07_tendermint::header::test_util::get_dummy_tendermint_header; - use crate::core::ics02_client::handler::{dispatch, ClientResult}; use crate::core::ics02_client::msgs::create_client::MsgCreateClient; - use crate::core::ics02_client::msgs::ClientMsg; use crate::core::ics02_client::trust_threshold::TrustThreshold; use crate::core::ics23_commitment::specs::ProofSpecs; use crate::core::ics24_host::identifier::ClientId; - use crate::handler::HandlerOutput; use crate::mock::client_state::{client_type as mock_client_type, MockClientState}; use crate::mock::consensus_state::MockConsensusState; use crate::mock::context::MockContext; @@ -206,7 +205,7 @@ mod tests { #[test] fn test_create_client_ok() { - let ctx = MockContext::default(); + let mut ctx = MockContext::default(); let signer = get_dummy_account_id(); let height = Height::new(0, 42).unwrap(); @@ -216,106 +215,31 @@ mod tests { signer, ); - let output = dispatch(&ctx, ClientMsg::CreateClient(msg.clone())); - - match output { - Ok(HandlerOutput { result, .. }) => { - let expected_client_id = ClientId::new(mock_client_type(), 0).unwrap(); - match result { - ClientResult::Create(create_result) => { - assert_eq!(create_result.client_type, mock_client_type()); - assert_eq!(create_result.client_id, expected_client_id); - assert_eq!( - create_result.client_state.as_ref().clone_into(), - msg.client_state - ); - assert_eq!( - create_result.consensus_state.as_ref().clone_into(), - msg.consensus_state - ); - } - _ => { - panic!("unexpected result type: expected ClientResult::CreateResult!"); - } - } - } - Err(err) => { - panic!("unexpected error: {}", err); - } - } - } + let client_type = mock_client_type(); - #[test] - fn test_create_client_ok_multiple() { - let existing_client_id = ClientId::default(); - let signer = get_dummy_account_id(); - let height_1 = Height::new(0, 80).unwrap(); - let height_2 = Height::new(0, 42).unwrap(); - let height_3 = Height::new(0, 50).unwrap(); - - let ctx = MockContext::default().with_client(&existing_client_id, height_1); - - let create_client_msgs: Vec = vec![ - MsgCreateClient::new( - MockClientState::new(MockHeader::new(height_2)).into(), - MockConsensusState::new(MockHeader::new(height_2)).into(), - signer.clone(), - ), - MsgCreateClient::new( - MockClientState::new(MockHeader::new(height_2)).into(), - MockConsensusState::new(MockHeader::new(height_2)).into(), - signer.clone(), - ), - MsgCreateClient::new( - MockClientState::new(MockHeader::new(height_3)).into(), - MockConsensusState::new(MockHeader::new(height_3)).into(), - signer, - ), - ] - .into_iter() - .collect(); - - // The expected client id that will be generated will be identical to "9999-mock-0" for all - // tests. This is because we're not persisting any client results (which is done via the - // tests for `ics26_routing::dispatch`. - let expected_client_id = ClientId::new(mock_client_type(), 0).unwrap(); - - for msg in create_client_msgs { - let output = dispatch(&ctx, ClientMsg::CreateClient(msg.clone())); - - match output { - Ok(HandlerOutput { result, .. }) => match result { - ClientResult::Create(create_res) => { - assert_eq!( - create_res.client_type, - create_res.client_state.client_type() - ); - assert_eq!(create_res.client_id, expected_client_id); - assert_eq!( - create_res.client_state.as_ref().clone_into(), - msg.client_state - ); - assert_eq!( - create_res.consensus_state.as_ref().clone_into(), - msg.consensus_state - ); - } - _ => { - panic!("expected result of type ClientResult::CreateResult"); - } - }, - Err(err) => { - panic!("unexpected error: {}", err); - } - } - } + let client_id = { + let id_counter = ctx.client_counter().unwrap(); + ClientId::new(client_type.clone(), id_counter).unwrap() + }; + + let res = validate(&ctx, msg.clone()); + + assert!(res.is_ok(), "validation happy path"); + + let res = execute(&mut ctx, msg.clone()); + + assert!(res.is_ok(), "execution happy path"); + + let expected_client_state = ctx.decode_client_state(msg.client_state).unwrap(); + assert_eq!(expected_client_state.client_type(), client_type); + assert_eq!(ctx.client_state(&client_id).unwrap(), expected_client_state); } #[test] fn test_tm_create_client_ok() { let signer = get_dummy_account_id(); - let ctx = MockContext::default(); + let mut ctx = MockContext::default(); let tm_header = get_dummy_tendermint_header(); @@ -337,38 +261,27 @@ mod tests { .unwrap() .into(); + let client_type = tm_client_type(); + + let client_id = { + let id_counter = ctx.client_counter().unwrap(); + ClientId::new(client_type.clone(), id_counter).unwrap() + }; + let msg = MsgCreateClient::new( tm_client_state, TmConsensusState::try_from(tm_header).unwrap().into(), signer, ); - let output = dispatch(&ctx, ClientMsg::CreateClient(msg.clone())); - - match output { - Ok(HandlerOutput { result, .. }) => { - let expected_client_id = ClientId::new(tm_client_type(), 0).unwrap(); - match result { - ClientResult::Create(create_res) => { - assert_eq!(create_res.client_type, tm_client_type()); - assert_eq!(create_res.client_id, expected_client_id); - assert_eq!( - create_res.client_state.as_ref().clone_into(), - msg.client_state - ); - assert_eq!( - create_res.consensus_state.as_ref().clone_into(), - msg.consensus_state - ); - } - _ => { - panic!("expected result of type ClientResult::CreateResult"); - } - } - } - Err(err) => { - panic!("unexpected error: {}", err); - } - } + let res = validate(&ctx, msg.clone()); + assert!(res.is_ok(), "tendermint client validation happy path"); + + let res = execute(&mut ctx, msg.clone()); + assert!(res.is_ok(), "tendermint client execution happy path"); + + let expected_client_state = ctx.decode_client_state(msg.client_state).unwrap(); + assert_eq!(expected_client_state.client_type(), client_type); + assert_eq!(ctx.client_state(&client_id).unwrap(), expected_client_state); } } diff --git a/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs b/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs index c46d085bf4..93f7acbfe5 100644 --- a/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs +++ b/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs @@ -127,14 +127,12 @@ mod tests { use crate::clients::ics07_tendermint::header::Header as TmHeader; use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; use crate::core::ics02_client::client_type::ClientType; - use crate::core::ics02_client::error::ClientError; - use crate::core::ics02_client::handler::dispatch; - use crate::core::ics02_client::handler::ClientResult; + use crate::core::ics02_client::handler::misbehaviour::execute; + use crate::core::ics02_client::handler::misbehaviour::validate; use crate::core::ics02_client::msgs::misbehaviour::MsgSubmitMisbehaviour; - use crate::core::ics02_client::msgs::ClientMsg; use crate::core::ics24_host::identifier::{ChainId, ClientId}; + use crate::core::ValidationContext; use crate::events::IbcEvent; - use crate::handler::HandlerOutput; use crate::mock::client_state::client_type as mock_client_type; use crate::mock::context::MockContext; use crate::mock::header::MockHeader; @@ -145,45 +143,21 @@ mod tests { use crate::Height; use crate::{downcast, prelude::*}; - /// Panics unless the given handler output satisfies the following conditions - - /// * The output result is of type client misbehaviour - /// * The specified client is frozen and it's frozen height is set correctly - /// * Only a single misbehaviour event was emitted with the specified client id and type - /// * No logs were emitted - fn ensure_misbehaviour_result( - output: Result, ClientError>, - client_id: &ClientId, - client_type: &ClientType, - ) { - match output { - Ok(HandlerOutput { - result: ClientResult::Misbehaviour(res), - mut events, - log, - }) => { - // check result - assert_eq!(&res.client_id, client_id); - assert!(res.client_state.is_frozen()); - assert_eq!( - res.client_state.frozen_height(), - Some(Height::new(0, 1).unwrap()) - ); - - // check events - let misbehaviour_client_event = - downcast!(events.pop().unwrap() => IbcEvent::ClientMisbehaviour).unwrap(); - assert!(events.is_empty()); - assert_eq!(misbehaviour_client_event.client_id(), client_id); - assert_eq!(misbehaviour_client_event.client_type(), client_type); - - // check logs - assert!(log.is_empty()); - } - _ => panic!( - "Expected client to be frozen due to misbehaviour, instead got result: {:?}", - output - ), - } + fn ensure_misbehaviour(ctx: &MockContext, client_id: &ClientId, client_type: &ClientType) { + let client_state = ctx.client_state(client_id).unwrap(); + + assert!(client_state.is_frozen()); + assert_eq!( + client_state.frozen_height(), + Some(Height::new(0, 1).unwrap()) + ); + + // check events + let misbehaviour_client_event = + downcast!(ctx.events.first().unwrap() => IbcEvent::ClientMisbehaviour).unwrap(); + assert_eq!(ctx.events.len(), 1); + assert_eq!(misbehaviour_client_event.client_id(), client_id); + assert_eq!(misbehaviour_client_event.client_type(), client_type); } /// Tests misbehaviour handling for the mock client. @@ -205,9 +179,14 @@ mod tests { signer: get_dummy_account_id(), }; - let ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); - let output = dispatch(&ctx, ClientMsg::Misbehaviour(msg)); - ensure_misbehaviour_result(output, &client_id, &mock_client_type()); + let mut ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); + + let res = validate(&ctx, msg.clone()); + assert!(res.is_ok()); + let res = execute(&mut ctx, msg); + assert!(res.is_ok()); + + ensure_misbehaviour(&ctx, &client_id, &mock_client_type()); } /// Tests misbehaviour handling failure for a non-existent client @@ -227,46 +206,8 @@ mod tests { }; let ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); - let output = dispatch(&ctx, ClientMsg::Misbehaviour(msg.clone())); - match output { - Err(ClientError::ClientNotFound { client_id }) => assert_eq!(client_id, msg.client_id), - _ => panic!("expected ClientNotFound error, instead got {:?}", output), - } - } - - /// Tests misbehaviour handling for multiple clients - #[test] - fn test_misbehaviour_client_ok_multiple() { - let client_ids = vec![ - ClientId::from_str("mockclient1").unwrap(), - ClientId::from_str("mockclient2").unwrap(), - ClientId::from_str("mockclient3").unwrap(), - ]; - let signer = get_dummy_account_id(); - let initial_height = Height::new(0, 45).unwrap(); - let misbehaviour_height = Height::new(0, 49).unwrap(); - - let mut ctx = MockContext::default(); - - for cid in &client_ids { - ctx = ctx.with_client(cid, initial_height); - } - - for client_id in &client_ids { - let msg = MsgSubmitMisbehaviour { - client_id: client_id.clone(), - misbehaviour: MockMisbehaviour { - client_id: client_id.clone(), - header1: MockHeader::new(misbehaviour_height), - header2: MockHeader::new(misbehaviour_height), - } - .into(), - signer: signer.clone(), - }; - - let output = dispatch(&ctx, ClientMsg::Misbehaviour(msg.clone())); - ensure_misbehaviour_result(output, client_id, &mock_client_type()); - } + let res = validate(&ctx, msg); + assert!(res.is_err()); } /// Tests misbehaviour handling for the synthetic Tendermint client. @@ -279,7 +220,7 @@ mod tests { let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); // Create a mock context for chain-A with a synthetic tendermint light client for chain-B - let ctx_a = MockContext::new( + let mut ctx_a = MockContext::new( ChainId::new("mockgaiaA".to_string(), 1), HostType::Mock, 5, @@ -327,8 +268,11 @@ mod tests { signer: get_dummy_account_id(), }; - let output = dispatch(&ctx_a, ClientMsg::Misbehaviour(msg)); - ensure_misbehaviour_result(output, &client_id, &tm_client_type()); + let res = validate(&ctx_a, msg.clone()); + assert!(res.is_ok()); + let res = execute(&mut ctx_a, msg); + assert!(res.is_ok()); + ensure_misbehaviour(&ctx_a, &client_id, &tm_client_type()); } #[test] @@ -339,7 +283,7 @@ mod tests { let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); // Create a mock context for chain-A with a synthetic tendermint light client for chain-B - let ctx_a = MockContext::new( + let mut ctx_a = MockContext::new( ChainId::new("mockgaiaA".to_string(), 1), HostType::Mock, 5, @@ -387,7 +331,10 @@ mod tests { signer: get_dummy_account_id(), }; - let output = dispatch(&ctx_a, ClientMsg::Misbehaviour(msg)); - ensure_misbehaviour_result(output, &client_id, &tm_client_type()); + let res = validate(&ctx_a, msg.clone()); + assert!(res.is_ok()); + let res = execute(&mut ctx_a, msg); + assert!(res.is_ok()); + ensure_misbehaviour(&ctx_a, &client_id, &tm_client_type()); } } diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index f3e1044fdf..964427b557 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -232,14 +232,11 @@ mod tests { use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::core::ics02_client::client_state::ClientState; use crate::core::ics02_client::consensus_state::downcast_consensus_state; - use crate::core::ics02_client::error::ClientError; - use crate::core::ics02_client::handler::dispatch; - use crate::core::ics02_client::handler::ClientResult::Update; + use crate::core::ics02_client::handler::update_client::{execute, validate}; use crate::core::ics02_client::msgs::update_client::MsgUpdateClient; - use crate::core::ics02_client::msgs::ClientMsg; use crate::core::ics24_host::identifier::{ChainId, ClientId}; + use crate::core::ValidationContext; use crate::events::IbcEvent; - use crate::handler::HandlerOutput; use crate::mock::client_state::client_type as mock_client_type; use crate::mock::client_state::MockClientState; use crate::mock::context::MockContext; @@ -257,40 +254,25 @@ mod tests { let timestamp = Timestamp::now(); - let ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); + let mut ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); let height = Height::new(0, 46).unwrap(); let msg = MsgUpdateClient { - client_id: client_id.clone(), + client_id, header: MockHeader::new(height).with_timestamp(timestamp).into(), signer, }; - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg)); - - match output { - Ok(HandlerOutput { - result, - events: _, - log, - }) => { - assert!(log.is_empty()); - // Check the result - match result { - Update(upd_res) => { - assert_eq!(upd_res.client_id, client_id); - assert_eq!( - upd_res.client_state, - MockClientState::new(MockHeader::new(height).with_timestamp(timestamp)) - .into_box() - ) - } - _ => panic!("update handler result has incorrect type"), - } - } - Err(err) => { - panic!("unexpected error: {:?}", err); - } - } + let res = validate(&ctx, msg.clone()); + + assert!(res.is_ok(), "validation happy path"); + + let res = execute(&mut ctx, msg.clone()); + assert!(res.is_ok(), "execution happy path"); + + assert_eq!( + ctx.client_state(&msg.client_id).unwrap(), + MockClientState::new(MockHeader::new(height).with_timestamp(timestamp)).into_box() + ); } #[test] @@ -306,57 +288,9 @@ mod tests { signer, }; - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg.clone())); + let res = validate(&ctx, msg); - match output { - Err(ClientError::ClientNotFound { client_id }) => { - assert_eq!(client_id, msg.client_id); - } - _ => { - panic!("expected ClientNotFound error, instead got {:?}", output) - } - } - } - - #[test] - fn test_update_client_ok_multiple() { - let client_ids = vec![ - ClientId::from_str("mockclient1").unwrap(), - ClientId::from_str("mockclient2").unwrap(), - ClientId::from_str("mockclient3").unwrap(), - ]; - let signer = get_dummy_account_id(); - let initial_height = Height::new(0, 45).unwrap(); - let update_height = Height::new(0, 49).unwrap(); - - let mut ctx = MockContext::default(); - - for cid in &client_ids { - ctx = ctx.with_client(cid, initial_height); - } - - for cid in &client_ids { - let msg = MsgUpdateClient { - client_id: cid.clone(), - header: MockHeader::new(update_height).into(), - signer: signer.clone(), - }; - - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg.clone())); - - match output { - Ok(HandlerOutput { - result: _, - events: _, - log, - }) => { - assert!(log.is_empty()); - } - Err(err) => { - panic!("unexpected error: {:?}", err); - } - } - } + assert!(res.is_err()); } #[test] @@ -366,7 +300,7 @@ mod tests { let update_height = Height::new(1, 21).unwrap(); let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); - let ctx = MockContext::new( + let mut ctx = MockContext::new( ChainId::new("mockgaiaA".to_string(), 1), HostType::Mock, 5, @@ -389,34 +323,20 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { - client_id: client_id.clone(), + client_id, header: block.into(), signer, }; - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg)); - - match output { - Ok(HandlerOutput { - result, - events: _, - log, - }) => { - assert!(log.is_empty()); - // Check the result - match result { - Update(upd_res) => { - assert_eq!(upd_res.client_id, client_id); - assert!(!upd_res.client_state.is_frozen()); - assert_eq!(upd_res.client_state.latest_height(), latest_header_height,) - } - _ => panic!("update handler result has incorrect type"), - } - } - Err(err) => { - panic!("unexpected error: {:?}", err); - } - } + let res = validate(&ctx, msg.clone()); + assert!(res.is_ok()); + + let res = execute(&mut ctx, msg.clone()); + assert!(res.is_ok(), "result: {res:?}"); + + let client_state = ctx.client_state(&msg.client_id).unwrap(); + assert!(!client_state.is_frozen()); + assert_eq!(client_state.latest_height(), latest_header_height); } #[test] @@ -426,7 +346,7 @@ mod tests { let update_height = Height::new(1, 21).unwrap(); let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); - let ctx = MockContext::new( + let mut ctx = MockContext::new( ChainId::new("mockgaiaA".to_string(), 1), HostType::Mock, 5, @@ -450,34 +370,20 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { - client_id: client_id.clone(), + client_id, header: block.into(), signer, }; - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg)); - - match output { - Ok(HandlerOutput { - result, - events: _, - log, - }) => { - assert!(log.is_empty()); - // Check the result - match result { - Update(upd_res) => { - assert_eq!(upd_res.client_id, client_id); - assert!(!upd_res.client_state.is_frozen()); - assert_eq!(upd_res.client_state.latest_height(), latest_header_height,) - } - _ => panic!("update handler result has incorrect type"), - } - } - Err(err) => { - panic!("unexpected error: {:?}", err); - } - } + let res = validate(&ctx, msg.clone()); + assert!(res.is_ok()); + + let res = execute(&mut ctx, msg.clone()); + assert!(res.is_ok(), "result: {res:?}"); + + let client_state = ctx.client_state(&msg.client_id).unwrap(); + assert!(!client_state.is_frozen()); + assert_eq!(client_state.latest_height(), latest_header_height); } #[test] @@ -487,7 +393,7 @@ mod tests { let chain_start_height = Height::new(1, 11).unwrap(); - let ctx = MockContext::new( + let mut ctx = MockContext::new( ChainId::new("mockgaiaA".to_string(), 1), HostType::Mock, 5, @@ -525,35 +431,21 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { - client_id: client_id.clone(), + client_id, header: block.into(), signer, }; - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg)); - - match output { - Ok(HandlerOutput { - result, - events: _, - log, - }) => { - assert!(log.is_empty()); - // Check the result - match result { - Update(upd_res) => { - assert_eq!(upd_res.client_id, client_id); - assert!(!upd_res.client_state.is_frozen()); - assert_eq!(upd_res.client_state, ctx.latest_client_states(&client_id)); - assert_eq!(upd_res.client_state.latest_height(), latest_header_height,) - } - _ => panic!("update handler result has incorrect type"), - } - } - Err(err) => { - panic!("unexpected error: {:?}", err); - } - } + let res = validate(&ctx, msg.clone()); + assert!(res.is_ok()); + + let res = execute(&mut ctx, msg.clone()); + assert!(res.is_ok(), "result: {res:?}"); + + let client_state = ctx.client_state(&msg.client_id).unwrap(); + assert!(!client_state.is_frozen()); + assert_eq!(client_state.latest_height(), latest_header_height); + assert_eq!(client_state, ctx.latest_client_states(&msg.client_id)); } #[test] @@ -595,17 +487,8 @@ mod tests { signer, }; - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg)); - - match output { - Ok(_) => { - panic!("update handler result has incorrect type"); - } - Err(err) => match err { - ClientError::HeaderVerificationFailure { reason: _ } => {} - _ => panic!("unexpected error: {:?}", err), - }, - } + let res = validate(&ctx, msg); + assert!(res.is_err()); } #[test] @@ -615,7 +498,7 @@ mod tests { let timestamp = Timestamp::now(); - let ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); + let mut ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); let height = Height::new(0, 46).unwrap(); let header: Any = MockHeader::new(height).with_timestamp(timestamp).into(); let msg = MsgUpdateClient { @@ -624,9 +507,11 @@ mod tests { signer, }; - let output = dispatch(&ctx, ClientMsg::UpdateClient(msg)).unwrap(); + let res = execute(&mut ctx, msg); + assert!(res.is_ok()); + let update_client_event = - downcast!(output.events.first().unwrap() => IbcEvent::UpdateClient).unwrap(); + downcast!(ctx.events.first().unwrap() => IbcEvent::UpdateClient).unwrap(); assert_eq!(update_client_event.client_id(), &client_id); assert_eq!(update_client_event.client_type(), &mock_client_type()); diff --git a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs index 91ae7db9af..40245bca79 100644 --- a/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/upgrade_client.rs @@ -201,16 +201,16 @@ pub(crate) fn process( #[cfg(feature = "upgrade_client")] #[cfg(test)] mod tests { + use crate::core::ics02_client::handler::upgrade_client::execute; + use crate::core::ics24_host::path::ClientConsensusStatePath; + use crate::core::ValidationContext; use crate::events::IbcEvent; use crate::{downcast, prelude::*}; + use rstest::*; use core::str::FromStr; - use crate::core::ics02_client::error::ClientError; - use crate::core::ics02_client::handler::dispatch; - use crate::core::ics02_client::handler::ClientResult::Upgrade; use crate::core::ics02_client::msgs::upgrade_client::MsgUpgradeClient; - use crate::core::ics02_client::msgs::ClientMsg; use crate::core::ics24_host::identifier::ClientId; use crate::mock::client_state::client_type as mock_client_type; use crate::mock::client_state::MockClientState; @@ -220,132 +220,89 @@ mod tests { use crate::test_utils::get_dummy_account_id; use crate::Height; - #[test] - fn upgrade_client_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: MsgUpgradeClient, - want_pass: bool, - } - let client_id = ClientId::default(); - let signer = get_dummy_account_id(); - let ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); - let tests: Vec = vec![ - Test { - name: "Processing succeeds".to_string(), - ctx: ctx.clone(), - msg: MsgUpgradeClient { - client_id: client_id.clone(), - client_state: MockClientState::new(MockHeader::new( - Height::new(1, 26).unwrap(), - )) - .into(), - consensus_state: MockConsensusState::new(MockHeader::new( - Height::new(1, 26).unwrap(), - )) - .into(), - proof_upgrade_client: Default::default(), - proof_upgrade_consensus_state: Default::default(), - signer: signer.clone(), - }, - want_pass: true, - }, - Test { - name: "Processing fails for non existing client".to_string(), - ctx: ctx.clone(), - msg: MsgUpgradeClient { - client_id: ClientId::from_str("nonexistingclient").unwrap(), - client_state: MockClientState::new(MockHeader::new( - Height::new(1, 26).unwrap(), - )) - .into(), - consensus_state: MockConsensusState::new(MockHeader::new( - Height::new(1, 26).unwrap(), - )) - .into(), - proof_upgrade_client: Default::default(), - proof_upgrade_consensus_state: Default::default(), - signer: signer.clone(), - }, - want_pass: false, - }, - Test { - name: "Processing fails for low upgrade height".to_string(), - ctx, - msg: MsgUpgradeClient { - client_id: client_id.clone(), - client_state: MockClientState::new(MockHeader::new( - Height::new(0, 26).unwrap(), - )) - .into(), - consensus_state: MockConsensusState::new(MockHeader::new( - Height::new(0, 26).unwrap(), - )) - .into(), - proof_upgrade_client: Default::default(), - proof_upgrade_consensus_state: Default::default(), - signer, - }, - want_pass: false, - }, - ] - .into_iter() - .collect(); - - for test in tests { - let output = dispatch(&test.ctx, ClientMsg::UpgradeClient(test.msg.clone())); - let test_name = test.name; - match (test.want_pass, output) { - (true, Ok(output)) => { - let upgrade_client_event = - downcast!(output.events.first().unwrap() => IbcEvent::UpgradeClient) - .unwrap(); - assert_eq!(upgrade_client_event.client_id(), &client_id); - assert_eq!(upgrade_client_event.client_type(), &mock_client_type()); - assert_eq!( - upgrade_client_event.consensus_height(), - &Height::new(1, 26).unwrap() - ); - assert!(output.log.is_empty()); - match output.result { - Upgrade(upg_res) => { - assert_eq!(upg_res.client_id, client_id); - assert_eq!( - upg_res.client_state.as_ref().clone_into(), - test.msg.client_state - ); - assert_eq!( - upg_res.consensus_state.as_ref().clone_into(), - test.msg.consensus_state - ); - } - _ => panic!("upgrade handler result has incorrect type"), - } - } - (true, Err(e)) => panic!("unexpected error for test {test_name}, {e:?}"), - (false, Err(e)) => match e { - ClientError::ClientNotFound { client_id } => { - assert_eq!(client_id, test.msg.client_id) - } - ClientError::LowUpgradeHeight { - upgraded_height, - client_height, - } => { - assert_eq!(upgraded_height, Height::new(0, 42).unwrap()); - assert_eq!( - client_height, - MockClientState::try_from(test.msg.client_state) - .unwrap() - .latest_height() - ); - } - _ => panic!("unexpected error for test {test_name}, {e:?}"), - }, - (false, Ok(e)) => { - panic!("unexpected success for test {test_name}, result: {e:?}") - } - } - } + use super::validate; + + pub struct Fixture { + pub ctx: MockContext, + pub msg: MsgUpgradeClient, + } + + #[fixture] + fn fixture() -> Fixture { + let ctx = + MockContext::default().with_client(&ClientId::default(), Height::new(0, 42).unwrap()); + + let msg = MsgUpgradeClient { + client_id: ClientId::default(), + client_state: MockClientState::new(MockHeader::new(Height::new(1, 26).unwrap())).into(), + consensus_state: MockConsensusState::new(MockHeader::new(Height::new(1, 26).unwrap())) + .into(), + proof_upgrade_client: Default::default(), + proof_upgrade_consensus_state: Default::default(), + signer: get_dummy_account_id(), + }; + + Fixture { ctx, msg } + } + + #[rstest] + fn upgrade_client_ok(fixture: Fixture) { + let Fixture { mut ctx, msg } = fixture; + + let res = validate(&ctx, msg.clone()); + assert!(res.is_ok(), "validation happy path"); + + let res = execute(&mut ctx, msg.clone()); + assert!(res.is_ok(), "execution happy path"); + + let upgrade_client_event = + downcast!(ctx.events.first().unwrap() => IbcEvent::UpgradeClient).unwrap(); + assert_eq!(upgrade_client_event.client_id(), &msg.client_id); + assert_eq!(upgrade_client_event.client_type(), &mock_client_type()); + assert_eq!( + upgrade_client_event.consensus_height(), + &Height::new(1, 26).unwrap() + ); + + let client_state = ctx.client_state(&msg.client_id).unwrap(); + assert_eq!(client_state.as_ref().clone_into(), msg.client_state); + + let consensus_state = ctx + .consensus_state(&ClientConsensusStatePath { + client_id: msg.client_id, + epoch: 1, + height: 26, + }) + .unwrap(); + assert_eq!(consensus_state.as_ref().clone_into(), msg.consensus_state); + } + + #[rstest] + fn upgrade_client_fail_nonexisting_client(fixture: Fixture) { + let Fixture { ctx, mut msg } = fixture; + + msg.client_id = ClientId::from_str("nonexistingclient").unwrap(); + + let res = validate(&ctx, msg); + assert!( + res.is_err(), + "validation fails because the client is non-existing" + ); + } + + #[rstest] + fn upgrade_client_fail_low_upgrade_height(fixture: Fixture) { + let Fixture { ctx, mut msg } = fixture; + + msg.client_state = + MockClientState::new(MockHeader::new(Height::new(0, 26).unwrap())).into(); + msg.consensus_state = + MockConsensusState::new(MockHeader::new(Height::new(0, 26).unwrap())).into(); + + let res = validate(&ctx, msg); + assert!( + res.is_err(), + "validation fails because the upgrade height is too low" + ); } } diff --git a/crates/ibc/src/core/ics04_channel/context.rs b/crates/ibc/src/core/ics04_channel/context.rs index 638eb24667..21d0dbdec0 100644 --- a/crates/ibc/src/core/ics04_channel/context.rs +++ b/crates/ibc/src/core/ics04_channel/context.rs @@ -6,6 +6,7 @@ use crate::core::ics24_host::path::{ AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqAckPath, SeqRecvPath, SeqSendPath, }; +use crate::core::{ContextError, ValidationContext}; use core::time::Duration; use num_traits::float::FloatCore; @@ -151,21 +152,22 @@ pub trait ChannelReader { pub trait SendPacketReader { /// Returns the ChannelEnd for the given `port_id` and `chan_id`. - fn channel_end(&self, channel_end_path: &ChannelEndPath) -> Result; + fn channel_end(&self, channel_end_path: &ChannelEndPath) -> Result; /// Returns the ConnectionState for the given identifier `connection_id`. - fn connection_end(&self, connection_id: &ConnectionId) -> Result; + fn connection_end(&self, connection_id: &ConnectionId) -> Result; /// Returns the ClientState for the given identifier `client_id`. Necessary dependency towards /// proof verification. - fn client_state(&self, client_id: &ClientId) -> Result, PacketError>; + fn client_state(&self, client_id: &ClientId) -> Result, ContextError>; fn client_consensus_state( &self, client_cons_state_path: &ClientConsensusStatePath, - ) -> Result, PacketError>; + ) -> Result, ContextError>; - fn get_next_sequence_send(&self, seq_send_path: &SeqSendPath) -> Result; + fn get_next_sequence_send(&self, seq_send_path: &SeqSendPath) + -> Result; fn hash(&self, value: &[u8]) -> Vec; @@ -192,34 +194,36 @@ pub trait SendPacketReader { impl SendPacketReader for T where - T: ChannelReader, + T: ValidationContext, { - fn channel_end(&self, chan_end_path: &ChannelEndPath) -> Result { - ChannelReader::channel_end(self, chan_end_path).map_err(PacketError::Channel) + fn channel_end(&self, channel_end_path: &ChannelEndPath) -> Result { + self.channel_end(channel_end_path) } - fn connection_end(&self, connection_id: &ConnectionId) -> Result { - ChannelReader::connection_end(self, connection_id).map_err(PacketError::Channel) + fn connection_end(&self, connection_id: &ConnectionId) -> Result { + self.connection_end(connection_id) } - fn client_state(&self, client_id: &ClientId) -> Result, PacketError> { - ChannelReader::client_state(self, client_id).map_err(PacketError::Channel) + fn client_state(&self, client_id: &ClientId) -> Result, ContextError> { + self.client_state(client_id) } fn client_consensus_state( &self, client_cons_state_path: &ClientConsensusStatePath, - ) -> Result, PacketError> { - ChannelReader::client_consensus_state(self, client_cons_state_path) - .map_err(PacketError::Channel) + ) -> Result, ContextError> { + self.consensus_state(client_cons_state_path) } - fn get_next_sequence_send(&self, seq_send_path: &SeqSendPath) -> Result { - ChannelReader::get_next_sequence_send(self, seq_send_path) + fn get_next_sequence_send( + &self, + seq_send_path: &SeqSendPath, + ) -> Result { + self.get_next_sequence_send(seq_send_path) } fn hash(&self, value: &[u8]) -> Vec { - ChannelReader::hash(self, value) + self.hash(value) } } diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index e4148230e7..782e63e82a 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -268,6 +268,12 @@ pub(crate) fn process( #[cfg(test)] mod tests { + use crate::core::ics04_channel::handler::acknowledgement::validate; + use crate::core::ics24_host::identifier::ChannelId; + use crate::core::ics24_host::identifier::PortId; + use crate::core::ValidationContext; + use crate::prelude::*; + use rstest::*; use test_log::test; use crate::core::ics02_client::height::Height; @@ -276,26 +282,25 @@ mod tests { use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics03_connection::version::get_compatible_versions; use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, Order, State}; - use crate::core::ics04_channel::context::ChannelReader; - use crate::core::ics04_channel::handler::acknowledgement::process; + use crate::core::ics04_channel::commitment::PacketCommitment; use crate::core::ics04_channel::msgs::acknowledgement::test_util::get_dummy_raw_msg_acknowledgement; use crate::core::ics04_channel::msgs::acknowledgement::MsgAcknowledgement; use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; - use crate::events::IbcEvent; use crate::mock::context::MockContext; - use crate::prelude::*; use crate::timestamp::ZERO_DURATION; - #[test] - fn ack_packet_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: MsgAcknowledgement, - want_pass: bool, - } + pub struct Fixture { + pub context: MockContext, + pub client_height: Height, + pub msg: MsgAcknowledgement, + pub packet_commitment: PacketCommitment, + pub conn_end_on_a: ConnectionEnd, + pub chan_end_on_a: ChannelEnd, + } + #[fixture] + fn fixture() -> Fixture { let context = MockContext::default(); let client_height = Height::new(0, 2).unwrap(); @@ -306,7 +311,7 @@ mod tests { .unwrap(); let packet = msg.packet.clone(); - let data = context.packet_commitment( + let packet_commitment = context.packet_commitment( &packet.data, &packet.timeout_height_on_b, &packet.timeout_timestamp_on_b, @@ -315,7 +320,7 @@ mod tests { let chan_end_on_a = ChannelEnd::new( State::Open, Order::default(), - Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b.clone())), + Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b)), vec![ConnectionId::default()], Version::new("ics20-1".to_string()), ); @@ -332,67 +337,79 @@ mod tests { ZERO_DURATION, ); - let tests: Vec = vec![ - Test { - name: "Processing fails because no channel exists in the context".to_string(), - ctx: context.clone(), - msg: msg.clone(), - want_pass: false, - }, - Test { - name: "Good parameters".to_string(), - ctx: context - .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), conn_end_on_a) - .with_channel( - packet.port_on_a.clone(), - packet.chan_on_a.clone(), - chan_end_on_a, - ) - .with_packet_commitment( - packet.port_on_a, - packet.chan_on_a, - packet.sequence, - data, - ) //with_ack_sequence required for ordered channels - .with_ack_sequence(packet.port_on_b, packet.chan_on_b, 1.into()), - msg, - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - let res = process(&test.ctx, &test.msg); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "ack_packet: 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. - - for e in proto_output.events.iter() { - assert!(matches!(e, &IbcEvent::AcknowledgePacket(_))); - } - } - Err(e) => { - assert!( - !test.want_pass, - "ack_packet: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg.clone(), - test.ctx.clone(), - e, - ); - } - } + Fixture { + context, + client_height, + msg, + packet_commitment, + conn_end_on_a, + chan_end_on_a, } } + + #[rstest] + fn ack_fail_no_channel(fixture: Fixture) { + let Fixture { context, msg, .. } = fixture; + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because no channel exists in the context" + ) + } + + /// NO-OP case + #[rstest] + fn ack_success_no_packet_commitment(fixture: Fixture) { + let Fixture { + context, + msg, + conn_end_on_a, + chan_end_on_a, + client_height, + .. + } = fixture; + let context = context + .with_client(&ClientId::default(), client_height) + .with_channel(PortId::default(), ChannelId::default(), chan_end_on_a) + .with_connection(ConnectionId::default(), conn_end_on_a); + + let res = validate(&context, &msg); + + assert!( + res.is_ok(), + "Validation should succeed when no packet commitment is present" + ) + } + + #[rstest] + fn ack_success_happy_path(fixture: Fixture) { + let Fixture { + context, + msg, + packet_commitment, + conn_end_on_a, + chan_end_on_a, + client_height, + .. + } = fixture; + let context = context + .with_client(&ClientId::default(), client_height) + .with_channel(PortId::default(), ChannelId::default(), chan_end_on_a) + .with_connection(ConnectionId::default(), conn_end_on_a) + .with_packet_commitment( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + packet_commitment, + ); + + let res = validate(&context, &msg); + + assert!( + res.is_ok(), + "Happy path: validation should succeed. err: {res:?}" + ) + } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index 1b1eaa9c1f..adb4df7dd5 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -202,10 +202,9 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::msgs::chan_close_confirm::test_util::get_dummy_raw_msg_chan_close_confirm; use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm; - use crate::core::ics04_channel::msgs::ChannelMsg; + use crate::core::ValidationContext; use crate::prelude::*; use crate::core::ics03_connection::connection::ConnectionEnd; @@ -216,7 +215,6 @@ mod tests { use crate::core::ics04_channel::channel::{ ChannelEnd, Counterparty, Order, State as ChannelState, }; - use crate::core::ics04_channel::handler::channel_dispatch; use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; @@ -224,6 +222,8 @@ mod tests { use crate::mock::context::MockContext; use crate::timestamp::ZERO_DURATION; + use super::validate; + #[test] fn chan_close_confirm_event_height() { let client_id = ClientId::new(mock_client_type(), 24).unwrap(); @@ -264,6 +264,10 @@ mod tests { chan_end, ); - channel_dispatch(&context, &ChannelMsg::CloseConfirm(msg_chan_close_confirm)).unwrap(); + let res = validate(&context, &msg_chan_close_confirm); + assert!( + res.is_ok(), + "Validation expected to succeed (happy path). Error: {res:?}" + ); } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs index 9472270292..3aab224680 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs @@ -101,10 +101,9 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::msgs::chan_close_init::test_util::get_dummy_raw_msg_chan_close_init; use crate::core::ics04_channel::msgs::chan_close_init::MsgChannelCloseInit; - use crate::core::ics04_channel::msgs::ChannelMsg; + use crate::core::ValidationContext; use crate::prelude::*; use crate::core::ics03_connection::connection::ConnectionEnd; @@ -115,7 +114,6 @@ mod tests { use crate::core::ics04_channel::channel::{ ChannelEnd, Counterparty, Order, State as ChannelState, }; - use crate::core::ics04_channel::handler::channel_dispatch; use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; @@ -123,6 +121,8 @@ mod tests { use crate::mock::context::MockContext; use crate::timestamp::ZERO_DURATION; + use super::validate; + #[test] fn chan_close_init_event_height() { let client_id = ClientId::new(mock_client_type(), 24).unwrap(); @@ -164,6 +164,10 @@ mod tests { ) }; - channel_dispatch(&context, &ChannelMsg::CloseInit(msg_chan_close_init)).unwrap(); + let res = validate(&context, &msg_chan_close_init); + assert!( + res.is_ok(), + "Validation expected to succeed (happy path). Error: {res:?}" + ); } } 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 19f7f35a3b..5a0eff9fd2 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 @@ -206,206 +206,185 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use core::str::FromStr; + use crate::core::ics03_connection::msgs::test_util::get_dummy_raw_counterparty; + use crate::core::ics04_channel::channel::Order; + use crate::core::ics04_channel::handler::chan_open_ack::validate; + use crate::core::ics24_host::identifier::ClientId; + use crate::prelude::*; + use crate::timestamp::ZERO_DURATION; + use rstest::*; use test_log::test; 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::MsgConnectionOpenInit; - 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}; - use crate::core::ics04_channel::handler::channel_dispatch; use crate::core::ics04_channel::msgs::chan_open_ack::test_util::get_dummy_raw_msg_chan_open_ack; use crate::core::ics04_channel::msgs::chan_open_ack::MsgChannelOpenAck; - 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::ics24_host::identifier::ConnectionId; + use crate::mock::client_state::client_type as mock_client_type; use crate::mock::context::MockContext; - use crate::prelude::*; use crate::Height; - // TODO: The tests here are very fragile and complex. - // Should be adapted to use the same structure as `handler::chan_open_try::tests`. - #[test] - fn chan_open_ack_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ChannelMsg, - want_pass: bool, - } - let proof_height = 10; - let client_consensus_state_height = 10; - let host_chain_height = Height::new(0, 35).unwrap(); + pub struct Fixture { + pub context: MockContext, + pub msg: MsgChannelOpenAck, + pub client_id_on_a: ClientId, + pub conn_id_on_a: ConnectionId, + pub conn_end_on_a: ConnectionEnd, + pub chan_end_on_a: ChannelEnd, + pub proof_height: u64, + } + #[fixture] + fn fixture() -> Fixture { + let proof_height = 10; let context = MockContext::default(); - let msg_conn_init = MsgConnectionOpenInit::new_dummy(); - - let conn_end = ConnectionEnd::new( + let client_id_on_a = ClientId::new(mock_client_type(), 45).unwrap(); + let conn_id_on_a = ConnectionId::new(2); + let conn_end_on_a = ConnectionEnd::new( ConnectionState::Open, - msg_conn_init.client_id_on_a.clone(), - ConnectionCounterparty::new( - msg_conn_init.counterparty.client_id().clone(), - Some(ConnectionId::from_str("defaultConnection-1").unwrap()), - msg_conn_init.counterparty.prefix().clone(), - ), + client_id_on_a.clone(), + ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), get_compatible_versions(), - msg_conn_init.delay_period, + ZERO_DURATION, ); - let ccid = ::from_str("defaultConnection-0"); - let cid = match ccid { - Ok(v) => v, - Err(_e) => ConnectionId::default(), - }; - - let mut connection_vec0 = Vec::new(); - connection_vec0.insert( - 0, - match ::from_str("defaultConnection-0") { - Ok(a) => a, - _ => unreachable!(), - }, - ); - - let msg_conn_try = MsgConnectionOpenTry::new_dummy( - client_consensus_state_height, - host_chain_height.revision_height(), - ); - - let msg_chan_ack = + let msg = MsgChannelOpenAck::try_from(get_dummy_raw_msg_chan_open_ack(proof_height)).unwrap(); - let msg_chan_try = - MsgChannelOpenTry::try_from(get_dummy_raw_msg_chan_open_try(proof_height)).unwrap(); - - let chan_end = ChannelEnd::new( + let chan_end_on_a = ChannelEnd::new( State::Init, - 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_supported_on_a.clone(), + Order::Unordered, + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b.clone())), + vec![conn_id_on_a.clone()], + msg.version_on_b.clone(), ); - let failed_chan_end = ChannelEnd::new( + Fixture { + context, + msg, + client_id_on_a, + conn_id_on_a, + conn_end_on_a, + chan_end_on_a, + proof_height, + } + } + + #[rstest] + fn chan_open_ack_fail_no_channel(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_a, + conn_id_on_a, + conn_end_on_a, + proof_height, + .. + } = fixture; + let context = context + .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_a, conn_end_on_a); + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because no channel exists in the context" + ) + } + + #[rstest] + fn chan_open_ack_fail_channel_wrong_state(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_a, + conn_id_on_a, + conn_end_on_a, + proof_height, + .. + } = fixture; + + let wrong_chan_end = ChannelEnd::new( State::Open, - 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_supported_on_a, + Order::Unordered, + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b.clone())), + vec![conn_id_on_a.clone()], + msg.version_on_b.clone(), ); + let context = context + .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_a, conn_end_on_a) + .with_channel( + msg.port_id_on_a.clone(), + msg.chan_id_on_a.clone(), + wrong_chan_end, + ); + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because channel is in the wrong state" + ) + } - let tests: Vec = vec![ - Test { - name: "Processing fails because no channel exists in the context".to_string(), - ctx: context.clone(), - msg: ChannelMsg::OpenAck(msg_chan_ack.clone()), - want_pass: false, - }, - Test { - name: "Processing fails because the channel is in the wrong state".to_string(), - ctx: context - .clone() - .with_client( - &msg_conn_try.client_id_on_b, - Height::new(0, client_consensus_state_height).unwrap(), - ) - .with_channel( - msg_chan_ack.port_id_on_a.clone(), - msg_chan_ack.chan_id_on_a.clone(), - failed_chan_end, - ), - msg: ChannelMsg::OpenAck(msg_chan_ack.clone()), - want_pass: false, - }, - Test { - name: "Processing fails because a connection does exist".to_string(), - ctx: context - .clone() - .with_client( - &msg_conn_try.client_id_on_b, - Height::new(0, client_consensus_state_height).unwrap(), - ) - .with_channel( - msg_chan_ack.port_id_on_a.clone(), - msg_chan_ack.chan_id_on_a.clone(), - chan_end.clone(), - ), - msg: ChannelMsg::OpenAck(msg_chan_ack.clone()), - want_pass: false, - }, - Test { - name: "Processing fails due to missing client state ".to_string(), - ctx: context - .clone() - .with_connection(cid.clone(), conn_end.clone()) - .with_channel( - msg_chan_ack.port_id_on_a.clone(), - msg_chan_ack.chan_id_on_a.clone(), - chan_end.clone(), - ), - msg: ChannelMsg::OpenAck(msg_chan_ack.clone()), - want_pass: false, - }, - Test { - name: "Good parameters".to_string(), - ctx: context // .clone() - .with_client( - &msg_conn_try.client_id_on_b, - Height::new(0, client_consensus_state_height).unwrap(), - ) - .with_connection(cid, conn_end) - .with_channel( - msg_chan_ack.port_id_on_a.clone(), - msg_chan_ack.chan_id_on_a.clone(), - chan_end, - ), - msg: ChannelMsg::OpenAck(msg_chan_ack), - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - let res = channel_dispatch(&test.ctx, &test.msg); - // Additionally check the events and the output objects in the result. - match res { - Ok((_, res)) => { - assert!( - test.want_pass, - "chan_open_ack: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg, - test.ctx.clone() - ); - - // The object in the output is a ConnectionEnd, should have init state. - //assert_eq!(res.channel_id, msg_chan_init.channel_id().clone()); - assert_eq!(res.channel_end.state().clone(), State::Open); - } - Err(e) => { - assert!( - !test.want_pass, - "chan_open_ack: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } - } - } + #[rstest] + fn chan_open_ack_fail_no_connection(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_a, + chan_end_on_a, + proof_height, + .. + } = fixture; + + let context = context + .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) + .with_channel( + msg.port_id_on_a.clone(), + msg.chan_id_on_a.clone(), + chan_end_on_a, + ); + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because no connection exists in the context" + ) + } + + #[rstest] + fn chan_open_ack_happy_path(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_a, + conn_id_on_a, + conn_end_on_a, + chan_end_on_a, + proof_height, + .. + } = fixture; + + let context = context + .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_a, conn_end_on_a) + .with_channel( + msg.port_id_on_a.clone(), + msg.chan_id_on_a.clone(), + chan_end_on_a, + ); + + let res = validate(&context, &msg); + + assert!(res.is_ok(), "Validation happy path") } } diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index e4b8bdfa99..d8f096a2a6 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -201,21 +201,20 @@ pub(crate) fn process( #[cfg(test)] mod tests { + use crate::core::ics04_channel::handler::chan_open_confirm::validate; + use crate::core::ics24_host::identifier::ChannelId; use crate::prelude::*; - + use rstest::*; use test_log::test; 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::context::ConnectionReader; 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, Counterparty, Order, State}; - use crate::core::ics04_channel::handler::channel_dispatch; use crate::core::ics04_channel::msgs::chan_open_confirm::test_util::get_dummy_raw_msg_chan_open_confirm; use crate::core::ics04_channel::msgs::chan_open_confirm::MsgChannelOpenConfirm; - use crate::core::ics04_channel::msgs::ChannelMsg; use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; use crate::mock::client_state::client_type as mock_client_type; @@ -223,93 +222,137 @@ mod tests { use crate::timestamp::ZERO_DURATION; use crate::Height; - // TODO: The tests here should use the same structure as `handler::chan_open_try::tests`. - #[test] - fn chan_open_confirm_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ChannelMsg, - want_pass: bool, - } - let client_id = ClientId::new(mock_client_type(), 24).unwrap(); - let conn_id = ConnectionId::new(2); + pub struct Fixture { + pub context: MockContext, + pub msg: MsgChannelOpenConfirm, + pub client_id_on_b: ClientId, + pub conn_id_on_b: ConnectionId, + pub conn_end_on_b: ConnectionEnd, + pub chan_end_on_b: ChannelEnd, + pub proof_height: u64, + } + + #[fixture] + fn fixture() -> Fixture { + let proof_height = 10; let context = MockContext::default(); - let client_consensus_state_height = - context.host_current_height().unwrap().revision_height(); - // The connection underlying the channel we're trying to open. - let conn_end = ConnectionEnd::new( + let client_id_on_b = ClientId::new(mock_client_type(), 45).unwrap(); + let conn_id_on_b = ConnectionId::new(2); + let conn_end_on_b = ConnectionEnd::new( ConnectionState::Open, - client_id.clone(), + client_id_on_b.clone(), ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), get_compatible_versions(), ZERO_DURATION, ); - let msg_chan_confirm = MsgChannelOpenConfirm::try_from( - get_dummy_raw_msg_chan_open_confirm(client_consensus_state_height), - ) - .unwrap(); + let msg = + MsgChannelOpenConfirm::try_from(get_dummy_raw_msg_chan_open_confirm(proof_height)) + .unwrap(); - let chan_end = ChannelEnd::new( + let chan_end_on_b = ChannelEnd::new( State::TryOpen, - Order::default(), - Counterparty::new( - msg_chan_confirm.port_id_on_b.clone(), - Some(msg_chan_confirm.chan_id_on_b.clone()), - ), - vec![conn_id.clone()], + Order::Unordered, + Counterparty::new(msg.port_id_on_b.clone(), Some(ChannelId::default())), + vec![conn_id_on_b.clone()], Version::default(), ); - let tests: Vec = vec![Test { - name: "Good parameters".to_string(), - ctx: context - .with_client( - &client_id, - Height::new(0, client_consensus_state_height).unwrap(), - ) - .with_connection(conn_id, conn_end) - .with_channel( - msg_chan_confirm.port_id_on_b.clone(), - msg_chan_confirm.chan_id_on_b.clone(), - chan_end, - ), - msg: ChannelMsg::OpenConfirm(msg_chan_confirm), - want_pass: true, - }] - .into_iter() - .collect(); - - for test in tests { - let res = channel_dispatch(&test.ctx, &test.msg); - // Additionally check the events and the output objects in the result. - match res { - Ok((_, res)) => { - assert!( - test.want_pass, - "chan_open_confirm: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg, - test.ctx.clone() - ); - - // The object in the output is a ConnectionEnd, should have init state. - //assert_eq!(res.channel_id, msg_chan_init.channel_id().clone()); - assert_eq!(res.channel_end.state().clone(), State::Open); - } - Err(e) => { - assert!( - !test.want_pass, - "chan_open_ack: did not pass test: {}, \nparams {:?} {:?}\nerror: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } - } + Fixture { + context, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + chan_end_on_b, + proof_height, } } + + #[rstest] + fn chan_open_confirm_fail_no_channel(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + proof_height, + .. + } = fixture; + let context = context + .with_client(&client_id_on_b, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_b, conn_end_on_b); + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because no channel exists in the context" + ) + } + + #[rstest] + fn chan_open_confirm_fail_channel_wrong_state(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + proof_height, + .. + } = fixture; + + let wrong_chan_end = ChannelEnd::new( + State::Init, + Order::Unordered, + Counterparty::new(msg.port_id_on_b.clone(), Some(ChannelId::default())), + vec![conn_id_on_b.clone()], + Version::default(), + ); + let context = context + .with_client(&client_id_on_b, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_b, conn_end_on_b) + .with_channel( + msg.port_id_on_b.clone(), + ChannelId::default(), + wrong_chan_end, + ); + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because channel is in the wrong state" + ) + } + + #[rstest] + fn chan_open_confirm_happy_path(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + chan_end_on_b, + proof_height, + .. + } = fixture; + + let context = context + .with_client(&client_id_on_b, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_b, conn_end_on_b) + .with_channel( + msg.port_id_on_b.clone(), + ChannelId::default(), + chan_end_on_b, + ); + + let res = validate(&context, &msg); + + assert!(res.is_ok(), "Validation happy path") + } } 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 817c2153a5..77f0cfe529 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 @@ -92,7 +92,9 @@ pub(crate) fn process( #[cfg(test)] mod tests { + use crate::core::ics04_channel::handler::chan_open_init::validate; use crate::prelude::*; + use rstest::*; use test_log::test; @@ -100,34 +102,26 @@ mod tests { use crate::core::ics03_connection::connection::State as ConnectionState; 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; - use crate::core::ics04_channel::handler::channel_dispatch; use crate::core::ics04_channel::msgs::chan_open_init::test_util::get_dummy_raw_msg_chan_open_init; use crate::core::ics04_channel::msgs::chan_open_init::MsgChannelOpenInit; - use crate::core::ics04_channel::msgs::ChannelMsg; use crate::core::ics24_host::identifier::ConnectionId; use crate::mock::context::MockContext; - #[test] - fn chan_open_init_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ChannelMsg, - want_pass: bool, - } - - let msg_chan_init = - MsgChannelOpenInit::try_from(get_dummy_raw_msg_chan_open_init(None)).unwrap(); + pub struct Fixture { + pub context: MockContext, + pub msg: MsgChannelOpenInit, + pub conn_end_on_a: ConnectionEnd, + } - let msg_chan_init_with_counterparty_chan_id_some = - MsgChannelOpenInit::try_from(get_dummy_raw_msg_chan_open_init(Some(0))).unwrap(); + #[fixture] + fn fixture() -> Fixture { + let msg = MsgChannelOpenInit::try_from(get_dummy_raw_msg_chan_open_init(None)).unwrap(); let context = MockContext::default(); let msg_conn_init = MsgConnectionOpenInit::new_dummy(); - let init_conn_end = ConnectionEnd::new( + let conn_end_on_a = ConnectionEnd::new( ConnectionState::Init, msg_conn_init.client_id_on_a.clone(), msg_conn_init.counterparty.clone(), @@ -135,66 +129,56 @@ mod tests { msg_conn_init.delay_period, ); - let cid = ConnectionId::default(); - - let tests: Vec = vec![ - Test { - name: "Processing fails because no connection exists in the context".to_string(), - ctx: context.clone(), - msg: ChannelMsg::OpenInit(msg_chan_init.clone()), - want_pass: false, - }, - Test { - name: "Good parameters".to_string(), - ctx: context - .clone() - .with_connection(cid.clone(), init_conn_end.clone()), - msg: ChannelMsg::OpenInit(msg_chan_init), - want_pass: true, - }, - Test { - name: "Good parameters even if counterparty channel id is set some by relayer" - .to_string(), - ctx: context.with_connection(cid, init_conn_end), - msg: ChannelMsg::OpenInit(msg_chan_init_with_counterparty_chan_id_some), - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - let res = channel_dispatch(&test.ctx, &test.msg); - // Additionally check the events and the output objects in the result. - match res { - Ok((_, res)) => { - assert!( - test.want_pass, - "chan_open_init: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg, - test.ctx.clone() - ); - - // The object in the output is a ChannelEnd, should have init state. - assert_eq!(res.channel_end.state().clone(), State::Init); - let msg_init = test.msg; - - if let ChannelMsg::OpenInit(msg_init) = msg_init { - assert_eq!(res.port_id.clone(), msg_init.port_id_on_a.clone()); - } - } - Err(e) => { - assert!( - !test.want_pass, - "chan_open_init: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg, - test.ctx.clone(), - e, - ); - } - } + Fixture { + context, + msg, + conn_end_on_a, } } + + #[rstest] + fn chan_open_init_fail_no_connection(fixture: Fixture) { + let Fixture { context, msg, .. } = fixture; + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because no connection exists in the context" + ) + } + + #[rstest] + fn chan_open_init_success_happy_path(fixture: Fixture) { + let Fixture { + context, + msg, + conn_end_on_a, + } = fixture; + + let context = context.with_connection(ConnectionId::default(), conn_end_on_a); + + let res = validate(&context, &msg); + + assert!(res.is_ok(), "Validation succeeds; good parameters") + } + + #[rstest] + fn chan_open_init_success_counterparty_chan_id_set(fixture: Fixture) { + let Fixture { + context, + conn_end_on_a, + .. + } = fixture; + + let context = context.with_connection(ConnectionId::default(), conn_end_on_a); + let msg = MsgChannelOpenInit::try_from(get_dummy_raw_msg_chan_open_init(Some(0))).unwrap(); + + let res = validate(&context, &msg); + + assert!( + res.is_ok(), + "Validation succeeds even if counterparty channel id is set by relayer" + ) + } } 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 eba1adb407..794bf3146b 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 @@ -205,226 +205,118 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use crate::core::ics04_channel::handler::chan_open_try; - use crate::downcast; use crate::prelude::*; - + use rstest::*; use test_log::test; - use crate::core::ics02_client::error as ics02_error; + use crate::core::ics04_channel::handler::chan_open_try; + 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::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, 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; - use crate::core::ics04_channel::msgs::ChannelMsg; - use crate::core::ics04_channel::Version; - use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId}; + use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; use crate::mock::client_state::client_type as mock_client_type; use crate::mock::context::MockContext; use crate::timestamp::ZERO_DURATION; use crate::Height; - #[test] - fn chan_open_try_msg_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: ChannelMsg, - want_pass: bool, - match_error: Box, - } + pub struct Fixture { + pub context: MockContext, + pub msg: MsgChannelOpenTry, + pub client_id_on_b: ClientId, + pub conn_id_on_b: ConnectionId, + pub conn_end_on_b: ConnectionEnd, + pub proof_height: u64, + } - // Some general-purpose variable to parametrize the messages and the context. + #[fixture] + fn fixture() -> Fixture { let proof_height = 10; - let conn_id = ConnectionId::new(2); - let client_id = ClientId::new(mock_client_type(), 45).unwrap(); - - // The context. We'll reuse this same one across all tests. - let context = MockContext::default(); + let conn_id_on_b = ConnectionId::new(2); + let client_id_on_b = ClientId::new(mock_client_type(), 45).unwrap(); // This is the connection underlying the channel we're trying to open. - let conn_end = ConnectionEnd::new( + let conn_end_on_b = ConnectionEnd::new( ConnectionState::Open, - client_id.clone(), + client_id_on_b.clone(), ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), get_compatible_versions(), ZERO_DURATION, ); // We're going to test message processing against this message. + // 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(); - let chan_id = ChannelId::new(24); - let hops = vec![conn_id.clone()]; + let hops = vec![conn_id_on_b.clone()]; 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`. - let correct_chan_end = ChannelEnd::new( - State::Init, - msg.ordering, - Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), - msg.connection_hops_on_b.clone(), - Version::empty(), - ); + let context = MockContext::default(); - let tests: Vec = vec![ - Test { - name: "Processing fails because no connection exists in the context".to_string(), - ctx: context.clone(), - msg: ChannelMsg::OpenTry(msg.clone()), - want_pass: false, - match_error: { - let connection_id = msg.connection_hops_on_b[0].clone(); - Box::new(move |e| match e { - error::ChannelError::Connection(e) => { - assert_eq!( - e.to_string(), - ics03_error::ConnectionError::ConnectionNotFound { connection_id } - .to_string() - ); - } - _ => { - panic!("Expected MissingConnection, instead got {}", e) - } - }) - }, - }, - Test { - name: "Processing fails b/c the context has no client state".to_string(), - ctx: context - .clone() - .with_connection(conn_id.clone(), conn_end.clone()) - .with_channel( - msg.port_id_on_b.clone(), - chan_id.clone(), - correct_chan_end.clone(), - ), - msg: ChannelMsg::OpenTry(msg.clone()), - want_pass: false, - match_error: Box::new(|e| match e { - error::ChannelError::Connection(e) => { - assert_eq!( - e.to_string(), - ics03_error::ConnectionError::Client( - ics02_error::ClientError::ClientNotFound { - client_id: ClientId::new(mock_client_type(), 45).unwrap() - } - ) - .to_string() - ); - } - _ => { - panic!("Expected MissingClientState, instead got {}", e) - } - }), - }, - Test { - name: "Processing is successful".to_string(), - ctx: context - .clone() - .with_client(&client_id, Height::new(0, proof_height).unwrap()) - .with_connection(conn_id.clone(), conn_end.clone()) - .with_channel(msg.port_id_on_b.clone(), chan_id, correct_chan_end), - msg: ChannelMsg::OpenTry(msg.clone()), - want_pass: true, - match_error: Box::new(|_| {}), - }, - Test { - name: "Processing is successful against an empty context (no preexisting channel)" - .to_string(), - ctx: context - .with_client(&client_id, Height::new(0, proof_height).unwrap()) - .with_connection(conn_id, conn_end), - msg: ChannelMsg::OpenTry(msg), - want_pass: true, - match_error: Box::new(|_| {}), - }, - ] - .into_iter() - .collect(); - - for test in tests { - let test_msg = downcast!(test.msg => ChannelMsg::OpenTry).unwrap(); - let res = chan_open_try::process(&test.ctx, &test_msg); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "chan_open_ack: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test_msg, - test.ctx.clone() - ); - - // The object in the output is a channel end, should have TryOpen state. - assert_eq!( - proto_output.result.channel_end.state().clone(), - State::TryOpen - ); - } - Err(e) => { - assert!( - !test.want_pass, - "chan_open_try: did not pass test: {}, \nparams:\n\tmsg={:?}\n\tcontext={:?}\nerror: {:?}", - test.name, - test_msg, - test.ctx.clone(), - e, - ); - - (test.match_error)(e); - } - } + Fixture { + context, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + proof_height, } } - /// Addresses [issue 219](https://github.com/cosmos/ibc-rs/issues/219) - #[test] - fn chan_open_try_invalid_counterparty_channel_id() { - let proof_height = 10; - let conn_id = ConnectionId::new(2); - let client_id = ClientId::new(mock_client_type(), 45).unwrap(); + #[rstest] + fn chan_open_try_fail_no_connection(fixture: Fixture) { + let Fixture { context, msg, .. } = fixture; - // This is the connection underlying the channel we're trying to open. - let conn_end = ConnectionEnd::new( - ConnectionState::Open, - client_id.clone(), - ConnectionCounterparty::try_from(get_dummy_raw_counterparty(Some(0))).unwrap(), - get_compatible_versions(), - ZERO_DURATION, - ); + let res = chan_open_try::validate(&context, &msg); - // We're going to test message processing against this message. - // 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(); - - let chan_id = ChannelId::new(24); - let hops = vec![conn_id.clone()]; - msg.connection_hops_on_b = hops; + assert!( + res.is_err(), + "Validation fails because no connection exists in the context" + ) + } - let chan_end = ChannelEnd::new( - State::Init, - msg.ordering, - Counterparty::new(msg.port_id_on_a.clone(), None), - msg.connection_hops_on_b.clone(), - Version::empty(), - ); - let context = MockContext::default() - .with_client(&client_id, Height::new(0, proof_height).unwrap()) - .with_connection(conn_id, conn_end) - .with_channel(msg.port_id_on_b.clone(), chan_id, chan_end); + #[rstest] + fn chan_open_try_fail_no_client_state(fixture: Fixture) { + let Fixture { + context, + msg, + conn_id_on_b, + conn_end_on_b, + .. + } = fixture; + let context = context.with_connection(conn_id_on_b, conn_end_on_b); + + let res = chan_open_try::validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because the context has no client state" + ) + } - // Makes sure we don't crash - let _ = chan_open_try::process(&context, &msg); + #[rstest] + fn chan_open_try_happy_path(fixture: Fixture) { + let Fixture { + context, + msg, + client_id_on_b, + conn_id_on_b, + conn_end_on_b, + proof_height, + .. + } = fixture; + + let context = context + .with_client(&client_id_on_b, Height::new(0, proof_height).unwrap()) + .with_connection(conn_id_on_b, conn_end_on_b); + + let res = chan_open_try::validate(&context, &msg); + + assert!(res.is_ok(), "Validation success: happy path") } } diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index 7af276a255..3cec8733e5 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -344,7 +344,10 @@ pub(crate) fn process( #[cfg(test)] mod tests { + use crate::core::ics04_channel::handler::recv_packet::validate; use crate::prelude::*; + use crate::Height; + use rstest::*; use test_log::test; @@ -353,9 +356,9 @@ mod tests { use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics03_connection::version::get_compatible_versions; use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, Order, State}; - use crate::core::ics04_channel::handler::recv_packet::process; use crate::core::ics04_channel::msgs::recv_packet::test_util::get_dummy_raw_msg_recv_packet; use crate::core::ics04_channel::msgs::recv_packet::MsgRecvPacket; + use crate::core::ics04_channel::packet::Packet; use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; use crate::mock::context::MockContext; @@ -363,17 +366,18 @@ mod tests { use crate::test_utils::get_dummy_account_id; use crate::timestamp::Timestamp; use crate::timestamp::ZERO_DURATION; - use crate::{core::ics04_channel::packet::Packet, events::IbcEvent}; - - #[test] - fn recv_packet_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: MsgRecvPacket, - want_pass: bool, - } + pub struct Fixture { + pub context: MockContext, + pub client_height: Height, + pub host_height: Height, + pub msg: MsgRecvPacket, + pub conn_end_on_b: ConnectionEnd, + pub chan_end_on_b: ChannelEnd, + } + + #[fixture] + fn fixture() -> Fixture { let context = MockContext::default(); let host_height = context.query_latest_height().unwrap().increment(); @@ -387,24 +391,6 @@ mod tests { let packet = msg.packet.clone(); - let packet_old = Packet { - sequence: 1.into(), - port_on_a: PortId::default(), - chan_on_a: ChannelId::default(), - port_on_b: PortId::default(), - chan_on_b: ChannelId::default(), - data: Vec::new(), - timeout_height_on_b: client_height.into(), - timeout_timestamp_on_b: Timestamp::from_nanoseconds(1).unwrap(), - }; - - let msg_packet_old = MsgRecvPacket::new( - packet_old, - msg.proof_commitment_on_a.clone(), - msg.proof_height_on_a, - get_dummy_account_id(), - ); - let chan_end_on_b = ChannelEnd::new( State::Open, Order::default(), @@ -425,84 +411,107 @@ mod tests { ZERO_DURATION, ); - let tests: Vec = vec![ - Test { - name: "Processing fails because no channel exists in the context".to_string(), - ctx: context.clone(), - msg: msg.clone(), - want_pass: false, - }, - Test { - name: "Good parameters".to_string(), - ctx: context - .clone() - .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), conn_end_on_b.clone()) - .with_channel( - packet.port_on_b.clone(), - packet.chan_on_b.clone(), - chan_end_on_b.clone(), - ) - .with_send_sequence( - packet.port_on_b.clone(), - packet.chan_on_b.clone(), - 1.into(), - ) - .with_height(host_height) - // This `with_recv_sequence` is required for ordered channels - .with_recv_sequence( - packet.port_on_b.clone(), - packet.chan_on_b.clone(), - packet.sequence, - ), - msg, - want_pass: true, - }, - Test { - name: "Packet timeout expired".to_string(), - ctx: context - .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), conn_end_on_b) - .with_channel(PortId::default(), ChannelId::default(), chan_end_on_b) - .with_send_sequence(PortId::default(), ChannelId::default(), 1.into()) - .with_height(host_height), - msg: msg_packet_old, - want_pass: false, - }, - ] - .into_iter() - .collect(); - - for test in tests { - let res = process(&test.ctx, &test.msg); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "recv_packet: test passed but was supposed to fail for test: {}, \nparams \n msg={:?}\nctx:{:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ); - - assert!(!proto_output.events.is_empty()); // Some events must exist. - - for e in proto_output.events.iter() { - assert!(matches!(e, &IbcEvent::ReceivePacket(_))); - } - } - Err(e) => { - assert!( - !test.want_pass, - "recv_packet: did not pass test: {}, \nparams \nmsg={:?}\nctx={:?}\nerror={:?}", - test.name, - test.msg.clone(), - test.ctx.clone(), - e, - ); - } - } + Fixture { + context, + client_height, + host_height, + msg, + conn_end_on_b, + chan_end_on_b, } } + + #[rstest] + fn recv_packet_fail_no_channel(fixture: Fixture) { + let Fixture { context, msg, .. } = fixture; + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because no channel exists in the context" + ) + } + + #[rstest] + fn recv_packet_happy_path(fixture: Fixture) { + let Fixture { + context, + msg, + conn_end_on_b, + chan_end_on_b, + client_height, + host_height, + .. + } = fixture; + + let packet = msg.packet.clone(); + let context = context + .with_client(&ClientId::default(), client_height) + .with_connection(ConnectionId::default(), conn_end_on_b) + .with_channel( + packet.port_on_b.clone(), + packet.chan_on_b.clone(), + chan_end_on_b, + ) + .with_send_sequence(packet.port_on_b.clone(), packet.chan_on_b.clone(), 1.into()) + .with_height(host_height) + // This `with_recv_sequence` is required for ordered channels + .with_recv_sequence( + packet.port_on_b.clone(), + packet.chan_on_b.clone(), + packet.sequence, + ); + + let res = validate(&context, &msg); + + assert!( + res.is_ok(), + "Happy path: validation should succeed. err: {res:?}" + ) + } + + #[rstest] + fn recv_packet_timeout_expired(fixture: Fixture) { + let Fixture { + context, + msg, + conn_end_on_b, + chan_end_on_b, + client_height, + host_height, + .. + } = fixture; + + let packet_old = Packet { + sequence: 1.into(), + port_on_a: PortId::default(), + chan_on_a: ChannelId::default(), + port_on_b: PortId::default(), + chan_on_b: ChannelId::default(), + data: Vec::new(), + timeout_height_on_b: client_height.into(), + timeout_timestamp_on_b: Timestamp::from_nanoseconds(1).unwrap(), + }; + + let msg_packet_old = MsgRecvPacket::new( + packet_old, + msg.proof_commitment_on_a.clone(), + msg.proof_height_on_a, + get_dummy_account_id(), + ); + let context = context + .with_client(&ClientId::default(), client_height) + .with_connection(ConnectionId::default(), conn_end_on_b) + .with_channel(PortId::default(), ChannelId::default(), chan_end_on_b) + .with_send_sequence(PortId::default(), ChannelId::default(), 1.into()) + .with_height(host_height); + + let res = validate(&context, &msg_packet_old); + + assert!( + res.is_err(), + "recv_packet validation should fail when the packet has timed out" + ) + } } diff --git a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs index 723ad0e579..fa25bb77e2 100644 --- a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs @@ -8,6 +8,7 @@ use crate::core::ics24_host::identifier::{ChannelId, PortId}; use crate::core::ics24_host::path::ChannelEndPath; use crate::core::ics24_host::path::ClientConsensusStatePath; use crate::core::ics24_host::path::SeqSendPath; +use crate::core::ContextError; use crate::events::IbcEvent; use crate::handler::{HandlerOutput, HandlerResult}; use crate::prelude::*; @@ -26,7 +27,7 @@ pub struct SendPacketResult { pub fn send_packet( ctx_a: &impl SendPacketReader, packet: Packet, -) -> HandlerResult { +) -> HandlerResult { let mut output = HandlerOutput::builder(); let chan_end_path_on_a = ChannelEndPath::new(&packet.port_on_a, &packet.chan_on_a); @@ -35,7 +36,8 @@ pub fn send_packet( if chan_end_on_a.state_matches(&State::Closed) { return Err(PacketError::ChannelClosed { channel_id: packet.chan_on_a, - }); + } + .into()); } let counterparty = Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b.clone())); @@ -44,7 +46,8 @@ pub fn send_packet( return Err(PacketError::InvalidPacketCounterparty { port_id: packet.port_on_b.clone(), channel_id: packet.chan_on_b, - }); + } + .into()); } let conn_id_on_a = &chan_end_on_a.connection_hops()[0]; let conn_end_on_a = ctx_a.connection_end(conn_id_on_a)?; @@ -57,7 +60,8 @@ pub fn send_packet( if client_state_of_b_on_a.is_frozen() { return Err(PacketError::FrozenClient { client_id: conn_end_on_a.client_id().clone(), - }); + } + .into()); } let latest_height_on_a = client_state_of_b_on_a.latest_height(); @@ -66,7 +70,8 @@ pub fn send_packet( return Err(PacketError::LowPacketHeight { chain_height: latest_height_on_a, timeout_height: packet.timeout_height_on_b, - }); + } + .into()); } let client_cons_state_path_on_a = @@ -75,7 +80,7 @@ pub fn send_packet( let latest_timestamp = consensus_state_of_b_on_a.timestamp(); let packet_timestamp = packet.timeout_timestamp_on_b; if let Expiry::Expired = latest_timestamp.check_expiry(&packet_timestamp) { - return Err(PacketError::LowPacketTimestamp); + return Err(PacketError::LowPacketTimestamp.into()); } let seq_send_path_on_a = SeqSendPath::new(&packet.port_on_a, &packet.chan_on_a); @@ -85,7 +90,8 @@ pub fn send_packet( return Err(PacketError::InvalidPacketSequence { given_sequence: packet.sequence, next_sequence: next_seq_send_on_a, - }); + } + .into()); } output.log("success: packet send "); diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index 75241b0cb8..9f3f7089c9 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -328,7 +328,8 @@ pub(crate) fn process( #[cfg(test)] mod tests { - use test_log::test; + use crate::prelude::*; + use rstest::*; use crate::core::ics02_client::height::Height; use crate::core::ics03_connection::connection::ConnectionEnd; @@ -336,35 +337,35 @@ mod tests { use crate::core::ics03_connection::connection::State as ConnectionState; use crate::core::ics03_connection::version::get_compatible_versions; use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, Order, State}; - use crate::core::ics04_channel::context::ChannelReader; - use crate::core::ics04_channel::handler::timeout::process; + use crate::core::ics04_channel::commitment::PacketCommitment; + use crate::core::ics04_channel::handler::timeout::validate; use crate::core::ics04_channel::msgs::timeout::test_util::get_dummy_raw_msg_timeout; use crate::core::ics04_channel::msgs::timeout::MsgTimeout; use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; - use crate::core::ics24_host::path::ChannelEndPath; - use crate::events::IbcEvent; + use crate::core::ValidationContext; use crate::mock::context::MockContext; - use crate::prelude::*; use crate::timestamp::ZERO_DURATION; - #[test] - fn timeout_packet_processing() { - struct Test { - name: String, - ctx: MockContext, - msg: MsgTimeout, - want_pass: bool, - } + pub struct Fixture { + pub context: MockContext, + pub client_height: Height, + pub msg: MsgTimeout, + pub packet_commitment: PacketCommitment, + pub conn_end_on_a: ConnectionEnd, + pub chan_end_on_a_unordered: ChannelEnd, + pub chan_end_on_a_ordered: ChannelEnd, + } + #[fixture] + fn fixture() -> Fixture { let context = MockContext::default(); + let client_height = Height::new(0, 2).unwrap(); let msg_proof_height = 2; let msg_timeout_height = 5; let timeout_timestamp = 5; - let client_height = Height::new(0, 2).unwrap(); - let msg = MsgTimeout::try_from(get_dummy_raw_msg_timeout( msg_proof_height, msg_timeout_height, @@ -373,25 +374,22 @@ mod tests { .unwrap(); let packet = msg.packet.clone(); - let mut msg_ok = msg.clone(); - msg_ok.packet.timeout_timestamp_on_b = Default::default(); - - let data = context.packet_commitment( - &msg_ok.packet.data, - &msg_ok.packet.timeout_height_on_b, - &msg_ok.packet.timeout_timestamp_on_b, + let packet_commitment = context.packet_commitment( + &msg.packet.data, + &msg.packet.timeout_height_on_b, + &msg.packet.timeout_timestamp_on_b, ); - let chan_end_on_a = ChannelEnd::new( + let chan_end_on_a_unordered = ChannelEnd::new( State::Open, - Order::default(), - Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b.clone())), + Order::Unordered, + Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b)), vec![ConnectionId::default()], Version::new("ics20-1".to_string()), ); - let mut source_ordered_channel_end = chan_end_on_a.clone(); - source_ordered_channel_end.ordering = Order::Ordered; + let mut chan_end_on_a_ordered = chan_end_on_a_unordered.clone(); + chan_end_on_a_ordered.ordering = Order::Ordered; let conn_end_on_a = ConnectionEnd::new( ConnectionState::Open, @@ -405,128 +403,166 @@ mod tests { ZERO_DURATION, ); - let tests: Vec = vec![ - Test { - name: "Processing fails because no channel exists in the context".to_string(), - ctx: context.clone(), - msg: msg.clone(), - want_pass: false, - }, - Test { - name: "Processing fails because the client does not have a consensus state for the required height" - .to_string(), - ctx: context.clone().with_channel( - PortId::default(), - ChannelId::default(), - chan_end_on_a.clone(), - ) - .with_connection(ConnectionId::default(), conn_end_on_a.clone()), - msg: msg.clone(), - want_pass: false, - }, - Test { - name: "Processing fails because the proof's timeout has not been reached " - .to_string(), - ctx: context.clone().with_channel( - PortId::default(), - ChannelId::default(), - chan_end_on_a.clone(), - ) - .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), conn_end_on_a.clone()), - msg, - want_pass: false, - }, - Test { - name: "Good parameters Unordered channel".to_string(), - ctx: context.clone() - .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), conn_end_on_a.clone()) - .with_channel( - packet.port_on_a.clone(), - packet.chan_on_a.clone(), - chan_end_on_a, - ) - .with_packet_commitment( - msg_ok.packet.port_on_a.clone(), - msg_ok.packet.chan_on_a.clone(), - msg_ok.packet.sequence, - data.clone(), - ), - msg: msg_ok.clone(), - want_pass: true, - }, - Test { - name: "Good parameters Ordered Channel".to_string(), - ctx: context - .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), conn_end_on_a) - .with_channel( - packet.port_on_a.clone(), - packet.chan_on_a.clone(), - source_ordered_channel_end, - ) - .with_packet_commitment( - msg_ok.packet.port_on_a.clone(), - msg_ok.packet.chan_on_a.clone(), - msg_ok.packet.sequence, - data, - ) - .with_ack_sequence( - packet.port_on_b, - packet.chan_on_b, - 1.into(), - ), - msg: msg_ok, - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - let res = process(&test.ctx, &test.msg); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "TO_packet: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.msg.clone(), - test.ctx.clone() - ); - - let events = proto_output.events; - let src_channel_end = test - .ctx - .channel_end(&ChannelEndPath::new(&packet.port_on_a, &packet.chan_on_a)) - .unwrap(); - - if src_channel_end.order_matches(&Order::Ordered) { - assert_eq!(events.len(), 2); - - assert!(matches!(events[0], IbcEvent::TimeoutPacket(_))); - assert!(matches!(events[1], IbcEvent::ChannelClosed(_))); - } else { - assert_eq!(events.len(), 1); - assert!(matches!( - events.first().unwrap(), - &IbcEvent::TimeoutPacket(_) - )); - } - } - Err(e) => { - assert!( - !test.want_pass, - "timeout_packet: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.msg.clone(), - test.ctx.clone(), - e, - ); - } - } + Fixture { + context, + client_height, + msg, + packet_commitment, + conn_end_on_a, + chan_end_on_a_unordered, + chan_end_on_a_ordered, } } + + #[rstest] + fn timeout_fail_no_channel(fixture: Fixture) { + let Fixture { + context, + msg, + client_height, + .. + } = fixture; + let context = context.with_client(&ClientId::default(), client_height); + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because no channel exists in the context" + ) + } + + #[rstest] + fn timeout_fail_no_consensus_state_for_height(fixture: Fixture) { + let Fixture { + context, + msg, + chan_end_on_a_unordered, + conn_end_on_a, + packet_commitment, + .. + } = fixture; + + let packet = msg.packet.clone(); + + let context = context + .with_channel( + PortId::default(), + ChannelId::default(), + chan_end_on_a_unordered, + ) + .with_connection(ConnectionId::default(), conn_end_on_a) + .with_packet_commitment( + packet.port_on_a, + packet.chan_on_a, + packet.sequence, + packet_commitment, + ); + + let res = validate(&context, &msg); + + assert!( + res.is_err(), + "Validation fails because the client does not have a consensus state for the required height" + ) + } + + #[rstest] + #[ignore = "implement and make clear that the timeout is indeed not reached"] + fn timeout_fail_proof_timeout_not_reached(_fixture: Fixture) { + // TODO + } + + /// NO-OP case + #[rstest] + fn timeout_success_no_packet_commitment(fixture: Fixture) { + let Fixture { + context, + msg, + conn_end_on_a, + chan_end_on_a_unordered, + .. + } = fixture; + let context = context + .with_channel( + PortId::default(), + ChannelId::default(), + chan_end_on_a_unordered, + ) + .with_connection(ConnectionId::default(), conn_end_on_a); + + let res = validate(&context, &msg); + + assert!( + res.is_ok(), + "Validation should succeed when no packet commitment is present" + ) + } + + #[rstest] + fn timeout_success_unordered_channel(fixture: Fixture) { + let Fixture { + context, + msg, + chan_end_on_a_unordered, + conn_end_on_a, + packet_commitment, + client_height, + .. + } = fixture; + + let packet = msg.packet.clone(); + + let context = context + .with_client(&ClientId::default(), client_height) + .with_connection(ConnectionId::default(), conn_end_on_a) + .with_channel( + PortId::default(), + ChannelId::default(), + chan_end_on_a_unordered, + ) + .with_packet_commitment( + packet.port_on_a, + packet.chan_on_a, + packet.sequence, + packet_commitment, + ); + + let res = validate(&context, &msg); + + assert!(res.is_ok(), "Good parameters for unordered channels") + } + + #[rstest] + fn timeout_success_ordered_channel(fixture: Fixture) { + let Fixture { + context, + msg, + chan_end_on_a_ordered, + conn_end_on_a, + packet_commitment, + client_height, + .. + } = fixture; + + let packet = msg.packet.clone(); + + let context = context + .with_client(&ClientId::default(), client_height) + .with_connection(ConnectionId::default(), conn_end_on_a) + .with_channel( + PortId::default(), + ChannelId::default(), + chan_end_on_a_ordered, + ) + .with_packet_commitment( + packet.port_on_a, + packet.chan_on_a, + packet.sequence, + packet_commitment, + ); + + let res = validate(&context, &msg); + + assert!(res.is_ok(), "Good parameters for unordered channels") + } } diff --git a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs index 9e24ca2d52..fed5b1c2f2 100644 --- a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs @@ -79,123 +79,3 @@ pub fn process( Ok(output.with_result(result)) } - -#[cfg(test)] -mod tests { - use crate::prelude::*; - - use test_log::test; - - use crate::core::ics02_client::height::Height; - 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::version::get_compatible_versions; - use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, Order, State}; - use crate::core::ics04_channel::handler::write_acknowledgement::process; - use crate::core::ics04_channel::packet::test_utils::get_dummy_raw_packet; - use crate::core::ics04_channel::Version; - use crate::core::ics24_host::identifier::{ClientId, ConnectionId}; - use crate::mock::context::MockContext; - use crate::timestamp::ZERO_DURATION; - use crate::{core::ics04_channel::packet::Packet, events::IbcEvent}; - - #[test] - fn write_ack_packet_processing() { - struct Test { - name: String, - ctx: MockContext, - packet: Packet, - ack: Vec, - want_pass: bool, - } - - let context = MockContext::default(); - - let client_height = Height::new(0, 1).unwrap(); - - let mut packet: Packet = get_dummy_raw_packet(1, 6).try_into().unwrap(); - packet.sequence = 1.into(); - packet.data = vec![0]; - - let ack = vec![0]; - - let dest_channel_end = ChannelEnd::new( - State::Open, - Order::default(), - Counterparty::new(packet.port_on_a.clone(), Some(packet.chan_on_a.clone())), - vec![ConnectionId::default()], - Version::new("ics20-1".to_string()), - ); - - let connection_end = ConnectionEnd::new( - ConnectionState::Open, - ClientId::default(), - ConnectionCounterparty::new( - ClientId::default(), - Some(ConnectionId::default()), - Default::default(), - ), - get_compatible_versions(), - ZERO_DURATION, - ); - - let tests: Vec = vec![ - Test { - name: "Processing fails because no channel exists in the context".to_string(), - ctx: context.clone(), - packet: packet.clone(), - ack: ack.clone(), - want_pass: false, - }, - Test { - name: "Good parameters".to_string(), - ctx: context - .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), connection_end) - .with_channel( - packet.port_on_b.clone(), - packet.chan_on_b.clone(), - dest_channel_end, - ), - packet, - ack, - want_pass: true, - }, - ] - .into_iter() - .collect(); - - for test in tests { - let res = process(&test.ctx, test.packet.clone(), test.ack.try_into().unwrap()); - // Additionally check the events and the output objects in the result. - match res { - Ok(proto_output) => { - assert!( - test.want_pass, - "write_ack: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}", - test.name, - test.packet.clone(), - test.ctx.clone() - ); - - assert!(!proto_output.events.is_empty()); // Some events must exist. - - for e in proto_output.events.iter() { - assert!(matches!(e, &IbcEvent::WriteAcknowledgement(_))); - } - } - Err(e) => { - assert!( - !test.want_pass, - "write_ack: did not pass test: {}, \nparams {:?} {:?} error: {:?}", - test.name, - test.packet.clone(), - test.ctx.clone(), - e, - ); - } - } - } - } -} diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index fe84191787..80ec694767 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -10,9 +10,8 @@ use crate::prelude::*; use alloc::collections::btree_map::BTreeMap; use alloc::sync::Arc; -use core::borrow::Borrow; use core::cmp::min; -use core::fmt::{Debug, Formatter}; +use core::fmt::Debug; use core::ops::{Add, Sub}; use core::time::Duration; use parking_lot::Mutex; @@ -24,26 +23,21 @@ use tracing::debug; use crate::clients::ics07_tendermint::client_state::test_util::get_dummy_tendermint_client_state; use crate::clients::ics07_tendermint::client_state::ClientState as TmClientState; use crate::core::context::ContextError; -use crate::core::context::Router as NewRouter; +use crate::core::context::Router; use crate::core::ics02_client::client_state::ClientState; use crate::core::ics02_client::client_type::ClientType; use crate::core::ics02_client::consensus_state::ConsensusState; -use crate::core::ics02_client::context::{ClientKeeper, ClientReader}; use crate::core::ics02_client::error::ClientError; use crate::core::ics02_client::header::Header; use crate::core::ics03_connection::connection::ConnectionEnd; -use crate::core::ics03_connection::context::{ConnectionKeeper, ConnectionReader}; use crate::core::ics03_connection::error::ConnectionError; use crate::core::ics04_channel::channel::ChannelEnd; use crate::core::ics04_channel::commitment::{AcknowledgementCommitment, PacketCommitment}; -use crate::core::ics04_channel::context::{ChannelKeeper, ChannelReader}; use crate::core::ics04_channel::error::{ChannelError, PacketError}; use crate::core::ics04_channel::packet::{Receipt, Sequence}; -use crate::core::ics05_port::context::PortReader; -use crate::core::ics05_port::error::PortError; use crate::core::ics23_commitment::commitment::CommitmentPrefix; use crate::core::ics24_host::identifier::{ChainId, ChannelId, ClientId, ConnectionId, PortId}; -use crate::core::ics26_routing::context::{Module, ModuleId, Router, RouterBuilder, RouterContext}; +use crate::core::ics26_routing::context::{Module, ModuleId}; use crate::core::ics26_routing::msgs::MsgEnvelope; use crate::core::{dispatch, ExecutionContext, ValidationContext}; use crate::events::IbcEvent; @@ -85,11 +79,8 @@ pub struct MockContext { /// An object that stores all IBC related data. pub ibc_store: Arc>, - /// ICS26 router impl - router: MockRouter, - /// To implement ValidationContext Router - new_router: BTreeMap>, + router: BTreeMap>, pub events: Vec, @@ -127,7 +118,6 @@ impl Clone for MockContext { block_time: self.block_time, ibc_store, router: self.router.clone(), - new_router: self.new_router.clone(), events: self.events.clone(), logs: self.logs.clone(), } @@ -190,8 +180,7 @@ impl MockContext { .collect(), block_time, ibc_store: Arc::new(Mutex::new(MockIbcStore::default())), - router: Default::default(), - new_router: BTreeMap::new(), + router: BTreeMap::new(), events: Vec::new(), logs: Vec::new(), } @@ -475,7 +464,7 @@ impl MockContext { } pub fn add_route(&mut self, module_id: ModuleId, module: impl Module) -> Result<(), String> { - match self.new_router.insert(module_id, Arc::new(module)) { + match self.router.insert(module_id, Arc::new(module)) { None => Ok(()), Some(_) => Err("Duplicate module_id".to_owned()), } @@ -661,815 +650,18 @@ pub struct MockIbcStore { pub packet_receipt: PortChannelIdMap>, } -#[derive(Default)] -pub struct MockRouterBuilder(MockRouter); - -impl RouterBuilder for MockRouterBuilder { - type Router = MockRouter; - - fn add_route(mut self, module_id: ModuleId, module: impl Module) -> Result { - match self.0 .0.insert(module_id, Arc::new(module)) { - None => Ok(self), - Some(_) => Err("Duplicate module_id".to_owned()), - } - } - - fn build(self) -> Self::Router { - self.0 - } -} - -#[derive(Clone, Default)] -pub struct MockRouter(BTreeMap>); - -impl Debug for MockRouter { - fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { - write!(f, "{:?}", self.0.keys().collect::>()) - } -} - -impl Router for MockRouter { - fn get_route_mut(&mut self, module_id: &impl Borrow) -> Option<&mut dyn Module> { - self.0.get_mut(module_id.borrow()).and_then(Arc::get_mut) - } - - fn has_route(&self, module_id: &impl Borrow) -> bool { - self.0.get(module_id.borrow()).is_some() - } -} - -impl RouterContext for MockContext { - type Router = MockRouter; - - fn router(&self) -> &Self::Router { - &self.router - } - - fn router_mut(&mut self) -> &mut Self::Router { - &mut self.router - } -} - -impl PortReader for MockContext { - fn lookup_module_by_port(&self, port_id: &PortId) -> Result { - match self.ibc_store.lock().port_to_module.get(port_id) { - Some(mod_id) => Ok(mod_id.clone()), - None => Err(PortError::UnknownPort { - port_id: port_id.clone(), - }), - } - } -} - -impl ChannelReader for MockContext { - fn channel_end(&self, chan_end_path: &ChannelEndPath) -> Result { - match self - .ibc_store - .lock() - .channels - .get(&chan_end_path.0) - .and_then(|map| map.get(&chan_end_path.1)) - { - Some(channel_end) => Ok(channel_end.clone()), - None => Err(ChannelError::ChannelNotFound { - port_id: chan_end_path.0.clone(), - channel_id: chan_end_path.1.clone(), - }), - } - } - - fn connection_end(&self, cid: &ConnectionId) -> Result { - ConnectionReader::connection_end(self, cid).map_err(ChannelError::Connection) - } - - fn connection_channels( - &self, - cid: &ConnectionId, - ) -> Result, ChannelError> { - match self.ibc_store.lock().connection_channels.get(cid) { - Some(pcid) => Ok(pcid.clone()), - None => Err(ChannelError::MissingChannel), - } - } - - fn client_state(&self, client_id: &ClientId) -> Result, ChannelError> { - ClientReader::client_state(self, client_id) - .map_err(|e| ChannelError::Connection(ConnectionError::Client(e))) - } - - fn client_consensus_state( - &self, - client_cons_state_path: &ClientConsensusStatePath, - ) -> Result, ChannelError> { - ClientReader::consensus_state(self, client_cons_state_path) - .map_err(|e| ChannelError::Connection(ConnectionError::Client(e))) - } - - fn get_next_sequence_send(&self, seq_send_path: &SeqSendPath) -> Result { - match self - .ibc_store - .lock() - .next_sequence_send - .get(&seq_send_path.0) - .and_then(|map| map.get(&seq_send_path.1)) - { - Some(sequence) => Ok(*sequence), - None => Err(PacketError::MissingNextSendSeq { - port_id: seq_send_path.0.clone(), - channel_id: seq_send_path.1.clone(), - }), - } - } - - fn get_next_sequence_recv(&self, seq_recv_path: &SeqRecvPath) -> Result { - match self - .ibc_store - .lock() - .next_sequence_recv - .get(&seq_recv_path.0) - .and_then(|map| map.get(&seq_recv_path.1)) - { - Some(sequence) => Ok(*sequence), - None => Err(PacketError::MissingNextRecvSeq { - port_id: seq_recv_path.0.clone(), - channel_id: seq_recv_path.1.clone(), - }), - } - } - - fn get_next_sequence_ack(&self, seq_acks_path: &SeqAckPath) -> Result { - match self - .ibc_store - .lock() - .next_sequence_ack - .get(&seq_acks_path.0) - .and_then(|map| map.get(&seq_acks_path.1)) - { - Some(sequence) => Ok(*sequence), - None => Err(PacketError::MissingNextAckSeq { - port_id: seq_acks_path.0.clone(), - channel_id: seq_acks_path.1.clone(), - }), - } - } - - fn get_packet_commitment( - &self, - commitment_path: &CommitmentPath, - ) -> Result { - match self - .ibc_store - .lock() - .packet_commitment - .get(&commitment_path.port_id) - .and_then(|map| map.get(&commitment_path.channel_id)) - .and_then(|map| map.get(&commitment_path.sequence)) - { - Some(commitment) => Ok(commitment.clone()), - None => Err(PacketError::PacketCommitmentNotFound { - sequence: commitment_path.sequence, - }), - } - } - - fn get_packet_receipt(&self, receipt_path: &ReceiptPath) -> Result { - match self - .ibc_store - .lock() - .packet_receipt - .get(&receipt_path.port_id) - .and_then(|map| map.get(&receipt_path.channel_id)) - .and_then(|map| map.get(&receipt_path.sequence)) - { - Some(receipt) => Ok(receipt.clone()), - None => Err(PacketError::PacketReceiptNotFound { - sequence: receipt_path.sequence, - }), - } - } - - fn get_packet_acknowledgement( - &self, - ack_path: &AckPath, - ) -> Result { - match self - .ibc_store - .lock() - .packet_acknowledgement - .get(&ack_path.port_id) - .and_then(|map| map.get(&ack_path.channel_id)) - .and_then(|map| map.get(&ack_path.sequence)) - { - Some(ack) => Ok(ack.clone()), - None => Err(PacketError::PacketAcknowledgementNotFound { - sequence: ack_path.sequence, - }), - } - } - - fn hash(&self, value: &[u8]) -> Vec { - sha2::Sha256::digest(value).to_vec() - } - - fn host_height(&self) -> Result { - Ok(self.latest_height()) - } - - fn host_timestamp(&self) -> Result { - ClientReader::host_timestamp(self).map_err(|e| ChannelError::Other { - description: e.to_string(), - }) - } - - fn host_consensus_state( - &self, - height: &Height, - ) -> Result, ChannelError> { - ConnectionReader::host_consensus_state(self, height).map_err(ChannelError::Connection) - } - - fn pending_host_consensus_state(&self) -> Result, ChannelError> { - ClientReader::pending_host_consensus_state(self) - .map_err(|e| ChannelError::Connection(ConnectionError::Client(e))) - } - - fn client_update_time( - &self, - client_id: &ClientId, - height: &Height, - ) -> Result { - match self - .ibc_store - .lock() - .client_processed_times - .get(&(client_id.clone(), *height)) - { - Some(time) => Ok(*time), - None => Err(ChannelError::ProcessedTimeNotFound { - client_id: client_id.clone(), - height: *height, - }), - } - } - - fn client_update_height( - &self, - client_id: &ClientId, - height: &Height, - ) -> Result { - match self - .ibc_store - .lock() - .client_processed_heights - .get(&(client_id.clone(), *height)) - { - Some(height) => Ok(*height), - None => Err(ChannelError::ProcessedHeightNotFound { - client_id: client_id.clone(), - height: *height, - }), - } - } - - fn channel_counter(&self) -> Result { - Ok(self.ibc_store.lock().channel_ids_counter) - } - - fn max_expected_time_per_block(&self) -> Duration { - self.block_time - } -} - -impl ChannelKeeper for MockContext { - fn store_packet_commitment( - &mut self, - port_id: PortId, - channel_id: ChannelId, - seq: Sequence, - commitment: PacketCommitment, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .packet_commitment - .entry(port_id) - .or_default() - .entry(channel_id) - .or_default() - .insert(seq, commitment); - Ok(()) - } - - fn store_packet_acknowledgement( - &mut self, - port_id: PortId, - channel_id: ChannelId, - seq: Sequence, - ack_commitment: AcknowledgementCommitment, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .packet_acknowledgement - .entry(port_id) - .or_default() - .entry(channel_id) - .or_default() - .insert(seq, ack_commitment); - Ok(()) - } - - fn delete_packet_acknowledgement( - &mut self, - port_id: &PortId, - channel_id: &ChannelId, - seq: &Sequence, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .packet_acknowledgement - .get_mut(port_id) - .and_then(|map| map.get_mut(channel_id)) - .and_then(|map| map.remove(seq)); - Ok(()) - } - - fn store_connection_channels( - &mut self, - cid: ConnectionId, - port_id: PortId, - channel_id: ChannelId, - ) -> Result<(), ChannelError> { - self.ibc_store - .lock() - .connection_channels - .entry(cid) - .or_insert_with(Vec::new) - .push((port_id, channel_id)); - Ok(()) - } - - fn store_channel( - &mut self, - port_id: PortId, - channel_id: ChannelId, - channel_end: ChannelEnd, - ) -> Result<(), ChannelError> { - self.ibc_store - .lock() - .channels - .entry(port_id) - .or_default() - .insert(channel_id, channel_end); - Ok(()) - } - - fn store_next_sequence_send( - &mut self, - port_id: PortId, - channel_id: ChannelId, - seq: Sequence, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .next_sequence_send - .entry(port_id) - .or_default() - .insert(channel_id, seq); - Ok(()) - } - - fn store_next_sequence_recv( - &mut self, - port_id: PortId, - channel_id: ChannelId, - seq: Sequence, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .next_sequence_recv - .entry(port_id) - .or_default() - .insert(channel_id, seq); - Ok(()) - } - - fn store_next_sequence_ack( - &mut self, - port_id: PortId, - channel_id: ChannelId, - seq: Sequence, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .next_sequence_ack - .entry(port_id) - .or_default() - .insert(channel_id, seq); - Ok(()) - } - - fn increase_channel_counter(&mut self) { - self.ibc_store.lock().channel_ids_counter += 1; - } - - fn delete_packet_commitment( - &mut self, - port_id: &PortId, - channel_id: &ChannelId, - seq: &Sequence, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .packet_commitment - .get_mut(port_id) - .and_then(|map| map.get_mut(channel_id)) - .and_then(|map| map.remove(seq)); - Ok(()) - } - - fn store_packet_receipt( - &mut self, - port_id: PortId, - channel_id: ChannelId, - seq: Sequence, - receipt: Receipt, - ) -> Result<(), PacketError> { - self.ibc_store - .lock() - .packet_receipt - .entry(port_id) - .or_default() - .entry(channel_id) - .or_default() - .insert(seq, receipt); - Ok(()) - } -} - -impl ConnectionReader for MockContext { - fn connection_end(&self, cid: &ConnectionId) -> Result { - match self.ibc_store.lock().connections.get(cid) { - Some(connection_end) => Ok(connection_end.clone()), - None => Err(ConnectionError::ConnectionNotFound { - connection_id: cid.clone(), - }), - } - } - - fn client_state(&self, client_id: &ClientId) -> Result, ConnectionError> { - // Forward method call to the Ics2 Client-specific method. - ClientReader::client_state(self, client_id).map_err(ConnectionError::Client) - } - - fn decode_client_state( - &self, - client_state: Any, - ) -> Result, ConnectionError> { - ClientReader::decode_client_state(self, client_state).map_err(ConnectionError::Client) - } - - fn host_current_height(&self) -> Result { - Ok(self.latest_height()) - } - - fn host_oldest_height(&self) -> Result { - // history must be non-empty, so `self.history[0]` is valid - Ok(self.history[0].height()) - } - - fn commitment_prefix(&self) -> CommitmentPrefix { - CommitmentPrefix::try_from(b"mock".to_vec()).unwrap() - } - - fn client_consensus_state( - &self, - client_cons_state_path: &ClientConsensusStatePath, - ) -> Result, ConnectionError> { - // Forward method call to the Ics2Client-specific method. - ClientReader::consensus_state(self, client_cons_state_path).map_err(ConnectionError::Client) - } - - fn host_consensus_state( - &self, - height: &Height, - ) -> Result, ConnectionError> { - ClientReader::host_consensus_state(self, height).map_err(ConnectionError::Client) - } - - fn connection_counter(&self) -> Result { - Ok(self.ibc_store.lock().connection_ids_counter) - } - - fn validate_self_client( - &self, - _client_state_of_host_on_counterparty: Any, - ) -> Result<(), ConnectionError> { - Ok(()) - } -} - -impl ConnectionKeeper for MockContext { - fn store_connection( - &mut self, - connection_id: ConnectionId, - connection_end: ConnectionEnd, - ) -> Result<(), ConnectionError> { - self.ibc_store - .lock() - .connections - .insert(connection_id, connection_end); - Ok(()) - } - - fn store_connection_to_client( - &mut self, - connection_id: ConnectionId, - client_id: ClientId, - ) -> Result<(), ConnectionError> { - self.ibc_store - .lock() - .client_connections - .insert(client_id, connection_id); - Ok(()) - } - - fn increase_connection_counter(&mut self) { - self.ibc_store.lock().connection_ids_counter += 1; - } -} - -impl ClientReader for MockContext { - fn client_type(&self, client_id: &ClientId) -> Result { - match self.ibc_store.lock().clients.get(client_id) { - Some(client_record) => Ok(client_record.client_type.clone()), - None => Err(ClientError::ClientNotFound { - client_id: client_id.clone(), - }), - } - } - - fn client_state(&self, client_id: &ClientId) -> Result, ClientError> { - match self.ibc_store.lock().clients.get(client_id) { - Some(client_record) => { - client_record - .client_state - .clone() - .ok_or_else(|| ClientError::ClientNotFound { - client_id: client_id.clone(), - }) - } - None => Err(ClientError::ClientNotFound { - client_id: client_id.clone(), - }), - } - } - - fn decode_client_state(&self, client_state: Any) -> Result, ClientError> { - if let Ok(client_state) = TmClientState::try_from(client_state.clone()) { - Ok(client_state.into_box()) - } else if let Ok(client_state) = MockClientState::try_from(client_state.clone()) { - Ok(client_state.into_box()) - } else { - Err(ClientError::UnknownClientStateType { - client_state_type: client_state.type_url, - }) - } - } - - fn consensus_state( - &self, - client_cons_state_path: &ClientConsensusStatePath, - ) -> Result, ClientError> { - let height = - Height::new(client_cons_state_path.epoch, client_cons_state_path.height).unwrap(); - match self - .ibc_store - .lock() - .clients - .get(&client_cons_state_path.client_id) - { - Some(client_record) => match client_record.consensus_states.get(&height) { - Some(consensus_state) => Ok(consensus_state.clone()), - None => Err(ClientError::ConsensusStateNotFound { - client_id: client_cons_state_path.client_id.clone(), - height, - }), - }, - None => Err(ClientError::ConsensusStateNotFound { - client_id: client_cons_state_path.client_id.clone(), - height, - }), - } - } - - /// Search for the lowest consensus state higher than `height`. - fn next_consensus_state( - &self, - next_client_cons_state_path: &ClientConsensusStatePath, - ) -> Result>, ClientError> { - let ibc_store = self.ibc_store.lock(); - let client_record = ibc_store - .clients - .get(&next_client_cons_state_path.client_id) - .ok_or_else(|| ClientError::ClientNotFound { - client_id: next_client_cons_state_path.client_id.clone(), - })?; - - let height = Height::new( - next_client_cons_state_path.epoch, - next_client_cons_state_path.height, - ) - .unwrap(); - - // Get the consensus state heights and sort them in ascending order. - let mut heights: Vec = client_record.consensus_states.keys().cloned().collect(); - heights.sort(); - - // Search for next state. - for h in heights { - if h > height { - // unwrap should never happen, as the consensus state for h must exist - return Ok(Some( - client_record.consensus_states.get(&h).unwrap().clone(), - )); - } - } - Ok(None) - } - - /// Search for the highest consensus state lower than `height`. - fn prev_consensus_state( - &self, - prev_client_cons_state_path: &ClientConsensusStatePath, - ) -> Result>, ClientError> { - let ibc_store = self.ibc_store.lock(); - let client_record = ibc_store - .clients - .get(&prev_client_cons_state_path.client_id) - .ok_or_else(|| ClientError::ClientNotFound { - client_id: prev_client_cons_state_path.client_id.clone(), - })?; - - let height = Height::new( - prev_client_cons_state_path.epoch, - prev_client_cons_state_path.height, - ) - .unwrap(); - - // Get the consensus state heights and sort them in descending order. - let mut heights: Vec = client_record.consensus_states.keys().cloned().collect(); - heights.sort_by(|a, b| b.cmp(a)); - - // Search for previous state. - for h in heights { - if h < height { - // unwrap should never happen, as the consensus state for h must exist - return Ok(Some( - client_record.consensus_states.get(&h).unwrap().clone(), - )); - } - } - Ok(None) - } - - fn host_height(&self) -> Result { - Ok(self.latest_height()) - } - - fn host_timestamp(&self) -> Result { - Ok(self - .history - .last() - .expect("history cannot be empty") - .timestamp() - .add(self.block_time) - .unwrap()) - } - - fn host_consensus_state( - &self, - height: &Height, - ) -> Result, ClientError> { - match self.host_block(height) { - Some(block_ref) => Ok(block_ref.clone().into()), - None => Err(ClientError::MissingLocalConsensusState { height: *height }), - } - } - - fn pending_host_consensus_state(&self) -> Result, ClientError> { - Err(ClientError::ImplementationSpecific) - } - - fn client_counter(&self) -> Result { - Ok(self.ibc_store.lock().client_ids_counter) - } -} - -impl ClientKeeper for MockContext { - fn store_client_type( - &mut self, - client_id: ClientId, - client_type: ClientType, - ) -> Result<(), ClientError> { - let mut ibc_store = self.ibc_store.lock(); - let client_record = ibc_store - .clients - .entry(client_id) - .or_insert(MockClientRecord { - client_type: client_type.clone(), - consensus_states: Default::default(), - client_state: Default::default(), - }); - - client_record.client_type = client_type; - Ok(()) - } - - fn store_client_state( - &mut self, - client_id: ClientId, - client_state: Box, - ) -> Result<(), ClientError> { - let mut ibc_store = self.ibc_store.lock(); - let client_record = ibc_store - .clients - .entry(client_id) - .or_insert(MockClientRecord { - client_type: client_state.client_type(), - consensus_states: Default::default(), - client_state: Default::default(), - }); - - client_record.client_state = Some(client_state); - Ok(()) - } - - fn store_consensus_state( - &mut self, - client_id: ClientId, - height: Height, - consensus_state: Box, - ) -> Result<(), ClientError> { - let mut ibc_store = self.ibc_store.lock(); - let client_record = ibc_store - .clients - .entry(client_id) - .or_insert(MockClientRecord { - client_type: mock_client_type(), - consensus_states: Default::default(), - client_state: Default::default(), - }); - - client_record - .consensus_states - .insert(height, consensus_state); - Ok(()) - } - - fn increase_client_counter(&mut self) { - self.ibc_store.lock().client_ids_counter += 1 - } - - fn store_update_time( - &mut self, - client_id: ClientId, - height: Height, - timestamp: Timestamp, - ) -> Result<(), ClientError> { - let _ = self - .ibc_store - .lock() - .client_processed_times - .insert((client_id, height), timestamp); - Ok(()) - } - - fn store_update_height( - &mut self, - client_id: ClientId, - height: Height, - host_height: Height, - ) -> Result<(), ClientError> { - let _ = self - .ibc_store - .lock() - .client_processed_heights - .insert((client_id, height), host_height); - Ok(()) - } -} - impl RelayerContext for MockContext { - fn query_latest_height(&self) -> Result { - self.host_current_height().map_err(RelayerError::Connection) + fn query_latest_height(&self) -> Result { + self.host_height() } fn query_client_full_state(&self, client_id: &ClientId) -> Option> { // Forward call to Ics2. - ClientReader::client_state(self, client_id).ok() + ValidationContext::client_state(self, client_id).ok() } fn query_latest_header(&self) -> Option> { - let block_ref = self.host_block(&self.host_current_height().unwrap()); + let block_ref = self.host_block(&self.host_height().unwrap()); block_ref.cloned().map(Header::into_box) } @@ -1478,20 +670,20 @@ impl RelayerContext for MockContext { } } -impl NewRouter for MockContext { +impl Router for MockContext { fn get_route(&self, module_id: &ModuleId) -> Option<&dyn Module> { - self.new_router.get(module_id).map(Arc::as_ref) + self.router.get(module_id).map(Arc::as_ref) } fn get_route_mut(&mut self, module_id: &ModuleId) -> Option<&mut dyn Module> { - self.new_router.get_mut(module_id).and_then(Arc::get_mut) + self.router.get_mut(module_id).and_then(Arc::get_mut) } fn has_route(&self, module_id: &ModuleId) -> bool { - self.new_router.get(module_id).is_some() + self.router.get(module_id).is_some() } fn lookup_module_by_port(&self, port_id: &PortId) -> Option { - ::lookup_module_by_port(self, port_id).ok() + self.ibc_store.lock().port_to_module.get(port_id).cloned() } } diff --git a/crates/ibc/src/mock/ics18_relayer/context.rs b/crates/ibc/src/mock/ics18_relayer/context.rs index 04e237b71a..8282f5ae2c 100644 --- a/crates/ibc/src/mock/ics18_relayer/context.rs +++ b/crates/ibc/src/mock/ics18_relayer/context.rs @@ -3,8 +3,8 @@ use crate::prelude::*; use crate::core::ics02_client::client_state::ClientState; use crate::core::ics02_client::header::Header; -use super::error::RelayerError; use crate::core::ics24_host::identifier::ClientId; +use crate::core::ContextError; use crate::signer::Signer; use crate::Height; @@ -15,7 +15,7 @@ use crate::Height; /// types, light client, RPC client, etc.) pub trait RelayerContext { /// Returns the latest height of the chain. - fn query_latest_height(&self) -> Result; + fn query_latest_height(&self) -> Result; /// Returns this client state for the given `client_id` on this chain. /// Wrapper over the `/abci_query?path=..` endpoint. diff --git a/crates/ibc/src/test_utils.rs b/crates/ibc/src/test_utils.rs index 18f11b16ff..acecd65c28 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -25,6 +25,7 @@ use crate::core::ics04_channel::Version; use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; use crate::core::ics24_host::path::{ChannelEndPath, ClientConsensusStatePath, SeqSendPath}; use crate::core::ics26_routing::context::{Module, ModuleOutputBuilder}; +use crate::core::ContextError; use crate::mock::context::MockIbcStore; use crate::prelude::*; use crate::signer::Signer; @@ -226,7 +227,7 @@ impl TokenTransferKeeper for DummyTransferModule { channel_id: ChannelId, seq: Sequence, commitment: PacketCommitment, - ) -> Result<(), PacketError> { + ) -> Result<(), ContextError> { self.ibc_store .lock() .packet_commitment @@ -243,7 +244,7 @@ impl TokenTransferKeeper for DummyTransferModule { port_id: PortId, channel_id: ChannelId, seq: Sequence, - ) -> Result<(), PacketError> { + ) -> Result<(), ContextError> { self.ibc_store .lock() .next_sequence_send @@ -309,7 +310,7 @@ impl TokenTransferReader for DummyTransferModule { } impl SendPacketReader for DummyTransferModule { - fn channel_end(&self, chan_end_path: &ChannelEndPath) -> Result { + fn channel_end(&self, chan_end_path: &ChannelEndPath) -> Result { match self .ibc_store .lock() @@ -321,11 +322,12 @@ impl SendPacketReader for DummyTransferModule { None => Err(PacketError::ChannelNotFound { port_id: chan_end_path.0.clone(), channel_id: chan_end_path.1.clone(), - }), + } + .into()), } } - fn connection_end(&self, cid: &ConnectionId) -> Result { + fn connection_end(&self, cid: &ConnectionId) -> Result { match self.ibc_store.lock().connections.get(cid) { Some(connection_end) => Ok(connection_end.clone()), None => Err(ConnectionError::ConnectionNotFound { @@ -333,9 +335,10 @@ impl SendPacketReader for DummyTransferModule { }), } .map_err(PacketError::Connection) + .map_err(ContextError::PacketError) } - fn client_state(&self, client_id: &ClientId) -> Result, PacketError> { + fn client_state(&self, client_id: &ClientId) -> Result, ContextError> { match self.ibc_store.lock().clients.get(client_id) { Some(client_record) => { client_record @@ -350,12 +353,13 @@ impl SendPacketReader for DummyTransferModule { }), } .map_err(|e| PacketError::Connection(ConnectionError::Client(e))) + .map_err(ContextError::PacketError) } fn client_consensus_state( &self, client_cons_state_path: &ClientConsensusStatePath, - ) -> Result, PacketError> { + ) -> Result, ContextError> { let height = Height::new(client_cons_state_path.epoch, client_cons_state_path.height).unwrap(); match self @@ -377,9 +381,13 @@ impl SendPacketReader for DummyTransferModule { }), } .map_err(|e| PacketError::Connection(ConnectionError::Client(e))) + .map_err(ContextError::PacketError) } - fn get_next_sequence_send(&self, seq_send_path: &SeqSendPath) -> Result { + fn get_next_sequence_send( + &self, + seq_send_path: &SeqSendPath, + ) -> Result { match self .ibc_store .lock() @@ -391,7 +399,8 @@ impl SendPacketReader for DummyTransferModule { None => Err(PacketError::MissingNextSendSeq { port_id: seq_send_path.0.clone(), channel_id: seq_send_path.1.clone(), - }), + } + .into()), } }