Flexible identifier selection #482
Conversation
Update ConnOpenInit to reflect changes in cosmos/ibc#482. An additional version field is added to the connection handshake and connection open init message to take in an optional field. If this field is empty, then the default versions are used for connection handshake version negotiation.
| @@ -306,6 +317,7 @@ function connOpenInit( | |||
| ```typescript | |||
| function connOpenTry( | |||
| desiredIdentifier: Identifier, | |||
| provedIdentifier: Identifier, | |||
There was a problem hiding this comment.
couldn't this just be a boolean indicating if the sending chain decided to choose the identifier or not?
There was a problem hiding this comment.
sure, though we'll still construct a "provedIdentifier" since we need to check it
| @@ -318,24 +330,27 @@ function connOpenTry( | |||
| abortTransactionUnless(validateConnectionIdentifier(desiredIdentifier)) | |||
| abortTransactionUnless(consensusHeight < getCurrentHeight()) | |||
| expectedConsensusState = getConsensusState(consensusHeight) | |||
| expected = ConnectionEnd{INIT, desiredIdentifier, getCommitmentPrefix(), counterpartyClientIdentifier, | |||
| abortTransationUnless( | |||
| provedIdentifier === desiredIdentifier || | |||
There was a problem hiding this comment.
This is a bit confusing to me.
proved identifier is empty or comes from the init chain.
Desired Identifier comes from the receiving chain. But where? There doesn't seem to be a way to say "use whatever name they suggest", or "use whatever name they suggest if it starts with 'foo'" for example.
Rather than comparing here, it would make more sense to have
function desiredIdentifier(provedIdentifier) -> Identifier {}
So the receiving module can make this decision and about if desired
There was a problem hiding this comment.
Right, that's the intended usage, it just doesn't need to happen "within" the IBC module, per se. I can add a note about it.
There was a problem hiding this comment.
Added a comment with your example.
* update connopeninit Update ConnOpenInit to reflect changes in cosmos/ibc#482. An additional version field is added to the connection handshake and connection open init message to take in an optional field. If this field is empty, then the default versions are used for connection handshake version negotiation. * add version compatibility check in open init * implement partial changes to conn open try * partial implementation of conn open ack changes * fix tests * add handshake tests * make proto * fix conflicts * fix lint * fix lint * increase code cov Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: colin axnér <[email protected]>
|
Merging this as we've implemented it downstream, but let me know if comments were not adequately addressed. |
* update connopeninit Update ConnOpenInit to reflect changes in cosmos/ibc#482. An additional version field is added to the connection handshake and connection open init message to take in an optional field. If this field is empty, then the default versions are used for connection handshake version negotiation. * add version compatibility check in open init * implement partial changes to conn open try * partial implementation of conn open ack changes * fix tests * add handshake tests * make proto * fix conflicts * fix lint * fix lint * increase code cov Co-authored-by: Federico Kunze <[email protected]>
Flexible identifier & version selection.
Closes #462
Closes #465