Remove height information from IbcEvent (informalsystems#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 informalsystems#2

* Fix get_all_events()

* fix e2e tests

Co-authored-by: Romain Ruetschi <romain@informal.systems>

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

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
 
 

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

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
 
 

relayer-cli/src/commands/listen.rs

@@ -117,7 +117,7 @@ pub fn listen(
                 let matching_events = batch
                     .events
                     .into_iter()
-                    .filter(|e| event_match(e, filters))
+                    .filter(|e| event_match(&e.event, filters))
                     .collect_vec();
 
                 if matching_events.is_empty() {

relayer-cli/src/commands/misbehaviour.rs

@@ -64,8 +64,8 @@ pub fn monitor_misbehaviour(
     while let Ok(event_batch) = subscription.recv() {
         match event_batch.deref() {
             Ok(event_batch) => {
-                for event in &event_batch.events {
-                    match event {
+                for event_with_height in &event_batch.events {
+                    match &event_with_height.event {
                         IbcEvent::UpdateClient(update) => {
                             debug!("{:?}", update);
                             misbehaviour_handling(
@@ -82,7 +82,7 @@ pub fn monitor_misbehaviour(
 
                         IbcEvent::ClientMisbehaviour(ref _misbehaviour) => {
                             // TODO - submit misbehaviour to the witnesses (our full node)
-                            return Ok(Some(event.clone()));
+                            return Ok(Some(event_with_height.event.clone()));
                         }
 
                         _ => {}
@@ -128,7 +128,7 @@ fn misbehaviour_handling<Chain: ChainHandle>(
         })?;
 
     let client = ForeignClient::restore(client_id, chain, counterparty_chain);
-    let result = client.detect_misbehaviour_and_submit_evidence(update);
+    let result = client.detect_misbehaviour_and_submit_evidence(update.as_ref());
     if let MisbehaviourResults::EvidenceSubmitted(events) = result {
         info!("evidence submission result {:?}", events);
     }

relayer-cli/src/commands/tx/client.rs

@@ -13,6 +13,7 @@ use ibc_relayer::chain::requests::{
     IncludeProof, PageRequest, QueryClientStateRequest, QueryClientStatesRequest, QueryHeight,
 };
 use ibc_relayer::config::Config;
+use ibc_relayer::event::IbcEventWithHeight;
 use ibc_relayer::foreign_client::{CreateOptions, ForeignClient};
 use tendermint_light_client_verifier::types::TrustThreshold;
 use tracing::debug;
@@ -93,12 +94,12 @@ impl Runnable for TxCreateClientCmd {
         };
 
         // Trigger client creation via the "build" interface, so that we obtain the resulting event
-        let res: Result<IbcEvent, Error> = client
+        let res: Result<IbcEventWithHeight, Error> = client
             .build_create_client_and_send(options)
             .map_err(Error::foreign_client);
 
         match res {
-            Ok(receipt) => Output::success(receipt).exit(),
+            Ok(receipt) => Output::success(receipt.event).exit(),
             Err(e) => Output::error(format!("{}", e)).exit(),
         }
     }

relayer/src/chain/cosmos.rs

@@ -52,8 +52,6 @@ use ibc::{
 };
 use ibc_proto::cosmos::staking::v1beta1::Params as StakingParams;
 
-use crate::account::Balance;
-use crate::chain::client::ClientSettings;
 use crate::chain::cosmos::batch::{
     send_batched_messages_and_wait_check_tx, send_batched_messages_and_wait_commit,
 };
@@ -67,7 +65,6 @@ use crate::chain::cosmos::query::tx::query_txs;
 use crate::chain::cosmos::query::{abci_query, fetch_version_specs, packet_query, QueryResponse};
 use crate::chain::cosmos::types::account::Account;
 use crate::chain::cosmos::types::config::TxConfig;
-use crate::chain::cosmos::types::events::channel as channel_events;
 use crate::chain::cosmos::types::gas::{default_gas_from_config, max_gas_from_config};
 use crate::chain::endpoint::{ChainEndpoint, ChainStatus, HealthCheck};
 use crate::chain::tracking::TrackedMsgs;
@@ -78,6 +75,8 @@ use crate::event::monitor::{EventMonitor, EventReceiver, TxMonitorCmd};
 use crate::keyring::{KeyEntry, KeyRing};
 use crate::light_client::tendermint::LightClient as TmLightClient;
 use crate::light_client::{LightClient, Verified};
+use crate::{account::Balance, event::IbcEventWithHeight};
+use crate::{chain::client::ClientSettings, event::ibc_event_try_from_abci_event};
 
 use super::requests::{
     IncludeProof, QueryBlockRequest, QueryChannelClientStateRequest, QueryChannelRequest,
@@ -408,7 +407,7 @@ impl CosmosSdkChain {
     async fn do_send_messages_and_wait_commit(
         &mut self,
         tracked_msgs: TrackedMsgs,
-    ) -> Result<Vec<IbcEvent>, Error> {
+    ) -> Result<Vec<IbcEventWithHeight>, Error> {
         crate::time!("send_messages_and_wait_commit");
 
         let _span =
@@ -601,7 +600,7 @@ impl ChainEndpoint for CosmosSdkChain {
     fn send_messages_and_wait_commit(
         &mut self,
         tracked_msgs: TrackedMsgs,
-    ) -> Result<Vec<IbcEvent>, Error> {
+    ) -> Result<Vec<IbcEventWithHeight>, Error> {
         let runtime = self.rt.clone();
 
         runtime.block_on(self.do_send_messages_and_wait_commit(tracked_msgs))
@@ -1452,7 +1451,7 @@ impl ChainEndpoint for CosmosSdkChain {
     ///    Therefore, for packets we perform one tx_search for each sequence.
     ///    Alternatively, a single query for all packets could be performed but it would return all
     ///    packets ever sent.
-    fn query_txs(&self, request: QueryTxRequest) -> Result<Vec<IbcEvent>, Error> {
+    fn query_txs(&self, request: QueryTxRequest) -> Result<Vec<IbcEventWithHeight>, Error> {
         crate::time!("query_txs");
         crate::telemetry!(query, self.id(), "query_txs");
 
@@ -1653,7 +1652,7 @@ fn filter_matching_event(
         return None;
     }
 
-    let ibc_event = channel_events::try_from_tx(&event)?;
+    let ibc_event = ibc_event_try_from_abci_event(&event).ok()?;
     match ibc_event {
         IbcEvent::SendPacket(ref send_ev) if matches_packet(request, seq, &send_ev.packet) => {
             Some(ibc_event)

relayer/src/chain/cosmos/batch.rs

@@ -1,6 +1,7 @@
 use core::mem;
 
 use ibc::events::IbcEvent;
+use ibc::Height;
 use ibc_proto::google::protobuf::Any;
 use prost::Message;
 use tendermint_rpc::endpoint::broadcast::tx_sync::Response;
@@ -12,6 +13,7 @@ use crate::chain::cosmos::types::tx::{TxStatus, TxSyncResult};
 use crate::chain::cosmos::wait::wait_for_block_commits;
 use crate::config::types::{MaxMsgNum, MaxTxSize, Memo};
 use crate::error::Error;
+use crate::event::IbcEventWithHeight;
 use crate::keyring::KeyEntry;
 
 pub async fn send_batched_messages_and_wait_commit(
@@ -22,7 +24,7 @@ pub async fn send_batched_messages_and_wait_commit(
     account: &mut Account,
     tx_memo: &Memo,
     messages: Vec<Any>,
-) -> Result<Vec<IbcEvent>, Error> {
+) -> Result<Vec<IbcEventWithHeight>, Error> {
     if messages.is_empty() {
         return Ok(Vec::new());
     }
@@ -106,10 +108,14 @@ async fn send_messages_as_batches(
             send_tx_with_account_sequence_retry(config, key_entry, account, tx_memo, batch).await?;
 
         if response.code.is_err() {
-            let events_per_tx = vec![IbcEvent::ChainError(format!(
+            // Note: we don't have any height information in this case. This hack will fix itself
+            // once we remove the `ChainError` event (which is not actually an event)
+            let height = Height::new(config.chain_id.version(), 1).unwrap();
+
+            let events_per_tx = vec![IbcEventWithHeight::new(IbcEvent::ChainError(format!(
                 "check_tx (broadcast_tx_sync) on chain {} for Tx hash {} reports error: code={:?}, log={:?}",
                 config.chain_id, response.hash, response.code, response.log
-            )); message_count];
+            )), height); message_count];
 
             let tx_sync_result = TxSyncResult {
                 response,

relayer/src/chain/cosmos/query/tx.rs

@@ -13,6 +13,7 @@ use crate::chain::requests::{
     QueryClientEventRequest, QueryHeight, QueryPacketEventDataRequest, QueryTxHash, QueryTxRequest,
 };
 use crate::error::Error;
+use crate::event::{ibc_event_try_from_abci_event, IbcEventWithHeight};
 
 /// This function queries transactions for events matching certain criteria.
 /// 1. Client Update request - returns a vector with at most one update client event
@@ -30,15 +31,15 @@ pub async fn query_txs(
     rpc_client: &HttpClient,
     rpc_address: &Url,
     request: QueryTxRequest,
-) -> Result<Vec<IbcEvent>, Error> {
+) -> Result<Vec<IbcEventWithHeight>, Error> {
     crate::time!("query_txs");
     crate::telemetry!(query, chain_id, "query_txs");
 
     match request {
         QueryTxRequest::Packet(request) => {
             crate::time!("query_txs: query packet events");
 
-            let mut result: Vec<IbcEvent> = vec![];
+            let mut result: Vec<IbcEventWithHeight> = vec![];
 
             for seq in &request.sequences {
                 // query first (and only) Tx that includes the event specified in the query request
@@ -144,7 +145,7 @@ fn update_client_from_tx_search_response(
     chain_id: &ChainId,
     request: &QueryClientEventRequest,
     response: TxResponse,
-) -> Result<Option<IbcEvent>, Error> {
+) -> Result<Option<IbcEventWithHeight>, Error> {
     let height = ICSHeight::new(chain_id.version(), u64::from(response.height))
         .map_err(|_| Error::invalid_height_no_source())?;
 
@@ -159,19 +160,16 @@ fn update_client_from_tx_search_response(
         .events
         .into_iter()
         .filter(|event| event.type_str == request.event_id.as_str())
-        .flat_map(|event| events::client::try_from_tx(&event))
+        .flat_map(|event| ibc_event_try_from_abci_event(&event).ok())
         .flat_map(|event| match event {
-            IbcEvent::UpdateClient(mut update) => {
-                update.common.height = height;
-                Some(update)
-            }
+            IbcEvent::UpdateClient(update) => Some(update),
             _ => None,
         })
         .find(|update| {
             update.common.client_id == request.client_id
                 && update.common.consensus_height == request.consensus_height
         })
-        .map(IbcEvent::UpdateClient))
+        .map(|update| IbcEventWithHeight::new(IbcEvent::UpdateClient(update), height)))
 }
 
 // Extract the packet events from the query_txs RPC response. For any given
@@ -186,7 +184,7 @@ fn packet_from_tx_search_response(
     request: &QueryPacketEventDataRequest,
     seq: Sequence,
     response: TxResponse,
-) -> Result<Option<IbcEvent>, Error> {
+) -> Result<Option<IbcEventWithHeight>, Error> {
     let height = ICSHeight::new(chain_id.version(), u64::from(response.height))
         .map_err(|_| Error::invalid_height_no_source())?;
 
@@ -200,7 +198,8 @@ fn packet_from_tx_search_response(
         .tx_result
         .events
         .into_iter()
-        .find_map(|ev| filter_matching_event(ev, request, seq)))
+        .find_map(|ev| filter_matching_event(ev, request, seq))
+        .map(|ibc_event| IbcEventWithHeight::new(ibc_event, height)))
 }
 
 fn filter_matching_event(
@@ -224,7 +223,7 @@ fn filter_matching_event(
         return None;
     }
 
-    let ibc_event = events::channel::try_from_tx(&event)?;
+    let ibc_event = ibc_event_try_from_abci_event(&event).ok()?;
     match ibc_event {
         IbcEvent::SendPacket(ref send_ev) if matches_packet(request, seq, &send_ev.packet) => {
             Some(ibc_event)
@@ -260,17 +259,20 @@ pub async fn query_tx_response(
 fn all_ibc_events_from_tx_search_response(
     chain_id: &ChainId,
     response: TxResponse,
-) -> Vec<IbcEvent> {
+) -> Vec<IbcEventWithHeight> {
     let height = ICSHeight::new(chain_id.version(), u64::from(response.height)).unwrap();
     let deliver_tx_result = response.tx_result;
 
     if deliver_tx_result.code.is_err() {
         // We can only return a single ChainError here because at this point
         // we have lost information about how many messages were in the transaction
-        vec![IbcEvent::ChainError(format!(
-            "deliver_tx for {} reports error: code={:?}, log={:?}",
-            response.hash, deliver_tx_result.code, deliver_tx_result.log
-        ))]
+        vec![IbcEventWithHeight::new(
+            IbcEvent::ChainError(format!(
+                "deliver_tx for {} reports error: code={:?}, log={:?}",
+                response.hash, deliver_tx_result.code, deliver_tx_result.log
+            )),
+            height,
+        )]
     } else {
         let result = deliver_tx_result
             .events