From e704665ffeac6b3ada7b76ddd049dfeccae4599d Mon Sep 17 00:00:00 2001 From: Julian Toledano Date: Wed, 14 Aug 2024 13:29:08 +0200 Subject: [PATCH 1/5] update: keeper audit --- x/mint/keeper/genesis_test.go | 16 ++++++++-------- x/mint/keeper/grpc_query_test.go | 4 ++-- x/mint/keeper/keeper_test.go | 10 ++++++---- x/mint/keeper/migrator.go | 2 +- x/mint/keeper/migrator_test.go | 33 ++++++++++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 15 deletions(-) create mode 100644 x/mint/keeper/migrator_test.go diff --git a/x/mint/keeper/genesis_test.go b/x/mint/keeper/genesis_test.go index 754055066c27..56b1171a5dcb 100644 --- a/x/mint/keeper/genesis_test.go +++ b/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) } diff --git a/x/mint/keeper/grpc_query_test.go b/x/mint/keeper/grpc_query_test.go index f2f80689b759..eb83b895a7e1 100644 --- a/x/mint/keeper/grpc_query_test.go +++ b/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, diff --git a/x/mint/keeper/keeper_test.go b/x/mint/keeper/keeper_test.go index 012917777901..215ac12b4ddd 100644 --- a/x/mint/keeper/keeper_test.go +++ b/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) diff --git a/x/mint/keeper/migrator.go b/x/mint/keeper/migrator.go index a2dc25e2e365..199757c1f1a2 100644 --- a/x/mint/keeper/migrator.go +++ b/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 } diff --git a/x/mint/keeper/migrator_test.go b/x/mint/keeper/migrator_test.go new file mode 100644 index 000000000000..ce585da36c60 --- /dev/null +++ b/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()) +} From 0022320b879f8f1270b1d0ce7fa3fd33ed2e6dc1 Mon Sep 17 00:00:00 2001 From: Julian Toledano Date: Wed, 14 Aug 2024 14:25:39 +0200 Subject: [PATCH 2/5] update: types audit --- x/mint/simulation/genesis_test.go | 2 - x/mint/types/minter.go | 16 ++----- x/mint/types/minter_test.go | 72 +++++++++++++++++++++++++++++++ x/mint/types/params.go | 2 +- x/mint/types/params_test.go | 43 ++++++++++++++++++ 5 files changed, 119 insertions(+), 16 deletions(-) diff --git a/x/mint/simulation/genesis_test.go b/x/mint/simulation/genesis_test.go index ba31c32e4b2a..167308d8ec3e 100644 --- a/x/mint/simulation/genesis_test.go +++ b/x/mint/simulation/genesis_test.go @@ -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) } } diff --git a/x/mint/types/minter.go b/x/mint/types/minter.go index 84be5698df4a..b377639b2e53 100644 --- a/x/mint/types/minter.go +++ b/x/mint/types/minter.go @@ -84,17 +84,7 @@ func (m Minter) BlockProvision(params Params) sdk.Coin { // IsEqual returns true if two minters are equal, it checks all the fields func (m Minter) IsEqual(minter Minter) bool { - if !m.Inflation.Equal(minter.Inflation) { - return false - } - - if !m.AnnualProvisions.Equal(minter.AnnualProvisions) { - return false - } - - if !bytes.Equal(m.Data, minter.Data) { - return false - } - - return true + return m.Inflation.Equal(minter.Inflation) && + m.AnnualProvisions.Equal(minter.AnnualProvisions) && + bytes.Equal(m.Data, minter.Data) } diff --git a/x/mint/types/minter_test.go b/x/mint/types/minter_test.go index 8bff2334995a..7e18a3e1b173 100644 --- a/x/mint/types/minter_test.go +++ b/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 diff --git a/x/mint/types/params.go b/x/mint/types/params.go index dfc00348b3db..af4a01d2daed 100644 --- a/x/mint/types/params.go +++ b/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 } } diff --git a/x/mint/types/params_test.go b/x/mint/types/params_test.go index b43c6212d058..25f34ee02571 100644 --- a/x/mint/types/params_test.go +++ b/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) + } + } + }) + } +} From d2923cb0b5b1210956f358db28c087f82f9fc730 Mon Sep 17 00:00:00 2001 From: Julian Toledano Date: Wed, 14 Aug 2024 14:39:25 +0200 Subject: [PATCH 3/5] update: module audit --- x/mint/CHANGELOG.md | 2 +- x/mint/epoch_hooks.go | 2 +- x/mint/module.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/mint/CHANGELOG.md b/x/mint/CHANGELOG.md index 014ac8759a9d..285fab6b5c3b 100644 --- a/x/mint/CHANGELOG.md +++ b/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 diff --git a/x/mint/epoch_hooks.go b/x/mint/epoch_hooks.go index 2a63245036e6..9c1ae6ff1c04 100644 --- a/x/mint/epoch_hooks.go +++ b/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 } diff --git a/x/mint/module.go b/x/mint/module.go index b0f6b2a14ec9..6607b371c16d 100644 --- a/x/mint/module.go +++ b/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, From b71b6a35293b23b7285b1220a404f0ac592e8377 Mon Sep 17 00:00:00 2001 From: Julian Toledano Date: Wed, 14 Aug 2024 16:48:32 +0200 Subject: [PATCH 4/5] fix: typos --- x/mint/README.md | 4 ++-- x/mint/simulation/genesis_test.go | 2 +- x/mint/simulation/proposals.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x/mint/README.md b/x/mint/README.md index 849164f04131..5a885208eca1 100644 --- a/x/mint/README.md +++ b/x/mint/README.md @@ -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 @@ -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 diff --git a/x/mint/simulation/genesis_test.go b/x/mint/simulation/genesis_test.go index 167308d8ec3e..a9ca7c6185fb 100644 --- a/x/mint/simulation/genesis_test.go +++ b/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{}) diff --git a/x/mint/simulation/proposals.go b/x/mint/simulation/proposals.go index 570bf203efcc..eb0b140f5e3e 100644 --- a/x/mint/simulation/proposals.go +++ b/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( From cfda361338d655c96038412eccd84615324264e2 Mon Sep 17 00:00:00 2001 From: Julian Toledano Date: Fri, 16 Aug 2024 12:24:28 +0200 Subject: [PATCH 5/5] revert IsEqual --- x/mint/types/minter.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/x/mint/types/minter.go b/x/mint/types/minter.go index b377639b2e53..84be5698df4a 100644 --- a/x/mint/types/minter.go +++ b/x/mint/types/minter.go @@ -84,7 +84,17 @@ func (m Minter) BlockProvision(params Params) sdk.Coin { // IsEqual returns true if two minters are equal, it checks all the fields func (m Minter) IsEqual(minter Minter) bool { - return m.Inflation.Equal(minter.Inflation) && - m.AnnualProvisions.Equal(minter.AnnualProvisions) && - bytes.Equal(m.Data, minter.Data) + if !m.Inflation.Equal(minter.Inflation) { + return false + } + + if !m.AnnualProvisions.Equal(minter.AnnualProvisions) { + return false + } + + if !bytes.Equal(m.Data, minter.Data) { + return false + } + + return true }