Skip to content

Commit

Permalink
refactor(x/mint): v0.52 audit x/mint (#21301)
Browse files Browse the repository at this point in the history
  • Loading branch information
JulianToledano authored Aug 16, 2024
1 parent f8ab520 commit 825e81b
Show file tree
Hide file tree
Showing 14 changed files with 173 additions and 25 deletions.
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)
}
}
})
}
}

0 comments on commit 825e81b

Please sign in to comment.