From 3430a2c38ca03a835bf82ea83a08aae7e0e90071 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Wed, 21 Oct 2020 10:31:23 +0200 Subject: [PATCH 01/12] Fix the flexible conn id in Try and Ack for connection --- .../ics03_connection/handler/conn_open_ack.rs | 9 ++++--- .../ics03_connection/msgs/conn_open_ack.rs | 25 +++++++++++++------ .../ics03_connection/msgs/conn_open_try.rs | 21 ++++++++++------ 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/modules/src/ics03_connection/handler/conn_open_ack.rs b/modules/src/ics03_connection/handler/conn_open_ack.rs index 5b1614c80d..9d487a976d 100644 --- a/modules/src/ics03_connection/handler/conn_open_ack.rs +++ b/modules/src/ics03_connection/handler/conn_open_ack.rs @@ -31,8 +31,9 @@ pub(crate) fn process( // 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().as_str().is_empty() - || old_conn_end.counterparty().connection_id() == msg.counterparty_connection_id(); + let counterparty_matches = msg.counterparty_connection_id().is_none() + || old_conn_end.counterparty().connection_id().clone() + == msg.counterparty_connection_id().unwrap(); if state_is_consistent && counterparty_matches { Ok(old_conn_end.clone()) @@ -118,7 +119,7 @@ mod tests { let msg_ack = MsgConnectionOpenAck::try_from(get_dummy_msg_conn_open_ack()).unwrap(); let counterparty = Counterparty::new( client_id.clone(), - msg_ack.counterparty_connection_id().clone(), + msg_ack.counterparty_connection_id().unwrap(), CommitmentPrefix::from(vec![]), ) .unwrap(); @@ -148,7 +149,7 @@ mod tests { // Build a connection end that will exercise the successful path. let correct_counterparty = Counterparty::new( client_id.clone(), - msg_ack.counterparty_connection_id().clone(), + msg_ack.counterparty_connection_id().unwrap(), CommitmentPrefix::from(b"ibc".to_vec()), ) .unwrap(); diff --git a/modules/src/ics03_connection/msgs/conn_open_ack.rs b/modules/src/ics03_connection/msgs/conn_open_ack.rs index e113e7210b..e5bea28653 100644 --- a/modules/src/ics03_connection/msgs/conn_open_ack.rs +++ b/modules/src/ics03_connection/msgs/conn_open_ack.rs @@ -24,7 +24,7 @@ pub const TYPE_MSG_CONNECTION_OPEN_ACK: &str = "connection_open_ack"; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct MsgConnectionOpenAck { connection_id: ConnectionId, - counterparty_connection_id: ConnectionId, + counterparty_connection_id: Option, client_state: Option, proofs: Proofs, version: String, @@ -38,8 +38,8 @@ impl MsgConnectionOpenAck { } /// Getter for accessing the connection identifier of this message. - pub fn counterparty_connection_id(&self) -> &ConnectionId { - &self.counterparty_connection_id + pub fn counterparty_connection_id(&self) -> Option { + self.counterparty_connection_id.clone() } /// Getter for accessing the client state. @@ -116,15 +116,22 @@ impl TryFrom for MsgConnectionOpenAck { Some(_) => Some(msg.proof_client.into()), }; + let counterparty_connection_id = if msg.counterparty_connection_id.is_empty() { + None + } else { + Some( + msg.counterparty_connection_id + .parse() + .map_err(|e| Kind::IdentifierError.context(e))?, + ) + }; + Ok(Self { connection_id: msg .connection_id .parse() .map_err(|e| Kind::IdentifierError.context(e))?, - counterparty_connection_id: msg - .counterparty_connection_id - .parse() - .map_err(|e| Kind::IdentifierError.context(e))?, + counterparty_connection_id, client_state: msg .client_state .map(AnyClientState::try_from) @@ -148,7 +155,9 @@ impl From for RawMsgConnectionOpenAck { fn from(ics_msg: MsgConnectionOpenAck) -> Self { RawMsgConnectionOpenAck { connection_id: ics_msg.connection_id.as_str().to_string(), - counterparty_connection_id: ics_msg.counterparty_connection_id.as_str().to_string(), + counterparty_connection_id: ics_msg + .counterparty_connection_id + .map_or_else(|| "".to_string(), |v| v.as_str().to_string()), client_state: ics_msg .client_state .map_or_else(|| None, |v| Some(v.into())), diff --git a/modules/src/ics03_connection/msgs/conn_open_try.rs b/modules/src/ics03_connection/msgs/conn_open_try.rs index b712b40a0b..211d35d813 100644 --- a/modules/src/ics03_connection/msgs/conn_open_try.rs +++ b/modules/src/ics03_connection/msgs/conn_open_try.rs @@ -26,7 +26,7 @@ pub struct MsgConnectionOpenTry { connection_id: ConnectionId, client_id: ClientId, client_state: Option, - counterparty_chosen_connection_id: ConnectionId, + counterparty_chosen_connection_id: Option, counterparty: Counterparty, counterparty_versions: Vec, proofs: Proofs, @@ -127,6 +127,17 @@ impl TryFrom for MsgConnectionOpenTry { Some(_) => Some(msg.proof_client.into()), }; + let counterparty_chosen_connection_id = if msg.counterparty_chosen_connection_id.is_empty() + { + None + } else { + Some( + msg.counterparty_chosen_connection_id + .parse() + .map_err(|e| Kind::IdentifierError.context(e))?, + ) + }; + Ok(Self { connection_id: msg .desired_connection_id @@ -141,10 +152,7 @@ impl TryFrom for MsgConnectionOpenTry { .map(AnyClientState::try_from) .transpose() .map_err(|e| Kind::InvalidProof.context(e))?, - counterparty_chosen_connection_id: msg - .counterparty_chosen_connection_id - .parse() - .map_err(|e| Kind::IdentifierError.context(e))?, + counterparty_chosen_connection_id, counterparty: msg .counterparty .ok_or_else(|| Kind::MissingCounterparty)? @@ -177,8 +185,7 @@ impl From for RawMsgConnectionOpenTry { proof_height: Some(ics_msg.proofs.height().into()), counterparty_chosen_connection_id: ics_msg .counterparty_chosen_connection_id - .as_str() - .to_string(), + .map_or_else(|| "".to_string(), |v| v.as_str().to_string()), proof_init: ics_msg.proofs.object_proof().clone().into(), proof_client: ics_msg .proofs From 7f69db469f4199f731708f0aa0d903f1cbc86d7c Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 21 Oct 2020 16:19:09 +0200 Subject: [PATCH 02/12] Avoid cloning in MsgConnectionOpenAck::counterparty_connection_id --- modules/src/ics03_connection/handler/conn_open_ack.rs | 4 ++-- modules/src/ics03_connection/msgs/conn_open_ack.rs | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/modules/src/ics03_connection/handler/conn_open_ack.rs b/modules/src/ics03_connection/handler/conn_open_ack.rs index 9d487a976d..887ff3ed8b 100644 --- a/modules/src/ics03_connection/handler/conn_open_ack.rs +++ b/modules/src/ics03_connection/handler/conn_open_ack.rs @@ -119,7 +119,7 @@ mod tests { let msg_ack = MsgConnectionOpenAck::try_from(get_dummy_msg_conn_open_ack()).unwrap(); let counterparty = Counterparty::new( client_id.clone(), - msg_ack.counterparty_connection_id().unwrap(), + msg_ack.counterparty_connection_id().unwrap().clone(), CommitmentPrefix::from(vec![]), ) .unwrap(); @@ -149,7 +149,7 @@ mod tests { // Build a connection end that will exercise the successful path. let correct_counterparty = Counterparty::new( client_id.clone(), - msg_ack.counterparty_connection_id().unwrap(), + msg_ack.counterparty_connection_id().unwrap().clone(), CommitmentPrefix::from(b"ibc".to_vec()), ) .unwrap(); diff --git a/modules/src/ics03_connection/msgs/conn_open_ack.rs b/modules/src/ics03_connection/msgs/conn_open_ack.rs index e5bea28653..6de25981c4 100644 --- a/modules/src/ics03_connection/msgs/conn_open_ack.rs +++ b/modules/src/ics03_connection/msgs/conn_open_ack.rs @@ -18,9 +18,7 @@ use crate::Height; /// Message type for the `MsgConnectionOpenAck` message. pub const TYPE_MSG_CONNECTION_OPEN_ACK: &str = "connection_open_ack"; -/// /// Message definition `MsgConnectionOpenAck` (i.e., `ConnOpenAck` datagram). -/// #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct MsgConnectionOpenAck { connection_id: ConnectionId, @@ -38,8 +36,8 @@ impl MsgConnectionOpenAck { } /// Getter for accessing the connection identifier of this message. - pub fn counterparty_connection_id(&self) -> Option { - self.counterparty_connection_id.clone() + pub fn counterparty_connection_id(&self) -> Option<&ConnectionId> { + self.counterparty_connection_id.as_ref() } /// Getter for accessing the client state. From d0ffbf3ba445169169cfba100f0c7c30bdc953af Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 21 Oct 2020 16:19:41 +0200 Subject: [PATCH 03/12] Avoid a (safe) use of unwrap --- modules/src/ics03_connection/handler/conn_open_ack.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/modules/src/ics03_connection/handler/conn_open_ack.rs b/modules/src/ics03_connection/handler/conn_open_ack.rs index 887ff3ed8b..ab4e29a3a2 100644 --- a/modules/src/ics03_connection/handler/conn_open_ack.rs +++ b/modules/src/ics03_connection/handler/conn_open_ack.rs @@ -31,9 +31,12 @@ pub(crate) fn process( // 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().clone() - == msg.counterparty_connection_id().unwrap(); + let counterparty_matches = + if let Some(counterparty_connection_id) = msg.counterparty_connection_id() { + old_conn_end.counterparty().connection_id() == counterparty_connection_id + } else { + true + }; if state_is_consistent && counterparty_matches { Ok(old_conn_end.clone()) From f1dacaae8132d2f697e4481df9b93aa31bee312a Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Wed, 21 Oct 2020 17:17:45 +0200 Subject: [PATCH 04/12] Add check for counterparty_connection_id new field in Try handler. Allow None in Counterparty structure. --- modules/src/ics03_connection/connection.rs | 28 +++++++++++++------ modules/src/ics03_connection/error.rs | 3 ++ .../ics03_connection/handler/conn_open_ack.rs | 14 ++++------ .../handler/conn_open_confirm.rs | 4 +-- .../ics03_connection/handler/conn_open_try.rs | 11 +++++++- .../src/ics03_connection/handler/verify.rs | 4 ++- .../ics03_connection/msgs/conn_open_try.rs | 5 ++++ relayer/src/tx/connection.rs | 2 +- 8 files changed, 48 insertions(+), 23 deletions(-) diff --git a/modules/src/ics03_connection/connection.rs b/modules/src/ics03_connection/connection.rs index 3cb519ca52..9b469a314e 100644 --- a/modules/src/ics03_connection/connection.rs +++ b/modules/src/ics03_connection/connection.rs @@ -120,7 +120,7 @@ impl ConnectionEnd { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Counterparty { client_id: ClientId, - connection_id: ConnectionId, + connection_id: Option, prefix: CommitmentPrefix, } @@ -130,15 +130,23 @@ impl TryFrom for Counterparty { type Error = anomaly::Error; fn try_from(value: RawCounterparty) -> Result { + let connection_id = if value.connection_id.is_empty() { + None + } else { + Some( + value + .connection_id + .parse() + .map_err(|e| Kind::IdentifierError.context(e))?, + ) + }; + Ok(Counterparty::new( value .client_id .parse() .map_err(|e| Kind::IdentifierError.context(e))?, - value - .connection_id - .parse() - .map_err(|e| Kind::IdentifierError.context(e))?, + connection_id, value .prefix .ok_or_else(|| Kind::MissingCounterparty)? @@ -152,7 +160,9 @@ impl From for RawCounterparty { fn from(value: Counterparty) -> Self { RawCounterparty { client_id: value.client_id.as_str().to_string(), - connection_id: value.connection_id.as_str().to_string(), + connection_id: value + .connection_id + .map_or_else(|| "".to_string(), |v| v.as_str().to_string()), prefix: Some(ibc_proto::ibc::core::commitment::v1::MerklePrefix { key_prefix: value.prefix.0, }), @@ -163,7 +173,7 @@ impl From for RawCounterparty { impl Counterparty { pub fn new( client_id: ClientId, - connection_id: ConnectionId, + connection_id: Option, prefix: CommitmentPrefix, ) -> Result { Ok(Self { @@ -179,8 +189,8 @@ impl Counterparty { } /// Getter for connection id. - pub fn connection_id(&self) -> &ConnectionId { - &self.connection_id + pub fn connection_id(&self) -> Option<&ConnectionId> { + self.connection_id.as_ref() } pub fn prefix(&self) -> &CommitmentPrefix { diff --git a/modules/src/ics03_connection/error.rs b/modules/src/ics03_connection/error.rs index ed720c95a6..fb3ad80c5c 100644 --- a/modules/src/ics03_connection/error.rs +++ b/modules/src/ics03_connection/error.rs @@ -53,6 +53,9 @@ pub enum Kind { #[error("invalid counterparty")] InvalidCounterparty, + #[error("counterparty chosen connection id {0} is different than the connection id {1}")] + ConnectionIdMismatch(ConnectionId, ConnectionId), + #[error("missing counterparty")] MissingCounterparty, diff --git a/modules/src/ics03_connection/handler/conn_open_ack.rs b/modules/src/ics03_connection/handler/conn_open_ack.rs index ab4e29a3a2..8c442b9058 100644 --- a/modules/src/ics03_connection/handler/conn_open_ack.rs +++ b/modules/src/ics03_connection/handler/conn_open_ack.rs @@ -31,12 +31,8 @@ pub(crate) fn process( // Check that if the msg's counterparty connection id is not empty then it matches // the old connection's counterparty. - let counterparty_matches = - if let Some(counterparty_connection_id) = msg.counterparty_connection_id() { - old_conn_end.counterparty().connection_id() == counterparty_connection_id - } else { - true - }; + let counterparty_matches = msg.counterparty_connection_id().is_none() + || old_conn_end.counterparty().connection_id() == msg.counterparty_connection_id(); if state_is_consistent && counterparty_matches { Ok(old_conn_end.clone()) @@ -62,7 +58,7 @@ pub(crate) fn process( Counterparty::new( // The counterparty is the local chain. new_conn_end.client_id().clone(), // The local client identifier. - msg.connection_id().clone(), // Local connection id. + msg.counterparty_connection_id().cloned(), // This chain's connection id as known on counterparty. ctx.commitment_prefix(), // Local commitment prefix. )?, vec![msg.version().clone()], @@ -122,7 +118,7 @@ mod tests { let msg_ack = MsgConnectionOpenAck::try_from(get_dummy_msg_conn_open_ack()).unwrap(); let counterparty = Counterparty::new( client_id.clone(), - msg_ack.counterparty_connection_id().unwrap().clone(), + msg_ack.counterparty_connection_id().cloned(), CommitmentPrefix::from(vec![]), ) .unwrap(); @@ -152,7 +148,7 @@ mod tests { // Build a connection end that will exercise the successful path. let correct_counterparty = Counterparty::new( client_id.clone(), - msg_ack.counterparty_connection_id().unwrap().clone(), + msg_ack.counterparty_connection_id().cloned(), CommitmentPrefix::from(b"ibc".to_vec()), ) .unwrap(); diff --git a/modules/src/ics03_connection/handler/conn_open_confirm.rs b/modules/src/ics03_connection/handler/conn_open_confirm.rs index f0e6d968a4..220646d210 100644 --- a/modules/src/ics03_connection/handler/conn_open_confirm.rs +++ b/modules/src/ics03_connection/handler/conn_open_confirm.rs @@ -43,7 +43,7 @@ pub(crate) fn process( Counterparty::new( // The counterparty is the local chain. new_conn_end.client_id().clone(), // The local client identifier. - msg.connection_id().clone(), // Local connection id. + Some(msg.connection_id().clone()), // Local connection id. ctx.commitment_prefix(), // Local commitment prefix. )?, new_conn_end.versions(), @@ -103,7 +103,7 @@ mod tests { MsgConnectionOpenConfirm::try_from(get_dummy_msg_conn_open_confirm()).unwrap(); let counterparty = Counterparty::new( client_id.clone(), - msg_confirm.connection_id().clone(), + Some(msg_confirm.connection_id().clone()), CommitmentPrefix::from(vec![]), ) .unwrap(); diff --git a/modules/src/ics03_connection/handler/conn_open_try.rs b/modules/src/ics03_connection/handler/conn_open_try.rs index 176123301e..ec4f4cc481 100644 --- a/modules/src/ics03_connection/handler/conn_open_try.rs +++ b/modules/src/ics03_connection/handler/conn_open_try.rs @@ -18,6 +18,15 @@ pub(crate) fn process( // Check that consensus height (for client proof) in message is not too advanced nor too old. check_client_consensus_height(ctx, msg.consensus_height())?; + if let Some(chosen_id) = msg.counterparty_chosen_connection_id() { + if chosen_id != msg.connection_id().clone() { + return Err(Into::::into(Kind::ConnectionIdMismatch( + chosen_id, + msg.connection_id().clone(), + ))); + } + } + // Unwrap the old connection end (if any) and validate it against the message. let mut new_connection_end = match ctx.connection_end(msg.connection_id()) { Some(old_conn_end) => { @@ -53,7 +62,7 @@ pub(crate) fn process( msg.counterparty().client_id().clone(), Counterparty::new( msg.client_id().clone(), - msg.connection_id().clone(), + msg.counterparty_chosen_connection_id(), ctx.commitment_prefix(), )?, msg.counterparty_versions(), diff --git a/modules/src/ics03_connection/handler/verify.rs b/modules/src/ics03_connection/handler/verify.rs index d506ddaa42..0dd11b4b71 100644 --- a/modules/src/ics03_connection/handler/verify.rs +++ b/modules/src/ics03_connection/handler/verify.rs @@ -92,13 +92,15 @@ pub fn verify_connection_proof( let client_def = AnyClient::from_client_type(client_state.client_type()); // Verify the proof for the connection state against the expected connection end. + // A counterparty connection id of None causes `unwrap()` below and indicates an internal + // error as this is the connection id on the counterparty chain that must always be present. Ok(client_def .verify_connection_state( &client_state, proof_height, connection_end.counterparty().prefix(), proof, - connection_end.counterparty().connection_id(), + &connection_end.counterparty().connection_id().unwrap(), expected_conn, ) .map_err(|_| Kind::InvalidProof.context(id.to_string()))?) diff --git a/modules/src/ics03_connection/msgs/conn_open_try.rs b/modules/src/ics03_connection/msgs/conn_open_try.rs index 211d35d813..d72f15ab9d 100644 --- a/modules/src/ics03_connection/msgs/conn_open_try.rs +++ b/modules/src/ics03_connection/msgs/conn_open_try.rs @@ -49,6 +49,11 @@ impl MsgConnectionOpenTry { self.client_state.clone() } + /// Getter for accessing the counterparty connection identifier of this message. + pub fn counterparty_chosen_connection_id(&self) -> Option { + self.counterparty_chosen_connection_id.clone() + } + /// Getter for accesing the whole counterparty of this message. Returns a `clone()`. pub fn counterparty(&self) -> Counterparty { self.counterparty.clone() diff --git a/relayer/src/tx/connection.rs b/relayer/src/tx/connection.rs index f840a75b4c..acc55b9afa 100644 --- a/relayer/src/tx/connection.rs +++ b/relayer/src/tx/connection.rs @@ -33,7 +33,7 @@ pub fn conn_init(opts: ConnectionOpenInitOptions) -> Result<(), Error> { let counterparty = Counterparty::new( opts.dest_client_id, - opts.dest_connection_id, + Some(opts.dest_connection_id), CommitmentPrefix::from(vec![]), ); From 53933944ff5de517bbdaf33565994feddee3c557 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Wed, 21 Oct 2020 18:28:31 +0200 Subject: [PATCH 05/12] fix fmt --- modules/src/ics03_connection/handler/conn_open_ack.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/src/ics03_connection/handler/conn_open_ack.rs b/modules/src/ics03_connection/handler/conn_open_ack.rs index 8c442b9058..78b4af56ec 100644 --- a/modules/src/ics03_connection/handler/conn_open_ack.rs +++ b/modules/src/ics03_connection/handler/conn_open_ack.rs @@ -59,7 +59,7 @@ pub(crate) fn process( // The counterparty is the local chain. new_conn_end.client_id().clone(), // The local client identifier. msg.counterparty_connection_id().cloned(), // This chain's connection id as known on counterparty. - ctx.commitment_prefix(), // Local commitment prefix. + ctx.commitment_prefix(), // Local commitment prefix. )?, vec![msg.version().clone()], )?; From aa6b37344a23241687b9449d065f911cc99d5afd Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Thu, 22 Oct 2020 12:51:10 +0200 Subject: [PATCH 06/12] Make the dest conn id optional in the tx raw cli --- relayer-cli/src/commands/tx/connection.rs | 10 ++++++---- relayer/src/tx/connection.rs | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/relayer-cli/src/commands/tx/connection.rs b/relayer-cli/src/commands/tx/connection.rs index 4fdd9dfeb9..1b3d316e24 100644 --- a/relayer-cli/src/commands/tx/connection.rs +++ b/relayer-cli/src/commands/tx/connection.rs @@ -22,7 +22,7 @@ pub struct TxRawConnInitCmd { #[options(free, help = "identifier of the source connection")] src_connection_id: Option, - #[options(free, help = "identifier of the destination connection")] + #[options(help = "identifier of the destination connection", short = "d")] dest_connection_id: Option, } @@ -74,9 +74,11 @@ impl TxRawConnInitCmd { let dest_connection_id = self .dest_connection_id .as_ref() - .ok_or_else(|| "missing destination connection identifier".to_string())? - .parse() - .map_err(|_| "bad destination connection identifier".to_string())?; + .map(|v| { + v.parse() + .map_err(|_| "bad destination connection identifier".to_string()) + }) + .transpose()?; let opts = ConnectionOpenInitOptions { src_client_id, diff --git a/relayer/src/tx/connection.rs b/relayer/src/tx/connection.rs index acc55b9afa..efac87bddd 100644 --- a/relayer/src/tx/connection.rs +++ b/relayer/src/tx/connection.rs @@ -15,7 +15,7 @@ pub struct ConnectionOpenInitOptions { pub src_client_id: ClientId, pub dest_client_id: ClientId, pub src_connection_id: ConnectionId, - pub dest_connection_id: ConnectionId, + pub dest_connection_id: Option, pub src_chain_config: ChainConfig, pub dest_chain_config: ChainConfig, } @@ -33,7 +33,7 @@ pub fn conn_init(opts: ConnectionOpenInitOptions) -> Result<(), Error> { let counterparty = Counterparty::new( opts.dest_client_id, - Some(opts.dest_connection_id), + opts.dest_connection_id, CommitmentPrefix::from(vec![]), ); From 3ae1bd670861f67f8a21763e8b0e233c2dd82a32 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Thu, 22 Oct 2020 12:56:58 +0200 Subject: [PATCH 07/12] Update changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1af29876eb..b45e9c1caa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,12 @@ ### FEATURES -- [changelog] Added "unreleased" section in `CHANGELOG.MD` to help streamline releases ([#274]). +- [changelog] Added "unreleased" section in `CHANGELOG.MD` to help streamline releases ([#274]) - [relayer] Integrate relayer spike into relayer crate ([#335]) +- [modules] Implement flexible connection id selection ([#332]) [#274]: https://github.com/informalsystems/ibc-rs/issues/274 +[#332]: https://github.com/informalsystems/ibc-rs/issues/332 [#335]: https://github.com/informalsystems/ibc-rs/pulls/335 ### IMPROVEMENTS From b0e86086f1d75d8555f5c3615e4dd1bdb71ae494 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Thu, 22 Oct 2020 13:12:51 +0200 Subject: [PATCH 08/12] Update modules/src/ics03_connection/msgs/conn_open_try.rs Co-authored-by: Romain Ruetschi --- modules/src/ics03_connection/msgs/conn_open_try.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/modules/src/ics03_connection/msgs/conn_open_try.rs b/modules/src/ics03_connection/msgs/conn_open_try.rs index d72f15ab9d..cd6bfea4ef 100644 --- a/modules/src/ics03_connection/msgs/conn_open_try.rs +++ b/modules/src/ics03_connection/msgs/conn_open_try.rs @@ -132,17 +132,7 @@ impl TryFrom for MsgConnectionOpenTry { Some(_) => Some(msg.proof_client.into()), }; - let counterparty_chosen_connection_id = if msg.counterparty_chosen_connection_id.is_empty() - { - None - } else { - Some( - msg.counterparty_chosen_connection_id - .parse() - .map_err(|e| Kind::IdentifierError.context(e))?, - ) - }; - + let counterparty_chosen_connection_id = msg.counterparty_chosen_connection_id.map(|s| s.parse().map_err(|e| Kind::IdentifierError.context(e))).transpose()?; Ok(Self { connection_id: msg .desired_connection_id From 38db324da647b8a15478c4d496f944d665e42ff0 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Thu, 22 Oct 2020 13:13:10 +0200 Subject: [PATCH 09/12] Update modules/src/ics03_connection/msgs/conn_open_ack.rs Co-authored-by: Romain Ruetschi --- modules/src/ics03_connection/msgs/conn_open_ack.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/modules/src/ics03_connection/msgs/conn_open_ack.rs b/modules/src/ics03_connection/msgs/conn_open_ack.rs index 6de25981c4..1007ce0125 100644 --- a/modules/src/ics03_connection/msgs/conn_open_ack.rs +++ b/modules/src/ics03_connection/msgs/conn_open_ack.rs @@ -114,16 +114,7 @@ impl TryFrom for MsgConnectionOpenAck { Some(_) => Some(msg.proof_client.into()), }; - let counterparty_connection_id = if msg.counterparty_connection_id.is_empty() { - None - } else { - Some( - msg.counterparty_connection_id - .parse() - .map_err(|e| Kind::IdentifierError.context(e))?, - ) - }; - + let counterparty_connection_id = msg.counterparty_connection_id.map(|s| s.parse().map_err(|e| Kind::IdentifierError.context(e))).transpose()?; Ok(Self { connection_id: msg .connection_id From a61b3d3ddfc9e7c722b08d3507b8604f60b5b535 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Thu, 22 Oct 2020 13:26:27 +0200 Subject: [PATCH 10/12] fix conversions --- modules/src/ics03_connection/msgs/conn_open_ack.rs | 8 +++++++- modules/src/ics03_connection/msgs/conn_open_try.rs | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/modules/src/ics03_connection/msgs/conn_open_ack.rs b/modules/src/ics03_connection/msgs/conn_open_ack.rs index 1007ce0125..6dfeaf76ba 100644 --- a/modules/src/ics03_connection/msgs/conn_open_ack.rs +++ b/modules/src/ics03_connection/msgs/conn_open_ack.rs @@ -114,7 +114,13 @@ impl TryFrom for MsgConnectionOpenAck { Some(_) => Some(msg.proof_client.into()), }; - let counterparty_connection_id = msg.counterparty_connection_id.map(|s| s.parse().map_err(|e| Kind::IdentifierError.context(e))).transpose()?; + let counterparty_connection_id = Some(msg.counterparty_connection_id) + .filter(|x| !x.is_empty()) + .as_ref() + .map(|s| FromStr::from_str(s)) + .transpose() + .map_err(|e| Kind::IdentifierError.context(e))?; + Ok(Self { connection_id: msg .connection_id diff --git a/modules/src/ics03_connection/msgs/conn_open_try.rs b/modules/src/ics03_connection/msgs/conn_open_try.rs index cd6bfea4ef..7411880f18 100644 --- a/modules/src/ics03_connection/msgs/conn_open_try.rs +++ b/modules/src/ics03_connection/msgs/conn_open_try.rs @@ -132,7 +132,13 @@ impl TryFrom for MsgConnectionOpenTry { Some(_) => Some(msg.proof_client.into()), }; - let counterparty_chosen_connection_id = msg.counterparty_chosen_connection_id.map(|s| s.parse().map_err(|e| Kind::IdentifierError.context(e))).transpose()?; + let counterparty_chosen_connection_id = Some(msg.counterparty_chosen_connection_id) + .filter(|x| !x.is_empty()) + .as_ref() + .map(|s| FromStr::from_str(s)) + .transpose() + .map_err(|e| Kind::IdentifierError.context(e))?; + Ok(Self { connection_id: msg .desired_connection_id From 96fb534d7e8d77a8af277c112fb2c2b4f7a54acc Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Thu, 22 Oct 2020 13:32:36 +0200 Subject: [PATCH 11/12] simplify more --- modules/src/ics03_connection/msgs/conn_open_ack.rs | 3 +-- modules/src/ics03_connection/msgs/conn_open_try.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/src/ics03_connection/msgs/conn_open_ack.rs b/modules/src/ics03_connection/msgs/conn_open_ack.rs index 6dfeaf76ba..1a27173fd4 100644 --- a/modules/src/ics03_connection/msgs/conn_open_ack.rs +++ b/modules/src/ics03_connection/msgs/conn_open_ack.rs @@ -116,8 +116,7 @@ impl TryFrom for MsgConnectionOpenAck { let counterparty_connection_id = Some(msg.counterparty_connection_id) .filter(|x| !x.is_empty()) - .as_ref() - .map(|s| FromStr::from_str(s)) + .map(|v| FromStr::from_str(v.as_str())) .transpose() .map_err(|e| Kind::IdentifierError.context(e))?; diff --git a/modules/src/ics03_connection/msgs/conn_open_try.rs b/modules/src/ics03_connection/msgs/conn_open_try.rs index 7411880f18..99e9f0c169 100644 --- a/modules/src/ics03_connection/msgs/conn_open_try.rs +++ b/modules/src/ics03_connection/msgs/conn_open_try.rs @@ -134,8 +134,7 @@ impl TryFrom for MsgConnectionOpenTry { let counterparty_chosen_connection_id = Some(msg.counterparty_chosen_connection_id) .filter(|x| !x.is_empty()) - .as_ref() - .map(|s| FromStr::from_str(s)) + .map(|v| FromStr::from_str(v.as_str())) .transpose() .map_err(|e| Kind::IdentifierError.context(e))?; From 10e75d5fec28497ec6effb28b55427934b017256 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Thu, 22 Oct 2020 15:53:32 +0200 Subject: [PATCH 12/12] review comments --- modules/src/ics03_connection/connection.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/modules/src/ics03_connection/connection.rs b/modules/src/ics03_connection/connection.rs index 9b469a314e..3c78aa4f64 100644 --- a/modules/src/ics03_connection/connection.rs +++ b/modules/src/ics03_connection/connection.rs @@ -1,5 +1,6 @@ use serde_derive::{Deserialize, Serialize}; use std::convert::{TryFrom, TryInto}; +use std::str::FromStr; use ibc_proto::ibc::core::connection::v1::{ ConnectionEnd as RawConnectionEnd, Counterparty as RawCounterparty, @@ -130,17 +131,11 @@ impl TryFrom for Counterparty { type Error = anomaly::Error; fn try_from(value: RawCounterparty) -> Result { - let connection_id = if value.connection_id.is_empty() { - None - } else { - Some( - value - .connection_id - .parse() - .map_err(|e| Kind::IdentifierError.context(e))?, - ) - }; - + let connection_id = Some(value.connection_id) + .filter(|x| !x.is_empty()) + .map(|v| FromStr::from_str(v.as_str())) + .transpose() + .map_err(|e| Kind::IdentifierError.context(e))?; Ok(Counterparty::new( value .client_id