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: upgrade cosmos-sdk v0.45.4 #356

Merged
merged 11 commits into from
Jun 30, 2022
Merged

Conversation

0xHansLee
Copy link
Contributor

@0xHansLee 0xHansLee commented Jun 22, 2022

Upgrade cosmos-sdk to v0.45.4

Final ver of each package

cosmos-sdk : v0.45.4
ibc-go : v2.0.3
wasmd : v0.24.0
wasmvm : v1.0.0-beta7

Main changes

  • SubtractsCoins and SetSupply are removed in x/bank module. So I modified x/burn module to burn coins using BurnCoins in x/bank module.

  • In v0.42.11, ibc module was in comsos-sdk, under x/. But it has been moved to its own repo.

  • The upgrade to v0.44 or upper(v0.45 as in this PR) can be done using upgrade handler as suggested in guide. So I've also registered upgrade handler in app.go under v2.2.0-alpha1

  • removed x/token module because the module is useless anymore as we discussed

  • added feegrant and authz modules

Main Breaking Changes

v0.43

v0.44.0

v0.44.4

  • Change the order of module migration by pushing x/auth to the end. Auth module depends on other modules and should be run last. We have updated the documentation to provide more details how to change module migration order. This is technically a breaking change, but only impacts updates between the upgrades with version change, hence migrating from the previous patch release doesn't cause new migration and doesn't break the state (fix: always move auth module migration to the end cosmos/cosmos-sdk#10608)

v0.45.0

v0.45.3

  • move unsafe-reset-all to tendermint subcommand

TODO

  • update medibloc/cosmos-sdk package after release. It is not released yet, so I'll update after merge and release this PR
  • check ibc url in swagger config.json

Copy link
Contributor Author

@0xHansLee 0xHansLee left a comment

Choose a reason for hiding this comment

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

Some comments to help your reviews. Thanks in advance

// numbers, checks signatures & account numbers, and deducts fees from the first
// signer.
// Copied from wasmd, https://github.com/CosmWasm/wasmd/blob/v0.24.0/app/ante.go
func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This NewAnteHandler is to receive all positional params as ante.HandlerOptions struct. If required fields aren't set, throws error accordingly. (https://github.com/cosmos/cosmos-sdk/pull/8682/files)

Plus, referred wasm's NewAnteHandler. (https://github.com/CosmWasm/wasmd/blob/v0.24.0/app/ante.go)

@@ -360,7 +389,7 @@ func New(
app.CrisisKeeper = crisiskeeper.NewKeeper(
app.GetSubspace(crisistypes.ModuleName), invCheckPeriod, app.BankKeeper, authtypes.FeeCollectorName,
)
app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath)
app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath, bApp)
Copy link
Contributor Author

@0xHansLee 0xHansLee Jun 23, 2022

Choose a reason for hiding this comment

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

