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: cosmos v0.47 upgrade #2039

Merged
merged 77 commits into from
Apr 29, 2024

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Apr 17, 2024

Description

NOTE: a lot of files changed in the diff, but most are cli docs md files changes from auto generation, and things like moving files and changing imports, so PR is not that big.

Ethermint commit used here is in this PR: zeta-chain/ethermint#2

Closes all issues from epic: #1928

Going through this doc: https://github.com/cosmos/cosmos-sdk/blob/v0.47.10/UPGRADING.md

Migration to comet bft is done, but tendermint dependency is still there because go-tss is using it in zetaclient, but upgrade doc suggest to remove it everywhere. Can we migrate in go-tss too?

RandomizedParams is cleaned up and empty implementation of ProposalMsgs is added (even though not needed).

This is added in app.go RegisterAPIRoutes.

These methods are removed.

Updated, also network tests are a bit updated with changes from cosmos 0.47 codebase.

Not sure app wiring is used, so this is not relevant.

Constructor is simplified, and also base app opts method is used, overriding mempool like this:

baseappOptions := server.DefaultBaseappOptions(appOpts)
	baseappOptions = append(baseappOptions, func(app *baseapp.BaseApp) {
		app.SetMempool(mempool.NoOpMempool{})
	})

Added modulesToExport argument as suggested.

This was already done.

Proto updates:

This is updated, and also using docker as suggested in makefile for proto gen.

Added zetachain/zetacore subdirectories to proto folder to be inline with this.

Nothing needed here.

Done in separate PR: #2001

This is not changed, as suggested when module name is not second in message route.

Done in separate PR: #2014 , but here relevant handler upgrade is added.

No code changes needed here it seems.

Optional migration for existing proposals, skipped at the moment.

Added to upgrade handler.

This is updated.

Not relevant.

Looks not relevant.
Closes:

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Important: tested also with upgrade e2e tests, by using current develop as OLD_VERSION.

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Checklist:

  • I have added unit tests that prove my fix feature works

