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

.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))

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,25 +83,33 @@ pub trait TokenTransferReader: SendPacketReader {
 
 impl<T> TokenTransferKeeper for T
 where
-    T: ChannelKeeper + BankKeeper,
+    T: ExecutionContext + BankKeeper,
 {
     fn store_packet_commitment(
         &mut self,
         port_id: PortId,
         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(
         &mut self,
         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)
     }
 }
 

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,
                 ..

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()

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(_)
+        ));
+    }
+}