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

Update protos to IBC v3.0.0-rc.0 and Cosmos SDK v0.45.1 #1825

Merged
merged 22 commits into from
Feb 18, 2022
Merged

Conversation

romac
Copy link
Member

@romac romac commented Jan 31, 2022

Closes: cosmos/ibc-proto-rs#5

Description

  • Update Protocol Buffers definitons to Cosmos SDK v0.45.1 et IBC v3.0.0-rc.0

  • Remove channel version negotiation step during channel handshake (done in Handle non-standard ports in channel handshake #1840)

    In IBC v2, relayers would query the NegotiateAppVersion endpoint before calling ChanOpenTry.
    Applications would then need to verify that the passed in version was correct.
    As of IBC v3, applications will perform this version negotiation during the channel handshake, thus removing the need for the relayer to query the version.

  • Handle non-standard ports in channel handshake (done in Handle non-standard ports in channel handshake #1840)


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@romac romac changed the title Update to IBC v3.0.0-alpha2 and Cosmos SDK v0.45 Update protos to IBC v3.0.0-alpha2 and Cosmos SDK v0.45 Jan 31, 2022
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super! Left a couple of suggestions for further cleanup, but I think we're pretty much ready.

We should also bump up the IBC_GO_MODULE_VERSION_REQ to match ibc-go v3

proto/src/prost/COSMOS_IBC_COMMIT Outdated Show resolved Hide resolved
relayer/src/channel/version.rs Outdated Show resolved Hide resolved
relayer/src/channel/version.rs Outdated Show resolved Hide resolved
@romac
Copy link
Member Author

romac commented Feb 1, 2022

We should also bump up the IBC_GO_MODULE_VERSION_REQ to match ibc-go v3

b0b29ed

@romac romac marked this pull request as ready for review February 1, 2022 09:42
@romac romac requested a review from adizere February 1, 2022 14:29
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, awesome work Romain!

I think we still have 3 sets of COSMOS_IBC|SDK_COMMIT files, would be cool to reduce the redundancy there.

If we decide to do a 0.11.1 release then maybe merge this PR only after the release.

In IBC v2, relayers would query the `NegotiateAppVersion` endpoint before calling `ChanOpenTry`.
Applications would then need to verify that the passed in version was correct.

As of IBC v3, applications will perform this version negotiation during the channel handshake,
thus removing the need for the relayer to query the version.
@romac
Copy link
Member Author

romac commented Feb 2, 2022

I think we still have 3 sets of COSMOS_IBC|SDK_COMMIT files, would be cool to reduce the redundancy there.

Done :)

@adizere adizere self-requested a review February 3, 2022 16:58
@adizere
Copy link
Member

adizere commented Feb 3, 2022

We might have moved a bit too fast with upgrading the files: https://github.com/cosmos/ibc-go/releases/tag/v3.0.0-beta1. I think we should regenerate the files from v3-beta1, or maybe even better, wait it out until v3 comes out (towards end of Febr)?

At a high level, when comparing to v3.0.0-alpha2 this release:

  • addresses all issues raised by Trail of Bits during an audit of ICS27
  • simplifies controller chain portID to only contain a prefix and the owner address
  • introduces JSON encoded metadata to ICS27 to store the version, connection sequences, encoding/txtype, and the interchain account address
  • the ics27 host chain will include the MsgResponse in the packet acknowledgement upon successful transaction execution. Please see the auth module documentation for more information.

@romac
Copy link
Member Author

romac commented Feb 4, 2022

Sure, that was expected. We don't want to merge this before v3 is out anyway :)

* Unify error messages casing

* Revert unrelated change

* Fix capitalization of CheckTx
* Use empty version for `ChanOpenInit` when port is non-standard and source channel version for `ChanOpenTry` steps

* Remove unused error variant

* Remove misleading comment

* Add changelog entry
@romac romac mentioned this pull request Feb 9, 2022
5 tasks
@romac romac marked this pull request as draft February 14, 2022 15:25
@romac romac changed the title Update protos to IBC v3.0.0-alpha2 and Cosmos SDK v0.45 Update protos to IBC v3.0.0-rc.0 and Cosmos SDK v0.45.1 Feb 18, 2022
@romac romac marked this pull request as ready for review February 18, 2022 10:47
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

We'll do more extensive manual testing, and also give the master branch for testing to a couple of operators once this is merged.

Great work Romain!

@romac romac merged commit 0edb6b7 into master Feb 18, 2022
@romac romac deleted the romac/1797-ibc-v3 branch February 18, 2022 12:05
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants