Remove height information from IbcEvent (#2542)

* Introduce IbcEventWithHeight * Remove IbcEvent height() and set_height() * before AbciEvent -> Attributes refactor * stop using try_from_tx in favor of IbcEvent::try_from * Client TryFrom AbciEvent done * TryFrom AbciEvent for connection * AbciEvent -> IbcEvent scaffolding done * Client Abci conversion done * connection abci -> IbcEvent done * channel abci conversion done * Before TxSyncResult use IbcEventWithHeight * Remove client height * Remove height from connection * channel heights removed * Move tests to modules * move connection tests to modules * move channel tests to modules * fix bug * changelog * Update .changelog/unreleased/improvements/2542-remove-ibcevent-height Co-authored-by: Romain Ruetschi <romain@informal.systems> * fix potential regression * Remove `event()` from IbcEventWithHeight * Remove `height()` from IbcEventWithHeight * Fix PrettyEvents * fix regression - try 2 * cargo doc fix * move AbciEvent -> IbcEvent to relayer * Move abci_event -> ibc_event logic to relayer #2 * Fix get_all_events() * fix e2e tests Co-authored-by: Romain Ruetschi <romain@informal.systems>
informalsystems · Aug 12, 2022 · 996405c · 996405c
1 parent e04b60a
commit 996405c
Show file tree

Hide file tree

Showing 70 changed files with 1,185 additions and 1,259 deletions.
diff --git a/.changelog/unreleased/improvements/2542-remove-ibcevent-height b/.changelog/unreleased/improvements/2542-remove-ibcevent-height
@@ -0,0 +1 @@
+- Remove height attribute from `IbcEvent` and its variants ([#2542](https://github.com/informalsystems/ibc-rs/issues/2542))
diff --git a/e2e/e2e/channel.py b/e2e/e2e/channel.py
@@ -12,7 +12,6 @@ class TxChanOpenInitRes:
     connection_id: ConnectionId
     counterparty_channel_id: Optional[ChannelId]
     counterparty_port_id: PortId
-    height: BlockHeight
     port_id: PortId
 
 
@@ -49,7 +48,6 @@ class TxChanOpenTryRes:
     connection_id: ConnectionId
     counterparty_channel_id: ChannelId
     counterparty_port_id: ChannelId
-    height: BlockHeight
     port_id: PortId
 
 
@@ -88,7 +86,6 @@ class TxChanOpenAckRes:
     connection_id: ConnectionId
     counterparty_channel_id: ChannelId
     counterparty_port_id: ChannelId
-    height: BlockHeight
     port_id: PortId
 
 
@@ -125,7 +122,6 @@ class TxChanOpenConfirmRes:
     connection_id: ConnectionId
     counterparty_channel_id: ChannelId
     counterparty_port_id: ChannelId
-    height: BlockHeight
     port_id: PortId
 
 
@@ -161,7 +157,6 @@ class TxChanCloseInitRes:
     connection_id: ConnectionId
     counterparty_channel_id: ChannelId
     counterparty_port_id: ChannelId
-    height: BlockHeight
     port_id: PortId
 
 
@@ -198,7 +193,6 @@ class TxChanCloseConfirmRes:
     connection_id: ConnectionId
     counterparty_channel_id: ChannelId
     counterparty_port_id: ChannelId
-    height: BlockHeight
     port_id: PortId
 
 

diff --git a/e2e/e2e/client.py b/e2e/e2e/client.py
@@ -9,7 +9,6 @@ class ClientCreated:
     client_id: ClientId
     client_type: ClientType
     consensus_height: Height
-    height: BlockHeight
 
 
 @dataclass
@@ -33,7 +32,6 @@ class ClientUpdated:
     client_id: ClientId
     client_type: ClientType
     consensus_height: Height
-    height: BlockHeight
 
 
 @dataclass

diff --git a/e2e/e2e/packet.py b/e2e/e2e/packet.py
@@ -18,7 +18,6 @@ class Packet:
 
 @dataclass
 class TxPacketSendRes:
-    height: BlockHeight
     packet: Packet
 
 
@@ -61,7 +60,6 @@ def process(self, result: Any) -> TxPacketSendRes:
 
 @dataclass
 class TxPacketRecvRes:
-    height: BlockHeight
     packet: Packet
     ack: Hex
 
@@ -86,7 +84,6 @@ def process(self, result: Any) -> TxPacketRecvRes:
 
 @dataclass
 class TxPacketTimeoutRes:
-    height: BlockHeight
     packet: Packet
 
 
@@ -111,7 +108,6 @@ def process(self, result: Any) -> TxPacketTimeoutRes:
 
 @dataclass
 class TxPacketAckRes:
-    height: BlockHeight
     packet: Packet
 
 

diff --git a/modules/src/core/ics02_client/events.rs b/modules/src/core/ics02_client/events.rs
@@ -11,9 +11,6 @@ use crate::core::ics24_host::identifier::ClientId;
 use crate::events::{IbcEvent, IbcEventType};
 use crate::prelude::*;
 
-/// The content of the `key` field for the attribute containing the height.
-pub const HEIGHT_ATTRIBUTE_KEY: &str = "height";
-
 /// The content of the `key` field for the attribute containing the client identifier.
 pub const CLIENT_ID_ATTRIBUTE_KEY: &str = "client_id";
 
@@ -53,7 +50,6 @@ impl From<NewBlock> for IbcEvent {
 
 #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Attributes {
-    pub height: Height,
     pub client_id: ClientId,
     pub client_type: ClientType,
     pub consensus_height: Height,
@@ -62,7 +58,6 @@ pub struct Attributes {
 impl Default for Attributes {
     fn default() -> Self {
         Attributes {
-            height: Height::new(0, 1).unwrap(),
             client_id: Default::default(),
             client_type: ClientType::Tendermint,
             consensus_height: Height::new(0, 1).unwrap(),
@@ -79,34 +74,26 @@ impl Default for Attributes {
 /// Once tendermint-rs improves the API of the `Key` and `Value` types,
 /// we will be able to remove the `.parse().unwrap()` calls.
 impl From<Attributes> for Vec<Tag> {
-    fn from(a: Attributes) -> Self {
-        let height = Tag {
-            key: HEIGHT_ATTRIBUTE_KEY.parse().unwrap(),
-            value: a.height.to_string().parse().unwrap(),
-        };
+    fn from(attrs: Attributes) -> Self {
         let client_id = Tag {
             key: CLIENT_ID_ATTRIBUTE_KEY.parse().unwrap(),
-            value: a.client_id.to_string().parse().unwrap(),
+            value: attrs.client_id.to_string().parse().unwrap(),
         };
         let client_type = Tag {
             key: CLIENT_TYPE_ATTRIBUTE_KEY.parse().unwrap(),
-            value: a.client_type.as_str().parse().unwrap(),
+            value: attrs.client_type.as_str().parse().unwrap(),
         };
         let consensus_height = Tag {
             key: CONSENSUS_HEIGHT_ATTRIBUTE_KEY.parse().unwrap(),
-            value: a.height.to_string().parse().unwrap(),
+            value: attrs.consensus_height.to_string().parse().unwrap(),
         };
-        vec![height, client_id, client_type, consensus_height]
+        vec![client_id, client_type, consensus_height]
     }
 }
 
 impl core::fmt::Display for Attributes {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
-        write!(
-            f,
-            "h: {}, cs_h: {}({})",
-            self.height, self.client_id, self.consensus_height
-        )
+        write!(f, "cs_h: {}({})", self.client_id, self.consensus_height)
     }
 }
 
@@ -118,12 +105,6 @@ impl CreateClient {
     pub fn client_id(&self) -> &ClientId {
         &self.0.client_id
     }
-    pub fn height(&self) -> Height {
-        self.0.height
-    }
-    pub fn set_height(&mut self, height: Height) {
-        self.0.height = height;
-    }
 }
 
 impl From<Attributes> for CreateClient {
@@ -170,14 +151,6 @@ impl UpdateClient {
         self.common.client_type
     }
 
-    pub fn height(&self) -> Height {
-        self.common.height
-    }
-
-    pub fn set_height(&mut self, height: Height) {
-        self.common.height = height;
-    }
-
     pub fn consensus_height(&self) -> Height {
         self.common.consensus_height
     }
@@ -236,12 +209,6 @@ impl ClientMisbehaviour {
     pub fn client_id(&self) -> &ClientId {
         &self.0.client_id
     }
-    pub fn height(&self) -> Height {
-        self.0.height
-    }
-    pub fn set_height(&mut self, height: Height) {
-        self.0.height = height;
-    }
 }
 
 impl From<Attributes> for ClientMisbehaviour {
@@ -271,12 +238,6 @@ impl From<ClientMisbehaviour> for AbciEvent {
 pub struct UpgradeClient(pub Attributes);
 
 impl UpgradeClient {
-    pub fn set_height(&mut self, height: Height) {
-        self.0.height = height;
-    }
-    pub fn height(&self) -> Height {
-        self.0.height
-    }
     pub fn client_id(&self) -> &ClientId {
         &self.0.client_id
     }

diff --git a/modules/src/core/ics02_client/handler/create_client.rs b/modules/src/core/ics02_client/handler/create_client.rs
@@ -56,7 +56,6 @@ pub fn process(
 
     let event_attributes = Attributes {
         client_id,
-        height: ctx.host_height(),
         ..Default::default()
     };
     output.emit(IbcEvent::CreateClient(event_attributes.into()));
@@ -78,7 +77,6 @@ mod tests {
     use crate::core::ics02_client::client_consensus::AnyConsensusState;
     use crate::core::ics02_client::client_state::ClientState;
     use crate::core::ics02_client::client_type::ClientType;
-    use crate::core::ics02_client::context::ClientReader;
     use crate::core::ics02_client::handler::{dispatch, ClientResult};
     use crate::core::ics02_client::msgs::create_client::MsgCreateAnyClient;
     use crate::core::ics02_client::msgs::ClientMsg;
@@ -118,7 +116,6 @@ mod tests {
                 assert!(
                     matches!(event, IbcEvent::CreateClient(ref e) if e.client_id() == &expected_client_id)
                 );
-                assert_eq!(event.height(), ctx.host_height());
                 match result {
                     ClientResult::Create(create_result) => {
                         assert_eq!(create_result.client_type, ClientType::Mock);
@@ -187,7 +184,6 @@ mod tests {
                     assert!(
                         matches!(event, IbcEvent::CreateClient(ref e) if e.client_id() == &expected_client_id)
                     );
-                    assert_eq!(event.height(), ctx.host_height());
                     match result {
                         ClientResult::Create(create_res) => {
                             assert_eq!(create_res.client_type, msg.client_state.client_type());
@@ -251,7 +247,6 @@ mod tests {
                 assert!(
                     matches!(event, IbcEvent::CreateClient(ref e) if e.client_id() == &expected_client_id)
                 );
-                assert_eq!(event.height(), ctx.host_height());
                 match result {
                     ClientResult::Create(create_res) => {
                         assert_eq!(create_res.client_type, ClientType::Tendermint);

diff --git a/modules/src/core/ics02_client/handler/update_client.rs b/modules/src/core/ics02_client/handler/update_client.rs
@@ -93,7 +93,6 @@ pub fn process(
 
     let event_attributes = Attributes {
         client_id,
-        height: ctx.host_height(),
         ..Default::default()
     };
     output.emit(IbcEvent::UpdateClient(event_attributes.into()));
@@ -109,7 +108,6 @@ mod tests {
     use crate::core::ics02_client::client_consensus::AnyConsensusState;
     use crate::core::ics02_client::client_state::{AnyClientState, ClientState};
     use crate::core::ics02_client::client_type::ClientType;
-    use crate::core::ics02_client::context::ClientReader;
     use crate::core::ics02_client::error::{Error, ErrorDetail};
     use crate::core::ics02_client::handler::dispatch;
     use crate::core::ics02_client::handler::ClientResult::Update;
@@ -157,7 +155,6 @@ mod tests {
                 assert!(
                     matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
                 );
-                assert_eq!(event.height(), ctx.host_height());
                 assert!(log.is_empty());
                 // Check the result
                 match result {
@@ -241,7 +238,6 @@ mod tests {
                     assert!(
                         matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
                     );
-                    assert_eq!(event.height(), ctx.host_height());
                     assert!(log.is_empty());
                 }
                 Err(err) => {
@@ -309,7 +305,6 @@ mod tests {
                 assert!(
                     matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
                 );
-                assert_eq!(event.height(), ctx.host_height());
                 assert!(log.is_empty());
                 // Check the result
                 match result {
@@ -387,7 +382,6 @@ mod tests {
                 assert!(
                     matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
                 );
-                assert_eq!(event.height(), ctx.host_height());
                 assert!(log.is_empty());
                 // Check the result
                 match result {
@@ -466,7 +460,6 @@ mod tests {
                 assert!(
                     matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
                 );
-                assert_eq!(event.height(), ctx.host_height());
                 assert!(log.is_empty());
                 // Check the result
                 match result {

diff --git a/modules/src/core/ics02_client/handler/upgrade_client.rs b/modules/src/core/ics02_client/handler/upgrade_client.rs
@@ -66,7 +66,6 @@ pub fn process(
     });
     let event_attributes = Attributes {
         client_id,
-        height: ctx.host_height(),
         ..Default::default()
     };
 
@@ -80,7 +79,6 @@ mod tests {
 
     use core::str::FromStr;
 
-    use crate::core::ics02_client::context::ClientReader;
     use crate::core::ics02_client::error::{Error, ErrorDetail};
     use crate::core::ics02_client::handler::dispatch;
     use crate::core::ics02_client::handler::ClientResult::Upgrade;
@@ -125,7 +123,6 @@ mod tests {
                 assert!(
                     matches!(event, IbcEvent::UpgradeClient(ref e) if e.client_id() == &msg.client_id)
                 );
-                assert_eq!(event.height(), ctx.host_height());
                 assert!(log.is_empty());
                 // Check the result
                 match result {