go.mod Show resolved Hide resolved
@@ -40,7 +82,7 @@ func SetupHandlers(app *App) {
}
if upgradeInfo.Name == releaseVersion && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
storeUpgrades := storetypes.StoreUpgrades{
Added: []string{authoritytypes.ModuleName, lightclienttypes.ModuleName},
Added: []string{authoritytypes.ModuleName, lightclienttypes.ModuleName, consensustypes.ModuleName, crisistypes.ModuleName},
Copy link
Contributor

Choose a reason for hiding this comment

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

Should crisis be added here ?

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 think so, saw it in simapp example in 0.47 codebase, but maybe there is a reason to not add it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm , checked on develop the crisis module is already present .
https://github.com/zeta-chain/zeta-node/blob/022ea57e25b4e0cb338b99bbd1c9e235f41ae954/app/app.go#L514-L514

Adding it again might result in a consensus version mismatch because the initial version has already been set . I am not sure though if there is a special reason for adding crisis again , the upgrade test might help when we run it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we still need to add this, because crisis module only has store now when params are migrated
this is when i tested upgrade without it:

failed to load latest version: version of store crisis mismatch root store's version; expected 89 got 0; new stores should be added using StoreUpgrades

and also from upgrading to 0.47 docs: https://github.com/cosmos/cosmos-sdk/blob/v0.47.10/UPGRADING.md#xcrisis

Base automatically changed from params-upgrade to feat-cosmos-sdk-0.47.10-upgrade April 25, 2024 12:38
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

Looks good, some small comments

app/app.go Show resolved Hide resolved
changelog.md Show resolved Hide resolved
github.com/nanmu42/etherscan-api v1.10.0
github.com/onrik/ethrpc v1.2.0
github.com/tendermint/tendermint v0.34.14
Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's create a tag in go-tss?

x/emissions/types/message_update_params_test.go Outdated Show resolved Hide resolved
x/emissions/types/params.go Outdated Show resolved Hide resolved
x/emissions/types/params.go Outdated Show resolved Hide resolved
x/emissions/types/params_legacy.go Show resolved Hide resolved
x/emissions/types/params_legacy.go Outdated Show resolved Hide resolved
@skosito skosito requested a review from kingpinXD April 29, 2024 12:43
@skosito skosito merged commit 5a410a4 into feat-cosmos-sdk-0.47.10-upgrade Apr 29, 2024
18 checks passed
@skosito skosito deleted the cosmos-v0.47-upgrade branch April 29, 2024 18:44
@skosito skosito mentioned this pull request Apr 30, 2024
10 tasks
lumtis added a commit that referenced this pull request May 3, 2024
* Remove broadcast mode block wip

* Fixes

* Changelog

* PR comments

* Lint fix

* refactor: remove fungible params (#2004)

* Remove unused params from fungible module

* Make generate

* Changelog

* Lint fix

* Lint fix

* Fix changelog

* Revert proto indexes

* Make generate

* refactor: deprecate x/params (#2014)

* Remove unused params from fungible module

* Make generate

* Changelog

* Lint fix

* Lint fix

* Migrate emissions params to store

* Migrate observer params to its own store

* Make generate

* Changelog

* Lint fixes

* Add observer slash amount param to migrate script

* Fix tests

* Lint fixes

* Fixes

* Lint fixes

* Fixes

* Tests

* Small cleanup

* Remove not needed params store mock

* Fixes after merge

* Add found flag to get params method

* Move ballot maturity blocks param to emissions module

* Remove params from observer module

* Fix build

* PR comments

* PR comment

* PR comments

* PR comments

* feat: cosmos v0.47 upgrade (#2039)

* proto fixes after merge

* Fix tx proto emissions

* fixes

* make generate

* remove tendermint usage

* Update changelog

* fix test

* add some logs to proto gen script

* permissions

* add more logs

* test

* test

* test

* test

* test

* test

* test

* test

* test

* test

* cleanup

* cleanup

* always format proto before generation

* generate explicit format deps

* set docker username to fix permissions in CI

* dummy proto change to test formatting

* set buf cache directory

* Revert "dummy proto change to test formatting"

This reverts commit 7e5c0dc.

* refactor: update go-tss (#2094)

* Remove unused params from fungible module

* Make generate

* Changelog

* Lint fix

* Lint fix

* Migrate emissions params to store

* Migrate observer params to its own store

* Make generate

* Changelog

* Lint fixes

* Add observer slash amount param to migrate script

* Fix tests

* Lint fixes

* Fixes

* Lint fixes

* Fixes

* Tests

* Small cleanup

* Try upgrade with simapp change

* Remove not needed params store mock

* fix compile errors

* proto changes

* Fixes

* Upgrade handler changes

* Fixes

* Fix e2e test

* Fixes after merge

* Add found flag to get params method

* Move ballot maturity blocks param to emissions module

* Remove params from observer module

* Fix build

* Regen proto

* Fixes for e2e tests

* Fix some unit tests

* Fix some lint errors

* Fix more unit tests

* PR comments

* Proto paths fixes

* Linters

* make typescript

* proto format

* make specs

* make docs

* make gen

* lint

* Fix some unit tests

* Fix cli test suites

* Fix lints

* Tmp comment unit test

* lint

* Use latest ethermint change and fix test

* PR comment

* PR comments

* Fix one more import

* Cleanup

* Lint fix

* Bump zetachain ethermint

* Remove cometbft replace

* go mod tidy

* bump ethermint

* Revert replace

* Bump ethermint

* Proto files updates after merge

* Make generate

* Bump ethermint

* Fix PR comments

* Formatting

* Mention observer params removal in breaking changes in changelog

* bump ethermint

* Remove tendermint dep after go-tss upgrade

* go mod tidy

* bump go-tss

* changelog

* changelog

---------

Co-authored-by: Alex Gartner <[email protected]>
Co-authored-by: Lucas Bertrand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants