Cleanup mock context for ICS3

modules/src/context.rs

@@ -35,7 +35,6 @@ impl From<MockHeader> for AnyConsensusState {
 }
 
 pub trait ChainReader {
-    fn chain_type(&self) -> SelfChainType;
     fn self_historical_info(&self, height: Height) -> Option<HistoricalInfo>;
     fn header(&self, height: Height) -> Option<AnyHeader>;
     fn fetch_self_consensus_state(&self, height: Height) -> Option<AnyConsensusState>;

modules/src/context_mock.rs

@@ -1,4 +1,4 @@
-use crate::context::{ChainKeeper, ChainReader, HistoricalInfo, SelfChainType, SelfHeader};
+use crate::context::{ChainKeeper, ChainReader, HistoricalInfo, SelfHeader};
 use crate::ics02_client::client_def::{AnyConsensusState, AnyHeader};
 use crate::mock_client::header::MockHeader;
 
@@ -91,10 +91,6 @@ impl MockChainContext {
 }
 
 impl ChainReader for MockChainContext {
-    fn chain_type(&self) -> SelfChainType {
-        SelfChainType::Mock
-    }
-
     fn self_historical_info(&self, height: Height) -> Option<HistoricalInfo> {
         let l = height.value() as usize;
         let h = self.latest.value() as usize;

modules/src/ics02_client/context.rs

@@ -1,3 +1,7 @@
+//! ICS2 (client) context. The two traits `ClientReader` and `ClientKeeper` define the interface
+//! that any host chain must implement to be able to process any `ClientMsg`. See
+//! "ADR 003: IBC protocol implementation" for more details.
+
 use crate::ics02_client::client_def::{AnyClientState, AnyConsensusState};
 use crate::ics02_client::client_type::ClientType;
 use crate::ics02_client::error::Error;
@@ -6,12 +10,14 @@ use crate::ics02_client::handler::ClientResult::{CreateResult, UpdateResult};
 use crate::ics24_host::identifier::ClientId;
 use tendermint::block::Height;
 
+/// Defines the read-only part of ICS2 (client functions) context.
 pub trait ClientReader {
     fn client_type(&self, client_id: &ClientId) -> Option<ClientType>;
     fn client_state(&self, client_id: &ClientId) -> Option<AnyClientState>;
     fn consensus_state(&self, client_id: &ClientId, height: Height) -> Option<AnyConsensusState>;
 }
 
+/// Defines the write-only part of ICS2 (client functions) context.
 pub trait ClientKeeper {
     fn store_client_result(&mut self, handler_res: ClientResult) -> Result<(), Error> {
         match handler_res {

modules/src/ics03_connection/context.rs

@@ -1,4 +1,7 @@
-use crate::context::SelfChainType;
+//! ICS3 (connection) context. The two traits `ConnectionReader` and `ConnectionKeeper` define
+//! the interface that any host chain must implement to be able to process any `ConnectionMsg`.
+//! See "ADR 003: IBC protocol implementation" for more details.
+
 use crate::ics02_client::client_def::{AnyClientState, AnyConsensusState};
 use crate::ics03_connection::connection::{ConnectionEnd, State};
 use crate::ics03_connection::error::Error;
@@ -7,34 +10,32 @@ use crate::ics23_commitment::commitment::CommitmentPrefix;
 use crate::ics24_host::identifier::{ClientId, ConnectionId};
 use tendermint::block::Height;
 
-/// A context supplying all the necessary read-only dependencies for processing any `ICS3Msg`.
+/// A context supplying all the necessary read-only dependencies for processing any `ConnectionMsg`.
 pub trait ConnectionReader {
     /// Returns the ConnectionEnd for the given identifier `conn_id`.
-    fn fetch_connection_end(&self, conn_id: &ConnectionId) -> Option<&ConnectionEnd>;
+    fn connection_end(&self, conn_id: &ConnectionId) -> Option<&ConnectionEnd>;
 
     /// Returns the ClientState for the given identifier `client_id`.
-    fn fetch_client_state(&self, client_id: &ClientId) -> Option<AnyClientState>;
+    fn client_state(&self, client_id: &ClientId) -> Option<AnyClientState>;
 
     /// Returns the current height of the local chain.
-    fn chain_current_height(&self) -> Height;
+    fn host_current_height(&self) -> Height;
 
     /// Returns the number of consensus state historical entries for the local chain.
     fn chain_consensus_states_history_size(&self) -> usize;
 
-    fn chain_type(&self) -> SelfChainType;
-
     /// Returns the prefix that the local chain uses in the KV store.
     fn commitment_prefix(&self) -> CommitmentPrefix;
 
     /// Returns the ConsensusState that the given client stores at a specific height.
-    fn fetch_client_consensus_state(
+    fn client_consensus_state(
         &self,
         client_id: &ClientId,
         height: Height,
     ) -> Option<AnyConsensusState>;
 
-    /// Returns the ConsensusState of the local chain at a specific height.
-    fn fetch_self_consensus_state(&self, height: Height) -> Option<AnyConsensusState>;
+    /// Returns the ConsensusState of the host (local) chain at a specific height.
+    fn host_consensus_state(&self, height: Height) -> Option<AnyConsensusState>;
 
     /// Function required by ICS 03. Returns the list of all possible versions that the connection
     /// handshake protocol supports.
@@ -45,8 +46,8 @@ pub trait ConnectionReader {
     fn pick_version(&self, counterparty_candidate_versions: Vec<String>) -> String;
 }
 
-/// A context supplying all the necessary write-only dependencies (i.e., storage functionalities)
-/// for processing any `ICS3Msg`.
+/// A context supplying all the necessary write-only dependencies (i.e., storage writing facility)
+/// for processing any `ConnectionMsg`.
 pub trait ConnectionKeeper {
     fn store_connection_result(&mut self, result: ConnectionResult) -> Result<(), Error> {
         match result.connection_end.state() {

modules/src/ics03_connection/context_mock.rs

@@ -1,4 +1,7 @@
-use crate::context::{ChainReader, SelfChainType, SelfHeader};
+// TODO: This module is superseded by MockContext.
+// Will be nuked soon with https://github.com/informalsystems/ibc-rs/issues/297.
+
+use crate::context::{ChainReader, SelfHeader};
 use crate::context_mock::MockChainContext;
 use crate::ics02_client::client_def::{AnyClientState, AnyConsensusState};
 use crate::ics02_client::context::ClientReader;
@@ -62,15 +65,15 @@ impl MockConnectionContext {
 }
 
 impl ConnectionReader for MockConnectionContext {
-    fn fetch_connection_end(&self, cid: &ConnectionId) -> Option<&ConnectionEnd> {
+    fn connection_end(&self, cid: &ConnectionId) -> Option<&ConnectionEnd> {
         self.connections.get(cid)
     }
 
-    fn fetch_client_state(&self, client_id: &ClientId) -> Option<AnyClientState> {
+    fn client_state(&self, client_id: &ClientId) -> Option<AnyClientState> {
         self.client_context().client_state(client_id)
     }
 
-    fn chain_current_height(&self) -> Height {
+    fn host_current_height(&self) -> Height {
         self.chain_context().latest
     }
 
@@ -79,23 +82,19 @@ impl ConnectionReader for MockConnectionContext {
         self.chain_context().max_size()
     }
 
-    fn chain_type(&self) -> SelfChainType {
-        SelfChainType::Mock
-    }
-
     fn commitment_prefix(&self) -> CommitmentPrefix {
         CommitmentPrefix::from(vec![])
     }
 
-    fn fetch_client_consensus_state(
+    fn client_consensus_state(
         &self,
         client_id: &ClientId,
         height: Height,
     ) -> Option<AnyConsensusState> {
         self.client_context().consensus_state(client_id, height)
     }
 
-    fn fetch_self_consensus_state(&self, height: Height) -> Option<AnyConsensusState> {
+    fn host_consensus_state(&self, height: Height) -> Option<AnyConsensusState> {
         let hi = self.chain_context().self_historical_info(height)?.header;
         match hi {
             #[cfg(test)]

modules/src/ics03_connection/error.rs

@@ -21,11 +21,11 @@ pub enum Kind {
     #[error("connection end for identifier {0} was never initialized")]
     UninitializedConnection(ConnectionId),
 
-    #[error("consensus height claimed by the client on the other party is too advanced: {0}")]
-    InvalidConsensusHeight(Height),
+    #[error("consensus height claimed by the client on the other party is too advanced: {0} (host chain current height: {1})")]
+    InvalidConsensusHeight(Height, Height),
 
-    #[error("consensus height claimed by the client on the other party falls outside of trusting period: {0}")]
-    StaleConsensusHeight(Height),
+    #[error("consensus height claimed by the client on the other party falls outside of trusting period: {0} (host chain current height: {1})")]
+    StaleConsensusHeight(Height, Height),
 
     #[error("identifier error")]
     IdentifierError,

modules/src/ics03_connection/handler.rs

@@ -1,4 +1,5 @@
-//! This module implements the the processing logic for ICS3 (connection open handshake) messages.
+//! This module implements the processing logic for ICS3 (connection open handshake) messages.
+
 use crate::handler::{Event, EventType, HandlerOutput};
 use crate::ics03_connection::connection::ConnectionEnd;
 use crate::ics03_connection::context::ConnectionReader;
@@ -10,7 +11,7 @@ pub mod conn_open_ack;
 pub mod conn_open_confirm;
 pub mod conn_open_init;
 pub mod conn_open_try;
-pub mod verify;
+mod verify;
 
 #[derive(Clone, Debug)]
 pub enum ConnectionEvent {

modules/src/ics03_connection/handler/conn_open_ack.rs

@@ -1,3 +1,5 @@
+//! Protocol logic specific to processing ICS3 messages of type `MsgConnectionOpenAck`.
+
 use crate::handler::{HandlerOutput, HandlerResult};
 use crate::ics03_connection::connection::{ConnectionEnd, Counterparty, State};
 use crate::ics03_connection::context::ConnectionReader;
@@ -7,7 +9,6 @@ use crate::ics03_connection::handler::ConnectionEvent::ConnOpenAck;
 use crate::ics03_connection::handler::ConnectionResult;
 use crate::ics03_connection::msgs::conn_open_ack::MsgConnectionOpenAck;
 
-/// Protocol logic specific to processing ICS3 messages of type `MsgConnectionOpenAck`.
 pub(crate) fn process(
     ctx: &dyn ConnectionReader,
     msg: MsgConnectionOpenAck,
@@ -18,7 +19,7 @@ pub(crate) fn process(
     check_client_consensus_height(ctx, msg.consensus_height())?;
 
     // Unwrap the old connection end & validate it.
-    let mut new_conn_end = match ctx.fetch_connection_end(msg.connection_id()) {
+    let mut new_conn_end = match ctx.connection_end(msg.connection_id()) {
         // A connection end must exist and must be Init or TryOpen; otherwise we return an error.
         Some(old_conn_end) => {
             if !((old_conn_end.state_matches(&State::Init)
@@ -84,41 +85,44 @@ mod tests {
     use crate::handler::EventType;
     use crate::ics03_connection::connection::{ConnectionEnd, Counterparty, State};
     use crate::ics03_connection::context::ConnectionReader;
-    use crate::ics03_connection::context_mock::MockConnectionContext;
     use crate::ics03_connection::handler::{dispatch, ConnectionResult};
     use crate::ics03_connection::msgs::conn_open_ack::test_util::get_dummy_msg_conn_open_ack;
     use crate::ics03_connection::msgs::conn_open_ack::MsgConnectionOpenAck;
     use crate::ics03_connection::msgs::ConnectionMsg;
     use crate::ics23_commitment::commitment::CommitmentPrefix;
     use crate::ics24_host::identifier::ClientId;
+    use crate::mock_context::MockContext;
     use std::convert::TryFrom;
     use std::str::FromStr;
+    use tendermint::block::Height;
 
     #[test]
     fn conn_open_ack_msg_processing() {
         struct Test {
             name: String,
-            ctx: MockConnectionContext,
+            ctx: MockContext,
             msg: ConnectionMsg,
             want_pass: bool,
         }
 
         let client_id = ClientId::from_str("mock_clientid").unwrap();
-        let dummy_msg = MsgConnectionOpenAck::try_from(get_dummy_msg_conn_open_ack()).unwrap();
+        let msg_ack = MsgConnectionOpenAck::try_from(get_dummy_msg_conn_open_ack()).unwrap();
         let counterparty = Counterparty::new(
             client_id.clone(),
-            dummy_msg.connection_id().clone(),
+            msg_ack.connection_id().clone(),
             CommitmentPrefix::from(vec![]),
         )
         .unwrap();
-        let default_context = MockConnectionContext::new(10, 3);
+
+        // This context has very small height, tests should not pass.
+        let incorrect_context = MockContext::new(5, Height::from(3_u32));
 
         // A connection end (with incorrect state `Open`) that will be part of the context.
         let incorrect_conn_end_state = ConnectionEnd::new(
             State::Open,
             client_id.clone(),
             counterparty,
-            default_context.get_compatible_versions(),
+            incorrect_context.get_compatible_versions(),
         )
         .unwrap();
 
@@ -131,64 +135,66 @@ mod tests {
         // counterparty) that will be part of the context to exercise unsuccessful path.
         let mut incorrect_conn_end_prefix = incorrect_conn_end_state.clone();
         incorrect_conn_end_prefix.set_state(State::Init);
-        incorrect_conn_end_prefix.set_version(dummy_msg.version().clone());
+        incorrect_conn_end_prefix.set_version(msg_ack.version().clone());
 
         // Build a connection end that will exercise the successful path.
         let correct_counterparty = Counterparty::new(
             client_id.clone(),
-            dummy_msg.connection_id().clone(),
+            msg_ack.connection_id().clone(),
             CommitmentPrefix::from(b"ibc".to_vec()),
         )
         .unwrap();
         let correct_conn_end = ConnectionEnd::new(
             State::Init,
             client_id.clone(),
             correct_counterparty,
-            vec![dummy_msg.version().clone()],
+            vec![msg_ack.version().clone()],
         )
         .unwrap();
 
+        // The proofs in Ack msg have height 10, so the host chain should have at least height 10.
+        let correct_context = MockContext::new(5, Height::from(10_u32));
+
         let tests: Vec<Test> = vec![
             Test {
                 name: "Processing fails due to missing connection in context".to_string(),
-                ctx: default_context.clone(),
-                msg: ConnectionMsg::ConnectionOpenAck(dummy_msg.clone()),
+                ctx: incorrect_context.clone(),
+                msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()),
                 want_pass: false,
             },
             Test {
                 name: "Processing fails due to connections mismatch (incorrect state)".to_string(),
-                ctx: default_context
+                ctx: incorrect_context
                     .clone()
-                    .with_client_state(&client_id, 10)
-                    .add_connection(dummy_msg.connection_id().clone(), incorrect_conn_end_state),
-                msg: ConnectionMsg::ConnectionOpenAck(dummy_msg.clone()),
+                    .with_client(&client_id, Height::from(10_u32))
+                    .with_connection(msg_ack.connection_id().clone(), incorrect_conn_end_state),
+                msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()),
                 want_pass: false,
             },
             Test {
                 name: "Processing fails due to connections mismatch (incorrect versions)"
                     .to_string(),
-                ctx: default_context
+                ctx: incorrect_context
                     .clone()
-                    .with_client_state(&client_id, 10)
-                    .add_connection(dummy_msg.connection_id().clone(), incorrect_conn_end_vers),
-                msg: ConnectionMsg::ConnectionOpenAck(dummy_msg.clone()),
+                    .with_client(&client_id, Height::from(10_u32))
+                    .with_connection(msg_ack.connection_id().clone(), incorrect_conn_end_vers),
+                msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()),
                 want_pass: false,
             },
             Test {
                 name: "Processing fails: ConsensusStateVerificationFailure due to empty counterparty prefix".to_string(),
-                ctx: default_context
-                    .clone()
-                    .with_client_state(&client_id, 10)
-                    .add_connection(dummy_msg.connection_id().clone(), incorrect_conn_end_prefix),
-                msg: ConnectionMsg::ConnectionOpenAck(dummy_msg.clone()),
+                ctx: incorrect_context
+                    .with_client(&client_id, Height::from(10_u32))
+                    .with_connection(msg_ack.connection_id().clone(), incorrect_conn_end_prefix),
+                msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()),
                 want_pass: false,
             },
             Test {
                 name: "Successful processing of Ack message".to_string(),
-                ctx: default_context
-                    .with_client_state(&client_id, 10)
-                    .add_connection(dummy_msg.connection_id().clone(), correct_conn_end),
-                msg: ConnectionMsg::ConnectionOpenAck(dummy_msg.clone()),
+                ctx: correct_context
+                    .with_client(&client_id, Height::from(10_u32))
+                    .with_connection(msg_ack.connection_id().clone(), correct_conn_end),
+                msg: ConnectionMsg::ConnectionOpenAck(msg_ack.clone()),
                 want_pass: true,
             },
         ]
@@ -203,7 +209,7 @@ mod tests {
                     assert_eq!(
                         test.want_pass,
                         true,
-                        "process_ics3_msg() test passed but was supposed to fail for test: {}, \nparams {:?} {:?}",
+                        "conn_open_ack: test passed but was supposed to fail for test: {}, \nparams {:?} {:?}",
                         test.name,
                         test.msg.clone(),
                         test.ctx.clone()
@@ -212,7 +218,7 @@ mod tests {
 
                     // The object in the output is a ConnectionEnd, should have OPEN state.
                     let res: ConnectionResult = proto_output.result;
-                    assert_eq!(res.connection_id, dummy_msg.connection_id().clone());
+                    assert_eq!(res.connection_id, msg_ack.connection_id().clone());
                     assert_eq!(res.connection_end.state().clone(), State::Open);
 
                     for e in proto_output.events.iter() {
@@ -223,7 +229,7 @@ mod tests {
                     assert_eq!(
                         test.want_pass,
                         false,
-                        "process_ics3_msg() failed for test: {}, \nparams {:?} {:?} error: {:?}",
+                        "conn_open_ack: failed for test: {}, \nparams {:?} {:?} error: {:?}",
                         test.name,
                         test.msg,
                         test.ctx.clone(),