Skip to content

Commit

Permalink
refactor(x/mint): remove staking as a required module (#21858)
Browse files Browse the repository at this point in the history
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
tac0turtle and julienrbrt committed Sep 24, 2024
1 parent f195a86 commit f6d7a92
Show file tree
Hide file tree
Showing 17 changed files with 101 additions and 122 deletions.
6 changes: 4 additions & 2 deletions server/v2/cometbft/mempool/noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

var _ Mempool[sdk.Tx] = (*NoOpMempool[sdk.Tx])(nil) // verify interface at compile time
var _ Mempool[transaction.Tx] = (*NoOpMempool[transaction.Tx])(nil)
var (
_ Mempool[sdk.Tx] = (*NoOpMempool[sdk.Tx])(nil) // verify interface at compile time
_ Mempool[transaction.Tx] = (*NoOpMempool[transaction.Tx])(nil)
)

// NoOpMempool defines a no-op mempool. Transactions are completely discarded and
// ignored when BaseApp interacts with the mempool.
Expand Down
9 changes: 6 additions & 3 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ type SimApp struct {
BankKeeper bankkeeper.BaseKeeper
StakingKeeper *stakingkeeper.Keeper
SlashingKeeper slashingkeeper.Keeper
MintKeeper mintkeeper.Keeper
MintKeeper *mintkeeper.Keeper
DistrKeeper distrkeeper.Keeper
GovKeeper govkeeper.Keeper
UpgradeKeeper *upgradekeeper.Keeper
Expand Down Expand Up @@ -372,7 +372,10 @@ func NewSimApp(
cometService,
)

app.MintKeeper = mintkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[minttypes.StoreKey]), logger.With(log.ModuleKey, "x/mint")), app.StakingKeeper, app.AuthKeeper, app.BankKeeper, authtypes.FeeCollectorName, govModuleAddr)
app.MintKeeper = mintkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[minttypes.StoreKey]), logger.With(log.ModuleKey, "x/mint")), app.AuthKeeper, app.BankKeeper, authtypes.FeeCollectorName, govModuleAddr)
if err := app.MintKeeper.SetMintFn(mintkeeper.DefaultMintFn(minttypes.DefaultInflationCalculationFn, app.StakingKeeper, app.MintKeeper)); err != nil {
panic(err)
}

app.PoolKeeper = poolkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[pooltypes.StoreKey]), logger.With(log.ModuleKey, "x/protocolpool")), app.AuthKeeper, app.BankKeeper, app.StakingKeeper, govModuleAddr)

Expand Down Expand Up @@ -467,7 +470,7 @@ func NewSimApp(
bank.NewAppModule(appCodec, app.BankKeeper, app.AuthKeeper),
feegrantmodule.NewAppModule(appCodec, app.FeeGrantKeeper, app.interfaceRegistry),
gov.NewAppModule(appCodec, &app.GovKeeper, app.AuthKeeper, app.BankKeeper, app.PoolKeeper),
mint.NewAppModule(appCodec, app.MintKeeper, app.AuthKeeper, nil),
mint.NewAppModule(appCodec, app.MintKeeper, app.AuthKeeper),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AuthKeeper, app.BankKeeper, app.StakingKeeper, app.interfaceRegistry, cometService),
distr.NewAppModule(appCodec, app.DistrKeeper, app.StakingKeeper),
staking.NewAppModule(appCodec, app.StakingKeeper),
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/example/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ func Example() {

// here bankkeeper and staking keeper is nil because we are not testing them
// subspace is nil because we don't test params (which is legacy anyway)
mintKeeper := mintkeeper.NewKeeper(encodingCfg.Codec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[minttypes.StoreKey]), logger), nil, accountKeeper, nil, authtypes.FeeCollectorName, authority)
mintModule := mint.NewAppModule(encodingCfg.Codec, mintKeeper, accountKeeper, nil)
mintKeeper := mintkeeper.NewKeeper(encodingCfg.Codec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[minttypes.StoreKey]), logger), accountKeeper, nil, authtypes.FeeCollectorName, authority)
mintModule := mint.NewAppModule(encodingCfg.Codec, mintKeeper, accountKeeper)

