LSM staking migration #17
Conversation
Yes it does now, I am just having some flashbacks when it didn't and our testnet crashed :D |
There was a problem hiding this comment.
Added some comments.
Don't you also need to bump the consensus version of the x/staking module when performing a migration?
// ConsensusVersion implements AppModule/ConsensusVersion.
func (AppModule) ConsensusVersion() uint64 { return 2 }
x/staking/migrations/v3/store.go
Outdated
|
|
||
| // migrateUBDEntries will remove the ubdEntries with same creation_height | ||
| // and create a new ubdEntry with updated balance and initial_balance | ||
| func migrateUBDEntries(ctx sdk.Context, store storetypes.KVStore, cdc codec.BinaryCodec, legacySubspace exported.Subspace) error { |
There was a problem hiding this comment.
Can we have a test for this?
I'm not sure it exists on cosmos-sdk v47?
It can be setup like so:
- create old entries -> have multiple entries at the same block height
- migrate entries -> confirm that entries at the same height got merged
I'm also willing to accept this if this function is tested somewhere, but I don't see that being the case.
There was a problem hiding this comment.
unit test added in latest commits
|
Beware that this will be a long migration (long as in taking a lot of time). The list of delegations is >800k records, and every delegation has multiple entries. |
Great catch! updated in above commit |
There was a problem hiding this comment.
Making a comment because I cannot review unchanged code.
Think you also need to register the upgrade handler in x/staking/module.go
// RegisterServices registers module services.
func (am AppModule) RegisterServices(cfg module.Configurator) {
types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper))
querier := keeper.Querier{Keeper: am.keeper}
types.RegisterQueryServer(cfg.QueryServer(), querier)
m := keeper.NewMigrator(am.keeper)
cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2)
+ if err := cfg.RegisterMigration(types.ModuleName, 2, m.Migrate2to3); err != nil {
+ panic(fmt.Sprintf("failed to migrate x/%s from version 2 to 3: %v", types.ModuleName, err))
}
}
ValidatorBondFactor,ValidatorLiquidStakingCap,GlobalLiquidStakingCapLiquidSharesandValidatorBondSharesDelegation: addsValidatorBondRefreshTotalLiquidStakedTokensto determine the total stake from ICA accountsmin_self_delegation?