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

Add Channel Upgrade ACK and CONFIRM steps #3462

Conversation

ljoss17
Copy link
Contributor

@ljoss17 ljoss17 commented Jul 4, 2023

Closes: #3255, closes: #3238

Description

This PR adds the implementation required to send a MsgChannelUpgradeAck and updates the channel upgrade test to run the ACK step.


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/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

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

… luca_joss/integration-tests-channel-upgradability-upgrade-try
…de-try' into luca_joss/integration-tests-channel-upgradability-upgrade-ack
…Proof::Yes' when querying channel ends in the integration tests
@ljoss17 ljoss17 added O: tests Objective: Test more aspect of the relayer O: channel-upgradeability Objective: related to channel upgradeability labels Jul 4, 2023
@ljoss17 ljoss17 changed the title Add Channel Upgrade ACK step Add Channel Upgrade ACK and CONFIRM steps Jul 7, 2023
@ljoss17 ljoss17 marked this pull request as ready for review July 19, 2023 13:48
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

Amazing work!

@ljoss17 ljoss17 merged commit 128e48e into sean/channel-upgradability Jul 24, 2023
@ljoss17 ljoss17 deleted the luca_joss/integration-tests-channel-upgradability-upgrade-ack branch July 24, 2023 10:12
romac added a commit that referenced this pull request May 31, 2024
* Add new channel states

* Change channel state enum discriminants

* Update channel state doc comment

* Stub out channel upgrade CLI commands

* Use tx_chan_cmd macro for TxChanOpenInit command

* Stub out build_channel_upgrade_init function

* Add MsgChannelUpgradeInit type

* Add more tests for MsgChannelUpgradeInit message type

* Add chan_upgrade_try file for MsgChannelUpgradeTry message type

* Add build_chan_upgrade_init_and_send method

* Add UpgradeInit and UpgradeTry ChannelEvents

* Fix compilation errors with UpgradeInit and UpgradeTry impls

* Stub out MsgChannelUpgradeTry

* Add missing fields to MsgChannelUpgradeInit

* Use ibc-proto branch with channel upgrade messages

* Fill in todos

* Comment out code in tx chan-upgrade-init and add todos

* Add `UpgradeTimeout` enum

* Fleshing out MsgChannelUpgradeTry

* Clean up MsgChannelUpgradeTry

* Clean up MsgChannelUpgradeTry

* Fix error conversion

* Formatting and clippy

* Disable tests and fix more clippy warnings

* Re-enable tests

* Wrap up channel upgrade init CLI

* Cargo fmt

* Remove unnecessary CLI parameter

* Pass an `UpgradeTimeout` to `.build_chan_upgrade_init_and_send`

* Fix bug when converting integer to channel state

* Proper implementation of `State::less_or_equal_progress`

* Fix broken test

* Ignore channel upgrade try tests for now

* Fix channel upgrade try tests

* Stub out build_channel_upgrade_try

* Stub out build_channel_upgrade_try

* Fix return type of build_chan_upgrade_try

* Cargo fmt

* Remove stub function

* Add missing clone

* Remove redundant clone

