Problem: cosmos-sdk 0.46 is not used

[yihuang]

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

• [x] fix build
• [x] fix go lint
• [x] fix unit tests
• [x] fix integration tests

PR Checklist:

• [ ] Have you read the CONTRIBUTING.md?
• [ ] Does your PR follow the C4 patch requirements?
• [ ] Have you rebased your work on top of the latest master?
• [ ] Have you checked your code compiles? (make)
• [ ] Have you included tests for any non-trivial functionality?
• [ ] Have you checked your code passes the unit tests? (make test)
• [ ] Have you checked your code formatting is correct? (go fmt)
• [ ] Have you checked your basic code style is fine? (golangci-lint run)
• [ ] If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
• [ ] If your changes affect the client infrastructure, have you run the integration test?
• [ ] If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
• [ ] If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
• [ ] If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

[codecov[bot]]

Codecov Report

Merging #828 (4f0cf37) into master (71f66aa) will decrease coverage by 1.21%. The diff coverage is 36.00%.

@@            Coverage Diff             @@
##           master     #828      +/-   ##
==========================================
- Coverage   20.26%   19.05%   -1.22%     
==========================================
  Files          99       94       -5     
  Lines       11457    11941     +484     
==========================================
- Hits         2322     2275      -47     
- Misses       8647     9179     +532     
+ Partials      488      487       -1     

Flag	Coverage Δ
integration_tests	16.78% <40.90%> (-1.43%)	:arrow_down:
unit_tests	7.02% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
app/export.go	0.00% <0.00%> (ø)
app/state.go	0.00% <0.00%> (ø)
app/test_helpers.go	0.00% <0.00%> (ø)
x/chainmain/client/cli/genaccounts.go	12.19% <0.00%> (-0.21%)	:arrow_down:
x/chainmain/client/cli/testnet.go	9.35% <0.00%> (-0.10%)	:arrow_down:
x/chainmain/keeper/keeper.go	80.00% <ø> (ø)
x/icaauth/keeper/keeper.go	25.00% <ø> (ø)
x/icaauth/module.go	71.87% <ø> (ø)
x/icaauth/module_ibc.go	7.69% <ø> (ø)
x/nft/keeper/keeper.go	79.71% <0.00%> (-2.90%)	:arrow_down:
... and 34 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[devashishdxt]

I was hoping to do these changes in multiple steps (so that code review is easier):

1. Remove wasmd
2. Change release scripts to not include dlls
3. Create an alpha release with cosmos-sdk 0.45.7 and ibc-go 4.0.0 (for croeseid-5)
4. Upgrade cosmos-sdk to v0.46.0 and ibc-go to v5.0.0 (without enabling native NFT module)
5. Add new functionality from cosmos-sdk to chain-main (e.g., x/group)
6. Migrate from custom NFT module to native NFT module

@yihuang @tomtau Can we wait to merge this until next alpha release? Otherwise, we'll have to do duplicate work on release/v4 branch.

[tomtau]

@devashishdxt fine to break it down in that sequence, but is there a need to wait for ibc-go 4.0.0? or is rc2 ok?

(similarly with v5.0.0 -- is the beta ok for now?)

[devashishdxt]

@devashishdxt fine to break it down in that sequence, but is there a need to wait for ibc-go 4.0.0? or is rc2 ok?

Final version will use SDK v0.45.7 (which has fixes for non-determinism in simulation tests).

[tomtau]

@devashishdxt fine to break it down in that sequence, but is there a need to wait for ibc-go 4.0.0? or is rc2 ok?

Final version will use SDK v0.45.7 (which has fixes for non-determinism in simulation tests).

that could be achieved with replace or a fork of ibc-go; no need to wait for the final release here (given it's for the alpha testnet?)

[devashishdxt]

that could be achieved with replace or a fork of ibc-go; no need to wait for the final release here (given it's for the alpha testnet?)

I've talked to the team, they're going to release rc3 today. We can use that instead of using replace/fork.

[tomtau]

@devashishdxt for

2. Change release scripts to not include dlls

are you going to submit a PR or waiting for @yihuang ?

[devashishdxt]

2. Change release scripts to not include dlls

Hi @yihuang, can you help me create a PR for this?

[yihuang]

2. Change release scripts to not include dlls

Hi @yihuang, can you help me create a PR for this?

revert to old goreleaser workflow, right?

[devashishdxt]

2. Change release scripts to not include dlls

Hi @yihuang, can you help me create a PR for this?

revert to old goreleaser workflow, right?

Yes. The PR has other changes too, so, I'm not sure which ones to keen and which ones to revert.

[yihuang]

2. Change release scripts to not include dlls

Hi @yihuang, can you help me create a PR for this?

revert to old goreleaser workflow, right?

Yes. The PR has other changes too, so, I'm not sure which ones to keen and which ones to revert.

sure, https://github.com/crypto-org-chain/chain-main/pull/832

[tomtau]

3. Create an alpha release with cosmos-sdk 0.45.7 and ibc-go 4.0.0 (for croeseid-5)

@devashishdxt are you going to tag this one?

[devashishdxt]

3. Create an alpha release with cosmos-sdk 0.45.7 and ibc-go 4.0.0 (for croeseid-5)

@devashishdxt are you going to tag this one?

Yes. Just running a few things locally. Will create a PR then for release/... branch.

[tomtau]

/runsim

[github-actions[bot]]

Simulation tests started and triggered by /runsim. Will update here when it succeeds or fails. Can further check progress here

[github-actions[bot]]

❌ /runsim simulation test has failed 😅 Can further check here

[tomtau]

@yihuang can you rebase on the latest branch?

[devashishdxt]

@devashishdxt should this PR also add x/group or should that be done separately?

It's better to do it separately. This PR should just upgrade the cosmos-sdk version without adding any extra functionality. New things in cosmos-sdk can be adding in separate PRs.

[tomtau]

https://github.com/crypto-org-chain/chain-main/pull/828#issuecomment-1209296723 so are you going to do it before the NFT module work?

[devashishdxt]

https://github.com/crypto-org-chain/chain-main/pull/828#issuecomment-1209296723

so are you going to do it before the NFT module work?

Yes

[yihuang]

@yihuang can you rebase on the latest branch?

conflicts fixed.

[tomtau]

/runsim

[github-actions[bot]]

Simulation tests started and triggered by /runsim. Will update here when it succeeds or fails. Can further check progress here

[github-actions[bot]]

❌ /runsim simulation test has failed 😅 Can further check here

[yihuang]

/runsim

[github-actions[bot]]

Simulation tests started and triggered by /runsim. Will update here when it succeeds or fails. Can further check progress here

[github-actions[bot]]

❌ /runsim simulation test has failed 😅 Can further check here

[yihuang]

the timeout change seems don't have effect

[tomtau]

maybe the timeout change may need to be merged to the upstream branch first?

[yihuang]

maybe the timeout change may need to be merged to the upstream branch first?

maybe, I'm running locally.