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

docs: channel upgrades packet flushing #5348

Merged
merged 353 commits into from
Jan 5, 2024

Conversation

charleenfei
Copy link
Contributor

Description

closes: #5269

Commit Message / Changelog Entry

type: commit message

see the guidelines for commit messages. (view raw markdown for examples)


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md).
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/).
  • Added relevant godoc comments.
  • Provide a commit message to be used for the changelog entry in the PR description for review.
  • Re-reviewed Files changed in the Github PR explorer.
  • Review Codecov Report in the comment section below once CI passes.

DimitrisJim and others added 30 commits June 19, 2023 17:03
* updating tests to use expError in favour of expPass bool

* move write fn under chanUpgradeAck

* make expPass construction two lines instead of one

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Charly <charly@interchain.io>
Co-authored-by: Charly <charly@interchain.io>
* update upgrade seq comparison

* pr suggestions
* writeupgradetimeout method, pull in util methods
* adding boilerplate skeleton for chanUpgradeAck handler

* updating msg servers args

* adding test scaffolding and syncing latest changes of feat branch

* configure both proposed upgrades to use mock.UpgradeVersion

* updating chanUpgradeAck test cases

* updating var naming for consistency, adding additional testcases

* rm msg server implementation

* adding invalid flush status err and rm lint ignore comment

* adding test helpers to endpoint for get/set channel upgrade

* lint it

* adding initial msg server impl skeleton

* pull in code for WriteUpgradeAckChannel

* adding result to MsgChannelUpgradeAckResponse

* add initial test cases

* adding additional testcases

* apply testcase naming review suggestions

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* apply error return wrapping suggestions from review

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* fix error to use Wrapf and correct channel id arg, adding success log

* correct testing imports and satisy linter

* apply self suggestions for testcase context with in-line comments

* updating test func to use path.EndpointA and chainA sender acc

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
… server (#3913)

* make error wrapping and logging consistent for upgrade try msg server

* standardise logging
* Add ChanUpgradeOpen core handler.

* Tests tests tests.

* Update upgrade open handler based on feedback.

* Reformat testing approach.

* Move counterpartyhops assignment inline.

* Check err of SetChannelState.

* Address feedback.

* Remove uneeded modification of version.
* chore: adding check for in flight packets in WriteUpgradeAckChannel

* added test for TestWriteUpgradeAckChannel

* linter

* add client update to UpgradeAckChannel test

* mv test

* merge

* fix post-merge

* fix merge issues

* review comment

---------

Co-authored-by: Charly <charly@interchain.berlin>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
…3895)

* Add implementation for message server handling of ChanUpgradeOpen.

* Add tests for msg_server.

* Address review feedback.

* Remove setting of flush status.

* Apply suggestions from code review

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Address rest of review comments.

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
* chore: update abort upgrade function to panic on error

* apply review suggestions

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
* Amend Ack packet to keep acknowledging if we're in the process of flushing pre-upgrade packets.

* Use handshake to reach correct state before mutating any fields.

* Add test to verify post-ack channel state after last in-flight packet.

* Remove unecessary modifications of version for non initializing channel end.

* Test both cases: final in-flight packet and non-final one.

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* Remove manual setting of flush status.

* Update test name, pass mock version to both channels.

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
* Amend timeout to handle in-flight packets.

* Update timeout handler per spec.

* Update tests to test for toggling of flush status.

* Fix small typos in docstring.

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
…UpgradeOpen. (#4052)

* Pass in counterparty portid, channelid.

* use direct check on err.

* Force distinct channel identifiers when testing UpgradeOpen.
DimitrisJim and others added 22 commits December 14, 2023 21:01
#5409)

* Add proto message for pruning acks, msgs, stubs for keeper/msg-server.

* Make pruning number of zero into error.

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Rename num_to_prune to limit.

* make proto-format, add documentation note on limit argument.

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
… is enabled (#5358)

* wip: happy path test upgrading a channel to wire fee middleware

* change package name

* trying to have the relayer started before executing governance proposal

* passing happy path channel upgrade test

* some refactoring

* use sdkmath import alias

* update docker images

---------

Co-authored-by: Charly <charly@interchain.io>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
* chore: update channel upgrades default timeout to 10 mins

* chore: make lint-fix
* imp: add upgrade sequence to identified channel

* Add ValidateBasic tests for IdentifiedChannel.

---------

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
…age (#5415)

* disallow changing ordering in ICA upgrade and increase test coverage

* linting

* review comments
# Conflicts:
#	e2e/go.mod
#	e2e/go.sum
#	testing/utils.go
* refactor: apply issue design to timeout type

* chore: add godocs

* tests: add tests for elapsed and ErrTimeoutElapsed functions

* rm: unfinished tests

* test: tests for ErrTimeoutNotReached

* test: add tests for ErrTimeoutNotReached

* review suggestion: switcharoo

* chore: add in-code comment

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
* Pass appVersion to callback if version can be parsed.

* Assert on rest of values passed.
* Move cast to Upgradable module _after_ checking app is set.

* Add test to check nil apps are allowed.
* fix sequencing logic and unit tests

* added tests

* reorder comments

* add test

* fix typo

* refactored a couple of tests

* relocate comment

* fix: defer incrementing acc sequencing such that is happens on both success and failure tx execution

* chore: update version strings in comments

* test: wrap test code in suite.Run and add additional assertions

* refactor: adapt crossing hellos cancellation test case to use suite.Run and add more assertions

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
…r controller. (#5467)

* Add passthrough implementations for OnChanUpgradeOpen and OnChanUpgradeRestore for controller.

* Drop unused expError from test functions.
…adeOpen` (#5438)

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
* add logic for setting next seq recv, update tests

* update next seq recv and ack logic to spec

* typos

* update test

* wip changes for spec

* wip set counterparty upgrade

* update naming

* update endpoint

* spec changes

* test that counterparty upgrade has been set

* update err message

* pr review

* naming nit

* feat: check counterparty next sequence send in recv packet

* add comment

* lint

* add error doc

* only do defensive check if counterpartyUpgrade is set

* remove unnecessary 0 check

* merge conflict fix: add back deleted test

---------

Co-authored-by: Charly <charly@interchain.berlin>
Co-authored-by: Charly <charly@interchain.io>
Co-authored-by: Aditya Sripal <adityasripal@gmail.com>
Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>
# Conflicts:
#	e2e/go.mod
#	e2e/go.sum
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Charly <charly@interchain.berlin>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: chatton <github.qpeyb@simplelogin.fr>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Copy link
Contributor

@chatton chatton left a comment

Choose a reason for hiding this comment

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

LGTM!

docs/docs/01-ibc/06-channel-upgrades.md Outdated Show resolved Hide resolved
docs/docs/01-ibc/06-channel-upgrades.md Outdated Show resolved Hide resolved
docs/docs/01-ibc/06-channel-upgrades.md Outdated Show resolved Hide resolved

`FLUSHING` and `FLUSHCOMPLETE` are additional states which have been added to enable the upgrade feature.

This is found in the channel state on `Channel`:
Copy link
Contributor

Choose a reason for hiding this comment

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

does it make more sense to actually paste the State instead of Channel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i can paste the list of possible State as well!

@charleenfei charleenfei changed the base branch from 04-channel-upgrades to main January 5, 2024 12:22
@charleenfei charleenfei enabled auto-merge (squash) January 5, 2024 12:28
@charleenfei charleenfei merged commit a79663b into main Jan 5, 2024
19 checks passed
@charleenfei charleenfei deleted the charly/issue#5269-document-packet-flushing branch January 5, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
channel-upgradability Channel upgradability feature docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.