Explicit assumptions in packet_from_tx_search_response

CHANGELOG.md

@@ -1,6 +1,13 @@
 # Changelog
 
-[comment]: <> (## Unreleased Changes)
+## Unreleased Changes
+
+### BUG FIXES:
+
+- [relayer-cli]
+  - Fix wrong acks sent with `tx raw packet-ack` in a 3-chain setup ([#614])
+
+[#614]: https://github.com/informalsystems/ibc-rs/issues/614
 
 ## v0.1.0
 *February 4, 2021*

relayer/src/chain/cosmos.rs

@@ -6,6 +6,7 @@ use std::{
 use anomaly::fail;
 use bitcoin::hashes::hex::ToHex;
 use crossbeam_channel as channel;
+use ibc::ics04_channel::events as ChannelEvents;
 use prost::Message;
 use prost_types::Any;
 use tendermint::abci::Path as TendermintABCIPath;
@@ -688,7 +689,7 @@ impl Chain for CosmosSDKChain {
                 ))
                 .unwrap(); // todo
 
-            let mut events = packet_from_tx_search_response(&request, *seq, &response)?
+            let mut events = packet_from_tx_search_response(&request, *seq, response)?
                 .map_or(vec![], |v| vec![v]);
             result.append(&mut events);
         }
@@ -839,28 +840,35 @@ fn packet_query(request: &QueryPacketEventDataRequest, seq: &Sequence) -> Result
     ))
 }
 
-// Extract the packet events from the query_tx RPC response. The response includes the full set of events
-// from the Tx-es where there is at least one request query match.
-// For example, the query request asks for the Tx for packet with sequence 3, and both 3 and 4 were
-// committed in one Tx. In this case the response includes the events for 3 and 4.
+// Extract the packet events from the query_txs RPC response. For any given
+// packet query, there is at most one Tx matching such query. Moreover, a Tx may
+// contain several events, but a single one must match the packet query.
+// For example, if we're querying for the packet with sequence 3 and this packet
+// was committed in some Tx along with the packet with sequence 4, the response
+// will include both packets. For this reason, we iterate all packets in the Tx,
+// searching for those that match (which must be a single one).
 fn packet_from_tx_search_response(
     request: &QueryPacketEventDataRequest,
     seq: Sequence,
-    response: &tendermint_rpc::endpoint::tx_search::Response,
+    mut response: tendermint_rpc::endpoint::tx_search::Response,
 ) -> Result<Option<IBCEvent>, Error> {
-    // TODO: remove loop as `response.txs.len() <= 1`
-    for r in response.txs.iter() {
+    assert!(
+        response.txs.len() <= 1,
+        "packet_from_tx_search_response: unexpected number of txs"
+    );
+    if let Some(r) = response.txs.pop() {
         let height = r.height;
         if height.value() > request.height.revision_height {
-            continue;
+            return Ok(None);
         }
 
-        for e in r.tx_result.events.iter() {
+        let mut matching = Vec::new();
+        for e in r.tx_result.events {
             if e.type_str != request.event_id.as_str() {
                 continue;
             }
 
-            let res = from_tx_response_event(e);
+            let res = ChannelEvents::try_from_tx(&e);
             if res.is_none() {
                 continue;
             }
@@ -885,10 +893,18 @@ fn packet_from_tx_search_response(
                 continue;
             }
 
-            return Ok(Some(event));
+            matching.push(event);
         }
+
+        assert_eq!(
+            matching.len(),
+            1,
+            "packet_from_tx_search_response: unexpected number of matching packets"
+        );
+        Ok(matching.pop())
+    } else {
+        Ok(None)
     }
-    Ok(None)
 }
 
 /// Perform a generic `abci_query`, and return the corresponding deserialized response data.