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

(cherry picked from commit 1313b63)

# Conflicts:
#	CHANGELOG.md
#	app/keepers/keepers.go
#	x/incentives/keeper/gauge.go
#	x/incentives/keeper/keeper.go
#	x/txfees/types/expected_keepers.go
  • Loading branch information
p0mvn authored and mergify[bot] committed Aug 13, 2022
1 parent d354ba1 commit 26e0c32
Show file tree
Hide file tree
Showing 20 changed files with 104 additions and 102 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,24 @@ This release contains minor CLI bug fixes.
* [#1671](https://github.com/osmosis-labs/osmosis/pull/1671) Add hourly epochs to `x/epochs` DefaultGenesis.
* [#1665](https://github.com/osmosis-labs/osmosis/pull/1665) Delete app/App interface, instead use simapp.App
* [#1630](https://github.com/osmosis-labs/osmosis/pull/1630) Delete the v043_temp module, now that we're on an updated SDK version.
<<<<<<< HEAD
=======
* [#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

* [#2387](https://github.com/osmosis-labs/osmosis/pull/2387) Upgrade to IBC v3.2.0, which allows for sending/receiving IBC tokens with slashes.
* [#2057](https://github.com/osmosis-labs/osmosis/pull/2057) Reduce block times to about three seconds
* [#1312] Stableswap: Createpool logic
* [#1230] Stableswap CFMM equations
* [#1429] solver for multi-asset CFMM
* [#1539] Superfluid: Combine superfluid and staking query on querying delegation by delegator
* [#2223] Tokenfactory: Add SetMetadata functionality
>>>>>>> 1313b635 (refactor: clean up keeper interfaces (#2394))
### Bug Fixes

Expand Down
15 changes: 15 additions & 0 deletions app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,21 @@ func (appKeepers *AppKeepers) InitNormalKeepers(

appKeepers.EpochsKeeper = epochskeeper.NewKeeper(appCodec, appKeepers.keys[epochstypes.StoreKey])

<<<<<<< HEAD
=======
txFeesKeeper := txfeeskeeper.NewKeeper(
appCodec,
appKeepers.AccountKeeper,
appKeepers.BankKeeper,
appKeepers.keys[txfeestypes.StoreKey],
appKeepers.GAMMKeeper,
appKeepers.GAMMKeeper,
txfeestypes.FeeCollectorName,
txfeestypes.NonNativeFeeCollectorName,
)
appKeepers.TxFeesKeeper = &txFeesKeeper

>>>>>>> 1313b635 (refactor: clean up keeper interfaces (#2394))
appKeepers.IncentivesKeeper = incentiveskeeper.NewKeeper(
appCodec,
appKeepers.keys[incentivestypes.StoreKey],
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
}
5 changes: 5 additions & 0 deletions x/incentives/keeper/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,12 @@ 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)
}
<<<<<<< HEAD
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(feeDenom, fee)), address); err != nil {
>>>>>>> 1313b635 (refactor: clean up keeper interfaces (#2394))
return err
}
return nil
Expand Down
14 changes: 14 additions & 0 deletions x/incentives/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,20 @@ type Keeper struct {
bk types.BankKeeper
lk types.LockupKeeper
ek types.EpochKeeper
<<<<<<< HEAD
dk types.DistrKeeper
}

// 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 {
=======
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, ck types.CommunityPoolKeeper, txfk types.TxFeesKeeper) *Keeper {
>>>>>>> 1313b635 (refactor: clean up keeper interfaces (#2394))
if !paramSpace.HasKeyTable() {
paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
}
Expand All @@ -37,7 +46,12 @@ func NewKeeper(cdc codec.Codec, storeKey sdk.StoreKey, paramSpace paramtypes.Sub
bk: bk,
lk: lk,
ek: ek,
<<<<<<< HEAD
dk: dk,
=======
ck: ck,
tk: txfk,
>>>>>>> 1313b635 (refactor: clean up keeper interfaces (#2394))
}
}

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 @@ -37,7 +33,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
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,16 +14,12 @@ 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
}

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
}

type GAMMKeeper interface {
Expand All @@ -39,6 +35,5 @@ type IncentivesKeeper interface {
}

type DistrKeeper 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 26e0c32

Please sign in to comment.