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

refactor(mint)!: migrate state management to collections #16329

Merged
merged 11 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions testutil/integration/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func Example() {
// register the message and query servers
authtypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), authkeeper.NewMsgServerImpl(accountKeeper))
minttypes.RegisterMsgServer(integrationApp.MsgServiceRouter(), mintkeeper.NewMsgServerImpl(mintKeeper))
minttypes.RegisterQueryServer(integrationApp.QueryHelper(), mintKeeper)
minttypes.RegisterQueryServer(integrationApp.QueryHelper(), mintkeeper.NewQueryServerImpl(mintKeeper))

params := minttypes.DefaultParams()
params.BlocksPerYear = 10000
Expand Down Expand Up @@ -98,7 +98,7 @@ func Example() {
sdkCtx := sdk.UnwrapSDKContext(integrationApp.Context())

// we should also check the state of the application
got, err := mintKeeper.GetParams(sdkCtx)
got, err := mintKeeper.Params.Get(sdkCtx)
if err != nil {
panic(err)
}
Expand Down
9 changes: 6 additions & 3 deletions x/mint/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ func BeginBlocker(ctx context.Context, k keeper.Keeper, ic types.InflationCalcul
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)

// fetch stored minter & params
minter, err := k.GetMinter(ctx)
minter, err := k.Minter.Get(ctx)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

path flow from Begin/EndBlock to a panic call
if err != nil {
return err
}

params, err := k.GetParams(ctx)
params, err := k.Params.Get(ctx)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

path flow from Begin/EndBlock to a panic call
if err != nil {
return err
}
Expand All @@ -30,7 +30,10 @@ func BeginBlocker(ctx context.Context, k keeper.Keeper, ic types.InflationCalcul
bondedRatio := k.BondedRatio(ctx)
minter.Inflation = ic(ctx, minter, params, bondedRatio)
minter.AnnualProvisions = minter.NextAnnualProvisions(params, totalStakingSupply)
k.SetMinter(ctx, minter)
err = k.Minter.Set(ctx, minter)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

path flow from Begin/EndBlock to a panic call
if err != nil {
return err
}

// mint coins, update supply
mintedCoin := minter.BlockProvision(params)
Expand Down
10 changes: 6 additions & 4 deletions x/mint/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import (

// InitGenesis new mint genesis
func (keeper Keeper) InitGenesis(ctx sdk.Context, ak types.AccountKeeper, data *types.GenesisState) {
keeper.SetMinter(ctx, data.Minter)
if err := keeper.Minter.Set(ctx, data.Minter); err != nil {
panic(err)
}

if err := keeper.SetParams(ctx, data.Params); err != nil {
if err := keeper.Params.Set(ctx, data.Params); err != nil {
panic(err)
}

Expand All @@ -18,12 +20,12 @@ func (keeper Keeper) InitGenesis(ctx sdk.Context, ak types.AccountKeeper, data *

// ExportGenesis returns a GenesisState for a given context and keeper.
func (keeper Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
minter, err := keeper.GetMinter(ctx)
minter, err := keeper.Minter.Get(ctx)
if err != nil {
panic(err)
}

params, err := keeper.GetParams(ctx)
params, err := keeper.Params.Get(ctx)
if err != nil {
panic(err)
}
Expand Down
9 changes: 5 additions & 4 deletions x/mint/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper_test
import (
"testing"

"cosmossdk.io/collections"
"cosmossdk.io/math"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -72,15 +73,15 @@ func (s *GenesisTestSuite) TestImportExportGenesis() {

s.keeper.InitGenesis(s.sdkCtx, s.accountKeeper, genesisState)

minter, err := s.keeper.GetMinter(s.sdkCtx)
minter, err := s.keeper.Minter.Get(s.sdkCtx)
s.Require().Equal(genesisState.Minter, minter)
s.Require().NoError(err)

invalidCtx := testutil.DefaultContextWithDB(s.T(), s.key, storetypes.NewTransientStoreKey("transient_test"))
_, err = s.keeper.GetMinter(invalidCtx.Ctx)
s.Require().EqualError(err, "stored minter should not have been nil")
_, err = s.keeper.Minter.Get(invalidCtx.Ctx)
s.Require().ErrorIs(err, collections.ErrNotFound)

params, err := s.keeper.GetParams(s.sdkCtx)
params, err := s.keeper.Params.Get(s.sdkCtx)
s.Require().Equal(genesisState.Params, params)
s.Require().NoError(err)

Expand Down
22 changes: 15 additions & 7 deletions x/mint/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,20 @@ import (
"github.com/cosmos/cosmos-sdk/x/mint/types"
)

var _ types.QueryServer = Keeper{}
var _ types.QueryServer = queryServer{}

func NewQueryServerImpl(k Keeper) types.QueryServer {
return queryServer{k}
}

type queryServer struct {
k Keeper
}

// Params returns params of the mint module.
func (k Keeper) Params(c context.Context, _ *types.QueryParamsRequest) (*types.QueryParamsResponse, error) {
func (q queryServer) Params(c context.Context, _ *types.QueryParamsRequest) (*types.QueryParamsResponse, error) {
ctx := sdk.UnwrapSDKContext(c)
params, err := k.GetParams(ctx)
params, err := q.k.Params.Get(ctx)
if err != nil {
return nil, err
}
Expand All @@ -21,9 +29,9 @@ func (k Keeper) Params(c context.Context, _ *types.QueryParamsRequest) (*types.Q
}

// Inflation returns minter.Inflation of the mint module.
func (k Keeper) Inflation(c context.Context, _ *types.QueryInflationRequest) (*types.QueryInflationResponse, error) {
func (q queryServer) Inflation(c context.Context, _ *types.QueryInflationRequest) (*types.QueryInflationResponse, error) {
ctx := sdk.UnwrapSDKContext(c)
minter, err := k.GetMinter(ctx)
minter, err := q.k.Minter.Get(ctx)
if err != nil {
return nil, err
}
Expand All @@ -32,9 +40,9 @@ func (k Keeper) Inflation(c context.Context, _ *types.QueryInflationRequest) (*t
}

// AnnualProvisions returns minter.AnnualProvisions of the mint module.
func (k Keeper) AnnualProvisions(c context.Context, _ *types.QueryAnnualProvisionsRequest) (*types.QueryAnnualProvisionsResponse, error) {
func (q queryServer) AnnualProvisions(c context.Context, _ *types.QueryAnnualProvisionsRequest) (*types.QueryAnnualProvisionsResponse, error) {
ctx := sdk.UnwrapSDKContext(c)
minter, err := k.GetMinter(ctx)
minter, err := q.k.Minter.Get(ctx)
if err != nil {
return nil, err
}
Expand Down
10 changes: 5 additions & 5 deletions x/mint/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,26 @@ func (suite *MintTestSuite) SetupTest() {
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

err := suite.mintKeeper.SetParams(suite.ctx, types.DefaultParams())
err := suite.mintKeeper.Params.Set(suite.ctx, types.DefaultParams())
suite.Require().NoError(err)
suite.mintKeeper.SetMinter(suite.ctx, types.DefaultInitialMinter())
suite.Require().NoError(suite.mintKeeper.Minter.Set(suite.ctx, types.DefaultInitialMinter()))

queryHelper := baseapp.NewQueryServerTestHelper(testCtx.Ctx, encCfg.InterfaceRegistry)
types.RegisterQueryServer(queryHelper, suite.mintKeeper)
types.RegisterQueryServer(queryHelper, keeper.NewQueryServerImpl(suite.mintKeeper))

suite.queryClient = types.NewQueryClient(queryHelper)
}

func (suite *MintTestSuite) TestGRPCParams() {
params, err := suite.queryClient.Params(gocontext.Background(), &types.QueryParamsRequest{})
suite.Require().NoError(err)
kparams, err := suite.mintKeeper.GetParams(suite.ctx)
kparams, err := suite.mintKeeper.Params.Get(suite.ctx)
suite.Require().NoError(err)
suite.Require().Equal(params.Params, kparams)

inflation, err := suite.queryClient.Inflation(gocontext.Background(), &types.QueryInflationRequest{})
suite.Require().NoError(err)
minter, err := suite.mintKeeper.GetMinter(suite.ctx)
minter, err := suite.mintKeeper.Minter.Get(suite.ctx)
suite.Require().NoError(err)
suite.Require().Equal(inflation.Inflation, minter.Inflation)

Expand Down
70 changes: 16 additions & 54 deletions x/mint/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"cosmossdk.io/collections"
"cosmossdk.io/log"
"cosmossdk.io/math"

Expand All @@ -25,6 +26,10 @@ type Keeper struct {
// the address capable of executing a MsgUpdateParams message. Typically, this
// should be the x/gov module account.
authority string

Schema collections.Schema
Params collections.Item[types.Params]
Minter collections.Item[types.Minter]
}

// NewKeeper creates a new mint Keeper instance
Expand All @@ -42,14 +47,24 @@ func NewKeeper(
panic(fmt.Sprintf("the x/%s module account has not been set", types.ModuleName))
}

return Keeper{
sb := collections.NewSchemaBuilder(storeService)
k := Keeper{
cdc: cdc,
storeService: storeService,
stakingKeeper: sk,
bankKeeper: bk,
feeCollectorName: feeCollectorName,
authority: authority,
Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)),
Minter: collections.NewItem(sb, types.MinterKey, "minter", codec.CollValue[types.Minter](cdc)),
}

schema, err := sb.Build()
if err != nil {
panic(err)
}
k.Schema = schema
return k
}

// GetAuthority returns the x/mint module's authority.
Expand All @@ -63,59 +78,6 @@ func (k Keeper) Logger(ctx context.Context) log.Logger {
return sdkCtx.Logger().With("module", "x/"+types.ModuleName)
}

// GetMinter returns the minter.
func (k Keeper) GetMinter(ctx context.Context) (types.Minter, error) {
testinginprod marked this conversation as resolved.
Show resolved Hide resolved
var minter types.Minter
store := k.storeService.OpenKVStore(ctx)
bz, err := store.Get(types.MinterKey)
if err != nil {
return minter, err
}

if bz == nil {
return minter, fmt.Errorf("stored minter should not have been nil")
}

err = k.cdc.Unmarshal(bz, &minter)
return minter, err
}

// SetMinter sets the minter.
func (k Keeper) SetMinter(ctx context.Context, minter types.Minter) error {
store := k.storeService.OpenKVStore(ctx)
bz, err := k.cdc.Marshal(&minter)
if err != nil {
return err
}
return store.Set(types.MinterKey, bz)
}

// SetParams sets the x/mint module parameters.
func (k Keeper) SetParams(ctx context.Context, params types.Params) error {
store := k.storeService.OpenKVStore(ctx)
bz, err := k.cdc.Marshal(&params)
if err != nil {
return err
}
return store.Set(types.ParamsKey, bz)
}

// GetParams returns the current x/mint module parameters.
func (k Keeper) GetParams(ctx context.Context) (p types.Params, err error) {
store := k.storeService.OpenKVStore(ctx)
bz, err := store.Get(types.ParamsKey)
if err != nil {
return p, err
}

if bz == nil {
return p, nil
Copy link
Contributor Author

Choose a reason for hiding this comment

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

returning empty params should be an error, because of invalid decs, I think that the fact it did not create issues was by "luck". Right now the behavioural change is that we'll error if params are missing.

}

err = k.cdc.Unmarshal(bz, &p)
return p, err
}

// StakingTokenSupply implements an alias call to the underlying staking keeper's
// StakingTokenSupply to be used in BeginBlocker.
func (k Keeper) StakingTokenSupply(ctx context.Context) math.Int {
Expand Down
57 changes: 2 additions & 55 deletions x/mint/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,66 +65,13 @@ func (s *IntegrationTestSuite) SetupTest() {
s.Require().Equal(testCtx.Ctx.Logger().With("module", "x/"+types.ModuleName),
s.mintKeeper.Logger(testCtx.Ctx))

err := s.mintKeeper.SetParams(s.ctx, types.DefaultParams())
err := s.mintKeeper.Params.Set(s.ctx, types.DefaultParams())
s.Require().NoError(err)
s.mintKeeper.SetMinter(s.ctx, types.DefaultInitialMinter())
s.Require().NoError(s.mintKeeper.Minter.Set(s.ctx, types.DefaultInitialMinter()))

s.msgServer = keeper.NewMsgServerImpl(s.mintKeeper)
}

func (s *IntegrationTestSuite) TestParams() {
testCases := []struct {
name string
input types.Params
expectErr bool
}{
{
name: "set invalid params (⚠️ not validated in keeper)",
input: types.Params{
MintDenom: sdk.DefaultBondDenom,
InflationRateChange: math.LegacyNewDecWithPrec(-13, 2),
InflationMax: math.LegacyNewDecWithPrec(20, 2),
InflationMin: math.LegacyNewDecWithPrec(7, 2),
GoalBonded: math.LegacyNewDecWithPrec(67, 2),
BlocksPerYear: uint64(60 * 60 * 8766 / 5),
},
expectErr: false,
},
{
name: "set full valid params",
input: types.Params{
MintDenom: sdk.DefaultBondDenom,
InflationRateChange: math.LegacyNewDecWithPrec(8, 2),
InflationMax: math.LegacyNewDecWithPrec(20, 2),
InflationMin: math.LegacyNewDecWithPrec(2, 2),
GoalBonded: math.LegacyNewDecWithPrec(37, 2),
BlocksPerYear: uint64(60 * 60 * 8766 / 5),
},
expectErr: false,
},
}

for _, tc := range testCases {
tc := tc

s.Run(tc.name, func() {
expected, err := s.mintKeeper.GetParams(s.ctx)
s.Require().NoError(err)
err = s.mintKeeper.SetParams(s.ctx, tc.input)
if tc.expectErr {
s.Require().Error(err)
} else {
expected = tc.input
s.Require().NoError(err)
}

p, err := s.mintKeeper.GetParams(s.ctx)
s.Require().NoError(err)
s.Require().Equal(expected, p)
})
}
}

func (s *IntegrationTestSuite) TestAliasFunctions() {
stakingTokenSupply := math.NewIntFromUint64(100000000000)
s.stakingKeeper.EXPECT().StakingTokenSupply(s.ctx).Return(stakingTokenSupply)
Expand Down
2 changes: 1 addition & 1 deletion x/mint/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (ms msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdatePara
}

ctx := sdk.UnwrapSDKContext(goCtx)
if err := ms.SetParams(ctx, msg.Params); err != nil {
if err := ms.Params.Set(ctx, msg.Params); err != nil {
return nil, err
}

Expand Down
4 changes: 2 additions & 2 deletions x/mint/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (AppModule) Name() string {
// module-specific gRPC queries.
func (am AppModule) RegisterServices(cfg module.Configurator) {
types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper))
types.RegisterQueryServer(cfg.QueryServer(), am.keeper)
types.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServerImpl(am.keeper))

m := keeper.NewMigrator(am.keeper, am.legacySubspace)

Expand Down Expand Up @@ -197,7 +197,7 @@ func (AppModule) ProposalMsgs(simState module.SimulationState) []simtypes.Weight

// RegisterStoreDecoder registers a decoder for mint module's types.
func (am AppModule) RegisterStoreDecoder(sdr simtypes.StoreDecoderRegistry) {
sdr[types.StoreKey] = simulation.NewDecodeStore(am.cdc)
sdr[types.StoreKey] = simtypes.NewStoreDecoderFuncFromCollectionsSchema(am.keeper.Schema)
}

// WeightedOperations doesn't return any mint module operation.
Expand Down
Loading