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 sdk 0.47.10 upgrade #2100

Merged
merged 47 commits into from
May 3, 2024
Merged

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Apr 30, 2024

Description

Contains changes from these PRs:
#2039
#2014

One more PR left: #2094 to remove tendermint dependency completely

Closes: #1928

@lumtis @kingpinXD since you reviewed original PRs, only changes here are:

  • fixing of unit test: 70ceefa to remove observer params

  • make generate didnt work properly when running in github actions with new docker setup for proto-gen, please check changes with make generate-ci, i noticed other projects used same workaround

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?

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

* Remove unused params from fungible module

* Make generate

* Changelog

* Lint fix

* Lint fix
* 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
Copy link

!!!WARNING!!!
nosec detected in the following files: rpc/types/utils.go, testutil/network/util.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

@github-actions github-actions bot added the nosec label Apr 30, 2024
Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.61%. Comparing base (c7f1650) to head (f07f7d0).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2100      +/-   ##
===========================================
- Coverage    70.74%   70.61%   -0.14%     
===========================================
  Files          249      245       -4     
  Lines        14069    13976      -93     
===========================================
- Hits          9953     9869      -84     
+ Misses        3657     3650       -7     
+ Partials       459      457       -2     

see 23 files with indirect coverage changes

@lumtis
Copy link
Member

lumtis commented Apr 30, 2024

make generate didnt work properly when running in github actions with new docker setup for proto-gen, please check changes with make generate-ci, i noticed other projects used same workaround

These are the modifications? (looking at the entire diff is buggy to me because of the PR size)
0375c80
aa43519

@gartnera maybe you can help on this one? 👀

@gartnera
Copy link
Member

gartnera commented Apr 30, 2024

@gartnera maybe you can help on this one? 👀

yep I'll take a look

proto-format needs a run it seems? I pushed a fix, let me know if you agree with my changes.

➜  node git:(feat-cosmos-sdk-0.47.10-upgrade) make proto-format
➜  node git:(feat-cosmos-sdk-0.47.10-upgrade) ✗ git diff --stat | cat
 proto/zetachain/zetacore/crosschain/genesis.proto  |  2 +-
 proto/zetachain/zetacore/crosschain/query.proto    | 22 +++++++++++-----------
 .../zetacore/crosschain/rate_limiter_flags.proto   |  2 +-
 proto/zetachain/zetacore/crosschain/tx.proto       |  8 +++++---
 proto/zetachain/zetacore/emissions/query.proto     |  7 +++----
 proto/zetachain/zetacore/emissions/tx.proto        |  2 +-
 proto/zetachain/zetacore/pkg/chains/chains.proto   | 17 ++++++++++-------
 7 files changed, 32 insertions(+), 28 deletions(-)

@skosito
Copy link
Contributor Author

skosito commented Apr 30, 2024

make generate didnt work properly when running in github actions with new docker setup for proto-gen, please check changes with make generate-ci, i noticed other projects used same workaround

These are the modifications? (looking at the entire diff is buggy to me because of the PR size) 0375c80 aa43519

@gartnera maybe you can help on this one? 👀

was referring to this one 6b8055c

github.com/zetachain/zetacore/* was not generated in project folder, i guess {{github.workspace}}, what should happen when protoc gen script is run, without using root user, maybe there is better way, but this works

running proto-format is not related, but it is good change, thanks for adding that

cc: @gartnera

@gartnera
Copy link
Member

github.com/zetachain/zetacore/* was not generated in project folder, i guess {{github.workspace}}, what should happen when protoc gen script is run, without using root user, maybe there is better way, but this works

Yes I will look into that too

@gartnera
Copy link
Member

make generate didnt work properly when running in github actions with new docker setup for proto-gen, please check changes with make generate-ci, i noticed other projects used same workaround

should be resolved now by:

  • set user and group of docker container to match host. this ensures file permissions are correct on both sides
  • set buf cache directory. if the host user uid/gid doesn't exist inside the container, there will be no $HOME/.cache directory.

@skosito
Copy link
Contributor Author

skosito commented May 1, 2024

set docker username to fix permissions in CI

nice, thank you

skosito and others added 4 commits May 2, 2024 09:33
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking:cli breaking:proto ci Changes to CI pipeline or github actions nosec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Cosmos SDK to v0.47.10
3 participants