test: add mock middleware to block app upgrades and provide test coverage in core #5406
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
…here are no packet commitments left (#3964)
* 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.
…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>
…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>
…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>
|
Ooof, changing base branch to main was a bad idea... RIP this PR 😅 |
…-ibc-appmodules-for-upgrades
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5406 +/- ##
==========================================
+ Coverage 81.09% 81.27% +0.18%
==========================================
Files 197 197
Lines 15234 15234
==========================================
+ Hits 12354 12382 +28
+ Misses 2412 2391 -21
+ Partials 468 461 -7 |
I had the same experience on mine 🙈 20+ merge conflicts |
AdityaSripal
approved these changes
Jan 3, 2024
chatton
approved these changes
Jan 3, 2024
charleenfei
approved these changes
Jan 5, 2024
damiannolan
deleted the
damian/5380-test-type-casting-ibc-appmodules-for-upgrades
branch
9 tasks
mergify bot
pushed a commit
that referenced
this pull request
Apr 19, 2024
## Description - Add panic test cases for `OnChanUpgradeOpen` and `OnChanUpgradeRestore` - Reuse mock middleware as the app which doesn't implement the `UpgradeableModule` interface from [#5406](#5406) closes: #5471 ### Commit Message / Changelog Entry ```text test: add panic cases in tests when channel upgrade open or restore ``` see the [guidelines](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) 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. - [x] Targeted PR against the correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#pull-request-targeting)). - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/build/building-modules/11-structure.md) and [Go style guide](../docs/dev/go-style-guide.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/main/testing/README.md#ibc-testing-package). - [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`). - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Provide a [commit message](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) 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. ## Summary by CodeRabbit - **Tests** - Enhanced testing for channel upgrades in interchain accounts, including new scenarios for error handling. - Added import for `cosmossdk.io/errors`. - Added `expPanic` field to test cases in `TestOnChanUpgradeOpen` function. - Updated the logic in `TestOnChanUpgradeOpen` to handle expected errors and panics based on test cases.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
UpgradeableModuleinterface.This mock middleware can be removed after v9 IF the
UpgradeableModuleinterface will be added to theIBCModuleinterface.closes: #5380
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.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.