Upgrade Keeper takes new argument ProtocolVersionSetter which implements setting a protocol version on baseapp. (cosmos/cosmos-sdk#8897)

@@ -372,7 +401,7 @@ func New(

// Create IBC Keeper
app.IBCKeeper = ibckeeper.NewKeeper(
appCodec, keys[ibchost.StoreKey], app.GetSubspace(ibchost.ModuleName), app.StakingKeeper, scopedIBCKeeper,
appCodec, keys[ibchost.StoreKey], app.GetSubspace(ibchost.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -381,7 +410,7 @@ func New(
AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)).
AddRoute(distrtypes.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)).
AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper)).
AddRoute(ibchost.RouterKey, ibcclient.NewClientUpdateProposalHandler(app.IBCKeeper.ClientKeeper))
AddRoute(ibchost.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
Copy link
Contributor Author

@0xHansLee 0xHansLee Jun 23, 2022

Choose a reason for hiding this comment

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

The ClientUpdateProposalHandler has been renamed to ClientProposalHandler. It handles both UpdateClientProposals and UpgradeProposals. (https://github.com/cosmos/ibc-go/blob/main/docs/migrations/sdk-to-v1.md#proposal-handler-registration)

@@ -440,7 +462,7 @@ func New(
&app.IBCKeeper.PortKeeper,
scopedWasmKeeper,
app.TransferKeeper,
app.Router(),
app.MsgServiceRouter(),
Copy link
Contributor Author

@0xHansLee 0xHansLee Jun 23, 2022

Choose a reason for hiding this comment

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

go.mod Outdated
@@ -39,4 +39,4 @@ replace google.golang.org/grpc => google.golang.org/grpc v1.33.2

replace github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1

replace github.com/cosmos/cosmos-sdk => github.com/medibloc/cosmos-sdk v0.42.11-panacea.1
//replace github.com/cosmos/cosmos-sdk => github.com/medibloc/cosmos-sdk v0.45.4-panacea.1
Copy link
Contributor Author

@0xHansLee 0xHansLee Jun 23, 2022

Choose a reason for hiding this comment

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

TODO: after release upgraded version of medibloc/cosmos-sdk for min-commission-rate, replace it

@@ -45,20 +46,15 @@ func (suite *BurnTestSuite) BeforeTest(_, _ string) {
bankKeeper := suite.BankKeeper

for _, addr := range address {
err := bankKeeper.AddCoins(ctx, addr, initCoins)
// mint coins and send to each account
err := simapp.FundAccount(suite.BankKeeper, suite.Ctx, addr, initCoins)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AddCoins is deleted in x/bank, so replace to simapp.FundAccount to mint and send coins to the account

Comment on lines -1 to -15
package expected

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/bank/exported"
)

type BankKeeperI interface {
GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins

SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error

GetSupply(ctx sdk.Context) exported.SupplyI
SetSupply(ctx sdk.Context, supply exported.SupplyI)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to be not used, so deleted

GetSupply(ctx sdk.Context) exported.SupplyI
SetSupply(ctx sdk.Context, supply exported.SupplyI)
BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
GetSupply(ctx sdk.Context, denom string) sdk.Coin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

GetSupply in x/bank takes one more arg, denom, and returns Coin.

@@ -164,3 +164,6 @@ func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {}
func (am AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate {
return []abci.ValidatorUpdate{}
}

// ConsensusVersion implements AppModule/ConsensusVersion.
func (AppModule) ConsensusVersion() uint64 { return 1 }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ConsensusVersion is a sequence number for state-breaking change of the module. It should be incremented on each consensus-breaking change introduced by the module. To avoid wrong/empty versions, the initial version should be set to 1.
(https://github.com/cosmos/cosmos-sdk/blob/ad9e5620fb3445c716e9de45cfcdb56e8f1745bf/types/module/module.go#L176)

"oracle": 1,
}

app.IBCKeeper.ConnectionKeeper.SetParams(ctx, ibcconnectiontypes.DefaultParams())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For in-place ibc migration, a new param, MaxExpectedTimePerBlock, should be set.

(https://github.com/cosmos/ibc-go/blob/main/docs/migrations/sdk-to-v1.md#in-place-store-migrations)

@0xHansLee
Copy link
Contributor Author

In this commit, default value of minimum-gas-prices is set to "5umed" as guided in our gitbook
We can also add other custom configs for Panacea for future.

Copy link
Contributor

@gyuguen gyuguen left a comment

Choose a reason for hiding this comment

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

Thank you for your hard working.
I left a few comment.

app/app.go Outdated Show resolved Hide resolved
cmd/panacead/cmd/init.go Outdated Show resolved Hide resolved
x/burn/keeper/burn.go Show resolved Hide resolved
x/burn/keeper/burn_test.go Outdated Show resolved Hide resolved
x/oracle/keeper/oracle_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@gyuguen gyuguen left a comment

Choose a reason for hiding this comment

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

LGTM 👍

types/testsuite/suite.go Outdated Show resolved Hide resolved
go.mod Outdated
@@ -3,35 +3,35 @@ module github.com/medibloc/panacea-core/v2
go 1.15
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be changed from 1.15 to 1.18.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.18? not 1.17?
Cosmos-sdk v0.45.4 requires min ver of go of 1.17.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha, I was talking about looking at cosmos-sdk v0.45.6.
As you said, 1.17 seems to be correct. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The required minimum go version is updated in 79db4e4

Copy link
Contributor

@youngjoon-lee youngjoon-lee left a comment

Choose a reason for hiding this comment

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

danke

Copy link

@inchori inchori left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks for your effort!
When upgrading next time, I could try it.

@0xHansLee
Copy link
Contributor Author

As I said offline, the version v2.2.0-alpha1(including upgrade handler) is only for upgrade from v2.1.0-alpha2.
When we release a new version of panacea and upgrade mainnet, we should register a new upgrade handler which enable to upgrade and migrate from v2.0.3.

@0xHansLee 0xHansLee merged commit 664d546 into master Jun 30, 2022
@0xHansLee 0xHansLee deleted the ft/na/v0.44-cosmos-sdk-upgrade branch June 30, 2022 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants