Skip to content

Commit

Permalink
refactor: clean up keeper interfaces (backport #2394) (#2399)
Browse files Browse the repository at this point in the history
Co-authored-by: Roman <[email protected]>
  • Loading branch information
mergify[bot] and p0mvn authored Aug 13, 2022
1 parent d354ba1 commit 162cfd4
Show file tree
Hide file tree
Showing 22 changed files with 66 additions and 117 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* [#1893](https://github.com/osmosis-labs/osmosis/pull/1893) Change `EpochsKeeper.SetEpochInfo` to `AddEpochInfo`, which has more safety checks with it. (Makes it suitable to be called within upgrades)
* [#2396](https://github.com/osmosis-labs/osmosis/pull/2396) x/mint remove unused mintCoins parameter from AfterDistributeMintedCoin
* [#2399](https://github.com/osmosis-labs/osmosis/pull/2399) Remove unused interface methods from expected keepers of each module

#### Bug Fixes
* [2291](https://github.com/osmosis-labs/osmosis/pull/2291) Remove liquidity event that was emitted twice per message.
Expand Down
1 change: 0 additions & 1 deletion app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,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 @@ -295,7 +295,7 @@ func (k Keeper) chargeFeeIfSufficientFeeDenomBalance(ctx sdk.Context, address sd
if accountBalance.LT(totalCost) {
return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "account's balance of %s (%s) is less than the total cost of the message (%s)", appparams.BaseCoinUnit, accountBalance, totalCost)
}
if err := k.dk.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, fee)), address); err != nil {
if err := k.ck.FundCommunityPool(ctx, sdk.NewCoins(sdk.NewCoin(appparams.BaseCoinUnit, 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 @@ -21,11 +21,11 @@ type Keeper struct {
bk types.BankKeeper
lk types.LockupKeeper
ek types.EpochKeeper
dk types.DistrKeeper
ck types.CommunityPoolKeeper
}

// 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) *Keeper {
func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, bk types.BankKeeper, lk types.LockupKeeper, ek types.EpochKeeper, ck types.CommunityPoolKeeper) *Keeper {
if !paramSpace.HasKeyTable() {
paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
}
Expand All @@ -37,7 +37,7 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub
bk: bk,
lk: lk,
ek: ek,
dk: dk,
ck: ck,
}
}

Expand Down
7 changes: 2 additions & 5 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,7 +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
Expand All @@ -37,7 +34,7 @@ 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
}
6 changes: 3 additions & 3 deletions x/lockup/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ 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{
cdc: cdc,
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 @@ -552,7 +552,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 @@ -28,8 +28,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
4 changes: 2 additions & 2 deletions x/pool-incentives/keeper/distr.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ func (k Keeper) FundCommunityPoolFromModule(ctx sdk.Context, asset sdk.Coin) err
return err
}

feePool := k.distrKeeper.GetFeePool(ctx)
feePool := k.communityPoolKeeper.GetFeePool(ctx)
feePool.CommunityPool = feePool.CommunityPool.Add(sdk.NewDecCoinsFromCoins(asset)...)
k.distrKeeper.SetFeePool(ctx, feePool)
k.communityPoolKeeper.SetFeePool(ctx, feePool)
return nil
}

Expand Down
18 changes: 9 additions & 9 deletions x/pool-incentives/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ type Keeper struct {

paramSpace paramtypes.Subspace

accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
incentivesKeeper types.IncentivesKeeper
distrKeeper types.DistrKeeper
accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
incentivesKeeper types.IncentivesKeeper
communityPoolKeeper types.CommunityPoolKeeper

communityPoolName string // name of the Community pool ModuleAccount (Maybe the distribution module)
}

func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, incentivesKeeper types.IncentivesKeeper, distrKeeper types.DistrKeeper, communityPoolName string) Keeper {
func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, incentivesKeeper types.IncentivesKeeper, distrKeeper types.CommunityPoolKeeper, communityPoolName string) Keeper {
// ensure pool-incentives module account is set
if addr := accountKeeper.GetModuleAddress(types.ModuleName); addr == nil {
panic(fmt.Sprintf("%s module account has not been set", types.ModuleName))
Expand All @@ -48,10 +48,10 @@ func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, paramSpace paramtyp

paramSpace: paramSpace,

accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
incentivesKeeper: incentivesKeeper,
distrKeeper: distrKeeper,
accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
incentivesKeeper: incentivesKeeper,
communityPoolKeeper: distrKeeper,

communityPoolName: communityPoolName,
}
Expand Down
4 changes: 1 addition & 3 deletions x/pool-incentives/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
)

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
}
Expand All @@ -38,7 +36,7 @@ type IncentivesKeeper interface {
AddToGaugeRewards(ctx sdk.Context, owner sdk.AccAddress, coins sdk.Coins, gaugeID uint64) error
}

type DistrKeeper interface {
type CommunityPoolKeeper interface {
GetFeePool(ctx sdk.Context) (feePool distrtypes.FeePool)
SetFeePool(ctx sdk.Context, feePool distrtypes.FeePool)
}
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
Loading

0 comments on commit 162cfd4

Please sign in to comment.