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

[Do-not-merge] Upgrade to Cosmos-SDK v0.50. #1448

Closed
wants to merge 27 commits into from
Closed

[Do-not-merge] Upgrade to Cosmos-SDK v0.50. #1448

wants to merge 27 commits into from

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Jun 13, 2023

🚧 WIP - early version

See #1438

⚠️ All broken currently ! 🤯

Failing tests are deactivated. Go mod is setup my local workspace.

I am waiting for a soution in ibc-go that fixes the: failed consensus state verification for client (07-tendermint-0): chained membership proof failed to verify membership of value:

See upgrade doc in the SDK

I have started with just the go.mod dependencies and some related changes to imports.

This blocked now on the ibc-go release that includes the capability module and fixed imports for sdk v0.50.

  • Notional is helping the team with a PR but still core integration of the testing framework does not work for me.
  • Either there are some issues with CometBFT or the way how I would use it for the extended ibc-go testing framework. See my issue

This is also blocked on on some modules in the SDK that are not released. See go.mod replacement statements for details

Some notes on the work

New modules

  • circuit

Moved to cosmossdk.io path

  • store
  • x/nft
  • x/evidence
  • x/feegrant
  • x/upgrade

Others

"github.com/cosmos/cosmos-sdk/snapshots -> "cosmossdk.io/store/snapshots
"github.com/cosmos/cosmos-sdk/client/grpc/tmservice -> "github.com/cosmos/cosmos-sdk/client/grpc/cmtservice
"github.com/cosmos/cosmos-sdk/x/capability -> "github.com/cosmos/ibc-go/v7/modules/capability

Removed: legacy proposals for x/upgrade

  • deprecate MakeEncodingConfig
  • wasm keeper: accept runtime.NewKVStoreService for keys[wasm.StoreKey],
  • handle storeservice.Get/Set errors
  • remove non requires sdk modules from begin/ endblocker
  • replace sdk.Context with stdlib context in keeper
  • use [(cosmos_proto.scalar) = "cosmos.AddressString"] for addresses in proto
  • set cosmos.msg.v1.service proto annotation
  • upgrade ante-handler
  • use new collections framework for params

@alpe alpe added the blocked label Jun 13, 2023
Comment on lines +1059 to +1066
for _, m := range app.ModuleManager.Modules {
if moduleWithName, ok := m.(module.HasName); ok {
moduleName := moduleWithName.Name()
if appModule, ok := moduleWithName.(appmodule.AppModule); ok {
modules[moduleName] = appModule
}
}
}

Check warning

Code scanning / CodeQL

Iteration over map

Iteration over map may be a possible source of non-determinism
Comment on lines +1089 to +1091
for _, key := range app.keys {
keys = append(keys, key)
}

Check warning

Code scanning / CodeQL

Iteration over map

Iteration over map may be a possible source of non-determinism
app/app.go Show resolved Hide resolved
@faddat
Copy link
Contributor

faddat commented Jul 16, 2023

@alpe might be able to make a pr to fix now.

@alpe
Copy link
Contributor Author

alpe commented Jul 17, 2023

This branch is still very early integration only. I am going to rebase this on main later this week and open develop_sdk50 as feature branch

* use a more stable ibc-go

* fix some small errors in accordance with what's working in ibc-go and wasmd

* temporarily disable rosetta

* add a few error checks and update some deprecated tendermint calls

* error checks in export.go
@faddat
Copy link
Contributor

faddat commented Jul 17, 2023

@alpe sounds good to me.

FYI:

  1. I am not sure if rebase is best, typically I fail with that.

  2. If you are sure that it is best, would you mind showing me how you do it, when you do it? Because if it is best, I'm knowledge deficient there.

should fix the Dockerfile's build

@alpe
Copy link
Contributor Author

alpe commented Jul 21, 2023

Closing this in favour of #1528

@alpe alpe closed this Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants