imp(ibc-testkit): remove field access of MockContext (#1046)

* move events and logs inside MockIbcStore

* add getters for ibc events and logs

* refactor tests

* add doc strings

* add changelog

.changelog/unreleased/improvements/1043-remove-field-access-of-mockcontext.md

@@ -0,0 +1,2 @@
+- [ibc-testkit] Remove field access of `MockContext`.
+  ([\#1043](https://github.com/cosmos/ibc-rs/issues/1043))

ibc-testkit/src/fixtures/core/context.rs

@@ -120,8 +120,6 @@ impl From<MockContextConfig> for MockContext {
             history,
             block_time: params.block_time,
             ibc_store: Arc::new(Mutex::new(MockIbcStore::default())),
-            events: Vec::new(),
-            logs: Vec::new(),
         }
     }
 }

ibc-testkit/src/testapp/ibc/core/core_ctx.rs

@@ -543,12 +543,12 @@ impl ExecutionContext for MockContext {
     }
 
     fn emit_ibc_event(&mut self, event: IbcEvent) -> Result<(), ContextError> {
-        self.events.push(event);
+        self.ibc_store.lock().events.push(event);
         Ok(())
     }
 
     fn log_message(&mut self, message: String) -> Result<(), ContextError> {
-        self.logs.push(message);
+        self.ibc_store.lock().logs.push(message);
         Ok(())
     }
 }

ibc-testkit/src/testapp/ibc/core/types.rs

@@ -91,8 +91,14 @@ pub struct MockIbcStore {
     /// Constant-size commitments to packets data fields
     pub packet_commitment: PortChannelIdMap<BTreeMap<Sequence, PacketCommitment>>,
 
-    // Used by unordered channel
+    /// Used by unordered channel
     pub packet_receipt: PortChannelIdMap<BTreeMap<Sequence, Receipt>>,
+
+    /// Emitted IBC events in order
+    pub events: Vec<IbcEvent>,
+
+    /// Logs of the IBC module
+    pub logs: Vec<String>,
 }
 
 /// A context implementing the dependencies necessary for testing any IBC module.
@@ -116,10 +122,6 @@ pub struct MockContext {
 
     /// An object that stores all IBC related data.
     pub ibc_store: Arc<Mutex<MockIbcStore>>,
-
-    pub events: Vec<IbcEvent>,
-
-    pub logs: Vec<String>,
 }
 
 #[derive(Debug, TypedBuilder)]
@@ -170,8 +172,6 @@ impl Clone for MockContext {
             history: self.history.clone(),
             block_time: self.block_time,
             ibc_store,
-            events: self.events.clone(),
-            logs: self.logs.clone(),
         }
     }
 }
@@ -232,8 +232,6 @@ impl MockContext {
                 .collect(),
             block_time,
             ibc_store: Arc::new(Mutex::new(MockIbcStore::default())),
-            events: Vec::new(),
-            logs: Vec::new(),
         }
     }
 
@@ -299,8 +297,6 @@ impl MockContext {
             history,
             block_time,
             ibc_store: Arc::new(Mutex::new(MockIbcStore::default())),
-            events: Vec::new(),
-            logs: Vec::new(),
         }
     }
 
@@ -781,6 +777,14 @@ impl MockContext {
         let block_ref = self.host_block(&self.host_height().expect("Never fails"));
         block_ref.cloned()
     }
+
+    pub fn get_events(&self) -> Vec<IbcEvent> {
+        self.ibc_store.lock().events.clone()
+    }
+
+    pub fn get_logs(&self) -> Vec<String> {
+        self.ibc_store.lock().logs.clone()
+    }
 }
 
 #[cfg(test)]

ibc-testkit/tests/core/ics02_client/update_client.rs

@@ -676,11 +676,13 @@ fn test_update_client_events() {
     let res = execute(&mut ctx, &mut router, msg_envelope);
     assert!(res.is_ok());
 
+    let ibc_events = ctx.get_events();
+
     assert!(matches!(
-        ctx.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Client)
     ));
-    let update_client_event = downcast!(&ctx.events[1] => IbcEvent::UpdateClient).unwrap();
+    let update_client_event = downcast!(&ibc_events[1] => IbcEvent::UpdateClient).unwrap();
 
     assert_eq!(update_client_event.client_id(), &client_id);
     assert_eq!(update_client_event.client_type(), &mock_client_type());
@@ -696,13 +698,14 @@ fn ensure_misbehaviour(ctx: &MockContext, client_id: &ClientId, client_type: &Cl
     assert!(status.is_frozen(), "client_state status: {status}");
 
     // check events
-    assert_eq!(ctx.events.len(), 2);
+    let ibc_events = ctx.get_events();
+    assert_eq!(ibc_events.len(), 2);
     assert!(matches!(
-        ctx.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Client),
     ));
     let misbehaviour_client_event =
-        downcast!(&ctx.events[1] => IbcEvent::ClientMisbehaviour).unwrap();
+        downcast!(&ibc_events[1] => IbcEvent::ClientMisbehaviour).unwrap();
     assert_eq!(misbehaviour_client_event.client_id(), client_id);
     assert_eq!(misbehaviour_client_event.client_type(), client_type);
 }

ibc-testkit/tests/core/ics02_client/upgrade_client.rs

@@ -94,12 +94,13 @@ fn upgrade_client_execute(fxt: &mut Fixture<MsgUpgradeClient>, expect: Expect) {
         }
         Expect::Success => {
             assert!(res.is_ok(), "{err_msg}");
+            let ibc_events = fxt.ctx.get_events();
             assert!(matches!(
-                fxt.ctx.events[0],
+                ibc_events[0],
                 IbcEvent::Message(MessageEvent::Client)
             ));
             let upgrade_client_event =
-                downcast!(&fxt.ctx.events[1] => IbcEvent::UpgradeClient).unwrap();
+                downcast!(&ibc_events[1] => IbcEvent::UpgradeClient).unwrap();
             let plan_height = Height::new(1, 26).unwrap();
 
             assert_eq!(upgrade_client_event.client_id(), &fxt.msg.client_id);

ibc-testkit/tests/core/ics03_connection/conn_open_ack.rs

@@ -130,14 +130,15 @@ fn conn_open_ack_execute(fxt: &mut Fixture<MsgConnectionOpenAck>, expect: Expect
             assert!(res.is_err(), "{err_msg}");
         }
         Expect::Success => {
+            let ibc_events = fxt.ctx.get_events();
             assert!(res.is_ok(), "{err_msg}");
-            assert_eq!(fxt.ctx.events.len(), 2);
+            assert_eq!(ibc_events.len(), 2);
 
             assert!(matches!(
-                fxt.ctx.events[0],
+                ibc_events[0],
                 IbcEvent::Message(MessageEvent::Connection)
             ));
-            let event = &fxt.ctx.events[1];
+            let event = &ibc_events[1];
             assert!(matches!(event, &IbcEvent::OpenAckConnection(_)));
 
             let conn_open_try_event = match event {

ibc-testkit/tests/core/ics03_connection/conn_open_confirm.rs

@@ -84,14 +84,15 @@ fn conn_open_confirm_execute(fxt: &mut Fixture<MsgConnectionOpenConfirm>, expect
             assert!(res.is_err(), "{err_msg}");
         }
         Expect::Success => {
+            let ibc_events = fxt.ctx.get_events();
             assert!(res.is_ok(), "{err_msg}");
-            assert_eq!(fxt.ctx.events.len(), 2);
+            assert_eq!(ibc_events.len(), 2);
 
             assert!(matches!(
-                fxt.ctx.events[0],
+                ibc_events[0],
                 IbcEvent::Message(MessageEvent::Connection)
             ));
-            let event = &fxt.ctx.events[1];
+            let event = &ibc_events[1];
             assert!(matches!(event, &IbcEvent::OpenConfirmConnection(_)));
 
             let conn_open_try_event = match event {

ibc-testkit/tests/core/ics03_connection/conn_open_init.rs

@@ -79,17 +79,19 @@ fn conn_open_init_execute(
             assert!(res.is_err(), "{err_msg}")
         }
         Expect::Success => {
+            let ibc_events = fxt.ctx.get_events();
+
             assert!(res.is_ok(), "{err_msg}");
 
             assert_eq!(fxt.ctx.connection_counter().unwrap(), 1);
 
-            assert_eq!(fxt.ctx.events.len(), 2);
+            assert_eq!(ibc_events.len(), 2);
 
             assert!(matches!(
-                fxt.ctx.events[0],
+                ibc_events[0],
                 IbcEvent::Message(MessageEvent::Connection)
             ));
-            let event = &fxt.ctx.events[1];
+            let event = &ibc_events[1];
             assert!(matches!(event, &IbcEvent::OpenInitConnection(_)));
 
             let conn_open_init_event = match event {

ibc-testkit/tests/core/ics03_connection/conn_open_try.rs

@@ -96,13 +96,15 @@ fn conn_open_try_execute(fxt: &mut Fixture<MsgConnectionOpenTry>, expect: Expect
 
             assert_eq!(fxt.ctx.connection_counter().unwrap(), 1);
 
-            assert_eq!(fxt.ctx.events.len(), 2);
+            let ibc_events = fxt.ctx.get_events();
+
+            assert_eq!(ibc_events.len(), 2);
 
             assert!(matches!(
-                fxt.ctx.events[0],
+                ibc_events[0],
                 IbcEvent::Message(MessageEvent::Connection)
             ));
-            let event = &fxt.ctx.events[1];
+            let event = &ibc_events[1];
             assert!(matches!(event, &IbcEvent::OpenTryConnection(_)));
 
             let conn_open_try_event = match event {

ibc-testkit/tests/core/ics04_channel/acknowledgement.rs

@@ -217,12 +217,14 @@ fn ack_unordered_chan_execute(fixture: Fixture) {
 
     assert!(res.is_ok());
 
-    assert_eq!(ctx.events.len(), 2);
+    let ibc_events = ctx.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
     assert!(matches!(
-        ctx.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
-    assert!(matches!(ctx.events[1], IbcEvent::AcknowledgePacket(_)));
+    assert!(matches!(ibc_events[1], IbcEvent::AcknowledgePacket(_)));
 }
 
 #[rstest]
@@ -256,10 +258,12 @@ fn ack_ordered_chan_execute(fixture: Fixture) {
 
     assert!(res.is_ok());
 
-    assert_eq!(ctx.events.len(), 2);
+    let ibc_events = ctx.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
     assert!(matches!(
-        ctx.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
-    assert!(matches!(ctx.events[1], IbcEvent::AcknowledgePacket(_)));
+    assert!(matches!(ibc_events[1], IbcEvent::AcknowledgePacket(_)));
 }

ibc-testkit/tests/core/ics04_channel/chan_close_confirm.rs

@@ -121,15 +121,14 @@ fn test_chan_close_confirm_execute() {
 
     assert!(res.is_ok(), "Execution success: happy path");
 
-    assert_eq!(context.events.len(), 2);
+    let ibc_events = context.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
 
     assert!(matches!(
-        context.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
 
-    assert!(matches!(
-        context.events[1],
-        IbcEvent::CloseConfirmChannel(_)
-    ));
+    assert!(matches!(ibc_events[1], IbcEvent::CloseConfirmChannel(_)));
 }

ibc-testkit/tests/core/ics04_channel/chan_close_init.rs

@@ -123,12 +123,14 @@ fn test_chan_close_init_execute() {
 
     assert!(res.is_ok(), "Execution happy path");
 
-    assert_eq!(context.events.len(), 2);
+    let ibc_events = context.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
 
     assert!(matches!(
-        context.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
 
-    assert!(matches!(context.events[1], IbcEvent::CloseInitChannel(_)));
+    assert!(matches!(ibc_events[1], IbcEvent::CloseInitChannel(_)));
 }

ibc-testkit/tests/core/ics04_channel/chan_open_ack.rs

@@ -134,12 +134,14 @@ fn chan_open_ack_execute_happy_path(fixture: Fixture) {
 
     assert!(res.is_ok(), "Execution happy path");
 
-    assert_eq!(context.events.len(), 2);
+    let ibc_events = context.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
     assert!(matches!(
-        context.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
-    assert!(matches!(context.events[1], IbcEvent::OpenAckChannel(_)));
+    assert!(matches!(ibc_events[1], IbcEvent::OpenAckChannel(_)));
 }
 
 #[rstest]

ibc-testkit/tests/core/ics04_channel/chan_open_confirm.rs

@@ -131,14 +131,16 @@ fn chan_open_confirm_execute_happy_path(fixture: Fixture) {
 
     assert!(res.is_ok(), "Execution happy path");
 
-    assert_eq!(context.events.len(), 2);
+    let ibc_events = context.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
 
     assert!(matches!(
-        context.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
 
-    assert!(matches!(context.events[1], IbcEvent::OpenConfirmChannel(_)));
+    assert!(matches!(ibc_events[1], IbcEvent::OpenConfirmChannel(_)));
 }
 
 #[rstest]

ibc-testkit/tests/core/ics04_channel/chan_open_init.rs

@@ -94,13 +94,15 @@ fn chan_open_init_execute_happy_path(fixture: Fixture) {
 
     assert_eq!(ctx.channel_counter().unwrap(), 1);
 
-    assert_eq!(ctx.events.len(), 2);
+    let ibc_events = ctx.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
 
     assert!(matches!(
-        ctx.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
-    assert!(matches!(ctx.events[1], IbcEvent::OpenInitChannel(_)));
+    assert!(matches!(ibc_events[1], IbcEvent::OpenInitChannel(_)));
 }
 
 #[rstest]

ibc-testkit/tests/core/ics04_channel/chan_open_try.rs

@@ -114,13 +114,15 @@ fn chan_open_try_execute_happy_path(fixture: Fixture) {
 
     assert_eq!(ctx.channel_counter().unwrap(), 1);
 
-    assert_eq!(ctx.events.len(), 2);
+    let ibc_events = ctx.get_events();
+
+    assert_eq!(ibc_events.len(), 2);
 
     assert!(matches!(
-        ctx.events[0],
+        ibc_events[0],
         IbcEvent::Message(MessageEvent::Channel)
     ));
-    assert!(matches!(ctx.events[1], IbcEvent::OpenTryChannel(_)));
+    assert!(matches!(ibc_events[1], IbcEvent::OpenTryChannel(_)));
 }
 
 #[rstest]

ibc-testkit/tests/core/ics04_channel/recv_packet.rs

@@ -231,15 +231,17 @@ fn recv_packet_execute_happy_path(fixture: Fixture) {
 
     assert!(res.is_ok());
 
-    assert_eq!(ctx.events.len(), 4);
+    let ibc_events = ctx.get_events();
+
+    assert_eq!(ibc_events.len(), 4);
     assert!(matches!(
-        &ctx.events[0],
+        &ibc_events[0],
         &IbcEvent::Message(MessageEvent::Channel)
     ));
-    assert!(matches!(&ctx.events[1], &IbcEvent::ReceivePacket(_)));
+    assert!(matches!(&ibc_events[1], &IbcEvent::ReceivePacket(_)));
     assert!(matches!(
-        &ctx.events[2],
+        &ibc_events[2],
         &IbcEvent::Message(MessageEvent::Channel)
     ));
-    assert!(matches!(&ctx.events[3], &IbcEvent::WriteAcknowledgement(_)));
+    assert!(matches!(&ibc_events[3], &IbcEvent::WriteAcknowledgement(_)));
 }