* Add integration tests for channel upgradability (#3247)

* Add test for ChannelUpgradeInit step

* Add conditional check 'channel-upgrade' to channel upgradability tests

* Improve info message

* Update event processing for UpgradeInit

* Update UpgradeInit step test

* Rename ChannelUpgradeInit test

* Improve channel upgrade steps assertion

* Add ChannelUpgradeAssertionAttributes struct for tests

* Update tests after merge

* Fix channel upgrade init step tests

* Fix check for new channel ordering

* Fix broken doc comment link

* Progress on build_chan_upgrade_try

* Progress on build_chan_upgrade_try

* Update ChanUpgradeInit step to use new design improvements

* Nicely report connection id parsing failures

* Document some fields of MsgChannelUpgradeTry

* Add build_chan_upgrade_try_and_send fn

* Cargo fmt

* Update UpgradeInit to mirror event attributers from 'channel_upgrade_init' event

* Treat timestamp 0 as no timestamp

* Fix parsing UpgradeAttributes and improve ChanUpgradeInit test

* Add Hermes data requirements document (#3262)

* Add Hermes data requirements document

* Update data requirement for some endpoints with actual data being used

* Add headers

* Fix Order -> Ordering doc comment links

* Apply suggestions from code review

Co-authored-by: Anca Zamfir <ancazamfir@users.noreply.github.com>
Signed-off-by: Romain Ruetschi <romain.ruetschi@gmail.com>

---------

Signed-off-by: Romain Ruetschi <romain.ruetschi@gmail.com>
Co-authored-by: Sean Chen <skypemaster007@gmail.com>
Co-authored-by: Anca Zamfir <ancazamfir@users.noreply.github.com>

* Update nix flake to use simd v7.0.0 (#3264)

* Address compiler errors for now

* Cargo fmt

* Add channel-upgrade job to the CI

* Format flake.nix

* Fix test

* Update guide templates

* Expose `tx chan-upgrade-try` command

* Update TRY ste implementation for channel upgrade

* Fix channel upgrade TRY step

* Update nix flake

* Add integration test for `ChanUpgradeTry` step (#3259)

* Add test for ChannelUpgradeInit step

* Add conditional check 'channel-upgrade' to channel upgradability tests

* Improve info message

* Update event processing for UpgradeInit

* Update UpgradeInit step test

* Rename ChannelUpgradeInit test

* Add ChanUpgradeTry test

* Improve channel upgrade steps assertion

* Add ChannelUpgradeAssertionAttributes struct for tests

* Update naming for modifiable channel attributes in tests

* Fix test doc string

* Restore commented lines for channel upgrade TRY step

* Improve channel upgrade tests

* Disable TRY assert in test

* Remove unnecessary test configuration for channel upgrade

* Merge channel upgrade Init and Try tests into a single test

* Improve channel upgrade test

* Use 'CountingAndCachingChainHandle' in integration tests and 'IncludeProof::Yes' when querying channel ends in the integration tests

* Updated channel upgrade TRY step tests

* Fix and improve channel upgrade TRY step test

* Updated nix flake

* Fix test-stable

* Update nix flake and TRY step test

* Fix interchain-security CI. Add domain type FlushStatus

* Fix FlushStatus display errors

* Add Channel Upgrade ACK and CONFIRM steps (#3462)

* Add test for ChannelUpgradeInit step

* Add conditional check 'channel-upgrade' to channel upgradability tests

* Improve info message

* Update event processing for UpgradeInit

* Update UpgradeInit step test

* Rename ChannelUpgradeInit test

* Add ChanUpgradeTry test

* Improve channel upgrade steps assertion

* Add ChannelUpgradeAssertionAttributes struct for tests

* Update naming for modifiable channel attributes in tests

* Add test for 'ChanUpgradeAck' step

* Fix test doc string

* Restore commented lines for channel upgrade TRY step

* Improve channel upgrade tests

* Fix after merge

* Disable TRY assert in test

* Remove unnecessary test configuration for channel upgrade

* Merge channel upgrade Init and Try tests into a single test

* Improve channel upgrade test

* Use 'CountingAndCachingChainHandle' in integration tests and 'IncludeProof::Yes' when querying channel ends in the integration tests

* Updated channel upgrade TRY step tests

* Fix and improve channel upgrade TRY step test

* Updated nix flake

* Fix test-stable

* Update nix flake and TRY step test

* Update ACK step assert function

* Fix interchain-security CI. Add domain type FlushStatus

* Fix FlushStatus display errors

* Add build and send MsgChannelUpgradeAck

* Add ACK step to Channel Upgrade test

* Implement Channel Upgrade OPEN step

* Update flake.lock to use latest simapp for channel upgrade

* Fix channel upgrade test OPEN step

* Fix assertion after channelupgradeopen

* Update nix flake

* Fix comments

* Implement PartialEq for Version

* Apply suggestions and improve documentation

* Add missing CLIs for channel upgrade (#3495)

* Add missing CLIs for channel upgrade

* Update CLI template

* Add cargo patch for check-guide test

* Add documentation TODO to check-guide Cargo.toml

* Set dst-channel flag to required for channel upgrade CLIs and updated templates

* Clean channel upgrade CLIs

* Update guide templates

* Update channel upgrade after spec rework

* Update nix flake

* Update nix flake

* Add test bootstrap compatibility for pre and post SDK v0.47.0

* Channel upgrade worker handshake (#3569)

* Add missing CLIs for channel upgrade

* Update CLI template

* Add cargo patch for check-guide test

* Add documentation TODO to check-guide Cargo.toml

* Set dst-channel flag to required for channel upgrade CLIs and updated templates

* Clean channel upgrade CLIs

* Update guide templates

* Update channel upgrade test to match ibc-go changes

* WIP: waiting for new channel states during upgrade to be implemented

* Implement channel upgrade handshake

* Add channel upgrade handshake test

* Removed unnecessary logs

* Update channel upgrade test doc

* Add test to transfer ics29 packet after channel upgrade

* Update tools/integration-test/src/tests/channel_upgrade/ics29.rs

Co-authored-by: Sean Chen <seanchen11235@gmail.com>
Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com>

* Add comments

* Fix 'query bank balances' CLI to handle SDK v0.50 changes

* Remove flush status from channel end

* Update Nix flake and use alpha release of channel upgrade simapp for CI

* Fix test-stable and change 'into_i32' to 'as_i32' for Channel State

* Fix python tests

* Use state comparison method 'less_or_equal_progress' where applicable

* Add tests for channel upgrade completion

* Improve less_or_equal method for channel states and add unit tests

* Use automatic link for URL in channel State documentation

* Add comment on channel upgrade open assertion

---------

Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com>
Co-authored-by: Sean Chen <seanchen11235@gmail.com>

* Update nix flake and remove log file

* Use governance proposal to initialise channel upgrade tests (#3680)

* Use governance proposal to initialise channel upgrade tests

* Fix issue with chains which don't have 'expedited_voting_period'

* Fix issues post merge

* Correctly determine if the channel is upgrading or not

* Remove CLI to init channel upgrade

* Display channel state as 'Open' regardless if it is upgrading or not

* Change serialize for channel state

* Fix python e2e channel handshake test

* feat: implement chan upgrade cancel and add timeout ack integration test (#3736)

* feat: implement chan-upgrade-cancel and add integration test for timeout on upgrade ack

* uncomment code

* formatting

* Fix channel upgrade timeout and cancel handling

* Fix clippy warnings

* Add channel upgrade cancel tests with supervisor

* Fix typo

* Fix upgrade cancel on confirm test and update guide

* Clean channel upgrade tests

* Additional clean-up in channel upgrade test assertions

* Temporarily disable one chan upgrade timeout test

* Remove incorrect timeout channel upgrade test

---------

Co-authored-by: Luca Joss <43531661+ljoss17@users.noreply.github.com>
Co-authored-by: Luca Joss <luca@informal.systems>

* Fix Docker workflow after update to upload-artifact@v4

* Fix Artifact name for Docker job

* feat: updates to upgrade fields and logic for `MsgTimeoutOnClose` and `MsgChannelCloseConfirm` (#3764)

* wip: update naming of fields

* add logic to add counterparty upgrade sequence to timeout on close and channel close confirm messages

* add integration test for: when ICA channel upgrades and afterwards packet times out, then channel is closed

* argo fmt --all

* remove empty comment

* Fix channel end parsing and test for ICA timeout after channel upgrade

* Update nix flake

* Improve documentation for upgraded ICA channel close test

* Improve build channel close methods

---------

Co-authored-by: Luca Joss <luca@informal.systems>

* feat: add support for `MsgChannelUpgradeTimeout` and integration test (#3773)

* feat: add support for msgchannelupgradetimeout and integration test

* fix build errors

* Update nix flake

* Update CLI templates

* cargo fmt

* Fix clippy warnings/errors

* Fix extracting upgrade_timeout attribute

* WIP

* Fix channel upgrade logic

* Change Height to QueryHeight for restore_from_state method

---------

Co-authored-by: Luca Joss <luca@informal.systems>

* fetch counterparty upgrade sequence from src chain (#3801)

* test(channel upgradability): add integration test that relays packets during flushing (#3786)

* test: add integration test that flushes packets during channel upgrade

* wip: add counterparty upgrade sequence to chan upgrade open

* update integration test

* cargo fmt

* Fix UpgradeInit parsing and Registering ICA account following simd changes

* handle new fields in messages: order in msg register interchain account and counterparty upgrade sequence in msg channel upgrade open

* fix warnings

* Update Nix flake

* address review comments

* cargo fmt

* some clippy warnings

* fix assert_eq!

* fix test

* cargo fmt

* chore: rename order to ordering in msg register interchain account

* Use ibc-go v8.1.0-rc.0 for channel upgrade tests

* Add and use legacy 'MsgRegisterInterchainAccount'

* Use different signer to initialise channel upgrade for 'test_channel_upgrade_handshake' test

* add test where ICA channel is upgraded to unordered

* cargo fmt + add debug log

* increase packet timeout

* Add case where Channel upgrade Try event is received by source chain while dst chain is open not upgrading

* Use user2 to signe channel upgrade init proposal in ICA unordered test

---------

Co-authored-by: Luca Joss <luca@informal.systems>

* Fix ICS29 timeout fee test compatibility for ibc-go v8.1+

* Add compatibility to ICS29 tests for ibc-go v8.1+

* Fix typos

* Update method used to verify if there is an ongoing channel upgrade

* Update channel query to correctly reflect if it is upgrading or not if the state is open

* Add upgrade handshake step if both channel ends are in open upgrading

* Fix channel state assertion for channel Init and Try steps

* Fix assertions in channel upgrade tests

* Fix verification if channel needs to be flushed

* Add test for packet flushing during channel upgrade

* Fix link creation when channel is upgrading

* test(channel-upgrades): add test where upgrade timeouts on acknowledge packet (#3828)

* add test where upgrade times out during packet ack

* cargo fmt

* Fix packet worker spawning if channel is flushing or flushcomplete

---------

Co-authored-by: Luca Joss <luca@informal.systems>

* Clean-up channel upgrade tests

* Improve channel upgrade workers

* Remove unused code

* Use ibc-proto-rs v0.42.0

* Remove unnecessary code

* Clean up code and remove guide entries for tx channel upgrade init CLI

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Romain Ruetschi <106849+romac@users.noreply.github.com>

* Remove unnecessary fields from IBC channel upgrade events

* cargo fmt

* Fix async ICQ test

* Fix cargo doc

* Remove redundant import

* Remove redundant import

* Correctly handle channel upgrade error

* Remove validate basic from channel open try

* Improve detection of channel upgrade cancel when one end is in Flushcomplete

* Add changelog entry

* Fix post-merge compilation issues

* Rename `State::Flushcomplete` to `State::FlushComplete`

* Small cleanup

* Sort imports

* Whitespace

* Remove patch override

* Apply suggestions from code review

Co-authored-by: Romain Ruetschi <romain@informal.systems>
Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com>

* cargo fmt

* Fix codespell errors

* Use IncludeProof for query_upgrade and query_upgrade_error

* Add a method to build and send channel upgrade open or cancel

* Remove unnecessary assertions

* Add handling of channel upgrade timeout event in channel worker

---------

Signed-off-by: Romain Ruetschi <romain.ruetschi@gmail.com>
Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com>
Signed-off-by: Romain Ruetschi <romain@informal.systems>
Co-authored-by: Romain Ruetschi <romain@informal.systems>
Co-authored-by: Luca Joss <43531661+ljoss17@users.noreply.github.com>
Co-authored-by: Luca Joss <luca@informal.systems>
Co-authored-by: Anca Zamfir <ancazamfir@users.noreply.github.com>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Romain Ruetschi <106849+romac@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: channel-upgradeability Objective: related to channel upgradeability O: tests Objective: Test more aspect of the relayer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants