Skip to content

Commit

Permalink
refactor: clean up keeper interfaces (#2394)
Browse files Browse the repository at this point in the history
* refactor: clean up keeper  interfaces

* rename community pool keeper

* changelog

* restore TxFeesKeeper and type check
  • Loading branch information
p0mvn authored Aug 13, 2022
1 parent e41d3e7 commit 1313b63
Show file tree
Hide file tree
Showing 20 changed files with 54 additions and 109 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#1667](https://github.com/osmosis-labs/osmosis/pull/1673) Move wasm-bindings code out of app package into its own root level package.
* [#2013](https://github.com/osmosis-labs/osmosis/pull/2013) Make `SetParams`, `SetPool`, `SetTotalLiquidity`, and `SetDenomLiquidity` GAMM APIs private
* [#1857](https://github.com/osmosis-labs/osmosis/pull/1857) x/mint rename GetLastHalvenEpochNum to GetLastReductionEpochNum
* [#2394](https://github.com/osmosis-labs/osmosis/pull/2394) Remove unused interface methods from expected keepers of each module
* [#2390](https://github.com/osmosis-labs/osmosis/pull/2390) x/mint remove unused mintCoins parameter from AfterDistributeMintedCoin

### Features
Expand Down
1 change: 0 additions & 1 deletion app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
appCodec,
appKeepers.AccountKeeper,
appKeepers.BankKeeper,
appKeepers.EpochsKeeper,
appKeepers.keys[txfeestypes.StoreKey],
appKeepers.GAMMKeeper,
appKeepers.GAMMKeeper,
Expand Down
14 changes: 7 additions & 7 deletions x/gamm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ type Keeper struct {
hooks types.GammHooks

// keepers
accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
distrKeeper types.DistrKeeper
accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
communityPoolKeeper types.CommunityPoolKeeper
}

func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, distrKeeper types.DistrKeeper) Keeper {
func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, communityPoolKeeper types.CommunityPoolKeeper) Keeper {
// Ensure that the module account are set.
moduleAddr, perms := accountKeeper.GetModuleAddressAndPermissions(types.ModuleName)
if moduleAddr == nil {
Expand All @@ -54,9 +54,9 @@ func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, paramSpace paramtyp
cdc: cdc,
paramSpace: paramSpace,
// keepers
accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
distrKeeper: distrKeeper,
accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
communityPoolKeeper: communityPoolKeeper,
}
}

Expand Down
2 changes: 1 addition & 1 deletion x/gamm/keeper/pool_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (k Keeper) CreatePool(ctx sdk.Context, msg types.CreatePoolMsg) (uint64, er

// send pool creation fee to community pool
params := k.GetParams(ctx)
if err := k.distrKeeper.FundCommunityPool(ctx, params.PoolCreationFee, sender); err != nil {
if err := k.communityPoolKeeper.FundCommunityPool(ctx, params.PoolCreationFee, sender); err != nil {
return 0, err
}

Expand Down
26 changes: 3 additions & 23 deletions x/gamm/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,18 @@ import (
// creating a x/gamm keeper.
type AccountKeeper interface {
NewAccount(sdk.Context, authtypes.AccountI) authtypes.AccountI
NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI

GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI
GetAllAccounts(ctx sdk.Context) []authtypes.AccountI
SetAccount(ctx sdk.Context, acc authtypes.AccountI)

IterateAccounts(ctx sdk.Context, process func(authtypes.AccountI) bool)

ValidatePermissions(macc authtypes.ModuleAccountI) error

GetModuleAddress(moduleName string) sdk.AccAddress
GetModuleAddressAndPermissions(moduleName string) (addr sdk.AccAddress, permissions []string)
GetModuleAccountAndPermissions(ctx sdk.Context, moduleName string) (authtypes.ModuleAccountI, []string)
GetModuleAccount(ctx sdk.Context, moduleName string) authtypes.ModuleAccountI
SetModuleAccount(ctx sdk.Context, macc authtypes.ModuleAccountI)

UnmarshalAccount(bz []byte) (authtypes.AccountI, error)
}

// BankKeeper defines the banking contract that must be fulfilled when
// creating a x/gamm keeper.
type BankKeeper interface {
SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsFromModuleToModule(ctx sdk.Context, senderPool, recipientPool string, amt sdk.Coins) error

SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error

SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
Expand All @@ -47,17 +35,9 @@ type BankKeeper interface {
// TODO: Look into golang syntax to make this "Everything in stakingtypes.bankkeeper + extra funcs"
// I think it has to do with listing another interface as the first line here?
GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin
// SetBalances(ctx sdk.Context, addr sdk.AccAddress, balances sdk.Coins) error
LockedCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
GetSupply(ctx sdk.Context, denom string) sdk.Coin
UndelegateCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
DelegateCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
IterateAllBalances(ctx sdk.Context, callback func(addr sdk.AccAddress, coin sdk.Coin) (stop bool))
}

// DistrKeeper defines the contract needed to be fulfilled for distribution keeper.
type DistrKeeper interface {
// CommunityPoolKeeper defines the contract needed to be fulfilled for distribution keeper.
type CommunityPoolKeeper interface {
FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
}
2 changes: 1 addition & 1 deletion x/incentives/keeper/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sd
return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "account's balance of %s (%s) is less than the total cost of the message (%s)", feeDenom, accountBalance, totalCost)
}

if err := k.dk.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(feeDenom, fee)), address); err != nil {
if err := k.ck.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(feeDenom, fee)), address); err != nil {
return err
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions x/incentives/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ type Keeper struct {
bk types.BankKeeper
lk types.LockupKeeper
ek types.EpochKeeper
dk types.DistrKeeper
ck types.CommunityPoolKeeper
tk types.TxFeesKeeper
}

// NewKeeper returns a new instance of the incentive module keeper struct.
func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, dk types.DistrKeeper, txfk types.TxFeesKeeper) *Keeper {
func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, ck types.CommunityPoolKeeper, txfk types.TxFeesKeeper) *Keeper {
if !paramSpace.HasKeyTable() {
paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
}
Expand All @@ -39,7 +39,7 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub
bk: bk,
lk: lk,
ek: ek,
dk: dk,
ck: ck,
tk: txfk,
}
}
Expand Down
8 changes: 2 additions & 6 deletions x/incentives/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@ import (

// BankKeeper defines the expected interface needed to retrieve account balances.
type BankKeeper interface {
GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin

HasSupply(ctx sdk.Context, denom string) bool

SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsFromModuleToManyAccounts(
ctx sdk.Context, senderModule string, recipientAddrs []sdk.AccAddress, amts []sdk.Coins,
) error
Expand All @@ -25,8 +23,6 @@ type BankKeeper interface {

// LockupKeeper defines the expected interface needed to retrieve locks.
type LockupKeeper interface {
GetSyntheticLockup(ctx sdk.Context, lockID uint64, suffix string) (*lockuptypes.SyntheticLock, error)
GetLocksPastTimeDenom(ctx sdk.Context, denom string, timestamp time.Time) []lockuptypes.PeriodLock
GetLocksLongerThanDurationDenom(ctx sdk.Context, denom string, duration time.Duration) []lockuptypes.PeriodLock
GetPeriodLocksAccumulation(ctx sdk.Context, query lockuptypes.QueryCondition) sdk.Int
GetAccountPeriodLocks(ctx sdk.Context, addr sdk.AccAddress) []lockuptypes.PeriodLock
Expand All @@ -38,8 +34,8 @@ type EpochKeeper interface {
GetEpochInfo(ctx sdk.Context, identifier string) epochstypes.EpochInfo
}

// DistrKeeper defines the contract needed to be fulfilled for distribution keeper.
type DistrKeeper interface {
// CommunityPoolKeeper defines the contract needed to be fulfilled for distribution keeper.
type CommunityPoolKeeper interface {
FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
}

Expand Down
6 changes: 3 additions & 3 deletions x/lockup/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ type Keeper struct {

ak types.AccountKeeper
bk types.BankKeeper
dk types.DistrKeeper
ck types.CommunityPoolKeeper
}

// NewKeeper returns an instance of Keeper.
func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, ak types.AccountKeeper, bk types.BankKeeper, dk types.DistrKeeper) *Keeper {
func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, ak types.AccountKeeper, bk types.BankKeeper, ck types.CommunityPoolKeeper) *Keeper {
return &Keeper{
storeKey: storeKey,
ak: ak,
bk: bk,
dk: dk,
ck: ck,
}
}

Expand Down
2 changes: 1 addition & 1 deletion x/lockup/keeper/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ func (k Keeper) SlashTokensFromLockByID(ctx sdk.Context, lockID uint64, coins sd
}

modAddr := k.ak.GetModuleAddress(types.ModuleName)
err = k.dk.FundCommunityPool(ctx, coins, modAddr)
err = k.ck.FundCommunityPool(ctx, coins, modAddr)
if err != nil {
return nil, err
}
Expand Down
6 changes: 1 addition & 5 deletions x/lockup/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,11 @@ type AccountKeeper interface {
// BankKeeper defines the expected interface needed to retrieve account balances.
type BankKeeper interface {
GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin
LockedCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins

SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
BurnCoins(ctx sdk.Context, name string, amt sdk.Coins) error
}

type DistrKeeper interface {
type CommunityPoolKeeper interface {
FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
}
42 changes: 21 additions & 21 deletions x/mint/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ import (

// Keeper of the mint store.
type Keeper struct {
cdc codec.BinaryCodec
storeKey sdk.StoreKey
paramSpace paramtypes.Subspace
accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
distrKeeper types.DistrKeeper
epochKeeper types.EpochKeeper
hooks types.MintHooks
feeCollectorName string
cdc codec.BinaryCodec
storeKey sdk.StoreKey
paramSpace paramtypes.Subspace
accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
communityPoolKeeper types.CommunityPoolKeeper
epochKeeper types.EpochKeeper
hooks types.MintHooks
feeCollectorName string
}

type invalidRatioError struct {
Expand All @@ -50,7 +50,7 @@ const emptyWeightedAddressReceiver = ""
// NewKeeper creates a new mint Keeper instance.
func NewKeeper(
cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace,
ak types.AccountKeeper, bk types.BankKeeper, dk types.DistrKeeper, epochKeeper types.EpochKeeper,
ak types.AccountKeeper, bk types.BankKeeper, ck types.CommunityPoolKeeper, epochKeeper types.EpochKeeper,
feeCollectorName string,
) Keeper {
// ensure mint module account is set
Expand All @@ -64,14 +64,14 @@ func NewKeeper(
}

return Keeper{
cdc: cdc,
storeKey: key,
paramSpace: paramSpace,
accountKeeper: ak,
bankKeeper: bk,
distrKeeper: dk,
epochKeeper: epochKeeper,
feeCollectorName: feeCollectorName,
cdc: cdc,
storeKey: key,
paramSpace: paramSpace,
accountKeeper: ak,
bankKeeper: bk,
communityPoolKeeper: ck,
epochKeeper: epochKeeper,
feeCollectorName: feeCollectorName,
}
}

Expand Down Expand Up @@ -223,7 +223,7 @@ func (k Keeper) DistributeMintedCoin(ctx sdk.Context, mintedCoin sdk.Coin) error

// subtract from original provision to ensure no coins left over after the allocations
communityPoolAmount := mintedCoin.Amount.Sub(stakingIncentivesAmount).Sub(poolIncentivesAmount).Sub(devRewardAmount)
err = k.distrKeeper.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(params.MintDenom, communityPoolAmount)), k.accountKeeper.GetModuleAddress(types.ModuleName))
err = k.communityPoolKeeper.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(params.MintDenom, communityPoolAmount)), k.accountKeeper.GetModuleAddress(types.ModuleName))
if err != nil {
return err
}
Expand Down Expand Up @@ -287,7 +287,7 @@ func (k Keeper) distributeDeveloperRewards(ctx sdk.Context, totalMintedCoin sdk.
// If no developer rewards receivers provided, fund the community pool from
// the developer vesting module account.
if len(developerRewardsReceivers) == 0 {
err = k.distrKeeper.FundCommunityPool(ctx, devRewardCoins, developerRewardsModuleAccountAddress)
err = k.communityPoolKeeper.FundCommunityPool(ctx, devRewardCoins, developerRewardsModuleAccountAddress)
if err != nil {
return sdk.Int{}, err
}
Expand All @@ -301,7 +301,7 @@ func (k Keeper) distributeDeveloperRewards(ctx sdk.Context, totalMintedCoin sdk.
devRewardPortionCoins := sdk.NewCoins(devPortionCoin)
// fund community pool when rewards address is empty.
if w.Address == emptyWeightedAddressReceiver {
err := k.distrKeeper.FundCommunityPool(ctx, devRewardPortionCoins,
err := k.communityPoolKeeper.FundCommunityPool(ctx, devRewardPortionCoins,
k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName))
if err != nil {
return sdk.Int{}, err
Expand Down
4 changes: 2 additions & 2 deletions x/mint/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ type BankKeeper interface {
AddSupplyOffset(ctx sdk.Context, denom string, offsetAmount sdk.Int)
}

// DistrKeeper defines the contract needed to be fulfilled for distribution keeper.
type DistrKeeper interface {
// CommunityPoolKeeper defines the contract needed to be fulfilled for distribution keeper.
type CommunityPoolKeeper interface {
FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
}

Expand Down
5 changes: 0 additions & 5 deletions x/pool-incentives/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,13 @@ import (

// AccountKeeper interface contains functions for getting accounts and the module address
type AccountKeeper interface {
GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI

GetModuleAddress(name string) sdk.AccAddress
GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI
}

// BankKeeper sends tokens across modules and is able to get account balances.
type BankKeeper interface {
GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin

SendCoinsFromModuleToModule(ctx sdk.Context, senderModule string, recipientModule string, amt sdk.Coins) error
}

// GAMMKeeper gets the pool interface from poolID.
Expand All @@ -43,7 +39,6 @@ type IncentivesKeeper interface {

// DistrKeeper handles pool-fees functionality - setting / getting fees and funding the community pool.
type DistrKeeper interface {
GetFeePool(ctx sdk.Context) (feePool distrtypes.FeePool)
SetFeePool(ctx sdk.Context, feePool distrtypes.FeePool)
FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
}
2 changes: 1 addition & 1 deletion x/superfluid/keeper/epoch.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (k Keeper) MoveSuperfluidDelegationRewardToGauges(ctx sdk.Context) {
// To avoid unexpected issues on WithdrawDelegationRewards and AddToGaugeRewards
// we use cacheCtx and apply the changes later
_ = osmoutils.ApplyFuncIfNoError(ctx, func(cacheCtx sdk.Context) error {
_, err := k.dk.WithdrawDelegationRewards(cacheCtx, addr, valAddr)
_, err := k.ck.WithdrawDelegationRewards(cacheCtx, addr, valAddr)
if errors.Is(err, distributiontypes.ErrEmptyDelegationDistInfo) {
ctx.Logger().Debug("no swaps occurred in this pool between last epoch and this epoch")
return nil
Expand Down
6 changes: 3 additions & 3 deletions x/superfluid/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Keeper struct {
ak authkeeper.AccountKeeper
bk types.BankKeeper
sk types.StakingKeeper
dk types.DistrKeeper
ck types.CommunityPoolKeeper
ek types.EpochKeeper
lk types.LockupKeeper
gk types.GammKeeper
Expand All @@ -35,7 +35,7 @@ type Keeper struct {
var _ govtypes.StakingKeeper = (*Keeper)(nil)

// NewKeeper returns an instance of Keeper.
func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, ak authkeeper.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, dk types.DistrKeeper, ek types.EpochKeeper, lk types.LockupKeeper, gk types.GammKeeper, ik types.IncentivesKeeper, lms types.LockupMsgServer) *Keeper {
func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, ak authkeeper.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, dk types.CommunityPoolKeeper, ek types.EpochKeeper, lk types.LockupKeeper, gk types.GammKeeper, ik types.IncentivesKeeper, lms types.LockupMsgServer) *Keeper {
// set KeyTable if it has not already been set
if !paramSpace.HasKeyTable() {
paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
Expand All @@ -48,7 +48,7 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub
ak: ak,
bk: bk,
sk: sk,
dk: dk,
ck: dk,
ek: ek,
lk: lk,
gk: gk,
Expand Down
Loading

0 comments on commit 1313b63

Please sign in to comment.