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(x/mint): v0.52 audit x/mint #21301

Merged
merged 5 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion x/mint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
### API Breaking Changes

* [#20363](https://github.com/cosmos/cosmos-sdk/pull/20363) Deprecated InflationCalculationFn in favor of MintFn, `keeper.DefaultMintFn` wrapper must be used in order to continue using it in `NewAppModule`. This is not breaking for depinject users, as both `MintFn` and `InflationCalculationFn` are accepted.
* [#19367](https://github.com/cosmos/cosmos-sdk/pull/19398) `appmodule.Environment` is received on the Keeper to get access to different application services
* [#19367](https://github.com/cosmos/cosmos-sdk/pull/19398) `appmodule.Environment` is received on the Keeper to get access to different application services.

### Bug Fixes
4 changes: 2 additions & 2 deletions x/mint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ related to minting (in the `data` field)
* Minter: `0x00 -> ProtocolBuffer(minter)`

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/ace7bca105a8d5363782cfd19c6f169b286cd3b2/x/mint/proto/cosmos/mint/v1beta1/mint.proto#L11-L29
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/mint/proto/cosmos/mint/v1beta1/mint.proto#L11-L29
```

### Params
Expand All @@ -122,7 +122,7 @@ A value of `0` indicates an unlimited supply.
* Params: `mint/params -> legacy_amino(params)`

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/7068d0da52d954430054768b2c56aff44666933b/x/mint/proto/cosmos/mint/v1beta1/mint.proto#L26-L68
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/mint/proto/cosmos/mint/v1beta1/mint.proto#L31-L73
```

## Epoch minting
Expand Down
2 changes: 1 addition & 1 deletion x/mint/epoch_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ func (am AppModule) BeforeEpochStart(ctx context.Context, epochIdentifier string
}

// AfterEpochEnd is a noop
func (am AppModule) AfterEpochEnd(ctx context.Context, epochIdentifier string, epochNumber int64) error {
func (am AppModule) AfterEpochEnd(_ context.Context, _ string, _ int64) error {
return nil
}
16 changes: 8 additions & 8 deletions x/mint/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,21 +75,21 @@ func (s *GenesisTestSuite) TestImportExportGenesis() {
)

err := s.keeper.InitGenesis(s.sdkCtx, s.accountKeeper, genesisState)
s.Require().NoError(err)
s.NoError(err)

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

invalidCtx := testutil.DefaultContextWithDB(s.T(), s.key, storetypes.NewTransientStoreKey("transient_test"))
_, err = s.keeper.Minter.Get(invalidCtx.Ctx)
s.Require().ErrorIs(err, collections.ErrNotFound)
s.ErrorIs(err, collections.ErrNotFound)

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

genesisState2, err := s.keeper.ExportGenesis(s.sdkCtx)
s.Require().NoError(err)
s.Require().Equal(genesisState, genesisState2)
s.NoError(err)
s.Equal(genesisState, genesisState2)
}
4 changes: 2 additions & 2 deletions x/mint/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type MintTestSuite struct {
func (suite *MintTestSuite) SetupTest() {
encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, mint.AppModule{})
key := storetypes.NewKVStoreKey(types.StoreKey)
storeService := runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger())
env := runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger())
testCtx := testutil.DefaultContextWithDB(suite.T(), key, storetypes.NewTransientStoreKey("transient_test"))
suite.ctx = testCtx.Ctx

Expand All @@ -48,7 +48,7 @@ func (suite *MintTestSuite) SetupTest() {

suite.mintKeeper = keeper.NewKeeper(
encCfg.Codec,
storeService,
env,
stakingKeeper,
accountKeeper,
bankKeeper,
Expand Down
10 changes: 6 additions & 4 deletions x/mint/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (s *KeeperTestSuite) TestDefaultMintFn() {
err = s.mintKeeper.DefaultMintFn(types.DefaultInflationCalculationFn)(s.ctx, s.mintKeeper.Environment, &minter, "block", 0)
s.NoError(err)

// set a maxsupply and call again
// set a maxSupply and call again. totalSupply will be bigger than maxSupply.
params, err := s.mintKeeper.Params.Get(s.ctx)
s.NoError(err)
params.MaxSupply = math.NewInt(10000000000)
Expand All @@ -122,14 +122,16 @@ func (s *KeeperTestSuite) TestDefaultMintFn() {
s.NoError(err)

// modify max supply to be almost reached
// we tried to mint 2059stake but we only get to mint 2000stake
// modify blocksPerYear to mint 2053 coins per block
// we tried to mint 2053stake, but we only get to mint 2000stake
params, err = s.mintKeeper.Params.Get(s.ctx)
s.NoError(err)
params.MaxSupply = math.NewInt(100000000000 + 2000)
params.BlocksPerYear = 2434275
err = s.mintKeeper.Params.Set(s.ctx, params)
s.NoError(err)

s.bankKeeper.EXPECT().MintCoins(s.ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin("stake", math.NewInt(792)))).Return(nil)
s.bankKeeper.EXPECT().MintCoins(s.ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin("stake", math.NewInt(2000)))).Return(nil)
s.bankKeeper.EXPECT().SendCoinsFromModuleToModule(s.ctx, types.ModuleName, authtypes.FeeCollectorName, gomock.Any()).Return(nil)

err = s.mintKeeper.DefaultMintFn(types.DefaultInflationCalculationFn)(s.ctx, s.mintKeeper.Environment, &minter, "block", 0)
Expand All @@ -143,7 +145,7 @@ func (s *KeeperTestSuite) TestBeginBlocker() {
s.bankKeeper.EXPECT().MintCoins(s.ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin("stake", math.NewInt(792)))).Return(nil)
s.bankKeeper.EXPECT().SendCoinsFromModuleToModule(s.ctx, types.ModuleName, authtypes.FeeCollectorName, gomock.Any()).Return(nil)

// get minter (it should get modified aftwerwards)
// get minter (it should get modified afterwards)
minter, err := s.mintKeeper.Minter.Get(s.ctx)
s.NoError(err)

Expand Down
2 changes: 1 addition & 1 deletion x/mint/keeper/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func NewMigrator(k Keeper) Migrator {
// version 2. Specifically, it takes the parameters that are currently stored
// and managed by the x/params modules and stores them directly into the x/mint
// module state.
func (m Migrator) Migrate1to2(ctx context.Context) error {
func (m Migrator) Migrate1to2(_ context.Context) error {
return nil
}

Expand Down
33 changes: 33 additions & 0 deletions x/mint/keeper/migrator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package keeper_test

import (
"cosmossdk.io/math"
"cosmossdk.io/x/mint/keeper"
"cosmossdk.io/x/mint/types"
)

func (s *KeeperTestSuite) TestMigrator_Migrate2to3() {
migrator := keeper.NewMigrator(s.mintKeeper)

params, err := s.mintKeeper.Params.Get(s.ctx)
s.NoError(err)

err = migrator.Migrate2to3(s.ctx)
s.NoError(err)

migratedParams, err := s.mintKeeper.Params.Get(s.ctx)
s.NoError(err)
s.Equal(params, migratedParams)

params.MaxSupply = math.NewInt(1000000)
err = s.mintKeeper.Params.Set(s.ctx, params)
s.NoError(err)

err = migrator.Migrate2to3(s.ctx)
s.NoError(err)

migratedParams, err = s.mintKeeper.Params.Get(s.ctx)
s.NoError(err)
s.NotEqual(params, migratedParams)
s.Equal(migratedParams, types.DefaultParams())
}
2 changes: 1 addition & 1 deletion x/mint/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type AppModule struct {
}

// NewAppModule creates a new AppModule object.
// If the mintFn argument is nil, then the SDK's default minting function will be used.
// If the mintFn argument is nil, then the default minting function will be used.
func NewAppModule(
cdc codec.Codec,
keeper keeper.Keeper,
Expand Down
4 changes: 1 addition & 3 deletions x/mint/simulation/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

// TestRandomizedGenState tests the normal scenario of applying RandomizedGenState.
// Abonormal scenarios are not tested here.
// Abnormal scenarios are not tested here.
func TestRandomizedGenState(t *testing.T) {
cdcOpts := codectestutil.CodecOptions{}
encCfg := moduletestutil.MakeTestEncodingConfig(cdcOpts, mint.AppModule{})
Expand Down Expand Up @@ -84,8 +84,6 @@ func TestRandomizedGenState1(t *testing.T) {
}

for _, tt := range tests {
tt := tt

require.Panicsf(t, func() { simulation.RandomizedGenState(&tt.simState) }, tt.panicMsg)
}
}
2 changes: 1 addition & 1 deletion x/mint/simulation/proposals.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const (
OpWeightMsgUpdateParams = "op_weight_msg_update_params"
)

// ProposalMsgs defines the module weighted proposals' contents
// ProposalMsgs defines the module's weighted proposals contents
func ProposalMsgs() []simtypes.WeightedProposalMsg {
return []simtypes.WeightedProposalMsg{
simulation.NewWeightedProposalMsgX(
Expand Down
72 changes: 72 additions & 0 deletions x/mint/types/minter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,78 @@ func TestValidateMinter(t *testing.T) {
}
}

func TestIsEqualMinter(t *testing.T) {
tests := []struct {
name string
a Minter
b Minter
equal bool
}{
{
name: "equal minters",
a: Minter{
Inflation: math.LegacyNewDec(10),
AnnualProvisions: math.LegacyNewDec(10),
Data: []byte("data"),
},
b: Minter{
Inflation: math.LegacyNewDec(10),
AnnualProvisions: math.LegacyNewDec(10),
Data: []byte("data"),
},
equal: true,
},
{
name: "different inflation",
a: Minter{
Inflation: math.LegacyNewDec(10),
AnnualProvisions: math.LegacyNewDec(10),
Data: []byte("data"),
},
b: Minter{
Inflation: math.LegacyNewDec(100),
AnnualProvisions: math.LegacyNewDec(10),
Data: []byte("data"),
},
equal: false,
},
{
name: "different Annual Provisions",
a: Minter{
Inflation: math.LegacyNewDec(10),
AnnualProvisions: math.LegacyNewDec(10),
Data: []byte("data"),
},
b: Minter{
Inflation: math.LegacyNewDec(10),
AnnualProvisions: math.LegacyNewDec(100),
Data: []byte("data"),
},
equal: false,
},
{
name: "different data",
a: Minter{
Inflation: math.LegacyNewDec(10),
AnnualProvisions: math.LegacyNewDec(10),
Data: []byte("data"),
},
b: Minter{
Inflation: math.LegacyNewDec(10),
AnnualProvisions: math.LegacyNewDec(10),
Data: []byte("no data"),
},
equal: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
equal := tt.a.IsEqual(tt.b)
require.Equal(t, tt.equal, equal)
})
}
}

// Benchmarking :)
// previously using math.Int operations:
// BenchmarkBlockProvision-4 5000000 220 ns/op
Expand Down
2 changes: 1 addition & 1 deletion x/mint/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func DefaultParams() Params {
InflationMax: math.LegacyNewDecWithPrec(5, 2),
InflationMin: math.LegacyNewDecWithPrec(0, 2),
GoalBonded: math.LegacyNewDecWithPrec(67, 2),
BlocksPerYear: uint64(60 * 60 * 8766 / 5), // assuming 5 second block times
BlocksPerYear: uint64(60 * 60 * 8766 / 5), // assuming 5-second block times
MaxSupply: math.ZeroInt(), // assuming zero is infinite
}
}
Expand Down
43 changes: 43 additions & 0 deletions x/mint/types/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,46 @@ func TestValidate(t *testing.T) {
err = params.Validate()
require.Error(t, err)
}

func Test_validateInflationFields(t *testing.T) {
fns := []func(dec math.LegacyDec) error{
validateInflationRateChange,
validateInflationMax,
validateInflationMin,
validateGoalBonded,
}
tests := []struct {
name string
v math.LegacyDec
wantErr bool
}{
{
name: "valid",
v: math.LegacyNewDecWithPrec(12, 2),
},
{
name: "nil",
v: math.LegacyDec{},
wantErr: true,
},
{
name: "negative",
v: math.LegacyNewDec(-1),
wantErr: true,
},
{
name: "greater than one",
v: math.LegacyOneDec().Add(math.LegacyOneDec()),
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for _, fn := range fns {
if err := fn(tt.v); (err != nil) != tt.wantErr {
t.Errorf("validateInflationRateChange() error = %v, wantErr %v", err, tt.wantErr)
}
}
})
}
}
Loading