-
Notifications
You must be signed in to change notification settings - Fork 45
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
Collin/v8 migration fixes #324
Merged
Merged
Changes from 34 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
9cf139d
Add missing SDK 0.47 upgrade steps
cbrit 442e5b8
Remove msg event boilerplate replace in 0.47
cbrit f7b844d
Add missing param migrations and additional IBC client type
cbrit 0fbf617
Fixing cellarfees params migration
cbrit f72ba15
More cellarfees params fixing
cbrit b8f1b66
Even more cellarfees params fixing
cbrit 84fcf65
Hopefully last cellarfees params fix
cbrit 03e7b7b
Hopefully the last last cellarfees fix
cbrit dc1e953
Remove write to old cellarfees param subspace
cbrit d8eb12c
Use correct method to retrieve legacy cellarfees subspace
cbrit fa58afd
Add missing IBC migration stuff
cbrit 216b255
Test commit for debugging
cbrit c9d50c5
More debugging
cbrit c16ff12
Please
cbrit 050b56b
Debugging
cbrit cdc8446
AHHHH
cbrit ee76e49
just trying stuff
cbrit 533371b
.
cbrit 033916f
..
cbrit 3178f24
Trying with legacySubspace passed in via app module, and deleting par…
cbrit a272498
Fix method call
cbrit 509babb
Fix method call
cbrit ed9ecc1
Move upgrade handler setup after store mount
cbrit 68554f8
Hardcode cellarfees params
cbrit bc18b56
Incentives params test
cbrit 5da80e6
Cellarfees params migration fix test
cbrit d8a3941
Clean up auction params migration
cbrit 7fa3a1f
Explicitly set new auction module account permissions
cbrit 0d64fc2
Fix axelarcork export genesis bug that results in null values for Cor…
cbrit 13f302c
Actually add the upgrade fix...
cbrit 6105c40
Move auction permission to end of upgrade handler
cbrit a680ea4
Wow.
cbrit a25cea5
Tweak auction test
cbrit 36113b5
Merge branch 'main' into collin/v8-migration-fixes
cbrit e48b362
Fix params validation in cellarfees
cbrit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,83 @@ | ||
package v8 | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/cosmos/cosmos-sdk/baseapp" | ||
"github.com/cosmos/cosmos-sdk/codec" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/types/module" | ||
authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" | ||
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" | ||
consensusparamkeeper "github.com/cosmos/cosmos-sdk/x/consensus/keeper" | ||
paramstypes "github.com/cosmos/cosmos-sdk/x/params/types" | ||
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" | ||
ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" | ||
ibckeeper "github.com/cosmos/ibc-go/v7/modules/core/keeper" | ||
ibctmmigrations "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint/migrations" | ||
auctiontypes "github.com/peggyjv/sommelier/v8/x/auction/types" | ||
) | ||
|
||
func CreateUpgradeHandler( | ||
mm *module.Manager, | ||
configurator module.Configurator, | ||
baseAppLegacySS *paramstypes.Subspace, | ||
consensusParamsKeeper *consensusparamkeeper.Keeper, | ||
ibcKeeper *ibckeeper.Keeper, | ||
cdc codec.BinaryCodec, | ||
clientKeeper ibctmmigrations.ClientKeeper, | ||
accountKeeper *authkeeper.AccountKeeper, | ||
) upgradetypes.UpgradeHandler { | ||
return func(ctx sdk.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { | ||
ctx.Logger().Info("v8 upgrade: entering handler and running migrations") | ||
|
||
// Include this when migrating to ibc-go v7 (optional) | ||
// source: https://github.com/cosmos/ibc-go/blob/v7.2.0/docs/migrations/v6-to-v7.md | ||
// prune expired tendermint consensus states to save storage space | ||
if _, err := ibctmmigrations.PruneExpiredConsensusStates(ctx, cdc, clientKeeper); err != nil { | ||
return nil, err | ||
} | ||
|
||
// new x/consensus module params migration | ||
baseapp.MigrateParams(ctx, baseAppLegacySS, consensusParamsKeeper) | ||
|
||
return mm.RunMigrations(ctx, configurator, vm) | ||
// explicitly update the IBC 02-client params, adding the localhost client type | ||
params := ibcKeeper.ClientKeeper.GetParams(ctx) | ||
params.AllowedClients = append(params.AllowedClients, ibcexported.Localhost) | ||
ibcKeeper.ClientKeeper.SetParams(ctx, params) | ||
|
||
vm, err := mm.RunMigrations(ctx, configurator, vm) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// add burner permission to auction account | ||
if err := MigrateAuctionAccountPermissions(ctx, accountKeeper); err != nil { | ||
return nil, err | ||
} | ||
|
||
return vm, nil | ||
} | ||
} | ||
|
||
func MigrateAuctionAccountPermissions(ctx sdk.Context, accountKeeper *authkeeper.AccountKeeper) error { | ||
ctx.Logger().Info("Migrating auction account permissions") | ||
oldAcctI := accountKeeper.GetModuleAccount(ctx, auctiontypes.ModuleName) | ||
|
||
if oldAcctI == nil { | ||
return fmt.Errorf("module account not found") | ||
} | ||
|
||
newAcct := authtypes.NewEmptyModuleAccount(auctiontypes.ModuleName, authtypes.Burner) | ||
newAcct.AccountNumber = oldAcctI.GetAccountNumber() | ||
newAcct.Address = oldAcctI.GetAddress().String() | ||
newAcct.Sequence = oldAcctI.GetSequence() | ||
newAcct.Name = oldAcctI.GetName() | ||
newAcctI := (accountKeeper.NewAccount(ctx, newAcct)).(authtypes.ModuleAccountI) | ||
|
||
accountKeeper.SetModuleAccount(ctx, newAcctI) | ||
|
||
ctx.Logger().Info("Auction account permissions migrated") | ||
|
||
return nil | ||
} | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simplify auction account permissions migration by updating existing account
Instead of creating a new module account and manually copying fields from the old account, you can simplify the migration by directly modifying the permissions of the existing account. This approach reduces complexity and minimizes the risk of introducing errors.
Here's a suggested change:
📝 Committable suggestion