-
Notifications
You must be signed in to change notification settings - Fork 390
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
ICS4: Channel Upgradability Spec #677
Conversation
if !IsCompatible(counterpartyChannel, proposedUpgradeChannel) { | ||
restoreChannel() | ||
return | ||
} | ||
|
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 seems like leftover code from crossing hellos case handled above?
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.
Indeed. counterpartyChannel
is not defined.
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.
Sorry my mistake. Needed to define and prove counterpartyChannel earlier. counterpartyChannel is the channel that initiator upgraded to.
proposedUpgradeChannel is the channel that TRY is going to upgrade to.
Similar to the connection upgrade channel there needs to be some check on compatibility that will not be specified here since it is upgrade-specific.
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
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.
Looks good. ACK the protocol. See my comments below.
|
||
// call modules onChanUpgradeAck callback | ||
module = lookupModule(portIdentifier) | ||
err = module.onChanUpgradeAck( |
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.
In #686, onChanUpgradeAck
returns no error.
Co-authored-by: Marius Poke <marius.poke@posteo.de>
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.
Great work. Just one minor comment.
No description provided.