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

feat: removed verifychannelupgradeErrorAbsence from the 03-connection #5532

Merged
merged 359 commits into from
Jan 8, 2024

Conversation

vishal-kanna
Copy link
Contributor

closes: #5477

chatton and others added 30 commits June 21, 2023 10:03
* writeupgradetimeout method, pull in util methods
…s#3849)

* 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 (cosmos#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>
…osmos#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. (cosmos#4052)

* Pass in counterparty portid, channelid.

* use direct check on err.

* Force distinct channel identifiers when testing UpgradeOpen.
…osmos#4019)

* WIP: adding fee upgrade cbs and testing

* imp: allow failure expectations when using chain.SendMsgs

* fixing import errors from cherry-pick

* updating tests and rm try code

* rm diff onChanUpgradeTry

* Update modules/apps/29-fee/ibc_middleware.go

* adding MetadataFromVersion func to pkg types

* addressing pr feedback, disable fees on err, rename args, adding testcase

* Update modules/apps/29-fee/ibc_middleware_test.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* abstract out expIsFeeEnabled check in tests

* adding additional error context to MetadataFromVersion

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
…osmos#4023)

* WIP: adding fee upgrade cbs and testing

* imp: allow failure expectations when using chain.SendMsgs

* fixing import errors from cherry-pick

* updating tests and rm try code

* rm diff onChanUpgradeTry

* Update modules/apps/29-fee/ibc_middleware.go

* adding OnChanUpgradeTry implementation for 29-fee, adding tests

* rm CR in test expectation

* remove goconst linter as discussed async

* adding MetadataFromVersion func to pkg types

* addressing pr feedback, disable fees on err, rename args, adding testcase

* Update modules/apps/29-fee/ibc_middleware_test.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* abstract out expIsFeeEnabled check in tests

* adding additional error context to MetadataFromVersion

* propagate changes from onChanUpgradeInit PR

* addressing test assertion feedback

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
…osmos#4028)

* WIP: adding fee upgrade cbs and testing

* imp: allow failure expectations when using chain.SendMsgs

* fixing import errors from cherry-pick

* updating tests and rm try code

* rm diff onChanUpgradeTry

* Update modules/apps/29-fee/ibc_middleware.go

* adding OnChanUpgradeTry implementation for 29-fee, adding tests

* rm CR in test expectation

* remove goconst linter as discussed async

* adding onChanUpgradeAck implementation to 29-fee, adding tests

* adding MetadataFromVersion func to pkg types

* addressing pr feedback, disable fees on err, rename args, adding testcase

* Update modules/apps/29-fee/ibc_middleware_test.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* abstract out expIsFeeEnabled check in tests

* adding additional error context to MetadataFromVersion

* propagate changes from onChanUpgradeInit PR

* addressing test assertion feedback

* updating to use types.MetadataFromVersion in OnChanUpgradeAck

* updating tests to add additional checks

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
…#4074)

* use counterparty conn hops.

* Clean up tests.

* Amend inline comment slightly.

* Address nits

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
DimitrisJim and others added 2 commits December 21, 2023 14:39
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>
@vishal-kanna vishal-kanna changed the title feat: removed verifychannelupgradeErrorAbsence from the 03-Connection feat: removed verifychannelupgradeErrorAbsence from the 03-connection Jan 6, 2024
@crodriguezvega crodriguezvega mentioned this pull request Jan 8, 2024
9 tasks
@damiannolan damiannolan changed the base branch from 04-channel-upgrades to main January 8, 2024 12:28
@damiannolan
Copy link
Member

Changing the base branch to main has introduced a large number of conflicts here for some reason. I think it would be easier to create a new PR personally. Thoughts?

@damiannolan damiannolan self-assigned this Jan 8, 2024
@DimitrisJim
Copy link
Contributor

Changing the base branch to main has introduced a large number of conflicts here for some reason. I think it would be easier to create a new PR personally. Thoughts?

this shows an empty diff for me? Up for closing/reopening. If you wanna force push here I'd be happy with that too.

@damiannolan
Copy link
Member

damiannolan commented Jan 8, 2024

this shows an empty diff for me?

Yep, correct! I used:

gh pr checkout 5532

git merge main -X theirs

And just re-did the commit to remove the code here 83785d9

@damiannolan damiannolan enabled auto-merge (squash) January 8, 2024 12:44
@damiannolan damiannolan merged commit e1df7a6 into cosmos:main Jan 8, 2024
59 of 60 checks passed
mergify bot pushed a commit that referenced this pull request Jan 8, 2024
…#5532)

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
(cherry picked from commit e1df7a6)
crodriguezvega pushed a commit that referenced this pull request Jan 8, 2024
…#5532) (#5545)

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
(cherry picked from commit e1df7a6)

Co-authored-by: Vishal Potpelliwar <71565171+vishal-kanna@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
channel-upgradability Channel upgradability feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove VerifyChannelUpgradeErrorAbsence from 03-connection