// create the application and register all the modules from the previous step
integrationApp := integration.NewIntegrationApp(
Expand Down
6 changes: 3 additions & 3 deletions x/accounts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ func (a Account) AuthRetroCompatibility(ctx context.Context, _ *authtypes.QueryL

## Usage Notes

- Implement this handler only for account types you want to expose via x/auth gRPC methods.
- The `info` field in the response can be nil if your account doesn't fit the `BaseAccount` structure.
* Implement this handler only for account types you want to expose via x/auth gRPC methods.
* The `info` field in the response can be nil if your account doesn't fit the `BaseAccount` structure.

# Genesis

Expand Down Expand Up @@ -102,4 +102,4 @@ For example, given the following `genesis.json` file:
}
```

The accounts module will run the lockup account initialization message.
The accounts module will run the lockup account initialization message.
8 changes: 6 additions & 2 deletions x/mint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

### Bug Fixes

### 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.

### Bug Fixes
* [#21858](https://github.com/cosmos/cosmos-sdk/pull/21858) `NewKeeper` now returns a pointer to `Keeper`.
* [#21858](https://github.com/cosmos/cosmos-sdk/pull/21858) `DefaultMintFn` now takes `StakingKeeper` and `MintKeeper` as arguments to avoid staking keeper being required by mint.
* `SetMintFn` is used to replace the default minting function.
* `InflationCalculationFn` is not passed through depinject any longer, a MintFn is required instead.
38 changes: 16 additions & 22 deletions x/mint/depinject.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package mint

import (
"fmt"

modulev1 "cosmossdk.io/api/cosmos/mint/module/v1"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/depinject"
Expand All @@ -21,28 +23,25 @@ func (am AppModule) IsOnePerModuleType() {}
func init() {
appconfig.RegisterModule(&modulev1.Module{},
appconfig.Provide(ProvideModule),
appconfig.Invoke(InvokeSetMintFn),
)
}

type ModuleInputs struct {
depinject.In

ModuleKey depinject.OwnModuleKey
Config *modulev1.Module
Environment appmodule.Environment
Cdc codec.Codec
MintFn types.MintFn `optional:"true"`
InflationCalculationFn types.InflationCalculationFn `optional:"true"` // deprecated
Config *modulev1.Module
Environment appmodule.Environment
Cdc codec.Codec

AccountKeeper types.AccountKeeper
BankKeeper types.BankKeeper
StakingKeeper types.StakingKeeper
}

type ModuleOutputs struct {
depinject.Out

MintKeeper keeper.Keeper
MintKeeper *keeper.Keeper
Module appmodule.AppModule
EpochHooks epochstypes.EpochHooksWrapper
}
Expand All @@ -67,28 +66,23 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
k := keeper.NewKeeper(
in.Cdc,
in.Environment,
in.StakingKeeper,
in.AccountKeeper,
in.BankKeeper,
feeCollectorName,
as,
)

if in.MintFn != nil && in.InflationCalculationFn != nil {
panic("MintFn and InflationCalculationFn cannot both be set")
}
m := NewAppModule(in.Cdc, k, in.AccountKeeper)

// if no mintFn is provided, use the default minting function
if in.MintFn == nil {
// if no inflationCalculationFn is provided, use the default inflation calculation function
if in.InflationCalculationFn == nil {
in.InflationCalculationFn = types.DefaultInflationCalculationFn
}
return ModuleOutputs{MintKeeper: k, Module: m, EpochHooks: epochstypes.EpochHooksWrapper{EpochHooks: m}}
}

in.MintFn = k.DefaultMintFn(in.InflationCalculationFn)
func InvokeSetMintFn(mintKeeper *keeper.Keeper, mintFn types.MintFn, stakingKeeper types.StakingKeeper) error {
if mintFn == nil && stakingKeeper == nil {
return fmt.Errorf("custom minting function or staking keeper must be supplied or available")
} else if mintFn == nil {
mintFn = keeper.DefaultMintFn(types.DefaultInflationCalculationFn, stakingKeeper, mintKeeper)
}

m := NewAppModule(in.Cdc, k, in.AccountKeeper, in.MintFn)

return ModuleOutputs{MintKeeper: k, Module: m, EpochHooks: epochstypes.EpochHooksWrapper{EpochHooks: m}}
return mintKeeper.SetMintFn(mintFn)
}
2 changes: 1 addition & 1 deletion x/mint/epoch_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (am AppModule) BeforeEpochStart(ctx context.Context, epochIdentifier string

oldMinter := minter

err = am.mintFn(ctx, am.keeper.Environment, &minter, epochIdentifier, epochNumber)
err = am.keeper.MintFn(ctx, &minter, epochIdentifier, epochNumber)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions x/mint/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

// BeginBlocker mints new tokens for the previous block.
func (k Keeper) BeginBlocker(ctx context.Context, mintFn types.MintFn) error {
func (k Keeper) BeginBlocker(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

// fetch stored minter & params
Expand All @@ -22,7 +22,7 @@ func (k Keeper) BeginBlocker(ctx context.Context, mintFn types.MintFn) error {

// we pass -1 as epoch number to indicate that this is not an epoch minting,
// but a regular block minting. Same with epoch id "block".
err = mintFn(ctx, k.Environment, &minter, "block", -1)
err = k.MintFn(ctx, &minter, "block", -1)
if err != nil {
return err
}
Expand Down
11 changes: 8 additions & 3 deletions x/mint/keeper/genesis_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package keeper_test

import (
"context"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/suite"

"cosmossdk.io/collections"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/log"
"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
Expand All @@ -30,7 +32,7 @@ type GenesisTestSuite struct {
suite.Suite

sdkCtx sdk.Context
keeper keeper.Keeper
keeper *keeper.Keeper
cdc codec.BinaryCodec
accountKeeper types.AccountKeeper
key *storetypes.KVStoreKey
Expand All @@ -51,14 +53,17 @@ func (s *GenesisTestSuite) SetupTest() {
s.sdkCtx = testCtx.Ctx
s.key = key

stakingKeeper := minttestutil.NewMockStakingKeeper(ctrl)
accountKeeper := minttestutil.NewMockAccountKeeper(ctrl)
bankKeeper := minttestutil.NewMockBankKeeper(ctrl)
s.accountKeeper = accountKeeper
accountKeeper.EXPECT().GetModuleAddress(minterAcc.Name).Return(minterAcc.GetAddress())
accountKeeper.EXPECT().GetModuleAccount(s.sdkCtx, minterAcc.Name).Return(minterAcc)

s.keeper = keeper.NewKeeper(s.cdc, runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()), stakingKeeper, accountKeeper, bankKeeper, "", "")
s.keeper = keeper.NewKeeper(s.cdc, runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()), accountKeeper, bankKeeper, "", "")
err := s.keeper.SetMintFn(func(ctx context.Context, env appmodule.Environment, minter *types.Minter, epochId string, epochNumber int64) error {
return nil
})
s.NoError(err)
}

func (s *GenesisTestSuite) TestImportExportGenesis() {
Expand Down
4 changes: 2 additions & 2 deletions x/mint/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import (

var _ types.QueryServer = queryServer{}

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

type queryServer struct {
k Keeper
k *Keeper
}

// Params returns params of the mint module.
Expand Down
4 changes: 1 addition & 3 deletions x/mint/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type MintTestSuite struct {

ctx sdk.Context
queryClient types.QueryClient
mintKeeper keeper.Keeper
mintKeeper *keeper.Keeper
}

func (suite *MintTestSuite) SetupTest() {
Expand All @@ -42,14 +42,12 @@ func (suite *MintTestSuite) SetupTest() {
ctrl := gomock.NewController(suite.T())
accountKeeper := minttestutil.NewMockAccountKeeper(ctrl)
bankKeeper := minttestutil.NewMockBankKeeper(ctrl)
stakingKeeper := minttestutil.NewMockStakingKeeper(ctrl)

accountKeeper.EXPECT().GetModuleAddress("mint").Return(sdk.AccAddress{})

suite.mintKeeper = keeper.NewKeeper(
encCfg.Codec,
env,
stakingKeeper,
accountKeeper,
bankKeeper,
authtypes.FeeCollectorName,
Expand Down
47 changes: 25 additions & 22 deletions x/mint/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type Keeper struct {
appmodule.Environment

cdc codec.BinaryCodec
stakingKeeper types.StakingKeeper
bankKeeper types.BankKeeper
feeCollectorName string
// the address capable of executing a MsgUpdateParams message. Typically, this
Expand All @@ -30,18 +29,21 @@ type Keeper struct {
Schema collections.Schema
Params collections.Item[types.Params]
Minter collections.Item[types.Minter]

// mintFn is used to mint new coins during BeginBlock. This function is in charge of
// minting new coins based on arbitrary logic, previously done through InflationCalculationFn.
mintFn types.MintFn
}

// NewKeeper creates a new mint Keeper instance
func NewKeeper(
cdc codec.BinaryCodec,
env appmodule.Environment,
sk types.StakingKeeper,
ak types.AccountKeeper,
bk types.BankKeeper,
feeCollectorName string,
authority string,
) Keeper {
) *Keeper {
// ensure mint module account is set
if addr := ak.GetModuleAddress(types.ModuleName); addr == nil {
panic(fmt.Sprintf("the x/%s module account has not been set", types.ModuleName))
Expand All @@ -51,7 +53,6 @@ func NewKeeper(
k := Keeper{
Environment: env,
cdc: cdc,
stakingKeeper: sk,
bankKeeper: bk,
feeCollectorName: feeCollectorName,
authority: authority,
Expand All @@ -64,29 +65,25 @@ func NewKeeper(
panic(err)
}
k.Schema = schema
return k
}

// GetAuthority returns the x/mint module's authority.
func (k Keeper) GetAuthority() string {
return k.authority
return &k
}

// 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, error) {
return k.stakingKeeper.StakingTokenSupply(ctx)
// SetMintFn is used to mint new coins during BeginBlock. The mintFn function is in charge of
// minting new coins based on arbitrary logic, previously done through InflationCalculationFn.
func (k *Keeper) SetMintFn(mintFn types.MintFn) error {
k.mintFn = mintFn
return nil
}

// BondedRatio implements an alias call to the underlying staking keeper's
// BondedRatio to be used in BeginBlocker.
func (k Keeper) BondedRatio(ctx context.Context) (math.LegacyDec, error) {
return k.stakingKeeper.BondedRatio(ctx)
// GetAuthority returns the x/mint module's authority.
func (k *Keeper) GetAuthority() string {
return k.authority
}

// MintCoins implements an alias call to the underlying supply keeper's
// MintCoins to be used in BeginBlocker.
func (k Keeper) MintCoins(ctx context.Context, newCoins sdk.Coins) error {
func (k *Keeper) MintCoins(ctx context.Context, newCoins sdk.Coins) error {
if newCoins.Empty() {
// skip as no coins need to be minted
return nil
Expand All @@ -97,24 +94,30 @@ func (k Keeper) MintCoins(ctx context.Context, newCoins sdk.Coins) error {

// AddCollectedFees implements an alias call to the underlying supply keeper's
// AddCollectedFees to be used in BeginBlocker.
func (k Keeper) AddCollectedFees(ctx context.Context, fees sdk.Coins) error {
func (k *Keeper) AddCollectedFees(ctx context.Context, fees sdk.Coins) error {
return k.bankKeeper.SendCoinsFromModuleToModule(ctx, types.ModuleName, k.feeCollectorName, fees)
}

func (k Keeper) DefaultMintFn(ic types.InflationCalculationFn) types.MintFn {
func (k *Keeper) MintFn(ctx context.Context, minter *types.Minter, epochId string, epochNumber int64) error {
return k.mintFn(ctx, k.Environment, minter, epochId, epochNumber)
}

// DefaultMintFn returns a default mint function. It requires the Staking module and the mint keeper.
// The default Mintfn has a requirement on staking as it uses bond to calculate inflation.
func DefaultMintFn(ic types.InflationCalculationFn, staking types.StakingKeeper, k *Keeper) types.MintFn {
return func(ctx context.Context, env appmodule.Environment, minter *types.Minter, epochId string, epochNumber int64) error {
// the default mint function is called every block, so we only check if epochId is "block" which is
// a special value to indicate that this is not an epoch minting, but a regular block minting.
if epochId != "block" {
return nil
}

stakingTokenSupply, err := k.StakingTokenSupply(ctx)
stakingTokenSupply, err := staking.StakingTokenSupply(ctx)
if err != nil {
return err
}

bondedRatio, err := k.BondedRatio(ctx)
bondedRatio, err := staking.BondedRatio(ctx)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit f6d7a92

Please sign in to comment.