refactor(x/mint): v0.52 audit x/mint (backport #21301) (#21330)

Co-authored-by: Julián Toledano <JulianToledano@users.noreply.github.com>

x/mint/CHANGELOG.md

@@ -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

x/mint/README.md

@@ -62,7 +62,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
@@ -75,7 +75,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

x/mint/epoch_hooks.go

@@ -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
 }

x/mint/keeper/genesis_test.go

@@ -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)
 }

x/mint/keeper/grpc_query_test.go

@@ -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
 
@@ -48,7 +48,7 @@ func (suite *MintTestSuite) SetupTest() {
 
 	suite.mintKeeper = keeper.NewKeeper(
 		encCfg.Codec,
-		storeService,
+		env,
 		stakingKeeper,
 		accountKeeper,
 		bankKeeper,

x/mint/keeper/keeper_test.go

@@ -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)
@@ -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)
@@ -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)
 

x/mint/keeper/migrator.go

@@ -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
 }
 

x/mint/keeper/migrator_test.go

@@ -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())
+}

x/mint/module.go

@@ -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,

x/mint/simulation/genesis_test.go

@@ -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{})
@@ -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)
 	}
 }

x/mint/simulation/proposals.go

@@ -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(

x/mint/types/minter_test.go

@@ -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

x/mint/types/params.go

@@ -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
 	}
 }

x/mint/types/params_test.go

@@ -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)
+				}
+			}
+		})
+	}
+}