From 98eed0ba2dc0bfe84db5af32b34348a65e5f85e4 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 09:10:45 -0700 Subject: [PATCH 01/75] migrate deductfeesdecorator to txfees module & set up second module account, fee routing logic, epoch hooks etc. --- app/ante.go | 4 +- app/keepers.go | 4 + .../cmd/balances_from_state_export.go | 38 ++++- x/txfees/keeper/feedecorator.go | 141 ++++++++++++++++++ x/txfees/keeper/hooks.go | 53 +++++++ x/txfees/keeper/keeper.go | 17 +++ x/txfees/types/expected_keepers.go | 33 ++++ x/txfees/types/keys.go | 21 +++ 8 files changed, 304 insertions(+), 7 deletions(-) create mode 100644 x/txfees/keeper/hooks.go diff --git a/app/ante.go b/app/ante.go index 45ace867c52..41c2ec743d9 100644 --- a/app/ante.go +++ b/app/ante.go @@ -31,6 +31,7 @@ func NewAnteHandler( ) sdk.AnteHandler { mempoolFeeOptions := txfeestypes.NewMempoolFeeOptions(appOpts) mempoolFeeDecorator := txfeeskeeper.NewMempoolFeeDecorator(*txFeesKeeper, mempoolFeeOptions) + deductFeeDecorator := txfeeskeeper.NewDeductFeeDecorator(*txFeesKeeper, ak, bankKeeper, nil) return sdk.ChainAnteDecorators( ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first wasmkeeper.NewLimitSimulationGasDecorator(wasmConfig.SimulationGasLimit), @@ -43,7 +44,8 @@ func NewAnteHandler( ante.TxTimeoutHeightDecorator{}, ante.NewValidateMemoDecorator(ak), ante.NewConsumeGasForTxSizeDecorator(ak), - ante.NewDeductFeeDecorator(ak, bankKeeper, nil), + // Replaced with version from our txfees module from auth (previously "ante.NewDeductFeeDecorator(ak, bankKeeper, nil)")" + deductFeeDecorator, ante.NewSetPubKeyDecorator(ak), // SetPubKeyDecorator must be called before all signature verification decorators ante.NewValidateSigCountDecorator(ak), ante.NewSigGasConsumeDecorator(ak, sigGasConsumer), diff --git a/app/keepers.go b/app/keepers.go index 3f539d7e4c7..df5ba0c830f 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -345,6 +345,8 @@ func (app *OsmosisApp) InitNormalKeepers( ) app.MintKeeper = &mintKeeper + + poolIncentivesKeeper := poolincentiveskeeper.NewKeeper( appCodec, keys[poolincentivestypes.StoreKey], @@ -358,6 +360,7 @@ func (app *OsmosisApp) InitNormalKeepers( ) app.PoolIncentivesKeeper = &poolIncentivesKeeper + // TO DO: update txfees keeper here (very similar to mintkeeper above) txFeesKeeper := txfeeskeeper.NewKeeper( appCodec, keys[txfeestypes.StoreKey], @@ -465,6 +468,7 @@ func (app *OsmosisApp) SetupHooks() { app.EpochsKeeper.SetHooks( epochstypes.NewMultiEpochHooks( // insert epoch hooks receivers here + // TO DO: add TxFeesKeeper.Hooks() for auto fee swaps app.SuperfluidKeeper.Hooks(), app.IncentivesKeeper.Hooks(), app.MintKeeper.Hooks(), diff --git a/cmd/osmosisd/cmd/balances_from_state_export.go b/cmd/osmosisd/cmd/balances_from_state_export.go index 13dcd477971..16a697914fb 100644 --- a/cmd/osmosisd/cmd/balances_from_state_export.go +++ b/cmd/osmosisd/cmd/balances_from_state_export.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" "os" + "time" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/server" @@ -12,13 +13,15 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - appparams "github.com/osmosis-labs/osmosis/v7/app/params" - "github.com/osmosis-labs/osmosis/v7/osmoutils" - claimtypes "github.com/osmosis-labs/osmosis/v7/x/claim/types" - gammtypes "github.com/osmosis-labs/osmosis/v7/x/gamm/types" - lockuptypes "github.com/osmosis-labs/osmosis/v7/x/lockup/types" + "github.com/osmosis-labs/osmosis/app" + appparams "github.com/osmosis-labs/osmosis/app/params" + "github.com/osmosis-labs/osmosis/osmotestutils" + claimtypes "github.com/osmosis-labs/osmosis/x/claim/types" + gammtypes "github.com/osmosis-labs/osmosis/x/gamm/types" + lockuptypes "github.com/osmosis-labs/osmosis/x/lockup/types" "github.com/spf13/cobra" tmjson "github.com/tendermint/tendermint/libs/json" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" tmtypes "github.com/tendermint/tendermint/types" ) @@ -170,7 +173,7 @@ Example: } selectBondedPoolIDs := []uint64{} if selectPoolIdsStr != "" { - selectBondedPoolIDs, err = osmoutils.ParseUint64SliceFromString(selectPoolIdsStr, ",") + selectBondedPoolIDs, err = osmotestutils.ParseUint64SliceFromString(selectPoolIdsStr, ",") if err != nil { return err } @@ -315,6 +318,29 @@ Example: snapshotAccs[addr] = account } + app := app.Setup(false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: 1, Time: time.Now().UTC()}) + + // scaffolding for map creation: + // accountsMap := make(map[string]authtypes.GenesisAccount) + + // Remove module accounts from snapshots + for addr, account := range snapshotAccs { + + // TO DO: change the addr -> account function to a map to genesis accounts using the sanitized gen accs generated above + accAddr, err := sdk.AccAddressFromBech32(account.Address) + if err != nil { + return err + } + acc := app.AccountKeeper.GetAccount(ctx, accAddr) + + if _, ok := acc.(*authtypes.ModuleAccount); ok { + //Else, remove account from list - how do we remove an item from a map in go? + //ASSUMING THAT "addr" IS THE KEY + delete(snapshotAccs, addr) + } + } + snapshot := DeriveSnapshot{ NumberAccounts: uint64(len(snapshotAccs)), Accounts: snapshotAccs, diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 516a0c8c130..d6c756b847f 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -1,11 +1,15 @@ package keeper import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper/txfee_filters" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" + + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // MempoolFeeDecorator will check if the transaction's fee is at least as large @@ -119,3 +123,140 @@ func (mfd MempoolFeeDecorator) GetMinBaseGasPriceForTx(ctx sdk.Context, baseDeno } return cfgMinGasPrice } + +// DeductFeeDecorator deducts fees from the first signer of the tx +// If the first signer does not have the funds to pay for the fees, return with InsufficientFunds error +// Call next AnteHandler if fees successfully deducted +// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator +type DeductFeeDecorator struct { + ak types.AccountKeeper + bankKeeper types.BankKeeper + feegrantKeeper types.FeegrantKeeper + txFeesKeeper Keeper +} + +func NewDeductFeeDecorator(tk Keeper, ak types.AccountKeeper, bk types.BankKeeper, fk types.FeegrantKeeper) DeductFeeDecorator { + return DeductFeeDecorator{ + ak: ak, + bankKeeper: bk, + feegrantKeeper: fk, + txFeesKeeper: tk, + } +} + +func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + + // checks to make sure feeTx is of the right type + feeTx, ok := tx.(sdk.FeeTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + // checks to make sure the module account has been set to collect fees in OSMO + // TO DO: add a second module account to send non-OSMO txn fees to for auto fees swaps + // Note: if we do add a second module account, we might need to update the baseapp account keeper + // since that's what's passed into this function in ante.go + if addr := dfd.ak.GetModuleAddress(types.FeeCollectorName); addr == nil { + return ctx, fmt.Errorf("Fee collector module account (%s) has not been set", types.FeeCollectorName) + } + + // checks to make sure a separate module account has been set to collect fees not in OSMO + if addrFoo := dfd.ak.GetModuleAddress(types.FooCollectorName); addrFoo == nil { + return ctx, fmt.Errorf("Foo collector module account (%s) has not been set", types.FooCollectorName) + } + + // fee can be in any denom (checked for validity later) + fee := feeTx.GetFee() + feePayer := feeTx.FeePayer() + feeGranter := feeTx.FeeGranter() + + // set the fee payer as the default address to deduct fees from + deductFeesFrom := feePayer + + // if feegranter set deduct fee from feegranter account. + // this works with only when feegrant enabled. + if feeGranter != nil { + if dfd.feegrantKeeper == nil { + return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants are not enabled") + } else if !feeGranter.Equals(feePayer) { + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, tx.GetMsgs()) + + if err != nil { + return ctx, sdkerrors.Wrapf(err, "%s not allowed to pay fees from %s", feeGranter, feePayer) + } + } + + // if no errors, change the account that is charged for fees to the fee granter + deductFeesFrom = feeGranter + } + + // pulls account from address and makes sure it exists + deductFeesFromAcc := dfd.ak.GetAccount(ctx, deductFeesFrom) + if deductFeesFromAcc == nil { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "fee payer address: %s does not exist", deductFeesFrom) + } + + // deducts the fees and transfer them to the module account + if !feeTx.GetFee().IsZero() { + err = DeductFees(dfd.txFeesKeeper, dfd.bankKeeper, ctx, deductFeesFromAcc, feeTx.GetFee()) + if err != nil { + return ctx, err + } + } // no additional action/else statment needed if the fee is zero since zero-gas transactions are allowed + + events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx, + sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()), + )} + ctx.EventManager().EmitEvents(events) + + return next(ctx, tx, simulate) +} + +// DeductFees deducts fees from the given account and transfers them to the set module account +func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { + + // Checks the validity of the fee tokens (i.e. that the coins are sorted, have positive amount, with a + // valid and unique denomination (no duplicates)) + if !fees.IsValid() { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) + } + + // for _, coin := range coins { + // // send coin + // } + // wrap rest in this for loop to iterate through fees (sdk.Coins type) + // UPDATE: not necessary since mempoolFeeDecorator makes sure one one fee token is paid per tx + + // pulls base denom from TxFeesKeeper (should be uOSMO) + baseDenom, err := txFeesKeeper.GetBaseDenom(ctx) + if err != nil { + return err + } + + // checks if input fee is uOSMO - if so, sends fee directly to fee collector module account to be distributed in the next block by the staking module + // NOTE: assumes only one fee token exists in the fees array (as per the check in mempoolFeeDecorator) + if fees[0].Denom == baseDenom { + // sends to FeeCollectorName module account + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) + if err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + } + } else { + // NOTE: fee token whitelist is already checked in mempoolFeeDecorator + + // sends to FooCollectorName module account + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FooCollectorName, fees) + if err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + } + } + + // TO DO (auto fee swaps): Else, + // 1. add a check to make sure the non-OSMO fee denoms are supported on the DEX with sufficient liquidity (small arbitrary threshold) + // 2. run the same transfer logic as above but send tokens to a separate module account where they will be swapped into OSMO once per epoch & sent to first one using SendCoinsFromModuleToModule + + // TO DO (auto fee swaps): Logic for auto swap once per epoch from the second module account (this might belong elsewhere) + // requires hook in txfees module + + return nil +} diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go new file mode 100644 index 00000000000..5789cc47330 --- /dev/null +++ b/x/txfees/keeper/hooks.go @@ -0,0 +1,53 @@ +package keeper + +import ( + // "fmt" + + // "github.com/cosmos/cosmos-sdk/telemetry" + sdk "github.com/cosmos/cosmos-sdk/types" + epochstypes "github.com/osmosis-labs/osmosis/v7/x/epochs/types" + // "github.com/osmosis-labs/osmosis/v7/x/mint/types" +) + +func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { +} + +// at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account +func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { + + addrFoo := k.accountKeeper.GetModuleAddress(k.fooCollectorName); + + accFoo := k.accountKeeper.GetAccount(ctx, addrFoo); + + // loop + // if OSMO, continue + // otherwise, swap to OSMO and transfer to fee module account (SendModuleToModule) + + for coins := range [where?] { + // how do i get the coins array in an account? + } + +} + +// ___________________________________________________________________________________________________ + +// Hooks wrapper struct for incentives keeper +type Hooks struct { + k Keeper +} + +var _ epochstypes.EpochHooks = Hooks{} + +// Return the wrapper struct +func (k Keeper) Hooks() Hooks { + return Hooks{k} +} + +// epochs hooks +func (h Hooks) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { + h.k.BeforeEpochStart(ctx, epochIdentifier, epochNumber) +} + +func (h Hooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { + h.k.AfterEpochEnd(ctx, epochIdentifier, epochNumber) +} diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index f0e5b021dcb..5a88b1a3aad 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -17,19 +17,36 @@ type ( cdc codec.Codec storeKey sdk.StoreKey + accountKeeper types.AccountKeeper + bankKeeper types.BankKeeper + epochKeeper types.EpochKeeper + spotPriceCalculator types.SpotPriceCalculator + + feeCollectorName string + fooCollectorName string } ) func NewKeeper( cdc codec.Codec, + accountKeeper types.AccountKeeper, + bankKeeper types.BankKeeper, + epochKeeper types.EpochKeeper, storeKey sdk.StoreKey, spotPriceCalculator types.SpotPriceCalculator, + feeCollectorName string, + fooCollectorName string, ) Keeper { return Keeper{ cdc: cdc, + accountKeeper: accountKeeper, + bankKeeper: bankKeeper, + epochKeeper: epochKeeper, storeKey: storeKey, spotPriceCalculator: spotPriceCalculator, + feeCollectorName: feeCollectorName, + fooCollectorName: fooCollectorName, } } diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index 713d485cf4e..053d8b9efe0 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -2,6 +2,8 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + epochstypes "github.com/osmosis-labs/osmosis/v7/x/epochs/types" ) // SpotPriceCalculator defines the contract that must be fulfilled by a spot price calculator @@ -9,3 +11,34 @@ import ( type SpotPriceCalculator interface { CalculateSpotPrice(ctx sdk.Context, poolId uint64, tokenInDenom, tokenOutDenom string) (sdk.Dec, error) } + +// AccountKeeper defines the contract needed for AccountKeeper related APIs. +// Interface provides support to use non-sdk AccountKeeper for AnteHandler's decorators. +type AccountKeeper interface { + GetParams(ctx sdk.Context) (params authtypes.Params) + GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI + SetAccount(ctx sdk.Context, acc authtypes.AccountI) + GetModuleAddress(moduleName string) sdk.AccAddress +} + +// FeegrantKeeper defines the expected feegrant keeper. +type FeegrantKeeper interface { + UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error +} + +// BankKeeper defines the contract needed for supply related APIs (noalias) +type BankKeeper interface { + SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error +} + +// TxFeesKeeper defines the expected transaction fee keeper +type TxFeesKeeper interface { + ConvertToBaseToken(ctx sdk.Context, inputFee sdk.Coin) (sdk.Coin, error) + GetBaseDenom(ctx sdk.Context) (denom string, err error) + GetFeeToken(ctx sdk.Context, denom string) (FeeToken, error) +} + +// EpochKeeper defines the contract needed to be fulfilled for epochs keeper +type EpochKeeper interface { + GetEpochInfo(ctx sdk.Context, identifier string) epochstypes.EpochInfo +} \ No newline at end of file diff --git a/x/txfees/types/keys.go b/x/txfees/types/keys.go index af4946db8c3..8b4e11ca17b 100644 --- a/x/txfees/types/keys.go +++ b/x/txfees/types/keys.go @@ -1,5 +1,9 @@ package types +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + const ( // ModuleName defines the module name ModuleName = "txfees" @@ -10,6 +14,12 @@ const ( // RouterKey is the message route for slashing RouterKey = ModuleName + // FeeCollectorName the root string for the fee collector account address + FeeCollectorName = "fee_collector" + + // FooCollectorName the root string for the $FOO collector account address (used for auto-swapping non-OSMO tx fees) + FooCollectorName = "foo_collector" + // QuerierRoute defines the module's query routing key QuerierRoute = ModuleName ) @@ -17,4 +27,15 @@ const ( var ( BaseDenomKey = []byte("base_denom") FeeTokensStorePrefix = []byte("fee_tokens") + + // AddressStoreKeyPrefix prefix for account-by-address store + AddressStoreKeyPrefix = []byte{0x01} + + // param key for global account number + GlobalAccountNumberKey = []byte("globalAccountNumber") ) + +// AddressStoreKey turn an address to key used to get it from the account store +func AddressStoreKey(addr sdk.AccAddress) []byte { + return append(AddressStoreKeyPrefix, addr.Bytes()...) +} \ No newline at end of file From bb1857cc5d81cd5535cf744e1d5702057e78db1c Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 17:16:37 -0700 Subject: [PATCH 02/75] add second module account to supported module accounts in app --- app/modules.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/modules.go b/app/modules.go index dca13f8c79c..26a62ae6352 100644 --- a/app/modules.go +++ b/app/modules.go @@ -186,6 +186,7 @@ var moduleAaccountPermissions = map[string][]string{ poolincentivestypes.ModuleName: nil, superfluidtypes.ModuleName: {authtypes.Minter, authtypes.Burner}, txfeestypes.ModuleName: nil, + txfeestypes.FooCollectorName: nil, wasm.ModuleName: {authtypes.Burner}, } From 88b44015e0e2d63b1f02aa9ced16c415484f7f5d Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 17:32:24 -0700 Subject: [PATCH 03/75] set up txFeesKeeper with new keeper interface in app --- app/keepers.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index df5ba0c830f..d4b5965a5e3 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -360,11 +360,16 @@ func (app *OsmosisApp) InitNormalKeepers( ) app.PoolIncentivesKeeper = &poolIncentivesKeeper - // TO DO: update txfees keeper here (very similar to mintkeeper above) + // Note: gammKeeper is expected to satisfy the SpotPriceCalculator interface parameter txFeesKeeper := txfeeskeeper.NewKeeper( appCodec, + app.AccountKeeper, + app.BankKeeper, + app.EpochsKeeper, keys[txfeestypes.StoreKey], - app.GAMMKeeper, + gammKeeper, + txfeestypes.FeeCollectorName, + txfeestypes.FooCollectorName, ) app.TxFeesKeeper = &txFeesKeeper From 2b7f32f4389f9ae7b4cb6fd7e804c71d22b26a71 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 17:35:30 -0700 Subject: [PATCH 04/75] add hook receiver to epoch hooks for txfees module --- app/keepers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/keepers.go b/app/keepers.go index d4b5965a5e3..d40013b1651 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -474,6 +474,7 @@ func (app *OsmosisApp) SetupHooks() { epochstypes.NewMultiEpochHooks( // insert epoch hooks receivers here // TO DO: add TxFeesKeeper.Hooks() for auto fee swaps + app.TxFeesKeeper.Hooks(), app.SuperfluidKeeper.Hooks(), app.IncentivesKeeper.Hooks(), app.MintKeeper.Hooks(), From 0becf2acbc4dcd1654b6fe8a1560fed763ab5019 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 21:27:09 -0700 Subject: [PATCH 05/75] set up AfterEpochEnd hook to swap all non-OSMO fees to OSMO --- x/txfees/keeper/hooks.go | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 5789cc47330..f65d6aecc5b 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -6,7 +6,7 @@ import ( // "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" epochstypes "github.com/osmosis-labs/osmosis/v7/x/epochs/types" - // "github.com/osmosis-labs/osmosis/v7/x/mint/types" + txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { @@ -14,19 +14,37 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - - addrFoo := k.accountKeeper.GetModuleAddress(k.fooCollectorName); - accFoo := k.accountKeeper.GetAccount(ctx, addrFoo); + // get module addres for FooCollectorName + addrFoo := k.accountKeeper.GetModuleAddress(txfeestypes.FooCollectorName) - // loop - // if OSMO, continue - // otherwise, swap to OSMO and transfer to fee module account (SendModuleToModule) + // get balances for all denoms in the module account + fooAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrFoo) - for coins := range [where?] { - // how do i get the coins array in an account? + // pulls base denom from TxFeesKeeper (should be uOSMO) + baseDenom, _ := k.GetBaseDenom(ctx) + + // iterate through the resulting array and swap each denom into OSMO using the GAMM module + for _, coin := range fooAccountBalances { + + if coin.Denom == baseDenom { + continue + } else { + // swap into OSMO + + // idea: createSwapEvent(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) + // in x/gamm/keeper/keeper.go + } } + // PLACEHOLDER – addrFee := k.accountKeeper.GetModuleAddress(txfeestypes.FeeCollectorName) + + // Potential error to test for: do swaps to OSMO consolidate into a single denom in fooAccountBalances? Should be yes but keep an eye out + + // send all OSMO to fee module account to be distributed in the next block + k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.FooCollectorName, txfeestypes.FeeCollectorName, fooAccountBalances) + + // Should events be emitted at the end here? } // ___________________________________________________________________________________________________ From ad8bd35e5e3594b28eece7d078ef449bd2f14e01 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 21:30:10 -0700 Subject: [PATCH 06/75] clean up feedecorator comments --- x/txfees/keeper/feedecorator.go | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index d6c756b847f..3d561d0e702 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -153,9 +153,6 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo } // checks to make sure the module account has been set to collect fees in OSMO - // TO DO: add a second module account to send non-OSMO txn fees to for auto fees swaps - // Note: if we do add a second module account, we might need to update the baseapp account keeper - // since that's what's passed into this function in ante.go if addr := dfd.ak.GetModuleAddress(types.FeeCollectorName); addr == nil { return ctx, fmt.Errorf("Fee collector module account (%s) has not been set", types.FeeCollectorName) } @@ -215,17 +212,10 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo // DeductFees deducts fees from the given account and transfers them to the set module account func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { - // Checks the validity of the fee tokens (i.e. that the coins are sorted, have positive amount, with a - // valid and unique denomination (no duplicates)) + // Checks the validity of the fee tokens (sorted, have positive amount, valid and unique denomination) if !fees.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - - // for _, coin := range coins { - // // send coin - // } - // wrap rest in this for loop to iterate through fees (sdk.Coins type) - // UPDATE: not necessary since mempoolFeeDecorator makes sure one one fee token is paid per tx // pulls base denom from TxFeesKeeper (should be uOSMO) baseDenom, err := txFeesKeeper.GetBaseDenom(ctx) @@ -233,16 +223,15 @@ func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ct return err } - // checks if input fee is uOSMO - if so, sends fee directly to fee collector module account to be distributed in the next block by the staking module - // NOTE: assumes only one fee token exists in the fees array (as per the check in mempoolFeeDecorator) + // checks if input fee is uOSMO (assumes only one fee token exists in the fees array (as per the check in mempoolFeeDecorator)) if fees[0].Denom == baseDenom { + // sends to FeeCollectorName module account err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) if err != nil { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } } else { - // NOTE: fee token whitelist is already checked in mempoolFeeDecorator // sends to FooCollectorName module account err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FooCollectorName, fees) @@ -251,12 +240,5 @@ func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ct } } - // TO DO (auto fee swaps): Else, - // 1. add a check to make sure the non-OSMO fee denoms are supported on the DEX with sufficient liquidity (small arbitrary threshold) - // 2. run the same transfer logic as above but send tokens to a separate module account where they will be swapped into OSMO once per epoch & sent to first one using SendCoinsFromModuleToModule - - // TO DO (auto fee swaps): Logic for auto swap once per epoch from the second module account (this might belong elsewhere) - // requires hook in txfees module - return nil } From c2c848e7f0bdc919f7cb3a45ef193ae2ec27e0b9 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 21:31:23 -0700 Subject: [PATCH 07/75] add relevant keepers to TxFeeKeeper struct --- x/txfees/keeper/keeper.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index 5a88b1a3aad..a54f3a0ade8 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -9,6 +9,8 @@ import ( "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" + bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" + "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) @@ -18,7 +20,7 @@ type ( storeKey sdk.StoreKey accountKeeper types.AccountKeeper - bankKeeper types.BankKeeper + bankKeeper *bankkeeper.BaseKeeper epochKeeper types.EpochKeeper spotPriceCalculator types.SpotPriceCalculator @@ -31,7 +33,7 @@ type ( func NewKeeper( cdc codec.Codec, accountKeeper types.AccountKeeper, - bankKeeper types.BankKeeper, + bankKeeper *bankkeeper.BaseKeeper, epochKeeper types.EpochKeeper, storeKey sdk.StoreKey, spotPriceCalculator types.SpotPriceCalculator, From 6467af64cf6b3faba3505d8bbfe71ff0bdb92186 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 22:00:42 -0700 Subject: [PATCH 08/75] revert accidental file changes due to branch mixup --- .../cmd/balances_from_state_export.go | 69 ++++++------------- 1 file changed, 22 insertions(+), 47 deletions(-) diff --git a/cmd/osmosisd/cmd/balances_from_state_export.go b/cmd/osmosisd/cmd/balances_from_state_export.go index 16a697914fb..ca89d080fcf 100644 --- a/cmd/osmosisd/cmd/balances_from_state_export.go +++ b/cmd/osmosisd/cmd/balances_from_state_export.go @@ -5,7 +5,6 @@ import ( "fmt" "io/ioutil" "os" - "time" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/server" @@ -13,15 +12,13 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/osmosis-labs/osmosis/app" - appparams "github.com/osmosis-labs/osmosis/app/params" - "github.com/osmosis-labs/osmosis/osmotestutils" - claimtypes "github.com/osmosis-labs/osmosis/x/claim/types" - gammtypes "github.com/osmosis-labs/osmosis/x/gamm/types" - lockuptypes "github.com/osmosis-labs/osmosis/x/lockup/types" + appparams "github.com/osmosis-labs/osmosis/v7/app/params" + "github.com/osmosis-labs/osmosis/v7/osmoutils" + claimtypes "github.com/osmosis-labs/osmosis/v7/x/claim/types" + gammtypes "github.com/osmosis-labs/osmosis/v7/x/gamm/types" + lockuptypes "github.com/osmosis-labs/osmosis/v7/x/lockup/types" "github.com/spf13/cobra" tmjson "github.com/tendermint/tendermint/libs/json" - tmproto "github.com/tendermint/tendermint/proto/tendermint/types" tmtypes "github.com/tendermint/tendermint/types" ) @@ -63,10 +60,10 @@ func underlyingCoins(originCoins sdk.Coins, pools map[string]gammtypes.PoolI) sd for _, coin := range originCoins { if pools[coin.Denom] != nil { pool := pools[coin.Denom] - assets := pool.GetAllPoolAssets() + assets := pool.GetTotalPoolLiquidity(sdk.Context{}) for _, asset := range assets { - balances = balances.Add(sdk.NewCoin(asset.Token.Denom, asset.Token.Amount.Mul(coin.Amount).Quo(pool.GetTotalShares().Amount))) - if pools[asset.Token.Denom] != nil { // this happens when there's a pool for LP token swap + balances = balances.Add(sdk.NewCoin(asset.Denom, asset.Amount.Mul(coin.Amount).Quo(pool.GetTotalShares()))) + if pools[asset.Denom] != nil { // this happens when there's a pool for LP token swap convertAgain = true } } @@ -82,12 +79,12 @@ func underlyingCoins(originCoins sdk.Coins, pools map[string]gammtypes.PoolI) sd } // pools is a map from LP share string -> pool. -// TODO: Make a separate type for this +// TODO: Make a separate type for this. func underlyingCoinsForSelectPools( originCoins sdk.Coins, pools map[string]gammtypes.PoolI, - selectPoolIDs []uint64) map[uint64]sdk.Coins { - + selectPoolIDs []uint64, +) map[uint64]sdk.Coins { balancesByPool := make(map[uint64]sdk.Coins) for _, coin := range originCoins { @@ -143,7 +140,8 @@ func getGenStateFromPath(genesisFilePath string) (map[string]json.RawMessage, er return genState, nil } -// ExportAirdropSnapshotCmd generates a snapshot.json from a provided exported genesis.json +// ExportAirdropSnapshotCmd generates a snapshot.json from a provided exported genesis.json. +//nolint:ineffassign // because of accounts = authtypes.SanitizeGenesisAccounts(accounts) func ExportDeriveBalancesCmd() *cobra.Command { cmd := &cobra.Command{ Use: "export-derive-balances [input-genesis-file] [output-snapshot-json]", @@ -173,14 +171,14 @@ Example: } selectBondedPoolIDs := []uint64{} if selectPoolIdsStr != "" { - selectBondedPoolIDs, err = osmotestutils.ParseUint64SliceFromString(selectPoolIdsStr, ",") + selectBondedPoolIDs, err = osmoutils.ParseUint64SliceFromString(selectPoolIdsStr, ",") if err != nil { return err } } authGenesis := authtypes.GenesisState{} - clientCtx.JSONCodec.MustUnmarshalJSON(genState["auth"], &authGenesis) + clientCtx.Codec.MustUnmarshalJSON(genState["auth"], &authGenesis) accounts, err := authtypes.UnpackAccounts(authGenesis.Accounts) if err != nil { panic(err) @@ -191,7 +189,7 @@ Example: snapshotAccs := make(map[string]DerivedAccount) bankGenesis := banktypes.GenesisState{} - clientCtx.JSONCodec.MustUnmarshalJSON(genState["bank"], &bankGenesis) + clientCtx.Codec.MustUnmarshalJSON(genState["bank"], &bankGenesis) for _, balance := range bankGenesis.Balances { address := balance.Address acc, ok := snapshotAccs[address] @@ -204,7 +202,7 @@ Example: } stakingGenesis := stakingtypes.GenesisState{} - clientCtx.JSONCodec.MustUnmarshalJSON(genState["staking"], &stakingGenesis) + clientCtx.Codec.MustUnmarshalJSON(genState["staking"], &stakingGenesis) for _, unbonding := range stakingGenesis.UnbondingDelegations { address := unbonding.DelegatorAddress acc, ok := snapshotAccs[address] @@ -245,7 +243,7 @@ Example: } lockupGenesis := lockuptypes.GenesisState{} - clientCtx.JSONCodec.MustUnmarshalJSON(genState["lockup"], &lockupGenesis) + clientCtx.Codec.MustUnmarshalJSON(genState["lockup"], &lockupGenesis) for _, lock := range lockupGenesis.Locks { address := lock.Owner @@ -259,7 +257,7 @@ Example: } claimGenesis := claimtypes.GenesisState{} - clientCtx.JSONCodec.MustUnmarshalJSON(genState["claim"], &claimGenesis) + clientCtx.Codec.MustUnmarshalJSON(genState["claim"], &claimGenesis) for _, record := range claimGenesis.ClaimRecords { address := record.Address @@ -287,7 +285,7 @@ Example: } gammGenesis := gammtypes.GenesisState{} - clientCtx.JSONCodec.MustUnmarshalJSON(genState["gamm"], &gammGenesis) + clientCtx.Codec.MustUnmarshalJSON(genState["gamm"], &gammGenesis) // collect gamm pools pools := make(map[string]gammtypes.PoolI) @@ -297,7 +295,7 @@ Example: if err != nil { panic(err) } - pools[pool.GetTotalShares().Denom] = pool + pools[gammtypes.GetPoolShareDenom(pool.GetId())] = pool } // convert balances to underlying coins and sum up balances to total balance @@ -318,29 +316,6 @@ Example: snapshotAccs[addr] = account } - app := app.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{Height: 1, Time: time.Now().UTC()}) - - // scaffolding for map creation: - // accountsMap := make(map[string]authtypes.GenesisAccount) - - // Remove module accounts from snapshots - for addr, account := range snapshotAccs { - - // TO DO: change the addr -> account function to a map to genesis accounts using the sanitized gen accs generated above - accAddr, err := sdk.AccAddressFromBech32(account.Address) - if err != nil { - return err - } - acc := app.AccountKeeper.GetAccount(ctx, accAddr) - - if _, ok := acc.(*authtypes.ModuleAccount); ok { - //Else, remove account from list - how do we remove an item from a map in go? - //ASSUMING THAT "addr" IS THE KEY - delete(snapshotAccs, addr) - } - } - snapshot := DeriveSnapshot{ NumberAccounts: uint64(len(snapshotAccs)), Accounts: snapshotAccs, @@ -354,7 +329,7 @@ Example: return fmt.Errorf("failed to marshal snapshot: %w", err) } - err = ioutil.WriteFile(snapshotOutput, snapshotJSON, 0644) + err = ioutil.WriteFile(snapshotOutput, snapshotJSON, 0o644) return err }, } From e5d46e12bcd564ef164cf570ebcea6445dcfc6ea Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 24 Mar 2022 22:06:06 -0700 Subject: [PATCH 09/75] revert accidental file changes due to branch mixup --- .../cmd/balances_from_state_export.go | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/cmd/osmosisd/cmd/balances_from_state_export.go b/cmd/osmosisd/cmd/balances_from_state_export.go index ca89d080fcf..13dcd477971 100644 --- a/cmd/osmosisd/cmd/balances_from_state_export.go +++ b/cmd/osmosisd/cmd/balances_from_state_export.go @@ -60,10 +60,10 @@ func underlyingCoins(originCoins sdk.Coins, pools map[string]gammtypes.PoolI) sd for _, coin := range originCoins { if pools[coin.Denom] != nil { pool := pools[coin.Denom] - assets := pool.GetTotalPoolLiquidity(sdk.Context{}) + assets := pool.GetAllPoolAssets() for _, asset := range assets { - balances = balances.Add(sdk.NewCoin(asset.Denom, asset.Amount.Mul(coin.Amount).Quo(pool.GetTotalShares()))) - if pools[asset.Denom] != nil { // this happens when there's a pool for LP token swap + balances = balances.Add(sdk.NewCoin(asset.Token.Denom, asset.Token.Amount.Mul(coin.Amount).Quo(pool.GetTotalShares().Amount))) + if pools[asset.Token.Denom] != nil { // this happens when there's a pool for LP token swap convertAgain = true } } @@ -79,12 +79,12 @@ func underlyingCoins(originCoins sdk.Coins, pools map[string]gammtypes.PoolI) sd } // pools is a map from LP share string -> pool. -// TODO: Make a separate type for this. +// TODO: Make a separate type for this func underlyingCoinsForSelectPools( originCoins sdk.Coins, pools map[string]gammtypes.PoolI, - selectPoolIDs []uint64, -) map[uint64]sdk.Coins { + selectPoolIDs []uint64) map[uint64]sdk.Coins { + balancesByPool := make(map[uint64]sdk.Coins) for _, coin := range originCoins { @@ -140,8 +140,7 @@ func getGenStateFromPath(genesisFilePath string) (map[string]json.RawMessage, er return genState, nil } -// ExportAirdropSnapshotCmd generates a snapshot.json from a provided exported genesis.json. -//nolint:ineffassign // because of accounts = authtypes.SanitizeGenesisAccounts(accounts) +// ExportAirdropSnapshotCmd generates a snapshot.json from a provided exported genesis.json func ExportDeriveBalancesCmd() *cobra.Command { cmd := &cobra.Command{ Use: "export-derive-balances [input-genesis-file] [output-snapshot-json]", @@ -178,7 +177,7 @@ Example: } authGenesis := authtypes.GenesisState{} - clientCtx.Codec.MustUnmarshalJSON(genState["auth"], &authGenesis) + clientCtx.JSONCodec.MustUnmarshalJSON(genState["auth"], &authGenesis) accounts, err := authtypes.UnpackAccounts(authGenesis.Accounts) if err != nil { panic(err) @@ -189,7 +188,7 @@ Example: snapshotAccs := make(map[string]DerivedAccount) bankGenesis := banktypes.GenesisState{} - clientCtx.Codec.MustUnmarshalJSON(genState["bank"], &bankGenesis) + clientCtx.JSONCodec.MustUnmarshalJSON(genState["bank"], &bankGenesis) for _, balance := range bankGenesis.Balances { address := balance.Address acc, ok := snapshotAccs[address] @@ -202,7 +201,7 @@ Example: } stakingGenesis := stakingtypes.GenesisState{} - clientCtx.Codec.MustUnmarshalJSON(genState["staking"], &stakingGenesis) + clientCtx.JSONCodec.MustUnmarshalJSON(genState["staking"], &stakingGenesis) for _, unbonding := range stakingGenesis.UnbondingDelegations { address := unbonding.DelegatorAddress acc, ok := snapshotAccs[address] @@ -243,7 +242,7 @@ Example: } lockupGenesis := lockuptypes.GenesisState{} - clientCtx.Codec.MustUnmarshalJSON(genState["lockup"], &lockupGenesis) + clientCtx.JSONCodec.MustUnmarshalJSON(genState["lockup"], &lockupGenesis) for _, lock := range lockupGenesis.Locks { address := lock.Owner @@ -257,7 +256,7 @@ Example: } claimGenesis := claimtypes.GenesisState{} - clientCtx.Codec.MustUnmarshalJSON(genState["claim"], &claimGenesis) + clientCtx.JSONCodec.MustUnmarshalJSON(genState["claim"], &claimGenesis) for _, record := range claimGenesis.ClaimRecords { address := record.Address @@ -285,7 +284,7 @@ Example: } gammGenesis := gammtypes.GenesisState{} - clientCtx.Codec.MustUnmarshalJSON(genState["gamm"], &gammGenesis) + clientCtx.JSONCodec.MustUnmarshalJSON(genState["gamm"], &gammGenesis) // collect gamm pools pools := make(map[string]gammtypes.PoolI) @@ -295,7 +294,7 @@ Example: if err != nil { panic(err) } - pools[gammtypes.GetPoolShareDenom(pool.GetId())] = pool + pools[pool.GetTotalShares().Denom] = pool } // convert balances to underlying coins and sum up balances to total balance @@ -329,7 +328,7 @@ Example: return fmt.Errorf("failed to marshal snapshot: %w", err) } - err = ioutil.WriteFile(snapshotOutput, snapshotJSON, 0o644) + err = ioutil.WriteFile(snapshotOutput, snapshotJSON, 0644) return err }, } From cfae2f09b0cd106746b9ef5b9d3038ae7bd35a92 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 25 Mar 2022 10:26:36 -0700 Subject: [PATCH 10/75] add rudimentary swapping logic --- x/txfees/keeper/hooks.go | 15 +++++++++++---- x/txfees/types/expected_keepers.go | 8 ++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index f65d6aecc5b..35b1841d6f4 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -15,7 +15,7 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - // get module addres for FooCollectorName + // get module address for FooCollectorName addrFoo := k.accountKeeper.GetModuleAddress(txfeestypes.FooCollectorName) // get balances for all denoms in the module account @@ -30,10 +30,17 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb if coin.Denom == baseDenom { continue } else { - // swap into OSMO + + // TO DO: figure out how to cast this or get the pool ID for the main OSMO paired pool + feetoken := coin.(txfeestypes.FeeToken) + + // calculate spot price to determine minimum out for swap + // question: is the order of input denom and output denom correct? + spotPrice, _ := k.spotPriceCalculator.CalculateSpotPrice(ctx, feetoken.GetPoolID(), coin.Denom, baseDenom) - // idea: createSwapEvent(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) - // in x/gamm/keeper/keeper.go + // swap into OSMO + // question: is spotPrice really the minimum out for the swap? + k.spotPriceCalculator.SwapExactAmountIn(ctx, addrFoo, feetoken.GetPoolId(), coin.Denom, baseDenom, k.spotPriceCalculator) } } diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index 053d8b9efe0..1f4a429a1b5 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -10,6 +10,14 @@ import ( // The x/gamm keeper is expected to satisfy this interface type SpotPriceCalculator interface { CalculateSpotPrice(ctx sdk.Context, poolId uint64, tokenInDenom, tokenOutDenom string) (sdk.Dec, error) + SwapExactAmountIn( + ctx sdk.Context, + sender sdk.AccAddress, + poolId uint64, + tokenIn sdk.Coin, + tokenOutDenom string, + tokenOutMinAmount sdk.Int, + ) (tokenOutAmount sdk.Int, spotPriceAfter sdk.Dec, err error) } // AccountKeeper defines the contract needed for AccountKeeper related APIs. From 528edfd0f13e6c7409b43dea2103e22c88942db6 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 25 Mar 2022 10:29:04 -0700 Subject: [PATCH 11/75] clean up comments and spacing --- app/keepers.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index d40013b1651..9dcb146a737 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -345,8 +345,6 @@ func (app *OsmosisApp) InitNormalKeepers( ) app.MintKeeper = &mintKeeper - - poolIncentivesKeeper := poolincentiveskeeper.NewKeeper( appCodec, keys[poolincentivestypes.StoreKey], @@ -473,7 +471,6 @@ func (app *OsmosisApp) SetupHooks() { app.EpochsKeeper.SetHooks( epochstypes.NewMultiEpochHooks( // insert epoch hooks receivers here - // TO DO: add TxFeesKeeper.Hooks() for auto fee swaps app.TxFeesKeeper.Hooks(), app.SuperfluidKeeper.Hooks(), app.IncentivesKeeper.Hooks(), From 9db3006bfa3dc69dd5ab677a0f02da6f5b8b8660 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 25 Mar 2022 11:38:51 -0700 Subject: [PATCH 12/75] add gammkeeper and move swap calls under it --- app/keepers.go | 1 + x/txfees/keeper/hooks.go | 8 +++++--- x/txfees/keeper/keeper.go | 5 ++++- x/txfees/types/expected_keepers.go | 4 ++++ 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index 9dcb146a737..28dd2d68687 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -366,6 +366,7 @@ func (app *OsmosisApp) InitNormalKeepers( app.EpochsKeeper, keys[txfeestypes.StoreKey], gammKeeper, + gammKeeper, txfeestypes.FeeCollectorName, txfeestypes.FooCollectorName, ) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 35b1841d6f4..8678b619449 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -32,15 +32,17 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb } else { // TO DO: figure out how to cast this or get the pool ID for the main OSMO paired pool - feetoken := coin.(txfeestypes.FeeToken) + feetoken := txfeestypes.FeeToken(coin) // calculate spot price to determine minimum out for swap // question: is the order of input denom and output denom correct? - spotPrice, _ := k.spotPriceCalculator.CalculateSpotPrice(ctx, feetoken.GetPoolID(), coin.Denom, baseDenom) + // spotPrice, _ := k.spotPriceCalculator.CalculateSpotPrice(ctx, feetoken.GetPoolID(), coin.Denom, baseDenom) + + placeholderMinimum := sdk.OneInt() // swap into OSMO // question: is spotPrice really the minimum out for the swap? - k.spotPriceCalculator.SwapExactAmountIn(ctx, addrFoo, feetoken.GetPoolId(), coin.Denom, baseDenom, k.spotPriceCalculator) + k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.GetPoolId(), coin, baseDenom, placeholderMinimum) } } diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index a54f3a0ade8..cd0b7a30450 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -22,6 +22,7 @@ type ( accountKeeper types.AccountKeeper bankKeeper *bankkeeper.BaseKeeper epochKeeper types.EpochKeeper + gammKeeper types.GammKeeper spotPriceCalculator types.SpotPriceCalculator @@ -36,6 +37,7 @@ func NewKeeper( bankKeeper *bankkeeper.BaseKeeper, epochKeeper types.EpochKeeper, storeKey sdk.StoreKey, + gammKeeper types.GammKeeper, spotPriceCalculator types.SpotPriceCalculator, feeCollectorName string, fooCollectorName string, @@ -45,7 +47,8 @@ func NewKeeper( accountKeeper: accountKeeper, bankKeeper: bankKeeper, epochKeeper: epochKeeper, - storeKey: storeKey, + storeKey: storeKey, + gammKeeper: gammKeeper, spotPriceCalculator: spotPriceCalculator, feeCollectorName: feeCollectorName, fooCollectorName: fooCollectorName, diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index 1f4a429a1b5..36e2574dd7d 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -10,6 +10,10 @@ import ( // The x/gamm keeper is expected to satisfy this interface type SpotPriceCalculator interface { CalculateSpotPrice(ctx sdk.Context, poolId uint64, tokenInDenom, tokenOutDenom string) (sdk.Dec, error) +} + +// GammKeeper defines the contract needed for AccountKeeper related APIs. +type GammKeeper interface { SwapExactAmountIn( ctx sdk.Context, sender sdk.AccAddress, From 88504855dc734fe204aed0df10355501abfd5df6 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 25 Mar 2022 12:11:22 -0700 Subject: [PATCH 13/75] comments for intended feedecorator tests --- x/txfees/keeper/feedecorator_test.go | 43 ++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 5fc04171160..092d5c25bd9 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -31,6 +31,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested uint64 isCheckTx bool expectPass bool + baseDenomGas bool }{ { name: "no min gas price - checktx", @@ -39,6 +40,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: true, expectPass: true, + baseDenomGas: true, }, { name: "no min gas price - delivertx", @@ -47,6 +49,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: false, expectPass: true, + baseDenomGas: true, }, { name: "works with valid basedenom fee", @@ -56,6 +59,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: true, expectPass: true, + baseDenomGas: true, }, { name: "doesn't work with not enough fee in checktx", @@ -65,6 +69,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: true, expectPass: false, + baseDenomGas: true, }, { name: "works with not enough fee in delivertx", @@ -74,6 +79,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: false, expectPass: true, + baseDenomGas: true, }, { name: "works with valid converted fee", @@ -83,6 +89,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: true, expectPass: true, + baseDenomGas: false, }, { name: "doesn't work with not enough converted fee in checktx", @@ -92,6 +99,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: true, expectPass: false, + baseDenomGas: false, }, { name: "works with not enough converted fee in delivertx", @@ -101,6 +109,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: false, expectPass: true, + baseDenomGas: false, }, { name: "multiple fee coins - checktx", @@ -109,6 +118,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: true, expectPass: false, + baseDenomGas: false, }, { name: "multiple fee coins - delivertx", @@ -117,6 +127,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: false, expectPass: false, + baseDenomGas: false, }, { name: "invalid fee denom", @@ -125,6 +136,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: false, expectPass: false, + baseDenomGas: false, }, { name: "mingasprice not containing basedenom gets treated as min gas price 0", @@ -133,6 +145,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: 10000, isCheckTx: true, expectPass: true, + baseDenomGas: false, }, { name: "tx with gas wanted more than allowed should not pass", @@ -141,6 +154,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: mempoolFeeOpts.MaxGasWantedPerTx + 1, isCheckTx: true, expectPass: false, + baseDenomGas: false, }, { name: "tx with high gas and not enough fee should no pass", @@ -149,6 +163,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: mempoolFeeOpts.HighGasTxThreshold, isCheckTx: true, expectPass: false, + baseDenomGas: false, }, { name: "tx with high gas and enough fee should pass", @@ -157,6 +172,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { gasRequested: mempoolFeeOpts.HighGasTxThreshold, isCheckTx: true, expectPass: true, + baseDenomGas: false, }, } @@ -171,12 +187,35 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { ), []legacytx.StdSignature{}, "") mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper, mempoolFeeOpts) - antehandler := sdk.ChainAnteDecorators(mfd) - _, err := antehandler(suite.ctx, tx, false) + antehandlerMFD := sdk.ChainAnteDecorators(mfd) + _, err := antehandlerMFD(suite.ctx, tx, false) if tc.expectPass { suite.Require().NoError(err, "test: %s", tc.name) } else { suite.Require().Error(err, "test: %s", tc.name) } + + /* DeductFeeDecorator tests: + + // Does FeeGrantKeeper need to be added to the TxFeesKeeper struct? + + dfd := keeper.NewDeductFeeDecorator(*suite.app.TxFeesKeeper, *suite.app.TxFeesKeeper.accountKeeper, *suite.app.TxFeesKeeper.bankKeeper, *suite.app.TxFeesKeeper.feegrantKeeper) + antehandlerDFD := sdk.ChainAnteDecorators(dfd) + _, err = antehandlerDFD(suite.ctx, tx, false) + if tc.expectPass { + if tc.baseDenomGas { + // check main module account osmo balance & require no non-OSMO + // also require that the balance of the second module account (just get AllBalances) has not changed (suite.Require().Equalf(123, 123, "error message %s", "formatted")) + } else { + // check second module account to make sure the right fee amount has flowed there + // also require that the balance of the main module account (just get AllBalances) has not changed (suite.Require().Equalf(123, 123, "error message %s", "formatted")) + } + suite.Require().NoError(err, "test: %s", tc.name) + } else { + suite.Require().Error(err, "test: %s", tc.name) + } + + */ + } } From 26c3316ea4d56f4ec0f7adcf3569e9ead5fc45c5 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 25 Mar 2022 13:39:34 -0700 Subject: [PATCH 14/75] fix swap parameters to get correct pool id and minimum output --- x/txfees/keeper/hooks.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 8678b619449..d4fd855a4c7 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -32,7 +32,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb } else { // TO DO: figure out how to cast this or get the pool ID for the main OSMO paired pool - feetoken := txfeestypes.FeeToken(coin) + feetoken, _ := k.GetFeeToken(ctx, coin.Denom) // calculate spot price to determine minimum out for swap // question: is the order of input denom and output denom correct? @@ -42,7 +42,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb // swap into OSMO // question: is spotPrice really the minimum out for the swap? - k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.GetPoolId(), coin, baseDenom, placeholderMinimum) + k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.PoolID, coin, baseDenom, placeholderMinimum) } } From 3f40623c8e9705b7d8b0360c65a1f9f291455ac2 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 25 Mar 2022 16:06:19 -0700 Subject: [PATCH 15/75] set up feegrantkeeper in suite app for testing --- app/keepers.go | 16 ++++++++++++++-- x/txfees/keeper/feedecorator_test.go | 23 ++++++++++++++++++----- x/txfees/keeper/hooks.go | 12 ++---------- x/txfees/types/expected_keepers.go | 2 ++ 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index 28dd2d68687..d09ffca34ae 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -56,6 +56,9 @@ import ( stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + // Staking: Allows the Tendermint validator set to be chosen based on bonded stake. + feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" + // Upgrade: Software upgrades handling and coordination. "github.com/cosmos/cosmos-sdk/x/upgrade" upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper" @@ -158,6 +161,7 @@ type appKeepers struct { MintKeeper *mintkeeper.Keeper PoolIncentivesKeeper *poolincentiveskeeper.Keeper TxFeesKeeper *txfeeskeeper.Keeper + FeeGrantKeeper *feegrantkeeper.Keeper SuperfluidKeeper *superfluidkeeper.Keeper GovKeeper *govkeeper.Keeper WasmKeeper *wasm.Keeper @@ -365,13 +369,21 @@ func (app *OsmosisApp) InitNormalKeepers( app.BankKeeper, app.EpochsKeeper, keys[txfeestypes.StoreKey], - gammKeeper, - gammKeeper, + app.GAMMKeeper, + app.GAMMKeeper, txfeestypes.FeeCollectorName, txfeestypes.FooCollectorName, ) app.TxFeesKeeper = &txFeesKeeper + // Note: gammKeeper is expected to satisfy the SpotPriceCalculator interface parameter + feeGrantKeeper := feegrantkeeper.NewKeeper( + appCodec, + keys[txfeestypes.StoreKey], + app.AccountKeeper, + ) + app.FeeGrantKeeper = &feeGrantKeeper + // The last arguments can contain custom message handlers, and custom query handlers, // if we want to allow any custom callbacks supportedFeatures := "iterator,staking,stargate" diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 092d5c25bd9..0e6858dd47a 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -12,6 +12,22 @@ import ( func (suite *KeeperTestSuite) TestFeeDecorator() { suite.SetupTest(false) + /* + // Note: gammKeeper is expected to satisfy the SpotPriceCalculator interface parameter + txFeesKeeper := txfeeskeeper.NewKeeper( + appCodec, + suite.app.AccountKeeper, + suite.app.BankKeeper, + suite.app.EpochsKeeper, + keys[txfeestypes.StoreKey], + app.GAMMKeeper, + app.GAMMKeeper, + txfeestypes.FeeCollectorName, + txfeestypes.FooCollectorName, + ) + app.TxFeesKeeper = &txFeesKeeper + */ + mempoolFeeOpts := types.NewDefaultMempoolFeeOptions() mempoolFeeOpts.MinGasPriceForHighGasTx = sdk.MustNewDecFromStr("0.0025") baseDenom, _ := suite.app.TxFeesKeeper.GetBaseDenom(suite.ctx) @@ -195,11 +211,11 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.Require().Error(err, "test: %s", tc.name) } - /* DeductFeeDecorator tests: + // DeductFeeDecorator tests: // Does FeeGrantKeeper need to be added to the TxFeesKeeper struct? - dfd := keeper.NewDeductFeeDecorator(*suite.app.TxFeesKeeper, *suite.app.TxFeesKeeper.accountKeeper, *suite.app.TxFeesKeeper.bankKeeper, *suite.app.TxFeesKeeper.feegrantKeeper) + dfd := keeper.NewDeductFeeDecorator(*suite.app.TxFeesKeeper, *suite.app.AccountKeeper, *suite.app.BankKeeper, *suite.app.FeeGrantKeeper) antehandlerDFD := sdk.ChainAnteDecorators(dfd) _, err = antehandlerDFD(suite.ctx, tx, false) if tc.expectPass { @@ -214,8 +230,5 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { } else { suite.Require().Error(err, "test: %s", tc.name) } - - */ - } } diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index d4fd855a4c7..2f886a8fb5f 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -34,20 +34,12 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb // TO DO: figure out how to cast this or get the pool ID for the main OSMO paired pool feetoken, _ := k.GetFeeToken(ctx, coin.Denom) - // calculate spot price to determine minimum out for swap - // question: is the order of input denom and output denom correct? - // spotPrice, _ := k.spotPriceCalculator.CalculateSpotPrice(ctx, feetoken.GetPoolID(), coin.Denom, baseDenom) - - placeholderMinimum := sdk.OneInt() - // swap into OSMO // question: is spotPrice really the minimum out for the swap? - k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.PoolID, coin, baseDenom, placeholderMinimum) + k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) } } - // PLACEHOLDER – addrFee := k.accountKeeper.GetModuleAddress(txfeestypes.FeeCollectorName) - // Potential error to test for: do swaps to OSMO consolidate into a single denom in fooAccountBalances? Should be yes but keep an eye out // send all OSMO to fee module account to be distributed in the next block @@ -71,8 +63,8 @@ func (k Keeper) Hooks() Hooks { } // epochs hooks +// Don't do anything pre epoch start func (h Hooks) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - h.k.BeforeEpochStart(ctx, epochIdentifier, epochNumber) } func (h Hooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index 36e2574dd7d..c68538f2859 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -48,6 +48,8 @@ type TxFeesKeeper interface { ConvertToBaseToken(ctx sdk.Context, inputFee sdk.Coin) (sdk.Coin, error) GetBaseDenom(ctx sdk.Context) (denom string, err error) GetFeeToken(ctx sdk.Context, denom string) (FeeToken, error) + + // add keepers here } // EpochKeeper defines the contract needed to be fulfilled for epochs keeper From 2d1ceb2bbbcdb222f817702a129c64194c475862 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Sun, 27 Mar 2022 16:35:53 -0700 Subject: [PATCH 16/75] Update x/txfees/keeper/feedecorator.go Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 3d561d0e702..e9e74608f7e 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -124,9 +124,10 @@ func (mfd MempoolFeeDecorator) GetMinBaseGasPriceForTx(ctx sdk.Context, baseDeno return cfgMinGasPrice } -// DeductFeeDecorator deducts fees from the first signer of the tx -// If the first signer does not have the funds to pay for the fees, return with InsufficientFunds error -// Call next AnteHandler if fees successfully deducted +// DeductFeeDecorator deducts fees from the first signer of the tx. +// If the first signer does not have the funds to pay for the fees, we return an InsufficientFunds error. +// We call next AnteHandler if fees successfully deducted. +// // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator type DeductFeeDecorator struct { ak types.AccountKeeper From 081d9a7a52a734cda28269a146a0735a128c950f Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Sun, 27 Mar 2022 16:36:06 -0700 Subject: [PATCH 17/75] Update x/txfees/keeper/feedecorator.go Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index e9e74608f7e..fe50c3570e7 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -147,7 +147,6 @@ func NewDeductFeeDecorator(tk Keeper, ak types.AccountKeeper, bk types.BankKeepe func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - // checks to make sure feeTx is of the right type feeTx, ok := tx.(sdk.FeeTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") From aa5775e347848529b75d68fff1e3c5d1720e4514 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Sun, 27 Mar 2022 16:38:55 -0700 Subject: [PATCH 18/75] Update x/txfees/keeper/feedecorator.go Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index fe50c3570e7..05b1b97f489 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -170,7 +170,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo // set the fee payer as the default address to deduct fees from deductFeesFrom := feePayer - // if feegranter set deduct fee from feegranter account. + // If a fee granter was set, deduct fee from the fee granter's account. // this works with only when feegrant enabled. if feeGranter != nil { if dfd.feegrantKeeper == nil { From 184b9392726940402bd0ee15e66778fa344d1d40 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Sun, 27 Mar 2022 16:39:02 -0700 Subject: [PATCH 19/75] Update x/txfees/keeper/feedecorator.go Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 05b1b97f489..81331e2fa41 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -171,7 +171,6 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo deductFeesFrom := feePayer // If a fee granter was set, deduct fee from the fee granter's account. - // this works with only when feegrant enabled. if feeGranter != nil { if dfd.feegrantKeeper == nil { return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants are not enabled") From 4bce525e7190036043f5897bc70f442e21484e45 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Sun, 27 Mar 2022 16:39:20 -0700 Subject: [PATCH 20/75] Update x/txfees/keeper/feedecorator.go Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 81331e2fa41..88e063fe270 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -173,7 +173,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo // If a fee granter was set, deduct fee from the fee granter's account. if feeGranter != nil { if dfd.feegrantKeeper == nil { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants are not enabled") + return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants is not enabled") } else if !feeGranter.Equals(feePayer) { err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, tx.GetMsgs()) From 5032ca1e3b8ce79452e091d5ff2a59a3b17421b1 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Sun, 27 Mar 2022 16:41:18 -0700 Subject: [PATCH 21/75] Apply suggestions from code review Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 88e063fe270..6ec144931a6 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -186,7 +186,6 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo deductFeesFrom = feeGranter } - // pulls account from address and makes sure it exists deductFeesFromAcc := dfd.ak.GetAccount(ctx, deductFeesFrom) if deductFeesFromAcc == nil { return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "fee payer address: %s does not exist", deductFeesFrom) @@ -198,7 +197,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo if err != nil { return ctx, err } - } // no additional action/else statment needed if the fee is zero since zero-gas transactions are allowed + } events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx, sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()), @@ -208,7 +207,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return next(ctx, tx, simulate) } -// DeductFees deducts fees from the given account and transfers them to the set module account +// DeductFees deducts fees from the given account and transfers them to the set module account. func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { // Checks the validity of the fee tokens (sorted, have positive amount, valid and unique denomination) From b9026d14202a1ec94525816794f4d7604efc8f91 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 27 Mar 2022 16:44:46 -0700 Subject: [PATCH 22/75] update comments --- app/ante.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app/ante.go b/app/ante.go index 41c2ec743d9..818c0555aab 100644 --- a/app/ante.go +++ b/app/ante.go @@ -44,7 +44,6 @@ func NewAnteHandler( ante.TxTimeoutHeightDecorator{}, ante.NewValidateMemoDecorator(ak), ante.NewConsumeGasForTxSizeDecorator(ak), - // Replaced with version from our txfees module from auth (previously "ante.NewDeductFeeDecorator(ak, bankKeeper, nil)")" deductFeeDecorator, ante.NewSetPubKeyDecorator(ak), // SetPubKeyDecorator must be called before all signature verification decorators ante.NewValidateSigCountDecorator(ak), From 349bc70baf28ef16bf346939769c2abc1ed07bbc Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 12:10:29 -0700 Subject: [PATCH 23/75] attempt to migrate test from legacytx to txbuilder --- x/txfees/keeper/feedecorator.go | 5 +-- x/txfees/keeper/feedecorator_test.go | 62 +++++++++++++++++++--------- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 6ec144931a6..eb97fb01c0e 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -199,10 +199,9 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo } } - events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx, + ctx.EventManager().EmitEvents(sdk.Events{sdk.NewEvent(sdk.EventTypeTx, sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()), - )} - ctx.EventManager().EmitEvents(events) + )}) return next(ctx, tx, simulate) } diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 0e6858dd47a..c7549ae6e98 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -4,30 +4,18 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" - "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" - + // "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" + + clienttx "github.com/cosmos/cosmos-sdk/client/tx" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" ) func (suite *KeeperTestSuite) TestFeeDecorator() { suite.SetupTest(false) - /* - // Note: gammKeeper is expected to satisfy the SpotPriceCalculator interface parameter - txFeesKeeper := txfeeskeeper.NewKeeper( - appCodec, - suite.app.AccountKeeper, - suite.app.BankKeeper, - suite.app.EpochsKeeper, - keys[txfeestypes.StoreKey], - app.GAMMKeeper, - app.GAMMKeeper, - txfeestypes.FeeCollectorName, - txfeestypes.FooCollectorName, - ) - app.TxFeesKeeper = &txFeesKeeper - */ - mempoolFeeOpts := types.NewDefaultMempoolFeeOptions() mempoolFeeOpts.MinGasPriceForHighGasTx = sdk.MustNewDecFromStr("0.0025") baseDenom, _ := suite.app.TxFeesKeeper.GetBaseDenom(suite.ctx) @@ -192,15 +180,50 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { }, } + // TxBuilder Setup Start + + priv0, _, addr0 := testdata.KeyTestPubAddr() + acc1 := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr0) + suite.app.AccountKeeper.SetAccount(suite.ctx, acc1) + msgs := []sdk.Msg{testdata.NewTestMsg(addr0)} + gasLimit := testdata.NewTestGasLimit() + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0} + signerData := authsigning.SignerData{ + ChainID: suite.ctx.ChainID(), + AccountNumber: accNums[0], + Sequence: accSeqs[0]} + + // TxBuilder Setup End + for _, tc := range tests { suite.ctx = suite.ctx.WithIsCheckTx(tc.isCheckTx) suite.ctx = suite.ctx.WithMinGasPrices(tc.minGasPrices) + txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() + + sigV2, _ := clienttx.SignWithPrivKey( + 1, + signerData, + txBuilder, + privs[0], + suite.clientCtx.TxConfig, + accSeqs[0]) + + txBuilder.SetMsgs(msgs[0]) + txBuilder.SetSignatures(sigV2) + txBuilder.SetMemo("") + txBuilder.SetFeeAmount(tc.txFee) + txBuilder.SetGasLimit(gasLimit) + + tx := txBuilder.GetTx() + + /* Legacy test tx (kept for reference) tx := legacytx.NewStdTx([]sdk.Msg{}, legacytx.NewStdFee( tc.gasRequested, tc.txFee, ), []legacytx.StdSignature{}, "") + */ mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper, mempoolFeeOpts) antehandlerMFD := sdk.ChainAnteDecorators(mfd) @@ -211,7 +234,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.Require().Error(err, "test: %s", tc.name) } - // DeductFeeDecorator tests: + /* DeductFeeDecorator tests: // Does FeeGrantKeeper need to be added to the TxFeesKeeper struct? @@ -230,5 +253,6 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { } else { suite.Require().Error(err, "test: %s", tc.name) } + */ } } From d8fbd81080cc7f1b56523dd10f85cc5e5471aa0e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 28 Mar 2022 13:55:32 -0600 Subject: [PATCH 24/75] updates --- x/txfees/keeper/keeper_test.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/x/txfees/keeper/keeper_test.go b/x/txfees/keeper/keeper_test.go index 6643b41fdc2..24e14147897 100644 --- a/x/txfees/keeper/keeper_test.go +++ b/x/txfees/keeper/keeper_test.go @@ -13,7 +13,7 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" - "github.com/osmosis-labs/osmosis/v7/app" + osmosisapp "github.com/osmosis-labs/osmosis/v7/app" balancertypes "github.com/osmosis-labs/osmosis/v7/x/gamm/pool-models/balancer" gammtypes "github.com/osmosis-labs/osmosis/v7/x/gamm/types" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" @@ -23,7 +23,7 @@ type KeeperTestSuite struct { suite.Suite ctx sdk.Context - app *app.OsmosisApp + app *osmosisapp.OsmosisApp clientCtx client.Context @@ -36,8 +36,12 @@ var ( acc3 = sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address().Bytes()) ) +func TestKeeperTestSuite(t *testing.T) { + suite.Run(t, new(KeeperTestSuite)) +} + func (suite *KeeperTestSuite) SetupTest(isCheckTx bool) { - app := app.Setup(isCheckTx) + app := osmosisapp.Setup(isCheckTx) ctx := app.BaseApp.NewContext(isCheckTx, tmproto.Header{Height: 1, ChainID: "osmosis-1", Time: time.Now().UTC()}) queryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry()) @@ -49,6 +53,13 @@ func (suite *KeeperTestSuite) SetupTest(isCheckTx bool) { suite.queryClient = queryClient + encodingConfig := osmosisapp.MakeEncodingConfig() + suite.clientCtx = client.Context{}. + WithInterfaceRegistry(encodingConfig.InterfaceRegistry). + WithTxConfig(encodingConfig.TxConfig). + WithLegacyAmino(encodingConfig.Amino). + WithJSONCodec(encodingConfig.Marshaler) + // Mint some assets to the accounts. for _, acc := range []sdk.AccAddress{acc1, acc2, acc3} { err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, @@ -65,10 +76,6 @@ func (suite *KeeperTestSuite) SetupTest(isCheckTx bool) { } } -func TestKeeperTestSuite(t *testing.T) { - suite.Run(t, new(KeeperTestSuite)) -} - func (suite *KeeperTestSuite) ExecuteUpgradeFeeTokenProposal(feeToken string, poolId uint64) error { upgradeProp := types.NewUpdateFeeTokenProposal( "Test Proposal", From 80025323b310549dddde14b8299961f7293b695f Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 13:16:27 -0700 Subject: [PATCH 25/75] update txbuilder setup --- x/txfees/keeper/feedecorator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index c7549ae6e98..12bd32250bf 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -186,7 +186,6 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { acc1 := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr0) suite.app.AccountKeeper.SetAccount(suite.ctx, acc1) msgs := []sdk.Msg{testdata.NewTestMsg(addr0)} - gasLimit := testdata.NewTestGasLimit() privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0} signerData := authsigning.SignerData{ ChainID: suite.ctx.ChainID(), @@ -202,6 +201,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() + gasLimit := tc.gasRequested sigV2, _ := clienttx.SignWithPrivKey( 1, signerData, From 53712296f07e3e2064fff202ded49160773af078 Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 14:27:35 -0700 Subject: [PATCH 26/75] update feedecorator test for sanity check on dfd --- x/txfees/keeper/feedecorator.go | 2 - x/txfees/keeper/feedecorator_test.go | 62 +++++++++------------------- 2 files changed, 19 insertions(+), 45 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index eb97fb01c0e..574692bfe5d 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -222,14 +222,12 @@ func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ct // checks if input fee is uOSMO (assumes only one fee token exists in the fees array (as per the check in mempoolFeeDecorator)) if fees[0].Denom == baseDenom { - // sends to FeeCollectorName module account err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) if err != nil { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } } else { - // sends to FooCollectorName module account err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FooCollectorName, fees) if err != nil { diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 12bd32250bf..c6717fe915e 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -1,16 +1,14 @@ package keeper_test import ( - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/osmosis-labs/osmosis/v7/x/txfees/types" - - // "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" - "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" - clienttx "github.com/cosmos/cosmos-sdk/client/tx" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" + "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) func (suite *KeeperTestSuite) TestFeeDecorator() { @@ -180,26 +178,20 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { }, } - // TxBuilder Setup Start - - priv0, _, addr0 := testdata.KeyTestPubAddr() - acc1 := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr0) - suite.app.AccountKeeper.SetAccount(suite.ctx, acc1) - msgs := []sdk.Msg{testdata.NewTestMsg(addr0)} - privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0} - signerData := authsigning.SignerData{ - ChainID: suite.ctx.ChainID(), - AccountNumber: accNums[0], - Sequence: accSeqs[0]} - - // TxBuilder Setup End - for _, tc := range tests { - suite.ctx = suite.ctx.WithIsCheckTx(tc.isCheckTx) suite.ctx = suite.ctx.WithMinGasPrices(tc.minGasPrices) txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() + priv0, _, addr0 := testdata.KeyTestPubAddr() + acc1 := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr0) + suite.app.AccountKeeper.SetAccount(suite.ctx, acc1) + msgs := []sdk.Msg{testdata.NewTestMsg(addr0)} + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0} + signerData := authsigning.SignerData{ + ChainID: suite.ctx.ChainID(), + AccountNumber: accNums[0], + Sequence: accSeqs[0]} gasLimit := tc.gasRequested sigV2, _ := clienttx.SignWithPrivKey( @@ -210,6 +202,10 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.clientCtx.TxConfig, accSeqs[0]) + // DEBUG: fmt.Print("\nbalances pre-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) + simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, tc.txFee) + // DEBUG: fmt.Print("\nbalances post-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) + txBuilder.SetMsgs(msgs[0]) txBuilder.SetSignatures(sigV2) txBuilder.SetMemo("") @@ -218,29 +214,10 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { tx := txBuilder.GetTx() - /* Legacy test tx (kept for reference) - tx := legacytx.NewStdTx([]sdk.Msg{}, legacytx.NewStdFee( - tc.gasRequested, - tc.txFee, - ), []legacytx.StdSignature{}, "") - */ - mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper, mempoolFeeOpts) - antehandlerMFD := sdk.ChainAnteDecorators(mfd) - _, err := antehandlerMFD(suite.ctx, tx, false) - if tc.expectPass { - suite.Require().NoError(err, "test: %s", tc.name) - } else { - suite.Require().Error(err, "test: %s", tc.name) - } - - /* DeductFeeDecorator tests: - - // Does FeeGrantKeeper need to be added to the TxFeesKeeper struct? - dfd := keeper.NewDeductFeeDecorator(*suite.app.TxFeesKeeper, *suite.app.AccountKeeper, *suite.app.BankKeeper, *suite.app.FeeGrantKeeper) - antehandlerDFD := sdk.ChainAnteDecorators(dfd) - _, err = antehandlerDFD(suite.ctx, tx, false) + antehandlerMFD := sdk.ChainAnteDecorators(mfd, dfd) + _, err := antehandlerMFD(suite.ctx, tx, false) if tc.expectPass { if tc.baseDenomGas { // check main module account osmo balance & require no non-OSMO @@ -253,6 +230,5 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { } else { suite.Require().Error(err, "test: %s", tc.name) } - */ } } From 4f1fded325d7354e581267f277383ed07de20879 Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 20:41:18 -0700 Subject: [PATCH 27/75] working second module acc tx tests --- x/txfees/keeper/feedecorator_test.go | 33 ++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index c6717fe915e..52dfa57e66c 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -1,6 +1,8 @@ package keeper_test import ( + "fmt" + clienttx "github.com/cosmos/cosmos-sdk/client/tx" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/simapp" @@ -202,9 +204,18 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.clientCtx.TxConfig, accSeqs[0]) - // DEBUG: fmt.Print("\nbalances pre-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) + /* DEBUG: + fmt.Print("\nbalances pre-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) + //fmt.Print("\nbalances pre-fund paid fee: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, tc.txFee[0].Denom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) + if !tc.txFee.IsZero() { + fmt.Print("\nDirect fee print: ", tc.txFee[0]) + } + // all fees are always in 0th index of tc.txFee + */ + simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, tc.txFee) - // DEBUG: fmt.Print("\nbalances post-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) + // DEBUG: + //fmt.Print("\nbalances post-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) txBuilder.SetMsgs(msgs[0]) txBuilder.SetSignatures(sigV2) @@ -214,17 +225,31 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { tx := txBuilder.GetTx() + // get osmo module acc fee balance + // get foo module acc fee balance + moduleAddr1 := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) + moduleAddr2 := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) + fmt.Print("\nModule balance pre-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr1, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr2, uion)) + mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper, mempoolFeeOpts) dfd := keeper.NewDeductFeeDecorator(*suite.app.TxFeesKeeper, *suite.app.AccountKeeper, *suite.app.BankKeeper, *suite.app.FeeGrantKeeper) antehandlerMFD := sdk.ChainAnteDecorators(mfd, dfd) _, err := antehandlerMFD(suite.ctx, tx, false) + fmt.Print("\nModule balance post-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr1, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr2, uion)) + if tc.expectPass { - if tc.baseDenomGas { + if tc.baseDenomGas && !tc.txFee.IsZero() { // check main module account osmo balance & require no non-OSMO // also require that the balance of the second module account (just get AllBalances) has not changed (suite.Require().Equalf(123, 123, "error message %s", "formatted")) - } else { + moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) + suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, baseDenom), tc.name) + suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FeeCollectorName, addr0, tc.txFee) + } else if !tc.txFee.IsZero() { // check second module account to make sure the right fee amount has flowed there // also require that the balance of the main module account (just get AllBalances) has not changed (suite.Require().Equalf(123, 123, "error message %s", "formatted")) + moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) + suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, tc.txFee[0].Denom), tc.name) + suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FooCollectorName, addr0, tc.txFee) } suite.Require().NoError(err, "test: %s", tc.name) } else { From b1b02cf70ea10900bf97e22d0613523b46d9f785 Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 20:43:00 -0700 Subject: [PATCH 28/75] clean up debugging and comments --- x/txfees/keeper/feedecorator_test.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 52dfa57e66c..ba38d135e59 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -1,8 +1,6 @@ package keeper_test import ( - "fmt" - clienttx "github.com/cosmos/cosmos-sdk/client/tx" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/simapp" @@ -204,18 +202,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.clientCtx.TxConfig, accSeqs[0]) - /* DEBUG: - fmt.Print("\nbalances pre-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) - //fmt.Print("\nbalances pre-fund paid fee: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, tc.txFee[0].Denom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) - if !tc.txFee.IsZero() { - fmt.Print("\nDirect fee print: ", tc.txFee[0]) - } - // all fees are always in 0th index of tc.txFee - */ - simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, tc.txFee) - // DEBUG: - //fmt.Print("\nbalances post-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, addr0, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, addr0, uion)) txBuilder.SetMsgs(msgs[0]) txBuilder.SetSignatures(sigV2) @@ -225,28 +212,17 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { tx := txBuilder.GetTx() - // get osmo module acc fee balance - // get foo module acc fee balance - moduleAddr1 := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) - moduleAddr2 := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) - fmt.Print("\nModule balance pre-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr1, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr2, uion)) - mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper, mempoolFeeOpts) dfd := keeper.NewDeductFeeDecorator(*suite.app.TxFeesKeeper, *suite.app.AccountKeeper, *suite.app.BankKeeper, *suite.app.FeeGrantKeeper) antehandlerMFD := sdk.ChainAnteDecorators(mfd, dfd) _, err := antehandlerMFD(suite.ctx, tx, false) - fmt.Print("\nModule balance post-fund: ", suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr1, baseDenom), suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr2, uion)) if tc.expectPass { if tc.baseDenomGas && !tc.txFee.IsZero() { - // check main module account osmo balance & require no non-OSMO - // also require that the balance of the second module account (just get AllBalances) has not changed (suite.Require().Equalf(123, 123, "error message %s", "formatted")) moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, baseDenom), tc.name) suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FeeCollectorName, addr0, tc.txFee) } else if !tc.txFee.IsZero() { - // check second module account to make sure the right fee amount has flowed there - // also require that the balance of the main module account (just get AllBalances) has not changed (suite.Require().Equalf(123, 123, "error message %s", "formatted")) moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, tc.txFee[0].Denom), tc.name) suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FooCollectorName, addr0, tc.txFee) From 2ddddd21c922401c617ce7e5b9a1fbbc30c0b54c Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 21:09:25 -0700 Subject: [PATCH 29/75] remove superfluous comments --- x/txfees/keeper/hooks.go | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 2f886a8fb5f..0571c14d10d 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -1,9 +1,6 @@ package keeper import ( - // "fmt" - - // "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" epochstypes "github.com/osmosis-labs/osmosis/v7/x/epochs/types" txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" @@ -15,34 +12,24 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - // get module address for FooCollectorName addrFoo := k.accountKeeper.GetModuleAddress(txfeestypes.FooCollectorName) - // get balances for all denoms in the module account fooAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrFoo) - // pulls base denom from TxFeesKeeper (should be uOSMO) baseDenom, _ := k.GetBaseDenom(ctx) - // iterate through the resulting array and swap each denom into OSMO using the GAMM module for _, coin := range fooAccountBalances { if coin.Denom == baseDenom { continue } else { - // TO DO: figure out how to cast this or get the pool ID for the main OSMO paired pool feetoken, _ := k.GetFeeToken(ctx, coin.Denom) - // swap into OSMO - // question: is spotPrice really the minimum out for the swap? k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) } } - - // Potential error to test for: do swaps to OSMO consolidate into a single denom in fooAccountBalances? Should be yes but keep an eye out - - // send all OSMO to fee module account to be distributed in the next block + k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.FooCollectorName, txfeestypes.FeeCollectorName, fooAccountBalances) // Should events be emitted at the end here? From 634c2b8e28544896a097fbd6f2a0e4235ed4914c Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 23:11:36 -0700 Subject: [PATCH 30/75] add tests for hooks.go --- x/txfees/keeper/feedecorator_test.go | 1 + x/txfees/keeper/hooks.go | 2 + x/txfees/keeper/hooks_test.go | 82 ++++++++++++++++++++++++++++ x/txfees/keeper/keeper_test.go | 2 + 4 files changed, 87 insertions(+) create mode 100644 x/txfees/keeper/hooks_test.go diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index ba38d135e59..776d7e5266a 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -182,6 +182,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.ctx = suite.ctx.WithIsCheckTx(tc.isCheckTx) suite.ctx = suite.ctx.WithMinGasPrices(tc.minGasPrices) + // TxBuilder components reset for every test case txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() priv0, _, addr0 := testdata.KeyTestPubAddr() acc1 := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr0) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 0571c14d10d..196c2ed063a 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -29,6 +29,8 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) } } + + fooAccountBalances = k.bankKeeper.GetAllBalances(ctx, addrFoo) k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.FooCollectorName, txfeestypes.FeeCollectorName, fooAccountBalances) diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go new file mode 100644 index 00000000000..1f11e23b624 --- /dev/null +++ b/x/txfees/keeper/hooks_test.go @@ -0,0 +1,82 @@ +package keeper_test + +import ( + // "time" + "fmt" + "time" + + "github.com/cosmos/cosmos-sdk/simapp" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/osmosis-labs/osmosis/v7/x/txfees/types" +) + +func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { + suite.SetupTest(false) + baseDenom, _ := suite.app.TxFeesKeeper.GetBaseDenom(suite.ctx) + + testCases := []struct { + name string + }{ + { + name: "tc name", + }, + } + + for _, tc := range testCases { + + fmt.Print("\n", tc) + + // we create three pools for three separate fee tokens + uion := "uion" + uionPoolId := suite.PreparePoolWithAssets( + sdk.NewInt64Coin(baseDenom, 500), + sdk.NewInt64Coin(uion, 500), + ) + suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) + + atom := "atom" + atomPoolId := suite.PreparePoolWithAssets( + sdk.NewInt64Coin(baseDenom, 500), + sdk.NewInt64Coin(atom, 500), + ) + suite.ExecuteUpgradeFeeTokenProposal(atom, atomPoolId) + + ust := "ust" + ustPoolId := suite.PreparePoolWithAssets( + sdk.NewInt64Coin(baseDenom, 500), + sdk.NewInt64Coin(ust, 500), + ) + suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) + + coins := sdk.NewCoins( + sdk.NewInt64Coin(uion, 10), + sdk.NewInt64Coin(atom, 20), + sdk.NewInt64Coin(ust, 14), + ) + + _, _, addr0 := testdata.KeyTestPubAddr() + simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) + suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.FooCollectorName, coins) + + moduleAddrFee := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) + moduleAddrFoo := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) + + fmt.Print("\n(pre) Main module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFee)) + fmt.Print("\n(pre) Second module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) + + // make sure module account is funded with test fee tokens + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[0])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[1])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[2])) + + params := suite.app.IncentivesKeeper.GetParams(suite.ctx) + futureCtx := suite.ctx.WithBlockTime(time.Now().Add(time.Minute)) + suite.app.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) + + suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) + + fmt.Print("\n(post) Main module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFee)) + fmt.Print("\n(post) Second module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) + } +} diff --git a/x/txfees/keeper/keeper_test.go b/x/txfees/keeper/keeper_test.go index 24e14147897..182d77fb488 100644 --- a/x/txfees/keeper/keeper_test.go +++ b/x/txfees/keeper/keeper_test.go @@ -67,6 +67,8 @@ func (suite *KeeperTestSuite) SetupTest(isCheckTx bool) { sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000000000)), sdk.NewCoin("uosmo", sdk.NewInt(100000000000000000)), // Needed for pool creation fee sdk.NewCoin("uion", sdk.NewInt(10000000)), + sdk.NewCoin("atom", sdk.NewInt(10000000)), + sdk.NewCoin("ust", sdk.NewInt(10000000)), sdk.NewCoin("foo", sdk.NewInt(10000000)), sdk.NewCoin("bar", sdk.NewInt(10000000)), )) From 573f48c414ba72a62c6744b2b2485cb5e373fa2e Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 28 Mar 2022 23:53:18 -0700 Subject: [PATCH 31/75] complete fee swap and distribution tests --- x/gamm/keeper/math.go | 16 ++++ x/txfees/keeper/hooks_test.go | 146 +++++++++++++++++++--------------- 2 files changed, 97 insertions(+), 65 deletions(-) diff --git a/x/gamm/keeper/math.go b/x/gamm/keeper/math.go index 7d3bf67d45d..293fdd2fe93 100644 --- a/x/gamm/keeper/math.go +++ b/x/gamm/keeper/math.go @@ -98,6 +98,22 @@ func calcOutGivenIn( return tokenAmountOut } +// exported version +func CalcOutGivenIn( + tokenBalanceIn, + tokenWeightIn, + tokenBalanceOut, + tokenWeightOut, + tokenAmountIn, + swapFee sdk.Dec, +) sdk.Dec { + // deduct swapfee on the in asset + tokenAmountInAfterFee := tokenAmountIn.Mul(sdk.OneDec().Sub(swapFee)) + // delta balanceOut is positive(tokens inside the pool decreases) + tokenAmountOut := solveConstantFunctionInvariant(tokenBalanceIn, tokenBalanceIn.Add(tokenAmountInAfterFee), tokenWeightIn, tokenBalanceOut, tokenWeightOut) + return tokenAmountOut +} + // calcInGivenOut calculates token to be provided, fee added, // given the swapped out amount, using solveConstantFunctionInvariant func calcInGivenOut( diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index 1f11e23b624..23b28e76185 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -2,12 +2,13 @@ package keeper_test import ( // "time" - "fmt" + "time" "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + gamm "github.com/osmosis-labs/osmosis/v7/x/gamm/keeper" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) @@ -15,68 +16,83 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { suite.SetupTest(false) baseDenom, _ := suite.app.TxFeesKeeper.GetBaseDenom(suite.ctx) - testCases := []struct { - name string - }{ - { - name: "tc name", - }, - } - - for _, tc := range testCases { - - fmt.Print("\n", tc) - - // we create three pools for three separate fee tokens - uion := "uion" - uionPoolId := suite.PreparePoolWithAssets( - sdk.NewInt64Coin(baseDenom, 500), - sdk.NewInt64Coin(uion, 500), - ) - suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) - - atom := "atom" - atomPoolId := suite.PreparePoolWithAssets( - sdk.NewInt64Coin(baseDenom, 500), - sdk.NewInt64Coin(atom, 500), - ) - suite.ExecuteUpgradeFeeTokenProposal(atom, atomPoolId) - - ust := "ust" - ustPoolId := suite.PreparePoolWithAssets( - sdk.NewInt64Coin(baseDenom, 500), - sdk.NewInt64Coin(ust, 500), - ) - suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) - - coins := sdk.NewCoins( - sdk.NewInt64Coin(uion, 10), - sdk.NewInt64Coin(atom, 20), - sdk.NewInt64Coin(ust, 14), - ) - - _, _, addr0 := testdata.KeyTestPubAddr() - simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) - suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.FooCollectorName, coins) - - moduleAddrFee := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) - moduleAddrFoo := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) - - fmt.Print("\n(pre) Main module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFee)) - fmt.Print("\n(pre) Second module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) - - // make sure module account is funded with test fee tokens - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[0])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[1])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[2])) - - params := suite.app.IncentivesKeeper.GetParams(suite.ctx) - futureCtx := suite.ctx.WithBlockTime(time.Now().Add(time.Minute)) - suite.app.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) - - suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) - - fmt.Print("\n(post) Main module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFee)) - fmt.Print("\n(post) Second module acc balances: ", suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) - } + // create pools for three separate fee tokens + + defaultPooledAssetAmount := int64(500) + + uion := "uion" + uionPoolId := suite.PreparePoolWithAssets( + sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), + sdk.NewInt64Coin(uion, defaultPooledAssetAmount), + ) + suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) + + atom := "atom" + atomPoolId := suite.PreparePoolWithAssets( + sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), + sdk.NewInt64Coin(atom, defaultPooledAssetAmount), + ) + suite.ExecuteUpgradeFeeTokenProposal(atom, atomPoolId) + + ust := "ust" + ustPoolId := suite.PreparePoolWithAssets( + sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), + sdk.NewInt64Coin(ust, defaultPooledAssetAmount), + ) + suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) + + coins := sdk.NewCoins( + sdk.NewInt64Coin(uion, 10), + sdk.NewInt64Coin(atom, 20), + sdk.NewInt64Coin(ust, 14), + ) + + gamm.CalcOutGivenIn(sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(coins[0].Amount), + sdk.NewDec(0), + ) + + expectedOutput1 := gamm.CalcOutGivenIn(sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(coins[0].Amount), + sdk.NewDec(0)).TruncateInt() + expectedOutput2 := gamm.CalcOutGivenIn(sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(coins[1].Amount), + sdk.NewDec(0)).TruncateInt() + expectedOutput3 := gamm.CalcOutGivenIn(sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(sdk.NewInt(defaultPooledAssetAmount)), + sdk.OneDec(), + sdk.NewDecFromInt(coins[2].Amount), + sdk.NewDec(0)).TruncateInt() + + fullExpectedOutput := expectedOutput1.Add(expectedOutput2).Add(expectedOutput3) + + _, _, addr0 := testdata.KeyTestPubAddr() + simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) + suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.FooCollectorName, coins) + + moduleAddrFee := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) + moduleAddrFoo := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) + + // make sure module account is funded with test fee tokens + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[0])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[1])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[2])) + + params := suite.app.IncentivesKeeper.GetParams(suite.ctx) + futureCtx := suite.ctx.WithBlockTime(time.Now().Add(time.Minute)) + + suite.app.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) + + suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) + suite.Require().True(suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddrFee, baseDenom).Amount.GTE(fullExpectedOutput)) } From a611c28db82d52e599a35262bd7c1fc281cb5f5b Mon Sep 17 00:00:00 2001 From: alpo Date: Tue, 29 Mar 2022 00:01:07 -0700 Subject: [PATCH 32/75] clean up comments --- x/txfees/keeper/hooks_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index 23b28e76185..d7efb6442a3 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -1,8 +1,6 @@ package keeper_test import ( - // "time" - "time" "github.com/cosmos/cosmos-sdk/simapp" From 5ffcf2790f2b6babbbd81f6b52c0747082362ad4 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Tue, 29 Mar 2022 00:03:14 -0700 Subject: [PATCH 33/75] remove import comments app/keepers.go Co-authored-by: Aleksandr Bezobchuk --- app/keepers.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index d09ffca34ae..99c3a0f82a1 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -55,11 +55,7 @@ import ( // Staking: Allows the Tendermint validator set to be chosen based on bonded stake. stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - - // Staking: Allows the Tendermint validator set to be chosen based on bonded stake. feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" - - // Upgrade: Software upgrades handling and coordination. "github.com/cosmos/cosmos-sdk/x/upgrade" upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" From 38a9609afba95aafc09891db309e46ce69dcc3b1 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Tue, 29 Mar 2022 20:10:25 -0700 Subject: [PATCH 34/75] Apply suggestions from code review Co-authored-by: Matt, Park <45252226+mattverse@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk --- app/keepers.go | 1 - x/gamm/keeper/math.go | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index 99c3a0f82a1..311b64bfd6e 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -372,7 +372,6 @@ func (app *OsmosisApp) InitNormalKeepers( ) app.TxFeesKeeper = &txFeesKeeper - // Note: gammKeeper is expected to satisfy the SpotPriceCalculator interface parameter feeGrantKeeper := feegrantkeeper.NewKeeper( appCodec, keys[txfeestypes.StoreKey], diff --git a/x/gamm/keeper/math.go b/x/gamm/keeper/math.go index 293fdd2fe93..ec8220e0ca1 100644 --- a/x/gamm/keeper/math.go +++ b/x/gamm/keeper/math.go @@ -98,7 +98,7 @@ func calcOutGivenIn( return tokenAmountOut } -// exported version +// CalcOutGivenIn ... func CalcOutGivenIn( tokenBalanceIn, tokenWeightIn, @@ -110,7 +110,13 @@ func CalcOutGivenIn( // deduct swapfee on the in asset tokenAmountInAfterFee := tokenAmountIn.Mul(sdk.OneDec().Sub(swapFee)) // delta balanceOut is positive(tokens inside the pool decreases) - tokenAmountOut := solveConstantFunctionInvariant(tokenBalanceIn, tokenBalanceIn.Add(tokenAmountInAfterFee), tokenWeightIn, tokenBalanceOut, tokenWeightOut) + tokenAmountOut := solveConstantFunctionInvariant( + tokenBalanceIn, + tokenBalanceIn.Add(tokenAmountInAfterFee), + tokenWeightIn, + tokenBalanceOut, + tokenWeightOut, + ) return tokenAmountOut } From f03ca7ec6b8f059bee7056c23302f9f77f2518c2 Mon Sep 17 00:00:00 2001 From: alpo Date: Tue, 29 Mar 2022 20:23:05 -0700 Subject: [PATCH 35/75] change name for second module account to AltFeeCollector --- app/keepers.go | 4 ++-- app/modules.go | 2 +- x/txfees/keeper/feedecorator.go | 8 ++++---- x/txfees/keeper/feedecorator_test.go | 4 ++-- x/txfees/keeper/hooks.go | 12 ++++++------ x/txfees/keeper/hooks_test.go | 12 ++++++------ x/txfees/keeper/keeper.go | 6 +++--- x/txfees/types/keys.go | 4 ++-- 8 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index 311b64bfd6e..dbbf9d2bfc6 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -53,9 +53,9 @@ import ( slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" // Staking: Allows the Tendermint validator set to be chosen based on bonded stake. + feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" "github.com/cosmos/cosmos-sdk/x/upgrade" upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" @@ -368,7 +368,7 @@ func (app *OsmosisApp) InitNormalKeepers( app.GAMMKeeper, app.GAMMKeeper, txfeestypes.FeeCollectorName, - txfeestypes.FooCollectorName, + txfeestypes.AltFeeCollectorName, ) app.TxFeesKeeper = &txFeesKeeper diff --git a/app/modules.go b/app/modules.go index 26a62ae6352..944f198dd0d 100644 --- a/app/modules.go +++ b/app/modules.go @@ -186,7 +186,7 @@ var moduleAaccountPermissions = map[string][]string{ poolincentivestypes.ModuleName: nil, superfluidtypes.ModuleName: {authtypes.Minter, authtypes.Burner}, txfeestypes.ModuleName: nil, - txfeestypes.FooCollectorName: nil, + txfeestypes.AltFeeCollectorName: nil, wasm.ModuleName: {authtypes.Burner}, } diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 574692bfe5d..b3f6ba7a399 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -158,8 +158,8 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo } // checks to make sure a separate module account has been set to collect fees not in OSMO - if addrFoo := dfd.ak.GetModuleAddress(types.FooCollectorName); addrFoo == nil { - return ctx, fmt.Errorf("Foo collector module account (%s) has not been set", types.FooCollectorName) + if addrAltFee := dfd.ak.GetModuleAddress(types.AltFeeCollectorName); addrAltFee == nil { + return ctx, fmt.Errorf("Alt Fee collector module account (%s) has not been set", types.AltFeeCollectorName) } // fee can be in any denom (checked for validity later) @@ -228,8 +228,8 @@ func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ct return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } } else { - // sends to FooCollectorName module account - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FooCollectorName, fees) + // sends to AltFeeCollectorName module account + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.AltFeeCollectorName, fees) if err != nil { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 776d7e5266a..29087ab7b83 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -224,9 +224,9 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, baseDenom), tc.name) suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FeeCollectorName, addr0, tc.txFee) } else if !tc.txFee.IsZero() { - moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) + moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.AltFeeCollectorName) suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, tc.txFee[0].Denom), tc.name) - suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FooCollectorName, addr0, tc.txFee) + suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.AltFeeCollectorName, addr0, tc.txFee) } suite.Require().NoError(err, "test: %s", tc.name) } else { diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 196c2ed063a..669b2fed172 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -12,13 +12,13 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - addrFoo := k.accountKeeper.GetModuleAddress(txfeestypes.FooCollectorName) + addrAltFee := k.accountKeeper.GetModuleAddress(txfeestypes.AltFeeCollectorName) - fooAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrFoo) + altFeeAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrAltFee) baseDenom, _ := k.GetBaseDenom(ctx) - for _, coin := range fooAccountBalances { + for _, coin := range altFeeAccountBalances { if coin.Denom == baseDenom { continue @@ -26,13 +26,13 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb feetoken, _ := k.GetFeeToken(ctx, coin.Denom) - k.gammKeeper.SwapExactAmountIn(ctx, addrFoo, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) + k.gammKeeper.SwapExactAmountIn(ctx, addrAltFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) } } - fooAccountBalances = k.bankKeeper.GetAllBalances(ctx, addrFoo) + altFeeAccountBalances = k.bankKeeper.GetAllBalances(ctx, addrAltFee) - k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.FooCollectorName, txfeestypes.FeeCollectorName, fooAccountBalances) + k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.AltFeeCollectorName, txfeestypes.FeeCollectorName, altFeeAccountBalances) // Should events be emitted at the end here? } diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index d7efb6442a3..b1eeb4b3c20 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -76,21 +76,21 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { _, _, addr0 := testdata.KeyTestPubAddr() simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) - suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.FooCollectorName, coins) + suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.AltFeeCollectorName, coins) moduleAddrFee := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) - moduleAddrFoo := suite.app.AccountKeeper.GetModuleAddress(types.FooCollectorName) + moduleAddrAltFee := suite.app.AccountKeeper.GetModuleAddress(types.AltFeeCollectorName) // make sure module account is funded with test fee tokens - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[0])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[1])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrFoo, coins[2])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrAltFee, coins[0])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrAltFee, coins[1])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrAltFee, coins[2])) params := suite.app.IncentivesKeeper.GetParams(suite.ctx) futureCtx := suite.ctx.WithBlockTime(time.Now().Add(time.Minute)) suite.app.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) - suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrFoo)) + suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrAltFee)) suite.Require().True(suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddrFee, baseDenom).Amount.GTE(fullExpectedOutput)) } diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index cd0b7a30450..1c3965f2590 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -27,7 +27,7 @@ type ( spotPriceCalculator types.SpotPriceCalculator feeCollectorName string - fooCollectorName string + altFeeCollectorName string } ) @@ -40,7 +40,7 @@ func NewKeeper( gammKeeper types.GammKeeper, spotPriceCalculator types.SpotPriceCalculator, feeCollectorName string, - fooCollectorName string, + altFeeCollectorName string, ) Keeper { return Keeper{ cdc: cdc, @@ -51,7 +51,7 @@ func NewKeeper( gammKeeper: gammKeeper, spotPriceCalculator: spotPriceCalculator, feeCollectorName: feeCollectorName, - fooCollectorName: fooCollectorName, + altFeeCollectorName: altFeeCollectorName, } } diff --git a/x/txfees/types/keys.go b/x/txfees/types/keys.go index 8b4e11ca17b..507766a5240 100644 --- a/x/txfees/types/keys.go +++ b/x/txfees/types/keys.go @@ -17,8 +17,8 @@ const ( // FeeCollectorName the root string for the fee collector account address FeeCollectorName = "fee_collector" - // FooCollectorName the root string for the $FOO collector account address (used for auto-swapping non-OSMO tx fees) - FooCollectorName = "foo_collector" + // AltFeeCollectorName the root string for the alt fee collector account address (used for auto-swapping non-OSMO tx fees) + AltFeeCollectorName = "alt_fee_collector" // QuerierRoute defines the module's query routing key QuerierRoute = ModuleName From 5cda003c86954626b8bebab5039e6e43782629a8 Mon Sep 17 00:00:00 2001 From: alpo Date: Tue, 29 Mar 2022 20:27:01 -0700 Subject: [PATCH 36/75] go docs for CalcOutGivenIn function --- x/gamm/keeper/math.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x/gamm/keeper/math.go b/x/gamm/keeper/math.go index ec8220e0ca1..4e765147f40 100644 --- a/x/gamm/keeper/math.go +++ b/x/gamm/keeper/math.go @@ -98,7 +98,9 @@ func calcOutGivenIn( return tokenAmountOut } -// CalcOutGivenIn ... +// CalcOutGivenIn calculates token to be provided, fee added, +// given the swapped out amount, using solveConstantFunctionInvariant. +// This version is exported and can be used to calculate the exact output of a swap. func CalcOutGivenIn( tokenBalanceIn, tokenWeightIn, From dccb1ddf70483472b9abe7f9b7092e29d90d5292 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 30 Mar 2022 10:46:11 -0400 Subject: [PATCH 37/75] updates --- app/keepers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/keepers.go b/app/keepers.go index dbbf9d2bfc6..fb556a98156 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -157,7 +157,7 @@ type appKeepers struct { MintKeeper *mintkeeper.Keeper PoolIncentivesKeeper *poolincentiveskeeper.Keeper TxFeesKeeper *txfeeskeeper.Keeper - FeeGrantKeeper *feegrantkeeper.Keeper + FeeGrantKeeper *feegrantkeeper.Keeper SuperfluidKeeper *superfluidkeeper.Keeper GovKeeper *govkeeper.Keeper WasmKeeper *wasm.Keeper From 7355d76386af53f78ab6e2eb77df8636c9f66787 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 30 Mar 2022 10:46:43 -0400 Subject: [PATCH 38/75] updates --- app/modules.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/modules.go b/app/modules.go index 944f198dd0d..c6c5a4b8d6a 100644 --- a/app/modules.go +++ b/app/modules.go @@ -186,7 +186,7 @@ var moduleAaccountPermissions = map[string][]string{ poolincentivestypes.ModuleName: nil, superfluidtypes.ModuleName: {authtypes.Minter, authtypes.Burner}, txfeestypes.ModuleName: nil, - txfeestypes.AltFeeCollectorName: nil, + txfeestypes.AltFeeCollectorName: nil, wasm.ModuleName: {authtypes.Burner}, } From 495be0a2262bccdbcc667affa97158dee8acd1e0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 30 Mar 2022 10:47:04 -0400 Subject: [PATCH 39/75] updates --- x/gamm/keeper/math.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x/gamm/keeper/math.go b/x/gamm/keeper/math.go index 4e765147f40..c608f61208b 100644 --- a/x/gamm/keeper/math.go +++ b/x/gamm/keeper/math.go @@ -113,11 +113,11 @@ func CalcOutGivenIn( tokenAmountInAfterFee := tokenAmountIn.Mul(sdk.OneDec().Sub(swapFee)) // delta balanceOut is positive(tokens inside the pool decreases) tokenAmountOut := solveConstantFunctionInvariant( - tokenBalanceIn, - tokenBalanceIn.Add(tokenAmountInAfterFee), - tokenWeightIn, - tokenBalanceOut, - tokenWeightOut, + tokenBalanceIn, + tokenBalanceIn.Add(tokenAmountInAfterFee), + tokenWeightIn, + tokenBalanceOut, + tokenWeightOut, ) return tokenAmountOut } From 7a41952dcdccc5c5a89be928b3933b1ac5e6399a Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Sun, 3 Apr 2022 22:43:46 -0700 Subject: [PATCH 40/75] Apply suggestions from code review Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 1 - x/txfees/types/keys.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index b3f6ba7a399..549eea75f27 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -146,7 +146,6 @@ func NewDeductFeeDecorator(tk Keeper, ak types.AccountKeeper, bk types.BankKeepe } func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - feeTx, ok := tx.(sdk.FeeTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") diff --git a/x/txfees/types/keys.go b/x/txfees/types/keys.go index 507766a5240..deca41f9543 100644 --- a/x/txfees/types/keys.go +++ b/x/txfees/types/keys.go @@ -14,10 +14,10 @@ const ( // RouterKey is the message route for slashing RouterKey = ModuleName - // FeeCollectorName the root string for the fee collector account address + // FeeCollectorName the module account name for the fee collector account address. FeeCollectorName = "fee_collector" - // AltFeeCollectorName the root string for the alt fee collector account address (used for auto-swapping non-OSMO tx fees) + // AltFeeCollectorName the module account name for the alt fee collector account address (used for auto-swapping non-OSMO tx fees). AltFeeCollectorName = "alt_fee_collector" // QuerierRoute defines the module's query routing key From d4836120c0d08324a20a504933861c2317789ab0 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Wed, 6 Apr 2022 11:28:30 -0700 Subject: [PATCH 41/75] Apply suggestions from code review Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 1 - x/txfees/keeper/hooks.go | 13 ++----------- x/txfees/keeper/keeper.go | 2 -- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 549eea75f27..0f509e083be 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -207,7 +207,6 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo // DeductFees deducts fees from the given account and transfers them to the set module account. func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error { - // Checks the validity of the fee tokens (sorted, have positive amount, valid and unique denomination) if !fees.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 669b2fed172..5b3d231660d 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -6,20 +6,15 @@ import ( txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) -func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { -} +func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { } // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - addrAltFee := k.accountKeeper.GetModuleAddress(txfeestypes.AltFeeCollectorName) - altFeeAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrAltFee) - baseDenom, _ := k.GetBaseDenom(ctx) for _, coin := range altFeeAccountBalances { - if coin.Denom == baseDenom { continue } else { @@ -37,7 +32,6 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb // Should events be emitted at the end here? } -// ___________________________________________________________________________________________________ // Hooks wrapper struct for incentives keeper type Hooks struct { @@ -51,10 +45,7 @@ func (k Keeper) Hooks() Hooks { return Hooks{k} } -// epochs hooks -// Don't do anything pre epoch start -func (h Hooks) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { -} +func (h Hooks) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) {} func (h Hooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { h.k.AfterEpochEnd(ctx, epochIdentifier, epochNumber) diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index 1c3965f2590..09ed8ec4a94 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -23,9 +23,7 @@ type ( bankKeeper *bankkeeper.BaseKeeper epochKeeper types.EpochKeeper gammKeeper types.GammKeeper - spotPriceCalculator types.SpotPriceCalculator - feeCollectorName string altFeeCollectorName string } From 82454ddb76a2365033e225e08f394802a30f0ade Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 6 Apr 2022 11:59:23 -0700 Subject: [PATCH 42/75] update second module name --- app/keepers.go | 2 +- app/modules.go | 2 +- x/txfees/keeper/feedecorator.go | 8 ++++---- x/txfees/keeper/feedecorator_test.go | 4 ++-- x/txfees/keeper/hooks.go | 14 ++++++-------- x/txfees/keeper/hooks_test.go | 12 ++++++------ x/txfees/keeper/keeper.go | 6 +++--- x/txfees/types/keys.go | 4 ++-- 8 files changed, 25 insertions(+), 27 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index fb556a98156..eb62c61092f 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -368,7 +368,7 @@ func (app *OsmosisApp) InitNormalKeepers( app.GAMMKeeper, app.GAMMKeeper, txfeestypes.FeeCollectorName, - txfeestypes.AltFeeCollectorName, + txfeestypes.NonNativeFeeCollectorName, ) app.TxFeesKeeper = &txFeesKeeper diff --git a/app/modules.go b/app/modules.go index c6c5a4b8d6a..9ab8467c205 100644 --- a/app/modules.go +++ b/app/modules.go @@ -186,7 +186,7 @@ var moduleAaccountPermissions = map[string][]string{ poolincentivestypes.ModuleName: nil, superfluidtypes.ModuleName: {authtypes.Minter, authtypes.Burner}, txfeestypes.ModuleName: nil, - txfeestypes.AltFeeCollectorName: nil, + txfeestypes.NonNativeFeeCollectorName: nil, wasm.ModuleName: {authtypes.Burner}, } diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 0f509e083be..23cde19420c 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -157,8 +157,8 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo } // checks to make sure a separate module account has been set to collect fees not in OSMO - if addrAltFee := dfd.ak.GetModuleAddress(types.AltFeeCollectorName); addrAltFee == nil { - return ctx, fmt.Errorf("Alt Fee collector module account (%s) has not been set", types.AltFeeCollectorName) + if addrNonNativeFee := dfd.ak.GetModuleAddress(types.NonNativeFeeCollectorName); addrNonNativeFee == nil { + return ctx, fmt.Errorf("Alt Fee collector module account (%s) has not been set", types.NonNativeFeeCollectorName) } // fee can be in any denom (checked for validity later) @@ -226,8 +226,8 @@ func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ct return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } } else { - // sends to AltFeeCollectorName module account - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.AltFeeCollectorName, fees) + // sends to NonNativeFeeCollectorName module account + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.NonNativeFeeCollectorName, fees) if err != nil { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 29087ab7b83..79c06cc6a14 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -224,9 +224,9 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, baseDenom), tc.name) suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FeeCollectorName, addr0, tc.txFee) } else if !tc.txFee.IsZero() { - moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.AltFeeCollectorName) + moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, tc.txFee[0].Denom), tc.name) - suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.AltFeeCollectorName, addr0, tc.txFee) + suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.NonNativeFeeCollectorName, addr0, tc.txFee) } suite.Require().NoError(err, "test: %s", tc.name) } else { diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 5b3d231660d..c1dc66c4bd4 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -10,26 +10,24 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - addrAltFee := k.accountKeeper.GetModuleAddress(txfeestypes.AltFeeCollectorName) - altFeeAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrAltFee) + addrNonNativeFee := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) + nonNativeFeeAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) baseDenom, _ := k.GetBaseDenom(ctx) - for _, coin := range altFeeAccountBalances { + for _, coin := range nonNativeFeeAccountBalances { if coin.Denom == baseDenom { continue } else { feetoken, _ := k.GetFeeToken(ctx, coin.Denom) - k.gammKeeper.SwapExactAmountIn(ctx, addrAltFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) + k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) } } - altFeeAccountBalances = k.bankKeeper.GetAllBalances(ctx, addrAltFee) + nonNativeFeeAccountBalances = k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) - k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.AltFeeCollectorName, txfeestypes.FeeCollectorName, altFeeAccountBalances) - - // Should events be emitted at the end here? + k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBalances) } diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index b1eeb4b3c20..cf39ecd8534 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -76,21 +76,21 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { _, _, addr0 := testdata.KeyTestPubAddr() simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) - suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.AltFeeCollectorName, coins) + suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.NonNativeFeeCollectorName, coins) moduleAddrFee := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) - moduleAddrAltFee := suite.app.AccountKeeper.GetModuleAddress(types.AltFeeCollectorName) + moduleAddrNonNativeFee := suite.app.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) // make sure module account is funded with test fee tokens - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrAltFee, coins[0])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrAltFee, coins[1])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrAltFee, coins[2])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrNonNativeFee, coins[0])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrNonNativeFee, coins[1])) + suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrNonNativeFee, coins[2])) params := suite.app.IncentivesKeeper.GetParams(suite.ctx) futureCtx := suite.ctx.WithBlockTime(time.Now().Add(time.Minute)) suite.app.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) - suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrAltFee)) + suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrNonNativeFee)) suite.Require().True(suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddrFee, baseDenom).Amount.GTE(fullExpectedOutput)) } diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index 09ed8ec4a94..b59678c658b 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -25,7 +25,7 @@ type ( gammKeeper types.GammKeeper spotPriceCalculator types.SpotPriceCalculator feeCollectorName string - altFeeCollectorName string + nonNativeFeeCollectorName string } ) @@ -38,7 +38,7 @@ func NewKeeper( gammKeeper types.GammKeeper, spotPriceCalculator types.SpotPriceCalculator, feeCollectorName string, - altFeeCollectorName string, + nonNativeFeeCollectorName string, ) Keeper { return Keeper{ cdc: cdc, @@ -49,7 +49,7 @@ func NewKeeper( gammKeeper: gammKeeper, spotPriceCalculator: spotPriceCalculator, feeCollectorName: feeCollectorName, - altFeeCollectorName: altFeeCollectorName, + nonNativeFeeCollectorName: nonNativeFeeCollectorName, } } diff --git a/x/txfees/types/keys.go b/x/txfees/types/keys.go index deca41f9543..29de91b5cf0 100644 --- a/x/txfees/types/keys.go +++ b/x/txfees/types/keys.go @@ -17,8 +17,8 @@ const ( // FeeCollectorName the module account name for the fee collector account address. FeeCollectorName = "fee_collector" - // AltFeeCollectorName the module account name for the alt fee collector account address (used for auto-swapping non-OSMO tx fees). - AltFeeCollectorName = "alt_fee_collector" + // NonNativeFeeCollectorName the module account name for the alt fee collector account address (used for auto-swapping non-OSMO tx fees). + NonNativeFeeCollectorName = "non_native_fee_collector" // QuerierRoute defines the module's query routing key QuerierRoute = ModuleName From 0c001f4b549972a15a8fbf3f087e5d30a6c3dab7 Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 6 Apr 2022 12:40:09 -0700 Subject: [PATCH 43/75] add error check for swaps --- x/txfees/keeper/hooks.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index c1dc66c4bd4..ca277442075 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -19,9 +19,10 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb continue } else { - feetoken, _ := k.GetFeeToken(ctx, coin.Denom) - - k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) + feetoken, err := k.GetFeeToken(ctx, coin.Denom) + if err == nil { + _, _, err = k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) + } } } From 5c8ec967b791f1cbf91a0cc7ec302b3586920a4c Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Wed, 6 Apr 2022 12:41:18 -0700 Subject: [PATCH 44/75] Apply comment suggestions from code review Co-authored-by: Dev Ojha --- x/txfees/keeper/hooks.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index ca277442075..78c5b879c37 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -26,7 +26,8 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb } } - nonNativeFeeAccountBalances = k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) + // Get all of the txfee payout denom in the module account + nonNativeFeeAccountBalances = k.bankKeeper.GetBalance(ctx, addrNonNativeFee, basedenom) k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBalances) } From 3cb4fc7dcbebd13d95f2803189c09059d5f9d868 Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 6 Apr 2022 12:49:49 -0700 Subject: [PATCH 45/75] add minTokenOut comments --- x/txfees/keeper/hooks.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 78c5b879c37..0a063a66fc2 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -21,15 +21,20 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb feetoken, err := k.GetFeeToken(ctx, coin.Denom) if err == nil { - _, _, err = k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) + // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, + // but even then the point is a bit moot. + // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. + // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. + minTokenOut := sdk.ZeroInt() + k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, minTokenOut) } } } // Get all of the txfee payout denom in the module account - nonNativeFeeAccountBalances = k.bankKeeper.GetBalance(ctx, addrNonNativeFee, basedenom) + nonNativeFeeAccountBaseDenomBalance := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, addrNonNativeFee, baseDenom)) - k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBalances) + k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBaseDenomBalance) } From d33ae1052fdbd52ae445a93224c3bb536d0f1c33 Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 6 Apr 2022 14:17:46 -0700 Subject: [PATCH 46/75] update swap error checking logic --- x/txfees/keeper/hooks.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 0a063a66fc2..f1fcd1bc180 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -20,14 +20,16 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb } else { feetoken, err := k.GetFeeToken(ctx, coin.Denom) - if err == nil { - // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, - // but even then the point is a bit moot. - // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. - // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. - minTokenOut := sdk.ZeroInt() - k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, minTokenOut) + if err != nil { + continue } + + // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, + // but even then the point is a bit moot. + // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. + // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. + minTokenOut := sdk.ZeroInt() + k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, minTokenOut) } } From 213d32eaa24b2b0dd8bad3a22a215f2e04a84066 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 7 Apr 2022 09:18:02 -0700 Subject: [PATCH 47/75] set up changes for hook tests --- x/txfees/keeper/hooks_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index cf39ecd8534..5b7ac260099 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -23,6 +23,8 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(uion, defaultPooledAssetAmount), ) + uionPoolI, err:= suite.app.GAMMKeeper.GetPool(suite.ctx, uionPoolId) + suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) atom := "atom" @@ -30,6 +32,8 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(atom, defaultPooledAssetAmount), ) + atomPoolI, err:= suite.app.GAMMKeeper.GetPool(suite.ctx, atomPoolId) + suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(atom, atomPoolId) ust := "ust" @@ -37,6 +41,8 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(ust, defaultPooledAssetAmount), ) + ustPoolI, err:= suite.app.GAMMKeeper.GetPool(suite.ctx, ustPoolId) + suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) coins := sdk.NewCoins( From e415a51470725eeedf545bf2771411340bb6d768 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 7 Apr 2022 10:19:05 -0700 Subject: [PATCH 48/75] merge related fixes --- x/txfees/keeper/hooks_test.go | 14 +++++++------- x/txfees/types/expected_keepers.go | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index e6cc7f9dc5c..24409ffc15e 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -51,20 +51,20 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { swapFee := sdk.NewDec(0) - expectedOutput1 := uionPoolI.CalcOutAmtGivenIn(suite.ctx, + expectedOutput1, err := uionPoolI.CalcOutAmtGivenIn(suite.ctx, sdk.NewCoins(sdk.NewInt64Coin(uion, 10)), baseDenom, - swapFee).Amount.TruncateInt() - expectedOutput2 := atomPoolI.CalcOutAmtGivenIn(suite.ctx, + swapFee) + expectedOutput2, err := atomPoolI.CalcOutAmtGivenIn(suite.ctx, sdk.NewCoins(sdk.NewInt64Coin(atom, 20)), baseDenom, - swapFee).Amount.TruncateInt() - expectedOutput3 := ustPoolI.CalcOutAmtGivenIn(suite.ctx, + swapFee) + expectedOutput3, err := ustPoolI.CalcOutAmtGivenIn(suite.ctx, sdk.NewCoins(sdk.NewInt64Coin(ust, 14)), baseDenom, - swapFee).TruncateInt() + swapFee) - fullExpectedOutput := expectedOutput1.Add(expectedOutput2).Add(expectedOutput3) + fullExpectedOutput := (expectedOutput1.Amount.TruncateInt()).Add(expectedOutput2.Amount.TruncateInt()).Add(expectedOutput3.Amount.TruncateInt()) _, _, addr0 := testdata.KeyTestPubAddr() simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index 883099bdcae..b8f784bc472 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -3,6 +3,7 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + epochstypes "github.com/osmosis-labs/osmosis/v7/x/epochs/types" ) @@ -21,7 +22,7 @@ type GammKeeper interface { tokenIn sdk.Coin, tokenOutDenom string, tokenOutMinAmount sdk.Int, - ) (tokenOutAmount sdk.Int, spotPriceAfter sdk.Dec, err error) + ) (tokenOutAmount sdk.Int, err error) } // AccountKeeper defines the contract needed for AccountKeeper related APIs. From 3fe3b63385a75dfdd71be0fdb9a198209eb3296e Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 7 Apr 2022 10:41:18 -0700 Subject: [PATCH 49/75] merge related updates --- x/txfees/keeper/hooks.go | 1 + x/txfees/keeper/hooks_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index f1fcd1bc180..30653447249 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -2,6 +2,7 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" + epochstypes "github.com/osmosis-labs/osmosis/v7/x/epochs/types" txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index 24409ffc15e..f9bfdea1957 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -64,7 +64,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { baseDenom, swapFee) - fullExpectedOutput := (expectedOutput1.Amount.TruncateInt()).Add(expectedOutput2.Amount.TruncateInt()).Add(expectedOutput3.Amount.TruncateInt()) + fullExpectedOutput := expectedOutput1.Add(expectedOutput2).Add(expectedOutput3) _, _, addr0 := testdata.KeyTestPubAddr() simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) @@ -84,5 +84,5 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { suite.app.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrNonNativeFee)) - suite.Require().True(suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddrFee, baseDenom).Amount.GTE(fullExpectedOutput)) + suite.Require().True(suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddrFee, baseDenom).Amount.GTE(fullExpectedOutput.Amount.TruncateInt())) } From 637b75c72b623fcd6b9385467d15fd5c9d0c3fb2 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 7 Apr 2022 10:46:37 -0700 Subject: [PATCH 50/75] gofmt updates --- x/txfees/keeper/keeper.go | 42 +++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index f1357a49e89..e3ebfcc9aa2 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -14,20 +14,18 @@ import ( "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) -type ( - Keeper struct { - cdc codec.Codec - storeKey sdk.StoreKey - - accountKeeper types.AccountKeeper - bankKeeper *bankkeeper.BaseKeeper - epochKeeper types.EpochKeeper - gammKeeper types.GammKeeper - spotPriceCalculator types.SpotPriceCalculator - feeCollectorName string - nonNativeFeeCollectorName string - } -) +type Keeper struct { + cdc codec.Codec + storeKey sdk.StoreKey + + accountKeeper types.AccountKeeper + bankKeeper *bankkeeper.BaseKeeper + epochKeeper types.EpochKeeper + gammKeeper types.GammKeeper + spotPriceCalculator types.SpotPriceCalculator + feeCollectorName string + nonNativeFeeCollectorName string +} func NewKeeper( cdc codec.Codec, @@ -41,14 +39,14 @@ func NewKeeper( nonNativeFeeCollectorName string, ) Keeper { return Keeper{ - cdc: cdc, - accountKeeper: accountKeeper, - bankKeeper: bankKeeper, - epochKeeper: epochKeeper, - storeKey: storeKey, - gammKeeper: gammKeeper, - spotPriceCalculator: spotPriceCalculator, - feeCollectorName: feeCollectorName, + cdc: cdc, + accountKeeper: accountKeeper, + bankKeeper: bankKeeper, + epochKeeper: epochKeeper, + storeKey: storeKey, + gammKeeper: gammKeeper, + spotPriceCalculator: spotPriceCalculator, + feeCollectorName: feeCollectorName, nonNativeFeeCollectorName: nonNativeFeeCollectorName, } } From f803faa66d8de8e98f128e3999f204500c1a6149 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Thu, 7 Apr 2022 18:46:05 -0500 Subject: [PATCH 51/75] Fix feekeeper test setup --- x/txfees/keeper/feedecorator_test.go | 38 ++++++++++---------- x/txfees/keeper/hooks_test.go | 54 ++++++++++++++-------------- x/txfees/keeper/keeper_test.go | 4 --- 3 files changed, 46 insertions(+), 50 deletions(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 4cea2e56456..5ef961b9555 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -187,26 +187,26 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { // TxBuilder components reset for every test case txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() priv0, _, addr0 := testdata.KeyTestPubAddr() - acc1 := suite.app.AccountKeeper.NewAccountWithAddress(suite.Ctx, addr0) - suite.app.AccountKeeper.SetAccount(suite.Ctx, acc1) + acc1 := suite.App.AccountKeeper.NewAccountWithAddress(suite.Ctx, addr0) + suite.App.AccountKeeper.SetAccount(suite.Ctx, acc1) msgs := []sdk.Msg{testdata.NewTestMsg(addr0)} privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0} signerData := authsigning.SignerData{ - ChainID: suite.Ctx.ChainID(), - AccountNumber: accNums[0], - Sequence: accSeqs[0]} + ChainID: suite.Ctx.ChainID(), + AccountNumber: accNums[0], + Sequence: accSeqs[0]} gasLimit := tc.gasRequested sigV2, _ := clienttx.SignWithPrivKey( - 1, + 1, signerData, - txBuilder, - privs[0], + txBuilder, + privs[0], suite.clientCtx.TxConfig, accSeqs[0]) - simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, tc.txFee) - + simapp.FundAccount(suite.App.BankKeeper, suite.Ctx, addr0, tc.txFee) + txBuilder.SetMsgs(msgs[0]) txBuilder.SetSignatures(sigV2) txBuilder.SetMemo("") @@ -215,20 +215,20 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { tx := txBuilder.GetTx() - mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper, mempoolFeeOpts) - dfd := keeper.NewDeductFeeDecorator(*suite.app.TxFeesKeeper, *suite.app.AccountKeeper, *suite.app.BankKeeper, *suite.app.FeeGrantKeeper) + mfd := keeper.NewMempoolFeeDecorator(*suite.App.TxFeesKeeper, mempoolFeeOpts) + dfd := keeper.NewDeductFeeDecorator(*suite.App.TxFeesKeeper, *suite.App.AccountKeeper, *suite.App.BankKeeper, *suite.App.FeeGrantKeeper) antehandlerMFD := sdk.ChainAnteDecorators(mfd, dfd) - _, err := antehandlerMFD(suite.ctx, tx, false) + _, err := antehandlerMFD(suite.Ctx, tx, false) if tc.expectPass { if tc.baseDenomGas && !tc.txFee.IsZero() { - moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) - suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, baseDenom), tc.name) - suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.FeeCollectorName, addr0, tc.txFee) + moduleAddr := suite.App.AccountKeeper.GetModuleAddress(types.FeeCollectorName) + suite.Require().Equal(tc.txFee[0], suite.App.BankKeeper.GetBalance(suite.Ctx, moduleAddr, baseDenom), tc.name) + suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.FeeCollectorName, addr0, tc.txFee) } else if !tc.txFee.IsZero() { - moduleAddr := suite.app.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) - suite.Require().Equal(tc.txFee[0], suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddr, tc.txFee[0].Denom), tc.name) - suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.NonNativeFeeCollectorName, addr0, tc.txFee) + moduleAddr := suite.App.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) + suite.Require().Equal(tc.txFee[0], suite.App.BankKeeper.GetBalance(suite.Ctx, moduleAddr, tc.txFee[0].Denom), tc.name) + suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.NonNativeFeeCollectorName, addr0, tc.txFee) } suite.Require().NoError(err, "test: %s", tc.name) } else { diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index f9bfdea1957..6aa87e6fbee 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -12,10 +12,10 @@ import ( func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { suite.SetupTest(false) - baseDenom, _ := suite.app.TxFeesKeeper.GetBaseDenom(suite.ctx) + baseDenom, _ := suite.App.TxFeesKeeper.GetBaseDenom(suite.Ctx) // create pools for three separate fee tokens - + defaultPooledAssetAmount := int64(500) uion := "uion" @@ -23,7 +23,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(uion, defaultPooledAssetAmount), ) - uionPoolI, err:= suite.app.GAMMKeeper.GetPool(suite.ctx, uionPoolId) + uionPoolI, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, uionPoolId) suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) @@ -32,7 +32,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(atom, defaultPooledAssetAmount), ) - atomPoolI, err:= suite.app.GAMMKeeper.GetPool(suite.ctx, atomPoolId) + atomPoolI, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, atomPoolId) suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(atom, atomPoolId) @@ -41,7 +41,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(ust, defaultPooledAssetAmount), ) - ustPoolI, err:= suite.app.GAMMKeeper.GetPool(suite.ctx, ustPoolId) + ustPoolI, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, ustPoolId) suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) @@ -51,38 +51,38 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { swapFee := sdk.NewDec(0) - expectedOutput1, err := uionPoolI.CalcOutAmtGivenIn(suite.ctx, - sdk.NewCoins(sdk.NewInt64Coin(uion, 10)), - baseDenom, + expectedOutput1, err := uionPoolI.CalcOutAmtGivenIn(suite.Ctx, + sdk.NewCoins(sdk.NewInt64Coin(uion, 10)), + baseDenom, swapFee) - expectedOutput2, err := atomPoolI.CalcOutAmtGivenIn(suite.ctx, - sdk.NewCoins(sdk.NewInt64Coin(atom, 20)), - baseDenom, + expectedOutput2, err := atomPoolI.CalcOutAmtGivenIn(suite.Ctx, + sdk.NewCoins(sdk.NewInt64Coin(atom, 20)), + baseDenom, swapFee) - expectedOutput3, err := ustPoolI.CalcOutAmtGivenIn(suite.ctx, - sdk.NewCoins(sdk.NewInt64Coin(ust, 14)), - baseDenom, + expectedOutput3, err := ustPoolI.CalcOutAmtGivenIn(suite.Ctx, + sdk.NewCoins(sdk.NewInt64Coin(ust, 14)), + baseDenom, swapFee) - + fullExpectedOutput := expectedOutput1.Add(expectedOutput2).Add(expectedOutput3) _, _, addr0 := testdata.KeyTestPubAddr() - simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr0, coins) - suite.app.BankKeeper.SendCoinsFromAccountToModule(suite.ctx, addr0, types.NonNativeFeeCollectorName, coins) + simapp.FundAccount(suite.App.BankKeeper, suite.Ctx, addr0, coins) + suite.App.BankKeeper.SendCoinsFromAccountToModule(suite.Ctx, addr0, types.NonNativeFeeCollectorName, coins) - moduleAddrFee := suite.app.AccountKeeper.GetModuleAddress(types.FeeCollectorName) - moduleAddrNonNativeFee := suite.app.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) + moduleAddrFee := suite.App.AccountKeeper.GetModuleAddress(types.FeeCollectorName) + moduleAddrNonNativeFee := suite.App.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) // make sure module account is funded with test fee tokens - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrNonNativeFee, coins[0])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrNonNativeFee, coins[1])) - suite.Require().True(suite.app.BankKeeper.HasBalance(suite.ctx, moduleAddrNonNativeFee, coins[2])) + suite.Require().True(suite.App.BankKeeper.HasBalance(suite.Ctx, moduleAddrNonNativeFee, coins[0])) + suite.Require().True(suite.App.BankKeeper.HasBalance(suite.Ctx, moduleAddrNonNativeFee, coins[1])) + suite.Require().True(suite.App.BankKeeper.HasBalance(suite.Ctx, moduleAddrNonNativeFee, coins[2])) - params := suite.app.IncentivesKeeper.GetParams(suite.ctx) - futureCtx := suite.ctx.WithBlockTime(time.Now().Add(time.Minute)) + params := suite.App.IncentivesKeeper.GetParams(suite.Ctx) + futureCtx := suite.Ctx.WithBlockTime(time.Now().Add(time.Minute)) - suite.app.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) + suite.App.EpochsKeeper.AfterEpochEnd(futureCtx, params.DistrEpochIdentifier, int64(1)) - suite.Require().Empty(suite.app.BankKeeper.GetAllBalances(suite.ctx, moduleAddrNonNativeFee)) - suite.Require().True(suite.app.BankKeeper.GetBalance(suite.ctx, moduleAddrFee, baseDenom).Amount.GTE(fullExpectedOutput.Amount.TruncateInt())) + suite.Require().Empty(suite.App.BankKeeper.GetAllBalances(suite.Ctx, moduleAddrNonNativeFee)) + suite.Require().True(suite.App.BankKeeper.GetBalance(suite.Ctx, moduleAddrFee, baseDenom).Amount.GTE(fullExpectedOutput.Amount.TruncateInt())) } diff --git a/x/txfees/keeper/keeper_test.go b/x/txfees/keeper/keeper_test.go index 624eca35704..004074eddde 100644 --- a/x/txfees/keeper/keeper_test.go +++ b/x/txfees/keeper/keeper_test.go @@ -19,10 +19,6 @@ import ( ) type KeeperTestSuite struct { - suite.Suite - - ctx sdk.Context - app *osmosisapp.OsmosisApp apptesting.KeeperTestHelper clientCtx client.Context From 273fc3b753982a123d5059500d1215d4b0dfadf7 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 8 Apr 2022 18:28:37 -0700 Subject: [PATCH 52/75] switch tests to cleaner approach for resetting --- x/txfees/keeper/feedecorator_test.go | 12 ++++++++++-- x/txfees/keeper/hooks_test.go | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 5ef961b9555..2bc611a70da 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -181,6 +181,14 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { for _, tc := range tests { + // reset pool and accounts for each test + suite.SetupTest(false) + uionPoolId := suite.PrepareUni2PoolWithAssets( + sdk.NewInt64Coin(sdk.DefaultBondDenom, 500), + sdk.NewInt64Coin(uion, 500), + ) + suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) + suite.Ctx = suite.Ctx.WithIsCheckTx(tc.isCheckTx) suite.Ctx = suite.Ctx.WithMinGasPrices(tc.minGasPrices) @@ -224,11 +232,11 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { if tc.baseDenomGas && !tc.txFee.IsZero() { moduleAddr := suite.App.AccountKeeper.GetModuleAddress(types.FeeCollectorName) suite.Require().Equal(tc.txFee[0], suite.App.BankKeeper.GetBalance(suite.Ctx, moduleAddr, baseDenom), tc.name) - suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.FeeCollectorName, addr0, tc.txFee) + //suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.FeeCollectorName, addr0, tc.txFee) } else if !tc.txFee.IsZero() { moduleAddr := suite.App.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) suite.Require().Equal(tc.txFee[0], suite.App.BankKeeper.GetBalance(suite.Ctx, moduleAddr, tc.txFee[0].Denom), tc.name) - suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.NonNativeFeeCollectorName, addr0, tc.txFee) + //suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.NonNativeFeeCollectorName, addr0, tc.txFee) } suite.Require().NoError(err, "test: %s", tc.name) } else { diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index 6aa87e6fbee..1de74f16e59 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -45,7 +45,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) - coins := sdk.NewCoins(sdk.NewInt64Coin(atom, 20), + coins := sdk.NewCoins(sdk.NewInt64Coin(uion, 10), sdk.NewInt64Coin(atom, 20), sdk.NewInt64Coin(ust, 14)) From d710ad0657ffb2a124c3ce8cf92bbd7c67f0253f Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 8 Apr 2022 18:43:35 -0700 Subject: [PATCH 53/75] refactor tx building into test suite --- app/apptesting/test_suite.go | 17 +++++++++++++++++ x/txfees/keeper/feedecorator_test.go | 8 +------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index 1996505c13a..f7760b6aef7 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -22,6 +22,9 @@ import ( lockupkeeper "github.com/osmosis-labs/osmosis/v7/x/lockup/keeper" lockuptypes "github.com/osmosis-labs/osmosis/v7/x/lockup/types" + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/stretchr/testify/suite" "github.com/tendermint/tendermint/crypto/ed25519" ) @@ -217,3 +220,17 @@ func (keeperTestHelper *KeeperTestHelper) LockTokens(addr sdk.AccAddress, coins keeperTestHelper.Require().NoError(err) return msgResponse.ID } + +func (keeperTestHelper *KeeperTestHelper) BuildTx(txBuilder client.TxBuilder, msgs []sdk.Msg, sigV2 signing.SignatureV2, memo string, txFee sdk.Coins, gasLimit uint64) (tx authsigning.Tx) { + err := txBuilder.SetMsgs(msgs[0]) + keeperTestHelper.Require().NoError(err) + + err = txBuilder.SetSignatures(sigV2) + keeperTestHelper.Require().NoError(err) + + txBuilder.SetMemo(memo) + txBuilder.SetFeeAmount(txFee) + txBuilder.SetGasLimit(gasLimit) + + return txBuilder.GetTx() +} diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 2bc611a70da..0423ca0e693 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -215,13 +215,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { simapp.FundAccount(suite.App.BankKeeper, suite.Ctx, addr0, tc.txFee) - txBuilder.SetMsgs(msgs[0]) - txBuilder.SetSignatures(sigV2) - txBuilder.SetMemo("") - txBuilder.SetFeeAmount(tc.txFee) - txBuilder.SetGasLimit(gasLimit) - - tx := txBuilder.GetTx() + tx := suite.BuildTx(txBuilder, msgs, sigV2, "", tc.txFee, gasLimit) mfd := keeper.NewMempoolFeeDecorator(*suite.App.TxFeesKeeper, mempoolFeeOpts) dfd := keeper.NewDeductFeeDecorator(*suite.App.TxFeesKeeper, *suite.App.AccountKeeper, *suite.App.BankKeeper, *suite.App.FeeGrantKeeper) From b6da3609a9ebb53f3050c5c0ea28cbacd555aa8e Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 8 Apr 2022 19:08:55 -0700 Subject: [PATCH 54/75] add error checks and run linters --- x/txfees/keeper/hooks.go | 16 ++++++++++++---- x/txfees/types/expected_keepers.go | 2 +- x/txfees/types/keys.go | 2 +- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 30653447249..0463b435dce 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -10,7 +10,7 @@ import ( func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { } // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account -func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { +func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error { addrNonNativeFee := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) nonNativeFeeAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) baseDenom, _ := k.GetBaseDenom(ctx) @@ -22,7 +22,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb feetoken, err := k.GetFeeToken(ctx, coin.Denom) if err != nil { - continue + return err } // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, @@ -30,14 +30,22 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. minTokenOut := sdk.ZeroInt() - k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, minTokenOut) + _, err = k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, minTokenOut) + if err != nil { + return err + } } } // Get all of the txfee payout denom in the module account nonNativeFeeAccountBaseDenomBalance := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, addrNonNativeFee, baseDenom)) - k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBaseDenomBalance) + err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBaseDenomBalance) + if err != nil { + return err + } + + return nil } diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index b8f784bc472..8ffbfe2b299 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -56,4 +56,4 @@ type TxFeesKeeper interface { // EpochKeeper defines the contract needed to be fulfilled for epochs keeper type EpochKeeper interface { GetEpochInfo(ctx sdk.Context, identifier string) epochstypes.EpochInfo -} \ No newline at end of file +} diff --git a/x/txfees/types/keys.go b/x/txfees/types/keys.go index 7e7f8984a4a..eef001ca2dd 100644 --- a/x/txfees/types/keys.go +++ b/x/txfees/types/keys.go @@ -38,4 +38,4 @@ var ( // AddressStoreKey turn an address to key used to get it from the account store func AddressStoreKey(addr sdk.AccAddress) []byte { return append(AddressStoreKeyPrefix, addr.Bytes()...) -} \ No newline at end of file +} From 8bdea1f4565054382a8540bc8acb13022cfeffc9 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 8 Apr 2022 19:12:12 -0700 Subject: [PATCH 55/75] formatting --- app/modules.go | 2 +- x/txfees/keeper/feedecorator.go | 4 ++-- x/txfees/keeper/hooks.go | 7 +++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/modules.go b/app/modules.go index fa4e6a15219..7cd805a2fee 100644 --- a/app/modules.go +++ b/app/modules.go @@ -130,7 +130,7 @@ var moduleAccountPermissions = map[string][]string{ poolincentivestypes.ModuleName: nil, superfluidtypes.ModuleName: {authtypes.Minter, authtypes.Burner}, txfeestypes.ModuleName: nil, - txfeestypes.NonNativeFeeCollectorName: nil, + txfeestypes.NonNativeFeeCollectorName: nil, wasm.ModuleName: {authtypes.Burner}, } diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 8877b1b1de6..afcca9ad5d2 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -143,7 +143,7 @@ func NewDeductFeeDecorator(tk Keeper, ak types.AccountKeeper, bk types.BankKeepe ak: ak, bankKeeper: bk, feegrantKeeper: fk, - txFeesKeeper: tk, + txFeesKeeper: tk, } } @@ -213,7 +213,7 @@ func DeductFees(txFeesKeeper types.TxFeesKeeper, bankKeeper types.BankKeeper, ct if !fees.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - + // pulls base denom from TxFeesKeeper (should be uOSMO) baseDenom, err := txFeesKeeper.GetBaseDenom(ctx) if err != nil { diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 0463b435dce..07bd96f1a4d 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -7,7 +7,7 @@ import ( txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) -func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) { } +func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) {} // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error { @@ -19,7 +19,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb if coin.Denom == baseDenom { continue } else { - + feetoken, err := k.GetFeeToken(ctx, coin.Denom) if err != nil { return err @@ -39,7 +39,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb // Get all of the txfee payout denom in the module account nonNativeFeeAccountBaseDenomBalance := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, addrNonNativeFee, baseDenom)) - + err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBaseDenomBalance) if err != nil { return err @@ -48,7 +48,6 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb return nil } - // Hooks wrapper struct for incentives keeper type Hooks struct { k Keeper From 903b2a205711df0952ae38463a77b65e9da3eb72 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 8 Apr 2022 19:41:29 -0700 Subject: [PATCH 56/75] fix linting related issues --- x/txfees/keeper/hooks.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 07bd96f1a4d..b31f32e1558 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -10,7 +10,7 @@ import ( func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) {} // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account -func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error { +func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { addrNonNativeFee := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) nonNativeFeeAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) baseDenom, _ := k.GetBaseDenom(ctx) @@ -19,10 +19,9 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb if coin.Denom == baseDenom { continue } else { - feetoken, err := k.GetFeeToken(ctx, coin.Denom) if err != nil { - return err + panic(err) } // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, @@ -32,7 +31,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb minTokenOut := sdk.ZeroInt() _, err = k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, minTokenOut) if err != nil { - return err + panic(err) } } } @@ -42,10 +41,8 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBaseDenomBalance) if err != nil { - return err + panic(err) } - - return nil } // Hooks wrapper struct for incentives keeper From 691dce4a062086717282750421ad680035c79c58 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Wed, 13 Apr 2022 08:45:20 -0700 Subject: [PATCH 57/75] Apply suggestions from code review Co-authored-by: Aleksandr Bezobchuk --- app/apptesting/test_suite.go | 8 +++++++- app/keepers.go | 2 -- x/txfees/keeper/feedecorator_test.go | 2 -- x/txfees/keeper/hooks.go | 9 ++++----- x/txfees/types/expected_keepers.go | 2 -- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index f7760b6aef7..059ea9eda5e 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -221,7 +221,13 @@ func (keeperTestHelper *KeeperTestHelper) LockTokens(addr sdk.AccAddress, coins return msgResponse.ID } -func (keeperTestHelper *KeeperTestHelper) BuildTx(txBuilder client.TxBuilder, msgs []sdk.Msg, sigV2 signing.SignatureV2, memo string, txFee sdk.Coins, gasLimit uint64) (tx authsigning.Tx) { +func (keeperTestHelper *KeeperTestHelper) BuildTx( + txBuilder client.TxBuilder, + msgs []sdk.Msg, + sigV2 signing.SignatureV2, + memo string, txFee sdk.Coins, + gasLimit uint64, +) (authsigning.Tx) { err := txBuilder.SetMsgs(msgs[0]) keeperTestHelper.Require().NoError(err) diff --git a/app/keepers.go b/app/keepers.go index 7853c6e70dc..594a02dfae1 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -27,8 +27,6 @@ import ( paramproposal "github.com/cosmos/cosmos-sdk/x/params/types/proposal" slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" - - // Staking: Allows the Tendermint validator set to be chosen based on bonded stake. feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 0423ca0e693..c2b1d1e5a07 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -226,11 +226,9 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { if tc.baseDenomGas && !tc.txFee.IsZero() { moduleAddr := suite.App.AccountKeeper.GetModuleAddress(types.FeeCollectorName) suite.Require().Equal(tc.txFee[0], suite.App.BankKeeper.GetBalance(suite.Ctx, moduleAddr, baseDenom), tc.name) - //suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.FeeCollectorName, addr0, tc.txFee) } else if !tc.txFee.IsZero() { moduleAddr := suite.App.AccountKeeper.GetModuleAddress(types.NonNativeFeeCollectorName) suite.Require().Equal(tc.txFee[0], suite.App.BankKeeper.GetBalance(suite.Ctx, moduleAddr, tc.txFee[0].Denom), tc.name) - //suite.App.BankKeeper.SendCoinsFromModuleToAccount(suite.Ctx, types.NonNativeFeeCollectorName, addr0, tc.txFee) } suite.Require().NoError(err, "test: %s", tc.name) } else { diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index b31f32e1558..0bc7aa98e25 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -11,8 +11,8 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { - addrNonNativeFee := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) - nonNativeFeeAccountBalances := k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) + nonNativeFeeAddr := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) + nonNativeBalances := k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) baseDenom, _ := k.GetBaseDenom(ctx) for _, coin := range nonNativeFeeAccountBalances { @@ -28,8 +28,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb // but even then the point is a bit moot. // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. - minTokenOut := sdk.ZeroInt() - _, err = k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, minTokenOut) + _, err = k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) if err != nil { panic(err) } @@ -37,7 +36,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb } // Get all of the txfee payout denom in the module account - nonNativeFeeAccountBaseDenomBalance := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, addrNonNativeFee, baseDenom)) + nonNativeCoins := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, addrNonNativeFee, baseDenom)) err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBaseDenomBalance) if err != nil { diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index 8ffbfe2b299..293519b29d3 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -49,8 +49,6 @@ type TxFeesKeeper interface { ConvertToBaseToken(ctx sdk.Context, inputFee sdk.Coin) (sdk.Coin, error) GetBaseDenom(ctx sdk.Context) (denom string, err error) GetFeeToken(ctx sdk.Context, denom string) (FeeToken, error) - - // add keepers here } // EpochKeeper defines the contract needed to be fulfilled for epochs keeper From 6eec71344a55ee994a7a4e9a73f2c6b505bed8b5 Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 13 Apr 2022 09:15:23 -0700 Subject: [PATCH 58/75] apply suggestions from review --- app/ante.go | 3 +-- x/txfees/keeper/hooks.go | 35 +++++++++++++++--------------- x/txfees/keeper/keeper.go | 2 +- x/txfees/types/expected_keepers.go | 3 +++ x/txfees/types/keys.go | 15 ------------- 5 files changed, 22 insertions(+), 36 deletions(-) diff --git a/app/ante.go b/app/ante.go index dcb83fb7dc8..784c765d9c1 100644 --- a/app/ante.go +++ b/app/ante.go @@ -7,7 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ante "github.com/cosmos/cosmos-sdk/x/auth/ante" "github.com/cosmos/cosmos-sdk/x/auth/signing" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" channelkeeper "github.com/cosmos/ibc-go/v2/modules/core/04-channel/keeper" ibcante "github.com/cosmos/ibc-go/v2/modules/core/ante" @@ -22,7 +21,7 @@ func NewAnteHandler( wasmConfig wasm.Config, txCounterStoreKey sdk.StoreKey, ak ante.AccountKeeper, - bankKeeper authtypes.BankKeeper, + bankKeeper txfeestypes.BankKeeper, txFeesKeeper *txfeeskeeper.Keeper, spotPriceCalculator txfeestypes.SpotPriceCalculator, sigGasConsumer ante.SignatureVerificationGasConsumer, diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index 0bc7aa98e25..c04a0ac10ad 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -12,33 +12,32 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { nonNativeFeeAddr := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) - nonNativeBalances := k.bankKeeper.GetAllBalances(ctx, addrNonNativeFee) + nonNativeBalances := k.bankKeeper.GetAllBalances(ctx, nonNativeFeeAddr) baseDenom, _ := k.GetBaseDenom(ctx) - for _, coin := range nonNativeFeeAccountBalances { + for _, coin := range nonNativeBalances { if coin.Denom == baseDenom { continue - } else { - feetoken, err := k.GetFeeToken(ctx, coin.Denom) - if err != nil { - panic(err) - } - - // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, - // but even then the point is a bit moot. - // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. - // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. - _, err = k.gammKeeper.SwapExactAmountIn(ctx, addrNonNativeFee, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) - if err != nil { - panic(err) - } + } + feetoken, err := k.GetFeeToken(ctx, coin.Denom) + if err != nil { + panic(err) + } + + // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, + // but even then the point is a bit moot. + // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. + // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. + _, err = k.gammKeeper.SwapExactAmountIn(ctx, nonNativeFeeAddr, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) + if err != nil { + panic(err) } } // Get all of the txfee payout denom in the module account - nonNativeCoins := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, addrNonNativeFee, baseDenom)) + nonNativeCoins := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, nonNativeFeeAddr, baseDenom)) - err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeFeeAccountBaseDenomBalance) + err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeCoins) if err != nil { panic(err) } diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index e3ebfcc9aa2..ebfd7ffd036 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -19,7 +19,7 @@ type Keeper struct { storeKey sdk.StoreKey accountKeeper types.AccountKeeper - bankKeeper *bankkeeper.BaseKeeper + bankKeeper types.BankKeeper epochKeeper types.EpochKeeper gammKeeper types.GammKeeper spotPriceCalculator types.SpotPriceCalculator diff --git a/x/txfees/types/expected_keepers.go b/x/txfees/types/expected_keepers.go index 293519b29d3..c8a3d2df789 100644 --- a/x/txfees/types/expected_keepers.go +++ b/x/txfees/types/expected_keepers.go @@ -42,6 +42,9 @@ type FeegrantKeeper interface { // BankKeeper defines the contract needed for supply related APIs (noalias) type BankKeeper interface { SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error + GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins + GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin + SendCoinsFromModuleToModule(ctx sdk.Context, senderModule, recipientModule string, amt sdk.Coins) error } // TxFeesKeeper defines the expected transaction fee keeper diff --git a/x/txfees/types/keys.go b/x/txfees/types/keys.go index eef001ca2dd..0978070fd39 100644 --- a/x/txfees/types/keys.go +++ b/x/txfees/types/keys.go @@ -1,9 +1,5 @@ package types -import ( - sdk "github.com/cosmos/cosmos-sdk/types" -) - const ( // ModuleName defines the module name. ModuleName = "txfees" @@ -27,15 +23,4 @@ const ( var ( BaseDenomKey = []byte("base_denom") FeeTokensStorePrefix = []byte("fee_tokens") - - // AddressStoreKeyPrefix prefix for account-by-address store - AddressStoreKeyPrefix = []byte{0x01} - - // param key for global account number - GlobalAccountNumberKey = []byte("globalAccountNumber") ) - -// AddressStoreKey turn an address to key used to get it from the account store -func AddressStoreKey(addr sdk.AccAddress) []byte { - return append(AddressStoreKeyPrefix, addr.Bytes()...) -} From 193f83299859e2e9da972558d9d5086346ed5fdb Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 13 Apr 2022 09:19:43 -0700 Subject: [PATCH 59/75] run linter --- app/apptesting/test_suite.go | 12 ++++++------ app/keepers.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index 059ea9eda5e..02902feec7f 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -222,12 +222,12 @@ func (keeperTestHelper *KeeperTestHelper) LockTokens(addr sdk.AccAddress, coins } func (keeperTestHelper *KeeperTestHelper) BuildTx( - txBuilder client.TxBuilder, - msgs []sdk.Msg, - sigV2 signing.SignatureV2, - memo string, txFee sdk.Coins, - gasLimit uint64, -) (authsigning.Tx) { + txBuilder client.TxBuilder, + msgs []sdk.Msg, + sigV2 signing.SignatureV2, + memo string, txFee sdk.Coins, + gasLimit uint64, +) authsigning.Tx { err := txBuilder.SetMsgs(msgs[0]) keeperTestHelper.Require().NoError(err) diff --git a/app/keepers.go b/app/keepers.go index 594a02dfae1..ce1a5082a7b 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -19,6 +19,7 @@ import ( distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" evidencekeeper "github.com/cosmos/cosmos-sdk/x/evidence/keeper" evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" + feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/cosmos/cosmos-sdk/x/params" @@ -27,7 +28,6 @@ import ( paramproposal "github.com/cosmos/cosmos-sdk/x/params/types/proposal" slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" - feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/cosmos/cosmos-sdk/x/upgrade" From 87bd397bfa42ec6007d45fc68c7ca4ac67f54dea Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 13 Apr 2022 09:26:42 -0700 Subject: [PATCH 60/75] fix import-related issues --- app/apptesting/test_suite.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index 5c28ad9cdeb..ea10e12335f 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -25,8 +25,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/types/tx/signing" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" - "github.com/stretchr/testify/suite" - "github.com/tendermint/tendermint/crypto/ed25519" ) type KeeperTestHelper struct { From 342761cfa91232395f8f51e55fe813d5982f76a9 Mon Sep 17 00:00:00 2001 From: alpo Date: Wed, 13 Apr 2022 09:45:50 -0700 Subject: [PATCH 61/75] fix merge conflicts --- x/txfees/keeper/hooks_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index 1de74f16e59..333474b52b1 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -23,7 +23,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(uion, defaultPooledAssetAmount), ) - uionPoolI, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, uionPoolId) + uionPoolI, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, uionPoolId) suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) @@ -32,7 +32,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(atom, defaultPooledAssetAmount), ) - atomPoolI, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, atomPoolId) + atomPoolI, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, atomPoolId) suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(atom, atomPoolId) @@ -41,7 +41,7 @@ func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), sdk.NewInt64Coin(ust, defaultPooledAssetAmount), ) - ustPoolI, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, ustPoolId) + ustPoolI, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, ustPoolId) suite.Require().NoError(err) suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) From c1228c39c2499fb5ce79d676687e2327036a6268 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Thu, 14 Apr 2022 08:21:07 -0700 Subject: [PATCH 62/75] Delete math.go --- x/gamm/keeper/math.go | 202 ------------------------------------------ 1 file changed, 202 deletions(-) delete mode 100644 x/gamm/keeper/math.go diff --git a/x/gamm/keeper/math.go b/x/gamm/keeper/math.go deleted file mode 100644 index 366f805442a..00000000000 --- a/x/gamm/keeper/math.go +++ /dev/null @@ -1,202 +0,0 @@ -package keeper - -import ( - "github.com/osmosis-labs/osmosis/v7/osmomath" - - sdk "github.com/cosmos/cosmos-sdk/types" -) - -// Don't EVER change after initializing -// TODO: Analyze choice here -var powPrecision, _ = sdk.NewDecFromStr("0.00000001") - -// Singletons -var zero sdk.Dec = sdk.ZeroDec() -var one_half sdk.Dec = sdk.MustNewDecFromStr("0.5") -var one sdk.Dec = sdk.OneDec() -var two sdk.Dec = sdk.MustNewDecFromStr("2") - -// calcSpotPrice returns the spot price of the pool -// This is the weight-adjusted balance of the tokens in the pool. -// so spot_price = (B_in / W_in) / (B_out / W_out) -func calcSpotPrice( - tokenBalanceIn, - tokenWeightIn, - tokenBalanceOut, - tokenWeightOut sdk.Dec, -) sdk.Dec { - number := tokenBalanceIn.Quo(tokenWeightIn) - denom := tokenBalanceOut.Quo(tokenWeightOut) - ratio := number.Quo(denom) - - return ratio -} - -// calcSpotPriceWithSwapFee returns the spot price of the pool accounting for -// the input taken by the swap fee. -// This is the weight-adjusted balance of the tokens in the pool. -// so spot_price = (B_in / W_in) / (B_out / W_out) -// and spot_price_with_fee = spot_price / (1 - swapfee) -func calcSpotPriceWithSwapFee( - tokenBalanceIn, - tokenWeightIn, - tokenBalanceOut, - tokenWeightOut, - swapFee sdk.Dec, -) sdk.Dec { - spotPrice := calcSpotPrice(tokenBalanceIn, tokenWeightIn, tokenBalanceOut, tokenWeightOut) - // Q: Why is this not just (1 - swapfee) - // A: Its because its being applied to the other asset. - // TODO: write this up more coherently - // 1 / (1 - swapfee) - scale := sdk.OneDec().Quo(sdk.OneDec().Sub(swapFee)) - - return spotPrice.Mul(scale) -} - -// solveConstantFunctionInvariant solves the constant function of an AMM -// that determines the relationship between the differences of two sides -// of assets inside the pool. -// For fixed balanceXBefore, balanceXAfter, weightX, balanceY, weightY, -// we could deduce the balanceYDelta, calculated by: -// balanceYDelta = balanceY * (1 - (balanceXBefore/balanceXAfter)^(weightX/weightY)) -// balanceYDelta is positive when the balance liquidity decreases. -// balanceYDelta is negative when the balance liquidity increases. -func solveConstantFunctionInvariant( - tokenBalanceFixedBefore, - tokenBalanceFixedAfter, - tokenWeightFixed, - tokenBalanceUnknownBefore, - tokenWeightUnknown sdk.Dec, -) sdk.Dec { - // weightRatio = (weightX/weightY) - weightRatio := tokenWeightFixed.Quo(tokenWeightUnknown) - - // y = balanceXBefore/balanceYAfter - y := tokenBalanceFixedBefore.Quo(tokenBalanceFixedAfter) - - // amountY = balanceY * (1 - (y ^ weightRatio)) - foo := osmomath.Pow(y, weightRatio) - multiplier := sdk.OneDec().Sub(foo) - return tokenBalanceUnknownBefore.Mul(multiplier) -} - -// calcOutGivenIn calculates token to be swapped out given -// the provided amount, fee deducted, using solveConstantFunctionInvariant -func calcOutGivenIn( - tokenBalanceIn, - tokenWeightIn, - tokenBalanceOut, - tokenWeightOut, - tokenAmountIn, - swapFee sdk.Dec, -) sdk.Dec { - // deduct swapfee on the in asset - tokenAmountInAfterFee := tokenAmountIn.Mul(sdk.OneDec().Sub(swapFee)) - // delta balanceOut is positive(tokens inside the pool decreases) - tokenAmountOut := solveConstantFunctionInvariant(tokenBalanceIn, tokenBalanceIn.Add(tokenAmountInAfterFee), tokenWeightIn, tokenBalanceOut, tokenWeightOut) - return tokenAmountOut -} - -// calcInGivenOut calculates token to be provided, fee added, -// given the swapped out amount, using solveConstantFunctionInvariant -func calcInGivenOut( - tokenBalanceIn, - tokenWeightIn, - tokenBalanceOut, - tokenWeightOut, - tokenAmountOut, - swapFee sdk.Dec, -) sdk.Dec { - // delta balanceIn is negative(amount of tokens inside the pool increases) - tokenAmountIn := solveConstantFunctionInvariant(tokenBalanceOut, tokenBalanceOut.Sub(tokenAmountOut), tokenWeightOut, tokenBalanceIn, tokenWeightIn).Neg() - // We deduct a swap fee on the input asset. The swap happens by following the invariant curve on the input * (1 - swap fee) - // and then the swap fee is added to the pool. - // Thus in order to give X amount out, we solve the invariant for the invariant input. However invariant input = (1 - swapfee) * trade input. - // Therefore we divide by (1 - swapfee) here - tokenAmountInBeforeFee := tokenAmountIn.Quo(sdk.OneDec().Sub(swapFee)) - return tokenAmountInBeforeFee -} - -func feeRatio( - normalizedWeight, - swapFee sdk.Dec, -) sdk.Dec { - zar := (sdk.OneDec().Sub(normalizedWeight)).Mul(swapFee) - return sdk.OneDec().Sub(zar) -} - -// calcSingleInGivenPoolOut calculates token to be provided, fee added, -// given the swapped out shares amount, using solveConstantFunctionInvariant -func calcSingleInGivenPoolOut( - tokenBalanceIn, - normalizedTokenWeightIn, - poolSupply, - poolAmountOut, - swapFee sdk.Dec, -) sdk.Dec { - // delta balanceIn is negative(tokens inside the pool increases) - // pool weight is always 1 - tokenAmountIn := solveConstantFunctionInvariant(poolSupply.Add(poolAmountOut), poolSupply, sdk.OneDec(), tokenBalanceIn, normalizedTokenWeightIn).Neg() - // deduct swapfee on the in asset - tokenAmountInBeforeFee := tokenAmountIn.Quo(feeRatio(normalizedTokenWeightIn, swapFee)) - return tokenAmountInBeforeFee -} - -// pAo -func calcPoolOutGivenSingleIn( - tokenBalanceIn, - normalizedTokenWeightIn, - poolSupply, - tokenAmountIn, - swapFee sdk.Dec, -) sdk.Dec { - // deduct swapfee on the in asset - tokenAmountInAfterFee := tokenAmountIn.Mul(feeRatio(normalizedTokenWeightIn, swapFee)) - // delta poolSupply is negative(total pool shares increases) - // pool weight is always 1 - poolAmountOut := solveConstantFunctionInvariant(tokenBalanceIn.Add(tokenAmountInAfterFee), tokenBalanceIn, normalizedTokenWeightIn, poolSupply, sdk.OneDec()).Neg() - return poolAmountOut -} - -// tAo -func calcSingleOutGivenPoolIn( - tokenBalanceOut, - normalizedTokenWeightOut, - poolSupply, - poolAmountIn, - swapFee sdk.Dec, - exitFee sdk.Dec, -) sdk.Dec { - // charge exit fee on the pool token side - // pAiAfterExitFee = pAi*(1-exitFee) - poolAmountInAfterExitFee := poolAmountIn.Mul(sdk.OneDec().Sub(exitFee)) - - // delta balanceOut is positive(tokens inside the pool decreases) - // pool weight is always 1 - tokenAmountOut := solveConstantFunctionInvariant(poolSupply.Sub(poolAmountInAfterExitFee), poolSupply, sdk.OneDec(), tokenBalanceOut, normalizedTokenWeightOut) - // deduct - tokenAmountOutAfterFee := tokenAmountOut.Mul(feeRatio(normalizedTokenWeightOut, swapFee)) - return tokenAmountOutAfterFee -} - -// pAi -func calcPoolInGivenSingleOut( - tokenBalanceOut, - normalizedTokenWeightOut, - poolSupply, - tokenAmountOut, - swapFee sdk.Dec, - exitFee sdk.Dec, -) sdk.Dec { - tokenAmountOutBeforeFee := tokenAmountOut.Quo(feeRatio(normalizedTokenWeightOut, swapFee)) - - // delta poolSupply is positive(total pool shares decreases) - // pool weight is always 1 - poolAmountIn := solveConstantFunctionInvariant(tokenBalanceOut.Sub(tokenAmountOutBeforeFee), tokenBalanceOut, normalizedTokenWeightOut, poolSupply, sdk.OneDec()) - - // charge exit fee on the pool token side - // pAi = pAiAfterExitFee/(1-exitFee) - poolAmountInBeforeFee := poolAmountIn.Quo(sdk.OneDec().Sub(exitFee)) - return poolAmountInBeforeFee -} From 1e43f4e3520b145cbc0254f9a9166d275070e6c3 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 15 Apr 2022 16:17:47 -0500 Subject: [PATCH 63/75] Fix build error --- app/ante.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/ante.go b/app/ante.go index 06691e04624..787c5509ca0 100644 --- a/app/ante.go +++ b/app/ante.go @@ -3,8 +3,6 @@ package app import ( wasm "github.com/CosmWasm/wasmd/x/wasm" wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper" - channelkeeper "github.com/cosmos/ibc-go/v2/modules/core/04-channel/keeper" - ibcante "github.com/cosmos/ibc-go/v2/modules/core/ante" servertypes "github.com/cosmos/cosmos-sdk/server/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -12,7 +10,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/signing" channelkeeper "github.com/cosmos/ibc-go/v2/modules/core/04-channel/keeper" ibcante "github.com/cosmos/ibc-go/v2/modules/core/ante" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" txfeeskeeper "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" From 709bceb14a0131cc3021ff07461f3278b151464a Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 15 Apr 2022 16:20:05 -0500 Subject: [PATCH 64/75] Fix formatting --- x/txfees/keeper/feedecorator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index afcca9ad5d2..0f8a6794e9c 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -177,7 +177,6 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants is not enabled") } else if !feeGranter.Equals(feePayer) { err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, tx.GetMsgs()) - if err != nil { return ctx, sdkerrors.Wrapf(err, "%s not allowed to pay fees from %s", feeGranter, feePayer) } From 05b3fbc972775cddd63f31463c37970fbfe5d27c Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 15 Apr 2022 16:25:30 -0500 Subject: [PATCH 65/75] Not sure why make format didn't catch this --- x/txfees/keeper/feedecorator_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 495aa41a7e9..c2b1d1e5a07 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -1,14 +1,12 @@ package keeper_test import ( - clienttx "github.com/cosmos/cosmos-sdk/client/tx" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" - "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" From 3d8e7dce80b831a4622c892ddc0a6917702f1d7b Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 15 Apr 2022 14:25:34 -0700 Subject: [PATCH 66/75] import fixes --- app/ante.go | 3 --- x/txfees/keeper/feedecorator_test.go | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/app/ante.go b/app/ante.go index 06691e04624..2f36d4eb552 100644 --- a/app/ante.go +++ b/app/ante.go @@ -10,9 +10,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ante "github.com/cosmos/cosmos-sdk/x/auth/ante" "github.com/cosmos/cosmos-sdk/x/auth/signing" - channelkeeper "github.com/cosmos/ibc-go/v2/modules/core/04-channel/keeper" - ibcante "github.com/cosmos/ibc-go/v2/modules/core/ante" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" txfeeskeeper "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 495aa41a7e9..2833fecb6d4 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -1,14 +1,13 @@ package keeper_test import ( - clienttx "github.com/cosmos/cosmos-sdk/client/tx" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" - "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" From 2b8140dd9a742e3f133067d73462027bd42382af Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 15 Apr 2022 16:37:07 -0500 Subject: [PATCH 67/75] Delete fee grant keeper --- app/keepers.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/keepers.go b/app/keepers.go index ce1a5082a7b..f8746ef1b1d 100644 --- a/app/keepers.go +++ b/app/keepers.go @@ -19,7 +19,6 @@ import ( distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" evidencekeeper "github.com/cosmos/cosmos-sdk/x/evidence/keeper" evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" - feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/cosmos/cosmos-sdk/x/params" @@ -104,7 +103,6 @@ type appKeepers struct { MintKeeper *mintkeeper.Keeper PoolIncentivesKeeper *poolincentiveskeeper.Keeper TxFeesKeeper *txfeeskeeper.Keeper - FeeGrantKeeper *feegrantkeeper.Keeper SuperfluidKeeper *superfluidkeeper.Keeper GovKeeper *govkeeper.Keeper WasmKeeper *wasm.Keeper @@ -354,13 +352,6 @@ func (app *OsmosisApp) InitNormalKeepers( ) app.TxFeesKeeper = &txFeesKeeper - feeGrantKeeper := feegrantkeeper.NewKeeper( - appCodec, - keys[txfeestypes.StoreKey], - app.AccountKeeper, - ) - app.FeeGrantKeeper = &feeGrantKeeper - // The last arguments can contain custom message handlers, and custom query handlers, // if we want to allow any custom callbacks supportedFeatures := "iterator,staking,stargate,osmosis" From faf48a548c8cd82c2e297e5dd4c68facfd71e884 Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 18 Apr 2022 02:40:07 -0700 Subject: [PATCH 68/75] fix tests to accommodate removal of fee grant keeper --- x/txfees/keeper/feedecorator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 2833fecb6d4..473ffa4ee0c 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -219,7 +219,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { tx := suite.BuildTx(txBuilder, msgs, sigV2, "", tc.txFee, gasLimit) mfd := keeper.NewMempoolFeeDecorator(*suite.App.TxFeesKeeper, mempoolFeeOpts) - dfd := keeper.NewDeductFeeDecorator(*suite.App.TxFeesKeeper, *suite.App.AccountKeeper, *suite.App.BankKeeper, *suite.App.FeeGrantKeeper) + dfd := keeper.NewDeductFeeDecorator(*suite.App.TxFeesKeeper, *suite.App.AccountKeeper, *suite.App.BankKeeper, nil) antehandlerMFD := sdk.ChainAnteDecorators(mfd, dfd) _, err := antehandlerMFD(suite.Ctx, tx, false) From ab34ca3d91179e714af8865f1901882d4fc2568c Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 18 Apr 2022 08:33:41 -0700 Subject: [PATCH 69/75] remove panic errors at epoch end and remove potential for unbounded denom loop --- x/txfees/keeper/hooks.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index c04a0ac10ad..bc1046c1897 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -3,6 +3,7 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/osmosis-labs/osmosis/v7/osmoutils" epochstypes "github.com/osmosis-labs/osmosis/v7/x/epochs/types" txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) @@ -12,35 +13,37 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { nonNativeFeeAddr := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) - nonNativeBalances := k.bankKeeper.GetAllBalances(ctx, nonNativeFeeAddr) + // nonNativeBalances := k.bankKeeper.GetAllBalances(ctx, nonNativeFeeAddr) baseDenom, _ := k.GetBaseDenom(ctx) + feeTokens := k.GetFeeTokens(ctx) - for _, coin := range nonNativeBalances { + for _, coin := range feeTokens { if coin.Denom == baseDenom { continue } + coinBalance := k.bankKeeper.GetBalance(ctx, nonNativeFeeAddr, coin.Denom) feetoken, err := k.GetFeeToken(ctx, coin.Denom) if err != nil { - panic(err) + break } // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, // but even then the point is a bit moot. // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. - _, err = k.gammKeeper.SwapExactAmountIn(ctx, nonNativeFeeAddr, feetoken.PoolID, coin, baseDenom, sdk.ZeroInt()) + _, err = k.gammKeeper.SwapExactAmountIn(ctx, nonNativeFeeAddr, feetoken.PoolID, coinBalance, baseDenom, sdk.ZeroInt()) if err != nil { - panic(err) + break } } // Get all of the txfee payout denom in the module account nonNativeCoins := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, nonNativeFeeAddr, baseDenom)) - err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeCoins) - if err != nil { - panic(err) - } + _ = osmoutils.ApplyFuncIfNoError(ctx, func(cacheCtx sdk.Context) error { + err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeCoins) + return err + }) } // Hooks wrapper struct for incentives keeper From 366da354be13d46077a27f8d23448e14d5770ac9 Mon Sep 17 00:00:00 2001 From: alpo <62043214+AlpinYukseloglu@users.noreply.github.com> Date: Mon, 18 Apr 2022 08:35:49 -0700 Subject: [PATCH 70/75] Apply suggestions from code review Co-authored-by: Aleksandr Bezobchuk --- x/txfees/keeper/feedecorator.go | 2 +- x/txfees/keeper/feedecorator_test.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 0f8a6794e9c..2710b845ff7 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -160,7 +160,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo // checks to make sure a separate module account has been set to collect fees not in OSMO if addrNonNativeFee := dfd.ak.GetModuleAddress(types.NonNativeFeeCollectorName); addrNonNativeFee == nil { - return ctx, fmt.Errorf("Alt Fee collector module account (%s) has not been set", types.NonNativeFeeCollectorName) + return ctx, fmt.Errorf("non native fee collector module account (%s) has not been set", types.NonNativeFeeCollectorName) } // fee can be in any denom (checked for validity later) diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 473ffa4ee0c..bd4613a8b26 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -203,7 +203,8 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { signerData := authsigning.SignerData{ ChainID: suite.Ctx.ChainID(), AccountNumber: accNums[0], - Sequence: accSeqs[0]} + Sequence: accSeqs[0], + } gasLimit := tc.gasRequested sigV2, _ := clienttx.SignWithPrivKey( @@ -212,7 +213,8 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { txBuilder, privs[0], suite.clientCtx.TxConfig, - accSeqs[0]) + accSeqs[0], + ) simapp.FundAccount(suite.App.BankKeeper, suite.Ctx, addr0, tc.txFee) From 84ab5337aaa46d4cb4ee769478f305ea9d16db32 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Mon, 18 Apr 2022 11:52:26 -0500 Subject: [PATCH 71/75] Update epoch code + DRY some of the test --- x/txfees/keeper/hooks.go | 34 ++++++++++---------- x/txfees/keeper/hooks_test.go | 60 ++++++++++++++++------------------- 2 files changed, 45 insertions(+), 49 deletions(-) diff --git a/x/txfees/keeper/hooks.go b/x/txfees/keeper/hooks.go index bc1046c1897..6cb662b36a7 100644 --- a/x/txfees/keeper/hooks.go +++ b/x/txfees/keeper/hooks.go @@ -13,35 +13,35 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN // at the end of each epoch, swap all non-OSMO fees into OSMO and transfer to fee module account func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) { nonNativeFeeAddr := k.accountKeeper.GetModuleAddress(txfeestypes.NonNativeFeeCollectorName) - // nonNativeBalances := k.bankKeeper.GetAllBalances(ctx, nonNativeFeeAddr) baseDenom, _ := k.GetBaseDenom(ctx) feeTokens := k.GetFeeTokens(ctx) - for _, coin := range feeTokens { - if coin.Denom == baseDenom { + for _, feetoken := range feeTokens { + if feetoken.Denom == baseDenom { continue } - coinBalance := k.bankKeeper.GetBalance(ctx, nonNativeFeeAddr, coin.Denom) - feetoken, err := k.GetFeeToken(ctx, coin.Denom) - if err != nil { - break + coinBalance := k.bankKeeper.GetBalance(ctx, nonNativeFeeAddr, feetoken.Denom) + if coinBalance.Amount.IsZero() { + continue } - // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, - // but even then the point is a bit moot. - // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. - // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. - _, err = k.gammKeeper.SwapExactAmountIn(ctx, nonNativeFeeAddr, feetoken.PoolID, coinBalance, baseDenom, sdk.ZeroInt()) - if err != nil { - break - } + // Do the swap of this fee token denom to base denom. + _ = osmoutils.ApplyFuncIfNoError(ctx, func(cacheCtx sdk.Context) error { + // We allow full slippage. Theres not really an effective way to bound slippage until TWAP's land, + // but even then the point is a bit moot. + // The only thing that could be done is a costly griefing attack to reduce the amount of osmo given as tx fees. + // However the idea of the txfees FeeToken gating is that the pool is sufficiently liquid for that base token. + minAmountOut := sdk.ZeroInt() + _, err := k.gammKeeper.SwapExactAmountIn(cacheCtx, nonNativeFeeAddr, feetoken.PoolID, coinBalance, baseDenom, minAmountOut) + return err + }) } // Get all of the txfee payout denom in the module account - nonNativeCoins := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, nonNativeFeeAddr, baseDenom)) + baseDenomCoins := sdk.NewCoins(k.bankKeeper.GetBalance(ctx, nonNativeFeeAddr, baseDenom)) _ = osmoutils.ApplyFuncIfNoError(ctx, func(cacheCtx sdk.Context) error { - err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, nonNativeCoins) + err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, txfeestypes.NonNativeFeeCollectorName, txfeestypes.FeeCollectorName, baseDenomCoins) return err }) } diff --git a/x/txfees/keeper/hooks_test.go b/x/txfees/keeper/hooks_test.go index 333474b52b1..6f1a0872a7f 100644 --- a/x/txfees/keeper/hooks_test.go +++ b/x/txfees/keeper/hooks_test.go @@ -7,62 +7,58 @@ import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + gammtypes "github.com/osmosis-labs/osmosis/v7/x/gamm/types" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) +var defaultPooledAssetAmount = int64(500) + +func (suite *KeeperTestSuite) preparePool(denom string) (poolID uint64, pool gammtypes.PoolI) { + baseDenom, _ := suite.App.TxFeesKeeper.GetBaseDenom(suite.Ctx) + poolID = suite.PrepareUni2PoolWithAssets( + sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), + sdk.NewInt64Coin(denom, defaultPooledAssetAmount), + ) + pool, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, poolID) + suite.Require().NoError(err) + suite.ExecuteUpgradeFeeTokenProposal(denom, poolID) + return poolID, pool +} + func (suite *KeeperTestSuite) TestTxFeesAfterEpochEnd() { suite.SetupTest(false) baseDenom, _ := suite.App.TxFeesKeeper.GetBaseDenom(suite.Ctx) // create pools for three separate fee tokens - - defaultPooledAssetAmount := int64(500) - uion := "uion" - uionPoolId := suite.PrepareUni2PoolWithAssets( - sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), - sdk.NewInt64Coin(uion, defaultPooledAssetAmount), - ) - uionPoolI, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, uionPoolId) - suite.Require().NoError(err) - suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) - + _, uionPool := suite.preparePool(uion) atom := "atom" - atomPoolId := suite.PrepareUni2PoolWithAssets( - sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), - sdk.NewInt64Coin(atom, defaultPooledAssetAmount), - ) - atomPoolI, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, atomPoolId) - suite.Require().NoError(err) - suite.ExecuteUpgradeFeeTokenProposal(atom, atomPoolId) - + _, atomPool := suite.preparePool(atom) ust := "ust" - ustPoolId := suite.PrepareUni2PoolWithAssets( - sdk.NewInt64Coin(baseDenom, defaultPooledAssetAmount), - sdk.NewInt64Coin(ust, defaultPooledAssetAmount), - ) - ustPoolI, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, ustPoolId) - suite.Require().NoError(err) - suite.ExecuteUpgradeFeeTokenProposal(ust, ustPoolId) + _, ustPool := suite.preparePool(ust) + // todo make this section onwards table driven coins := sdk.NewCoins(sdk.NewInt64Coin(uion, 10), sdk.NewInt64Coin(atom, 20), sdk.NewInt64Coin(ust, 14)) swapFee := sdk.NewDec(0) - expectedOutput1, err := uionPoolI.CalcOutAmtGivenIn(suite.Ctx, - sdk.NewCoins(sdk.NewInt64Coin(uion, 10)), + expectedOutput1, err := uionPool.CalcOutAmtGivenIn(suite.Ctx, + sdk.Coins{sdk.Coin{Denom: uion, Amount: coins.AmountOf(uion)}}, baseDenom, swapFee) - expectedOutput2, err := atomPoolI.CalcOutAmtGivenIn(suite.Ctx, - sdk.NewCoins(sdk.NewInt64Coin(atom, 20)), + suite.Require().NoError(err) + expectedOutput2, err := atomPool.CalcOutAmtGivenIn(suite.Ctx, + sdk.Coins{sdk.Coin{Denom: atom, Amount: coins.AmountOf(atom)}}, baseDenom, swapFee) - expectedOutput3, err := ustPoolI.CalcOutAmtGivenIn(suite.Ctx, - sdk.NewCoins(sdk.NewInt64Coin(ust, 14)), + suite.Require().NoError(err) + expectedOutput3, err := ustPool.CalcOutAmtGivenIn(suite.Ctx, + sdk.Coins{sdk.Coin{Denom: ust, Amount: coins.AmountOf(ust)}}, baseDenom, swapFee) + suite.Require().NoError(err) fullExpectedOutput := expectedOutput1.Add(expectedOutput2).Add(expectedOutput3) From f3c0eeefbeec86f6c77f99353a468160039efb32 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 21 Apr 2022 07:49:45 -0700 Subject: [PATCH 72/75] formatting --- app/apptesting/test_suite.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index 2b46cdfd683..23a22e9460f 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -251,7 +251,7 @@ func (keeperTestHelper *KeeperTestHelper) BuildTx( return txBuilder.GetTx() } - + // CreateRandomAccounts is a function return a list of randomly generated AccAddresses func CreateRandomAccounts(numAccts int) []sdk.AccAddress { testAddrs := make([]sdk.AccAddress, numAccts) From 85d215bf611ee6e696ab7e8f651d27827cef3387 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 21 Apr 2022 08:22:11 -0700 Subject: [PATCH 73/75] formatting --- x/txfees/keeper/keeper_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/txfees/keeper/keeper_test.go b/x/txfees/keeper/keeper_test.go index 2e6dddb0298..0303f69b5d6 100644 --- a/x/txfees/keeper/keeper_test.go +++ b/x/txfees/keeper/keeper_test.go @@ -8,7 +8,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" osmosisapp "github.com/osmosis-labs/osmosis/v7/app" - + "github.com/osmosis-labs/osmosis/v7/app/apptesting" "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) @@ -29,7 +29,6 @@ func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } - func (suite *KeeperTestSuite) SetupTest(isCheckTx bool) { suite.Setup() suite.queryClient = types.NewQueryClient(suite.QueryHelper) From 2c22499d48b2aead349204a4e764992e23ab905f Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Thu, 21 Apr 2022 23:41:08 -0500 Subject: [PATCH 74/75] Fix build & keeper import --- x/txfees/keeper/keeper.go | 4 +--- x/txfees/keeper/keeper_test.go | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/x/txfees/keeper/keeper.go b/x/txfees/keeper/keeper.go index ebfd7ffd036..1c60dd2add8 100644 --- a/x/txfees/keeper/keeper.go +++ b/x/txfees/keeper/keeper.go @@ -9,8 +9,6 @@ import ( "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" - bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" - "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) @@ -30,7 +28,7 @@ type Keeper struct { func NewKeeper( cdc codec.Codec, accountKeeper types.AccountKeeper, - bankKeeper *bankkeeper.BaseKeeper, + bankKeeper types.BankKeeper, epochKeeper types.EpochKeeper, storeKey sdk.StoreKey, gammKeeper types.GammKeeper, diff --git a/x/txfees/keeper/keeper_test.go b/x/txfees/keeper/keeper_test.go index 0303f69b5d6..52f7420ec51 100644 --- a/x/txfees/keeper/keeper_test.go +++ b/x/txfees/keeper/keeper_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/suite" + "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" osmosisapp "github.com/osmosis-labs/osmosis/v7/app" @@ -16,15 +17,10 @@ import ( type KeeperTestSuite struct { apptesting.KeeperTestHelper + clientCtx client.Context queryClient types.QueryClient } -var ( - acc1 = sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address().Bytes()) - acc2 = sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address().Bytes()) - acc3 = sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address().Bytes()) -) - func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } From f34115202953076177d8b9ed0b5ac52b6fc2f63a Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Thu, 21 Apr 2022 23:42:14 -0500 Subject: [PATCH 75/75] Apply suggestions from code review Co-authored-by: Matt, Park <45252226+mattverse@users.noreply.github.com> --- x/txfees/keeper/feedecorator.go | 4 ++-- x/txfees/keeper/feedecorator_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index 2710b845ff7..950f224f47c 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -153,12 +153,12 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - // checks to make sure the module account has been set to collect fees in OSMO + // checks to make sure the module account has been set to collect fees in base token if addr := dfd.ak.GetModuleAddress(types.FeeCollectorName); addr == nil { return ctx, fmt.Errorf("Fee collector module account (%s) has not been set", types.FeeCollectorName) } - // checks to make sure a separate module account has been set to collect fees not in OSMO + // checks to make sure a separate module account has been set to collect fees not in base token if addrNonNativeFee := dfd.ak.GetModuleAddress(types.NonNativeFeeCollectorName); addrNonNativeFee == nil { return ctx, fmt.Errorf("non native fee collector module account (%s) has not been set", types.NonNativeFeeCollectorName) } diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index bd4613a8b26..204918a77e4 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -190,7 +190,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() { ) suite.ExecuteUpgradeFeeTokenProposal(uion, uionPoolId) - suite.Ctx = suite.Ctx.WithIsCheckTx(tc.isCheckTx) + suite.Ctx = suite.Ctx.WithIsCheckTx(tc.isCheckTx).WithMinGasPrices(tc.minGasPrices) suite.Ctx = suite.Ctx.WithMinGasPrices(tc.minGasPrices) // TxBuilder components reset for every test case