Re-enable tests in ics02_client and foreign_client (#535)

* ClientId constructor; re-enabled ClientCreate tests

* Improved ics18; Fixed cfg cargo annotation.

* Prep for fixing ForeignClient tests.

* Enable tests in the foreign_client module

* Reuse Height parsing

* More idiomatic methods in ibc::mock::context::MockContext

* Improve assert in build_create_client_and_send

* Remove ibc::handler::Event

* Fix extract_connection_id

* Check IBCEvent produced in create_client

* Check IBCEvent produced in test_tm_create_client_ok

* update CHANGELOG

* relayer: improve tests in foreign_client

Co-authored-by: Vitor Enes <vitorenesduarte@gmail.com>

.gitignore

@@ -18,4 +18,7 @@ Cargo.lock
 .idea
 
 # Ignore VisualStudio
-.vscode
+.vscode
+
+# Ignore chain's data
+data

CHANGELOG.md

@@ -19,7 +19,7 @@
 - [relayer-cli]
   - Replace `ChannelConfig` in `Channel::new` ([#511])
   - Add `packet-send` CLI ([#470])
-  - UX improvements for relayer txs ([#536, 540])
+  - UX improvements for relayer txs ([#536, #540])
 
 - [relayer]
   - Performance improvements ([#514], [#537])
@@ -34,6 +34,11 @@
 - [modules]
   - Fix for storing `ClientType` upon 'create-client' ([#513])
 
+### BREAKING CHANGES:
+
+- [modules]
+  - The `ibc::handler::Event` is removed and handlers now produce `ibc::events::IBCEvent`s ([#535])
+
 [#94]: https://github.com/informalsystems/ibc-rs/issues/94
 [#306]: https://github.com/informalsystems/ibc-rs/issues/306
 [#470]: https://github.com/informalsystems/ibc-rs/issues/470
@@ -50,6 +55,7 @@
 [#536]: https://github.com/informalsystems/ibc-rs/issues/536
 [#537]: https://github.com/informalsystems/ibc-rs/issues/537
 [#540]: https://github.com/informalsystems/ibc-rs/issues/540
+[#535]: https://github.com/informalsystems/ibc-rs/issues/535
 
 
 ## v0.0.6

modules/src/events.rs

@@ -13,7 +13,6 @@ use std::collections::HashMap;
 use std::convert::{TryFrom, TryInto};
 use tendermint::block::Height;
 
-use tendermint::abci::Event;
 use tracing::warn;
 
 /// Events types
@@ -70,25 +69,24 @@ pub enum IBCEvent {
 }
 
 // This is tendermint specific
-pub fn from_tx_response_event(event: Event) -> Option<IBCEvent> {
+pub fn from_tx_response_event(event: &tendermint::abci::Event) -> Option<IBCEvent> {
     // Return the first hit we find
-    // Look for client event...
-    if let Some(client_res) = ClientEvents::try_from_tx(event.clone()) {
-        return Some(client_res);
-    // Look for connection event...
-    } else if let Some(conn_res) = ConnectionEvents::try_from_tx(event.clone()) {
-        return Some(conn_res);
+    if let Some(client_res) = ClientEvents::try_from_tx(event) {
+        Some(client_res)
+    } else if let Some(conn_res) = ConnectionEvents::try_from_tx(event) {
+        Some(conn_res)
     } else if let Some(chan_res) = ChannelEvents::try_from_tx(event) {
-        return Some(chan_res);
+        Some(chan_res)
+    } else {
+        None
     }
-
-    None
 }
 
 impl IBCEvent {
     pub fn to_json(&self) -> String {
         serde_json::to_string(self).unwrap()
     }
+
     pub fn height(&self) -> Height {
         match self {
             IBCEvent::NewBlock(bl) => bl.height,
@@ -102,6 +100,7 @@ impl IBCEvent {
             _ => unimplemented!(),
         }
     }
+
     pub fn set_height(&mut self, height: ICSHeight) {
         match self {
             IBCEvent::SendPacketChannel(ev) => {

modules/src/handler.rs

@@ -1,61 +1,13 @@
+use crate::events::IBCEvent;
 use std::marker::PhantomData;
 
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct Attribute {
-    key: String,
-    value: String,
-}
-
-impl Attribute {
-    pub fn new(key: String, value: String) -> Self {
-        Self { key, value }
-    }
-
-    pub fn value(&self) -> String {
-        self.value.clone()
-    }
-
-    pub fn key(&self) -> String {
-        self.key.clone()
-    }
-}
-
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub enum EventType {
-    Message,
-    Custom(String),
-}
-
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct Event {
-    pub tpe: EventType,
-    pub attributes: Vec<Attribute>,
-}
-
-impl Event {
-    pub fn new(tpe: EventType, attrs: Vec<(String, String)>) -> Self {
-        Self {
-            tpe,
-            attributes: attrs
-                .into_iter()
-                .map(|(k, v)| Attribute::new(k, v))
-                .collect(),
-        }
-    }
-
-    /// Returns a vector containing the values within all attributes of this event
-    pub fn attribute_values(&self) -> Vec<String> {
-        self.attributes.iter().map(|a| a.value.clone()).collect()
-    }
-}
-
 pub type HandlerResult<T, E> = Result<HandlerOutput<T>, E>;
 
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug)]
 pub struct HandlerOutput<T> {
     pub result: T,
     pub log: Vec<String>,
-    pub events: Vec<Event>,
+    pub events: Vec<IBCEvent>,
 }
 
 impl<T> HandlerOutput<T> {
@@ -64,10 +16,10 @@ impl<T> HandlerOutput<T> {
     }
 }
 
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, Default)]
 pub struct HandlerOutputBuilder<T> {
     log: Vec<String>,
-    events: Vec<Event>,
+    events: Vec<IBCEvent>,
     marker: PhantomData<T>,
 }
 
@@ -89,13 +41,13 @@ impl<T> HandlerOutputBuilder<T> {
         self.log.push(log.into());
     }
 
-    pub fn with_events(mut self, events: impl Into<Vec<Event>>) -> Self {
-        self.events.append(&mut events.into());
+    pub fn with_events(mut self, mut events: Vec<IBCEvent>) -> Self {
+        self.events.append(&mut events);
         self
     }
 
-    pub fn emit(&mut self, event: impl Into<Event>) {
-        self.events.push(event.into());
+    pub fn emit(&mut self, event: IBCEvent) {
+        self.events.push(event);
     }
 
     pub fn with_result(self, result: T) -> HandlerOutput<T> {

modules/src/ics02_client/client_type.rs

@@ -1,6 +1,9 @@
-use super::error;
+use std::fmt;
+
 use serde_derive::{Deserialize, Serialize};
 
+use super::error;
+
 /// Type of the client, depending on the specific consensus algorithm.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
 pub enum ClientType {
@@ -17,11 +20,17 @@ impl ClientType {
             Self::Tendermint => "07-tendermint",
 
             #[cfg(any(test, feature = "mocks"))]
-            Self::Mock => "mock",
+            Self::Mock => "9999-mock",
         }
     }
 }
 
+impl fmt::Display for ClientType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "ClientType({})", self.as_string())
+    }
+}
+
 impl std::str::FromStr for ClientType {
     type Err = error::Error;
 
@@ -39,9 +48,10 @@ impl std::str::FromStr for ClientType {
 
 #[cfg(test)]
 mod tests {
-    use super::ClientType;
     use std::str::FromStr;
 
+    use super::ClientType;
+
     #[test]
     fn parse_tendermint_client_type() {
         let client_type = ClientType::from_str("07-tendermint");

modules/src/ics02_client/context.rs

@@ -5,8 +5,7 @@
 use crate::ics02_client::client_def::{AnyClientState, AnyConsensusState};
 use crate::ics02_client::client_type::ClientType;
 use crate::ics02_client::error::Error;
-use crate::ics02_client::handler::ClientResult::{Create, Update};
-use crate::ics02_client::handler::{ClientEvent, ClientResult};
+use crate::ics02_client::handler::ClientResult::{self, Create, Update};
 use crate::ics24_host::identifier::ClientId;
 use crate::Height;
 
@@ -15,25 +14,28 @@ 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>;
+
+    /// Returns a natural number, counting how many clients have been created thus far.
+    /// The value of this counter should increase only via method `ClientKeeper::increase_client_counter`.
+    fn client_counter(&self) -> u64;
 }
 
 /// Defines the write-only part of ICS2 (client functions) context.
 pub trait ClientKeeper {
-    fn store_client_result(
-        &mut self,
-        handler_res: ClientResult,
-    ) -> Result<Vec<ClientEvent>, Error> {
+    fn store_client_result(&mut self, handler_res: ClientResult) -> Result<(), Error> {
         match handler_res {
             Create(res) => {
-                let client_id = self.next_client_id();
+                let client_id = res.client_id.clone();
+
                 self.store_client_type(client_id.clone(), res.client_type)?;
                 self.store_client_state(client_id.clone(), res.client_state.clone())?;
                 self.store_consensus_state(
-                    client_id.clone(),
+                    client_id,
                     res.client_state.latest_height(),
                     res.consensus_state,
                 )?;
-                Ok(vec![ClientEvent::ClientCreated(client_id)])
+                self.increase_client_counter();
+                Ok(())
             }
             Update(res) => {
                 self.store_client_state(res.client_id.clone(), res.client_state.clone())?;
@@ -42,13 +44,11 @@ pub trait ClientKeeper {
                     res.client_state.latest_height(),
                     res.consensus_state,
                 )?;
-                Ok(vec![ClientEvent::ClientUpdated(res.client_id)])
+                Ok(())
             }
         }
     }
 
-    fn next_client_id(&mut self) -> ClientId;
-
     /// Called upon successful client creation
     fn store_client_type(
         &mut self,
@@ -70,4 +70,9 @@ pub trait ClientKeeper {
         height: Height,
         consensus_state: AnyConsensusState,
     ) -> Result<(), Error>;
+
+    /// Called upon client creation.
+    /// Increases the counter which keeps track of how many clients have been created.
+    /// Should never fail.
+    fn increase_client_counter(&mut self);
 }

modules/src/ics02_client/error.rs

@@ -12,6 +12,9 @@ pub enum Kind {
     #[error("unknown client type: {0}")]
     UnknownClientType(String),
 
+    #[error("Client identifier constructor failed for type {0} with counter {1}")]
+    ClientIdentifierConstructor(ClientType, u64),
+
     #[error("client already exists: {0}")]
     ClientAlreadyExists(ClientId),