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.

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

Implement openUpgradeHandshake subprotocol #3644

Closed
3 tasks
colin-axner opened this issue May 24, 2023 · 4 comments
Closed
3 tasks

Implement openUpgradeHandshake subprotocol #3644

colin-axner opened this issue May 24, 2023 · 4 comments
Assignees

Comments

@colin-axner
Copy link
Contributor

colin-axner commented May 24, 2023

Summary

Implement openUpgradeHandshake subprotocol in the spec. The function name may be changed to align better with our codebase (cc @damiannolan if you have any ideas)

The check for deleting the counterparty last packet sequence send may be skipped for now as the mapping may not exist when this issue is implemented

Spec pr

Code from spec:

// caller must do all relevant checks before calling this function
function openUpgradeHandshake(
    portIdentifier: Identifier,
    channelIdentifier: Identifier,
) {
    currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier))
    upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier))

    // switch channel fields to upgrade fields
    // and set channel state to OPEN
    currentChannel.ordering = upgrade.fields.ordering
    currentChannel.version = upgrade.fields.version
    currentChannel.connectionHops = upgrade.fields.connectionHops
    currentchannel.state = OPEN
    currentChannel.flushStatus = NOTINFLUSH
    provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel)

    // delete auxilliary state
    provableStore.delete(channelUpgradePath(portIdentifier, channelIdentifier))
    privateStore.delete(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier))
}

Code is subject to change based on pr reviews at the time of writing. Please add a basic unit test


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@damiannolan
Copy link
Contributor

OpenUpgradeHandshake to me sounds a little bit synonymous to InitUpgradeHandshake in some ways. Maybe we could use something like "complete" or "finalise" instead. If I think of something else I'll drop it here

@colin-axner
Copy link
Contributor Author

Should we call this WriteUpgradeOpenChannel?

@damiannolan
Copy link
Contributor

damiannolan commented Jun 5, 2023

I guess we can roll with that for now. I still have some reservations with "open" terminology - there is also ChanOpenInit as part of the channel opening handshake.

I guess we can nail down namings later, but as a general remark part of me slightly feels that if we are changing one handler name (confirm->open) then it may be less confusing if we changed another making it two instead of one outlier.

Some food for thought could be:

func ChanUpgradePropose()

func ChanUpgradeTry()

func ChanUpgradeAck()

func ChanUpgradeAccept() / ChanUpgradeComplete()

or any other suitable namings. I like the idea of there not being just one outlier in terminology from the opening handshake and I think "accept" or "complete" terminology could fit well.

edit: I still think I prefer Init to Propose in the above tho

@DimitrisJim DimitrisJim moved this from In progress to In review in ibc-go Jun 5, 2023
@DimitrisJim
Copy link
Contributor

closed in #3759

@github-project-automation github-project-automation bot moved this from In review to Todo in ibc-go Jun 14, 2023
@DimitrisJim DimitrisJim moved this from Todo to Done in ibc-go Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants