Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix the flexible conn id for connection messages #334

Merged
merged 13 commits into from
Oct 22, 2020
Merged

Fix the flexible conn id for connection messages #334

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3430a2c
Fix the flexible conn id in Try and Ack for connection
ancazamfir Oct 21, 2020
7f69db4
Avoid cloning in MsgConnectionOpenAck::counterparty_connection_id
romac Oct 21, 2020
d0ffbf3
Avoid a (safe) use of unwrap
romac Oct 21, 2020
f1dacaa
Add check for counterparty_connection_id new field in Try handler.
ancazamfir Oct 21, 2020
5393394
fix fmt
ancazamfir Oct 21, 2020
aa6b373
Make the dest conn id optional in the tx raw cli
ancazamfir Oct 22, 2020
744093a
Merge branch 'master' into anca/flexible_id
ancazamfir Oct 22, 2020
3ae1bd6
Update changelog
ancazamfir Oct 22, 2020
b0e8608
Update modules/src/ics03_connection/msgs/conn_open_try.rs
ancazamfir Oct 22, 2020
38db324
Update modules/src/ics03_connection/msgs/conn_open_ack.rs
ancazamfir Oct 22, 2020
a61b3d3
fix conversions
ancazamfir Oct 22, 2020
96fb534
simplify more
ancazamfir Oct 22, 2020
10e75d5
review comments
ancazamfir Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions modules/src/ics03_connection/handler/conn_open_ack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +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().as_str().is_empty()
|| old_conn_end.counterparty().connection_id() == msg.counterparty_connection_id();
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())
Expand Down Expand Up @@ -118,7 +122,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().clone(),
CommitmentPrefix::from(vec![]),
)
.unwrap();
Expand Down Expand Up @@ -148,7 +152,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().clone(),
CommitmentPrefix::from(b"ibc".to_vec()),
)
.unwrap();
Expand Down
27 changes: 17 additions & 10 deletions modules/src/ics03_connection/msgs/conn_open_ack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ 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,
counterparty_connection_id: ConnectionId,
counterparty_connection_id: Option<ConnectionId>,
client_state: Option<AnyClientState>,
proofs: Proofs,
version: String,
Expand All @@ -38,8 +36,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<&ConnectionId> {
self.counterparty_connection_id.as_ref()
}

/// Getter for accessing the client state.
Expand Down Expand Up @@ -116,15 +114,22 @@ impl TryFrom<RawMsgConnectionOpenAck> 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))?,
)
};

ancazamfir marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -148,7 +153,9 @@ impl From<MsgConnectionOpenAck> 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())),
Expand Down
21 changes: 14 additions & 7 deletions modules/src/ics03_connection/msgs/conn_open_try.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub struct MsgConnectionOpenTry {
connection_id: ConnectionId,
client_id: ClientId,
client_state: Option<AnyClientState>,
counterparty_chosen_connection_id: ConnectionId,
counterparty_chosen_connection_id: Option<ConnectionId>,
counterparty: Counterparty,
counterparty_versions: Vec<String>,
proofs: Proofs,
Expand Down Expand Up @@ -127,6 +127,17 @@ impl TryFrom<RawMsgConnectionOpenTry> 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))?,
)
};

ancazamfir marked this conversation as resolved.
Show resolved Hide resolved
Ok(Self {
connection_id: msg
.desired_connection_id
Expand All @@ -141,10 +152,7 @@ impl TryFrom<RawMsgConnectionOpenTry> 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)?
Expand Down Expand Up @@ -177,8 +185,7 @@ impl From<MsgConnectionOpenTry> 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
Expand Down