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.

Sign up for GitHub

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

Jump to bottom

feat(ica): allow unordered ica channels (backport #5633) #5647

Merged
merged 8 commits into from
Jan 19, 2024
Merged

feat(ica): allow unordered ica channels (backport #5633) #5647

merged 8 commits into from
Jan 19, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jan 17, 2024

This is an automatic backport of pull request #5633 done by Mergify.
Cherry-pick of 6174822 has failed:

On branch mergify/bp/release/v8.1.x/pr-5633
Your branch is up to date with 'origin/release/v8.1.x'.

You are currently cherry-picking commit 61748221.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   modules/apps/27-interchain-accounts/controller/client/cli/tx.go
	modified:   modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go
	modified:   modules/apps/27-interchain-accounts/controller/keeper/account.go
	modified:   modules/apps/27-interchain-accounts/controller/keeper/handshake.go
	modified:   modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
	modified:   modules/apps/27-interchain-accounts/controller/keeper/msg_server.go
	modified:   modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go
	modified:   modules/apps/27-interchain-accounts/controller/types/msgs.go
	modified:   modules/apps/27-interchain-accounts/controller/types/tx.pb.go
	modified:   modules/apps/27-interchain-accounts/host/ibc_module_test.go
	modified:   modules/apps/27-interchain-accounts/host/keeper/handshake.go
	modified:   modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
	modified:   modules/apps/callbacks/callbacks_test.go
	modified:   proto/ibc/applications/interchain_accounts/controller/v1/tx.proto

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   docs/docs/02-apps/02-interchain-accounts/01-overview.md
	deleted by us:   docs/docs/02-apps/02-interchain-accounts/05-messages.md
	deleted by us:   docs/docs/02-apps/02-interchain-accounts/08-client.md
	deleted by us:   docs/docs/02-apps/02-interchain-accounts/09-active-channels.md
	deleted by us:   e2e/tests/core/04-channel/upgrades_test.go
	deleted by us:   e2e/tests/interchain_accounts/base_test.go
	deleted by us:   e2e/tests/interchain_accounts/gov_test.go
	deleted by us:   e2e/tests/interchain_accounts/groups_test.go
	deleted by us:   e2e/tests/interchain_accounts/incentivized_test.go
	deleted by us:   e2e/tests/interchain_accounts/localhost_test.go
	deleted by us:   e2e/tests/interchain_accounts/params_test.go
	deleted by us:   e2e/tests/upgrades/genesis_test.go
	deleted by us:   e2e/testsuite/testconfig.go
	both modified:   modules/apps/27-interchain-accounts/controller/types/msgs_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@srdtrk @mergify
feat(ica): allow unordered ica channels (#5633)
709cf8a 
* Remove order check for ICA host and controller upgrade callbacks (#5561)

* imp: remove the channel type = ordered checks from both host and controller (#5578)

* rm checks and tests, amend docustring

* rm unnecessary test

* When a channel reopens the ordering and metadata must not change (#5562)

* chore: require active channel be CLOSED before re-opening. (#5601)

* docs: Update ICA documentation with support for unordered channels (#5607)

* Allow specifying order when registering ICA account (#5608)

* proto: Add Order to MsgRegisterInterchainAccount.

* chore: apply proto changes to go files.

* Add ordering to cli tx for Register.

* Add documentation line for tx now accepting ordering.

* Address feedback review.

Co-authored-by: Carlos Rodriguez <[email protected]>

* Address Cian's feedback; spacing.

---------

Co-authored-by: Carlos Rodriguez <[email protected]>

* docs: ICA register CLI (#5625)

* imp(ica/host): removed previous version validation check (#5613)

* imp: removed validation check

* test: updated icahost test

* docs: added godocs

* docs: added godocs

* chore(ica/host): require active channel be CLOSED before re-opening (#5630)

* chore(ica/host): require active channel be CLOSED before re-opening

* Update modules/apps/27-interchain-accounts/host/keeper/handshake_test.go

Co-authored-by: DimitrisJim <[email protected]>

---------

Co-authored-by: DimitrisJim <[email protected]>

* e2e: ordered ica channel is upgraded to unordered (#5616)

* E2E test where unordered channel is used with ICA (#5566)

* test: add test to use an unordered ICA channel

* chore: add hard coded UNORDERED channel Order

* proto: Add Order to MsgRegisterInterchainAccount.

* chore: apply proto changes to go files.

* chore: apply proto changes to go files.

* chore: e2e test passing with hard coded ordered value

* Add ordering to cli tx for Register.

* Add documentation line for tx now accepting ordering.

* Address feedback review.

Co-authored-by: Carlos Rodriguez <[email protected]>

* Address Cian's feedback; spacing.

* Update e2e/tests/interchain_accounts/base_test.go

Co-authored-by: Charly <[email protected]>

---------

Co-authored-by: DimitrisJim <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Charly <[email protected]>

* fix: host chan open try test (#5632)

* chore: fix linter and merge main

* chore: doc lint issue fix

* docs: add extra information about ICA channel reopening (#5631)

* docs: add extra information about ICA channel reopening

* add link to active channels section

* Apply suggestions from code review

Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: srdtrk <[email protected]>

* Update 01-overview.md

---------

Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: srdtrk <[email protected]>

* chore: rm order checks reintroduced after merge conflict.

* e2e: comment out failing e2e

* chore: lintertroubles

---------

Co-authored-by: Cian Hatton <[email protected]>
Co-authored-by: Charly <[email protected]>
Co-authored-by: DimitrisJim <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: chatton <[email protected]>
Co-authored-by: Colin Axnér <[email protected]>
(cherry picked from commit 6174822)

# Conflicts:
#	docs/docs/02-apps/02-interchain-accounts/01-overview.md
#	docs/docs/02-apps/02-interchain-accounts/05-messages.md
#	docs/docs/02-apps/02-interchain-accounts/08-client.md
#	docs/docs/02-apps/02-interchain-accounts/09-active-channels.md
#	e2e/tests/core/04-channel/upgrades_test.go
#	e2e/tests/interchain_accounts/base_test.go
#	e2e/tests/interchain_accounts/gov_test.go
#	e2e/tests/interchain_accounts/groups_test.go
#	e2e/tests/interchain_accounts/incentivized_test.go
#	e2e/tests/interchain_accounts/localhost_test.go
#	e2e/tests/interchain_accounts/params_test.go
#	e2e/tests/upgrades/genesis_test.go
#	e2e/testsuite/testconfig.go
#	modules/apps/27-interchain-accounts/controller/types/msgs_test.go
@mergify mergify bot requested a review from colin-axner as a code owner January 17, 2024 18:57
@mergify mergify bot added the conflicts label Jan 17, 2024
@DimitrisJim
chore: rm docs, e2e folders.
6e29344
@DimitrisJim
chore: Fix conflicts.
f40c58f 
Conflicts in controller's msgs_tests.go due to 8.1 using old approach for getting Signers.
@DimitrisJim DimitrisJim self-assigned this Jan 17, 2024
@DimitrisJim
fix: Remove API breaks on backport.
9bb3c46 
Introduce NewMsgRegisterInterchainAccountWithOrder to allow Order when creating a new instance of the message and
remove Order argument from NewMsgRegisterInterchainAccount.

Amend testing function to use old function since they all hard-coded ORDERED as the argument for order.
@DimitrisJim DimitrisJim removed their assignment Jan 17, 2024
@crodriguezvega crodriguezvega added priority PRs that need prompt reviews and removed conflicts labels Jan 17, 2024
crodriguezvega
crodriguezvega approved these changes Jan 18, 2024
View reviewed changes
Copy link
Contributor

@crodriguezvega crodriguezvega left a comment

Choose a reason for hiding this comment

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

Thank you for fixing the conflicts, @DimitrisJim.

@crodriguezvega crodriguezvega merged commit 3c95be2 into release/v8.1.x Jan 19, 2024
19 checks passed
@crodriguezvega crodriguezvega deleted the mergify/bp/release/v8.1.x/pr-5633 branch January 19, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crodriguezvega crodriguezvega crodriguezvega approved these changes

@damiannolan damiannolan damiannolan approved these changes

@colin-axner colin-axner Awaiting requested review from colin-axner

@AdityaSripal AdityaSripal Awaiting requested review from AdityaSripal AdityaSripal is a code owner

@srdtrk srdtrk Awaiting requested review from srdtrk

@charleenfei charleenfei Awaiting requested review from charleenfei

@chatton chatton Awaiting requested review from chatton

@DimitrisJim DimitrisJim Awaiting requested review from DimitrisJim DimitrisJim is a code owner

Assignees
No one assigned
Labels
priority PRs that need prompt reviews
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@damiannolan @crodriguezvega @DimitrisJim @srdtrk