Allow a conn open ack to succeed in the happy case

modules/src/ics02_client/client_def.rs

@@ -73,7 +73,7 @@ pub trait ClientDef: Clone {
         height: Height,
         prefix: &CommitmentPrefix,
         proof: &CommitmentProofBytes,
-        connection_id: &ConnectionId,
+        connection_id: Option<&ConnectionId>,
         expected_connection_end: &ConnectionEnd,
     ) -> Result<(), Box<dyn std::error::Error>>;
 
@@ -472,7 +472,7 @@ impl ClientDef for AnyClient {
         height: Height,
         prefix: &CommitmentPrefix,
         proof: &CommitmentProofBytes,
-        connection_id: &ConnectionId,
+        connection_id: Option<&ConnectionId>,
         expected_connection_end: &ConnectionEnd,
     ) -> Result<(), Box<dyn std::error::Error>> {
         match self {

modules/src/ics03_connection/handler/conn_open_ack.rs

@@ -30,10 +30,19 @@ pub(crate) fn process(
                 || old_conn_end.state_matches(&State::TryOpen)
                     && old_conn_end.versions().get(0).eq(&Some(msg.version()));
 
-            // Check that if the msg's counterparty connection id is not empty then it matches
-            // the old connection's counterparty.
-            let counterparty_matches = msg.counterparty_connection_id().is_none()
-                || old_conn_end.counterparty().connection_id() == msg.counterparty_connection_id();
+            // Check that if we have a counterparty connection id, then it
+            // matches the one in the message
+            let counterparty_matches = if let Some(counterparty_connection_id) =
+                old_conn_end.counterparty().connection_id()
+            {
+                // it's okay to unwrap as `msg.counterparty_connection_id`
+                // should always be set
+                // TODO: `msg.counterparty_connection_id` should simply be a
+                //       `ConnectionId`, not an `Option<ConnectionId>`
+                msg.counterparty_connection_id.as_ref().unwrap() == counterparty_connection_id
+            } else {
+                true
+            };
 
             if state_is_consistent && counterparty_matches {
                 Ok(old_conn_end)
@@ -168,16 +177,6 @@ mod tests {
             CommitmentPrefix::from(vec![]), // incorrect field
         ));
 
-        // A connection end with correct state & prefix, but incorrect counterparty; exercises
-        // unsuccessful processing path.
-        let mut conn_end_cparty = conn_end_open.clone();
-        conn_end_cparty.set_state(State::Init);
-        conn_end_cparty.set_counterparty(Counterparty::new(
-            client_id.clone(),
-            None, // incorrect field
-            CommitmentPrefix::from(b"ibc".to_vec()),
-        ));
-
         let tests: Vec<Test> = vec![
             Test {
                 name: "Successful processing of an Ack message".to_string(),
@@ -216,15 +215,6 @@ mod tests {
                 want_pass: false,
                 error_kind: Some(Kind::ConsensusStateVerificationFailure(proof_height))
             },
-            Test {
-                name: "Processing fails due to mismatching counterparty conn id".to_string(),
-                ctx: default_context
-                    .with_client(&client_id, proof_height)
-                    .with_connection(conn_id.clone(), conn_end_cparty),
-                msg: ConnectionMsg::ConnectionOpenAck(Box::new(msg_ack)),
-                want_pass: false,
-                error_kind: Some(Kind::ConnectionMismatch(conn_id))
-            },
             /*
             Test {
                 name: "Processing fails due to MissingLocalConsensusState".to_string(),

modules/src/ics03_connection/handler/verify.rs

@@ -96,7 +96,7 @@ pub fn verify_connection_proof(
             proof_height,
             connection_end.counterparty().prefix(),
             proof,
-            &connection_end.counterparty().connection_id().unwrap(),
+            connection_end.counterparty().connection_id(),
             expected_conn,
         )
         .map_err(|_| Kind::InvalidProof)?)

modules/src/ics07_tendermint/client_def.rs

@@ -57,7 +57,7 @@ impl ClientDef for TendermintClient {
         _height: Height,
         _prefix: &CommitmentPrefix,
         _proof: &CommitmentProofBytes,
-        _connection_id: &ConnectionId,
+        _connection_id: Option<&ConnectionId>,
         _expected_connection_end: &ConnectionEnd,
     ) -> Result<(), Box<dyn std::error::Error>> {
         todo!()

modules/src/mock/client_def.rs

@@ -64,7 +64,7 @@ impl ClientDef for MockClient {
         _height: Height,
         _prefix: &CommitmentPrefix,
         _proof: &CommitmentProofBytes,
-        _connection_id: &ConnectionId,
+        _connection_id: Option<&ConnectionId>,
         _expected_connection_end: &ConnectionEnd,
     ) -> Result<(), Box<dyn std::error::Error>> {
         Ok(())