-
Notifications
You must be signed in to change notification settings - Fork 329
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.
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
Allow a conn open ack to succeed in the happy case #700
Conversation
conn_end_cparty.set_state(State::Init); | ||
conn_end_cparty.set_counterparty(Counterparty::new( | ||
client_id.clone(), | ||
None, // incorrect field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test has been removed as my understanding is that msg.counterparty_connection_id
should always be a Some
(which hints that it should not be an Option
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this field is no longer an Option
.
@@ -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(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the connection is in the Init
state, connection_end.counterparty().connection_id()
is a None
, and thus it's not okay to unwrap
.
Codecov Report
@@ Coverage Diff @@
## master #700 +/- ##
=========================================
+ Coverage 13.6% 44.1% +30.4%
=========================================
Files 69 150 +81
Lines 3752 9764 +6012
Branches 1374 0 -1374
=========================================
+ Hits 513 4312 +3799
- Misses 2618 5452 +2834
+ Partials 621 0 -621
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you Vitor.
* Allow a conn open ack to succeed * Remove invalid test * update CHANGELOG * Make MsgConnectionOpenAck.counterparty_connection_id not an Option
Closes: #699
Description
Currently, the conn open ack handler requires a connection to have the counterparty connection id set (line 36):
https://github.com/informalsystems/ibc-rs/blob/02aef9cf5055a3817579b555c84289b99d5d08a8/modules/src/ics03_connection/handler/conn_open_ack.rs#L33-L45
However, if the connection is in the
Init
state (the happy/normal case), the counterparty connection id is unknown (i.e. aNone
), in which case the conn open ack fails with aConnectionMismatch
.With this PR, we allow the counterparty connection id to be
None
, and only fail with aConnectionMismatch
in case it's aSome
(that doesn't match).Since the
MsgConnectionOpenAck.counterparty_connection_id
field is always required, this PR also changes it to simply be aConnectionId
(instead of anOption<ConnectionId>
).For contributor use:
docs/
) and code comments.Files changed
in the Github PR explorer.