diff --git a/CHANGELOG.md b/CHANGELOG.md index 6baaff0c9a8..0dc4f47dc58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,9 +44,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### State Breaking -* [#7250](https://github.com/osmosis-labs/osmosis/pull/7250) Further filter spam gauges from epoch distribution. +* [#7250](https://github.com/osmosis-labs/osmosis/pull/7250) Further filter spam gauges from epoch distribution * [#7472](https://github.com/osmosis-labs/osmosis/pull/7472) Refactor TWAP keys to only require a single key format. Significantly lowers TWAP-caused writes * [#7499](https://github.com/osmosis-labs/osmosis/pull/7499) Slight speed/gas improvements to CL CreatePosition and AddToPosition +* [#7564](https://github.com/osmosis-labs/osmosis/pull/7564) Move protorev dev account bank sends from every backrun to once per epoch * [#7508](https://github.com/osmosis-labs/osmosis/pull/7508) Improve protorev performance by removing iterator and storing base denoms as a single object rather than an array. * [#7509](https://github.com/osmosis-labs/osmosis/pull/7509) Distributing ProtoRev profits to the community pool and burn address * [#7524](https://github.com/osmosis-labs/osmosis/pull/7524) Poolmanager: ListPoolsByDenom will now skip pools that cannot correctly return their constituent denoms. diff --git a/x/protorev/keeper/developer_fees.go b/x/protorev/keeper/developer_fees.go index 5d8a186161a..96b43e2b6b5 100644 --- a/x/protorev/keeper/developer_fees.go +++ b/x/protorev/keeper/developer_fees.go @@ -3,13 +3,12 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/osmosis-labs/osmosis/osmomath" "github.com/osmosis-labs/osmosis/v23/x/protorev/types" ) // DistributeProfit sends the developer fee from the module account to the developer account // and burns the remaining profit if denominated in osmo. -func (k Keeper) DistributeProfit(ctx sdk.Context, arbProfit sdk.Coin) error { +func (k Keeper) DistributeProfit(ctx sdk.Context, arbProfits sdk.Coins) error { // Developer account must be set in order to be able to withdraw developer fees developerAccount, err := k.GetDeveloperAccount(ctx) if err != nil { @@ -22,38 +21,55 @@ func (k Keeper) DistributeProfit(ctx sdk.Context, arbProfit sdk.Coin) error { return err } - // Initialize the developer profit to 0 - devProfit := sdk.NewCoin(arbProfit.Denom, osmomath.ZeroInt()) + var ( + devProfit sdk.Coins + remainingProfit sdk.Coins + profitSplit int64 + ) - // Calculate the developer fee if daysSinceGenesis < types.Phase1Length { - devProfit.Amount = arbProfit.Amount.MulRaw(types.ProfitSplitPhase1).QuoRaw(100) + profitSplit = types.ProfitSplitPhase1 } else if daysSinceGenesis < types.Phase2Length { - devProfit.Amount = arbProfit.Amount.MulRaw(types.ProfitSplitPhase2).QuoRaw(100) + profitSplit = types.ProfitSplitPhase2 } else { - devProfit.Amount = arbProfit.Amount.MulRaw(types.ProfitSplitPhase3).QuoRaw(100) + profitSplit = types.ProfitSplitPhase3 + } + + // Calculate the developer fee from all arb profits + for _, arbProfit := range arbProfits { + devProfitAmount := arbProfit.Amount.MulRaw(profitSplit).QuoRaw(100) + devProfit = append(devProfit, sdk.NewCoin(arbProfit.Denom, devProfitAmount)) } // Send the developer profit to the developer account - if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, developerAccount, sdk.NewCoins(devProfit)); err != nil { + if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, developerAccount, devProfit); err != nil { return err } - // Burn the remaining profit by sending to the null address iff the profit is denominated in osmo. - remainingProfit := sdk.NewCoin(arbProfit.Denom, arbProfit.Amount.Sub(devProfit.Amount)) - if arbProfit.Denom == types.OsmosisDenomination { - return k.bankKeeper.SendCoinsFromModuleToAccount( + // Remove the developer profit from the remaining arb profits + remainingProfit = arbProfits.Sub(devProfit...) + + // If the remaining arb profits has the OSMO denom for one of the coins, burn the OSMO by sending to the null address + arbProfitsOsmoCoin := sdk.NewCoin(types.OsmosisDenomination, remainingProfit.AmountOf(types.OsmosisDenomination)) + if arbProfitsOsmoCoin.IsPositive() { + err := k.bankKeeper.SendCoinsFromModuleToAccount( ctx, types.ModuleName, types.DefaultNullAddress, - sdk.NewCoins(remainingProfit), + sdk.NewCoins(arbProfitsOsmoCoin), ) + if err != nil { + return err + } } - // Otherwise distribute the remaining profit to the community pool. + // Remove the burned OSMO from the remaining arb profits + remainingProfit = remainingProfit.Sub(arbProfitsOsmoCoin) + + // Send all remaining arb profits to the community pool return k.distributionKeeper.FundCommunityPool( ctx, - sdk.NewCoins(remainingProfit), + remainingProfit, k.accountKeeper.GetModuleAddress(types.ModuleName), ) } diff --git a/x/protorev/keeper/developer_fees_test.go b/x/protorev/keeper/developer_fees_test.go index 27a0c638d13..267c59577ef 100644 --- a/x/protorev/keeper/developer_fees_test.go +++ b/x/protorev/keeper/developer_fees_test.go @@ -135,7 +135,7 @@ func (suite *KeeperTestSuite) TestDistributeProfit() { tc.alterState() - err := suite.App.ProtoRevKeeper.DistributeProfit(suite.Ctx, sdk.NewCoin(tc.denom, arbProfit)) + err := suite.App.ProtoRevKeeper.DistributeProfit(suite.Ctx, sdk.NewCoins(sdk.NewCoin(tc.denom, arbProfit))) if tc.expectedErr { suite.Require().Error(err) return diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index b0724d8897e..a77758df177 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -8,6 +8,7 @@ import ( "github.com/osmosis-labs/osmosis/osmomath" gammtypes "github.com/osmosis-labs/osmosis/v23/x/gamm/types" "github.com/osmosis-labs/osmosis/v23/x/protorev/types" + epochstypes "github.com/osmosis-labs/osmosis/x/epochs/types" ) type Hooks struct { @@ -15,7 +16,8 @@ type Hooks struct { } var ( - _ gammtypes.GammHooks = Hooks{} + _ gammtypes.GammHooks = Hooks{} + _ epochstypes.EpochHooks = Hooks{} ) // Create new ProtoRev hooks. @@ -90,6 +92,24 @@ func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, h.k.StoreSwap(ctx, poolId, input[0].Denom, output[0].Denom) } +// ---------------------------------------------------------------------------- +// EPOCH HOOKS +// ---------------------------------------------------------------------------- + +// Hooks wrapper struct for incentives keeper + +func (h Hooks) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) error { + return h.k.BeforeEpochStart(ctx, epochIdentifier, epochNumber) +} + +func (h Hooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error { + return h.k.AfterEpochEnd(ctx, epochIdentifier, epochNumber) +} + +func (h Hooks) GetModuleName() string { + return epochstypes.ModuleName +} + // ---------------------------------------------------------------------------- // HELPER METHODS // ---------------------------------------------------------------------------- @@ -221,3 +241,48 @@ func (k Keeper) CompareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) } } + +func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) error { + return nil +} + +func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error { + // Get the current arb profits (only in base denoms to prevent spam vector) + profit, err := k.CurrentBaseDenomProfits(ctx) + if err != nil { + return err + } + + // Distribute profits to developer account, community pool, and burn osmo + err = k.DistributeProfit(ctx, profit) + if err != nil { + return err + } + + return nil +} + +// CalculateCurrentProfit retrieves the current balance of the protorev module account and filters for base denoms. +func (k Keeper) CurrentBaseDenomProfits(ctx sdk.Context) (sdk.Coins, error) { + moduleAcc := k.accountKeeper.GetModuleAddress(types.ModuleName) + + baseDenoms, err := k.GetAllBaseDenoms(ctx) + if err != nil { + return nil, err + } + + // Get the current protorev balance of all denoms + protorevBalanceAllDenoms := k.bankKeeper.GetAllBalances(ctx, moduleAcc) + + // Filter for base denoms + var protorevBalanceBaseDenoms sdk.Coins + + for _, baseDenom := range baseDenoms { + amountOfBaseDenom := protorevBalanceAllDenoms.AmountOf(baseDenom.Denom) + if !amountOfBaseDenom.IsZero() { + protorevBalanceBaseDenoms = append(protorevBalanceBaseDenoms, sdk.NewCoin(baseDenom.Denom, amountOfBaseDenom)) + } + } + + return protorevBalanceBaseDenoms.Sort(), nil +} diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 821d1a45a5e..4859e0e938f 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -2,8 +2,10 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" "github.com/osmosis-labs/osmosis/osmomath" + "github.com/osmosis-labs/osmosis/v23/app/apptesting" "github.com/osmosis-labs/osmosis/v23/x/gamm/pool-models/balancer" "github.com/osmosis-labs/osmosis/v23/x/gamm/pool-models/stableswap" poolmanagertypes "github.com/osmosis-labs/osmosis/v23/x/poolmanager/types" @@ -848,3 +850,110 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { }) } } + +func (s *KeeperTestSuite) TestAfterEpochEnd() { + tests := []struct { + name string + arbProfits sdk.Coins + }{ + { + name: "osmo denom only", + arbProfits: sdk.NewCoins(sdk.NewCoin("uosmo", osmomath.NewInt(100000000))), + }, + { + name: "osmo denom and another base denom", + arbProfits: sdk.NewCoins(sdk.NewCoin("uosmo", osmomath.NewInt(100000000)), + sdk.NewCoin("juno", osmomath.NewInt(100000000))), + }, + { + name: "osmo denom, another base denom, and a non base denom", + arbProfits: sdk.NewCoins(sdk.NewCoin("uosmo", osmomath.NewInt(100000000)), + sdk.NewCoin("juno", osmomath.NewInt(100000000)), + sdk.NewCoin("eth", osmomath.NewInt(100000000))), + }, + { + name: "no profits", + arbProfits: sdk.Coins{}, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.SetupTest() + + // Set base denoms + baseDenoms := []types.BaseDenom{ + { + Denom: types.OsmosisDenomination, + StepSize: osmomath.NewInt(1_000_000), + }, + { + Denom: "juno", + StepSize: osmomath.NewInt(1_000_000), + }, + } + s.App.ProtoRevKeeper.SetBaseDenoms(s.Ctx, baseDenoms) + + // Set protorev developer account + devAccount := apptesting.CreateRandomAccounts(1)[0] + s.App.ProtoRevKeeper.SetDeveloperAccount(s.Ctx, devAccount) + + err := s.App.BankKeeper.MintCoins(s.Ctx, types.ModuleName, tc.arbProfits) + s.Require().NoError(err) + + communityPoolBalancePre := s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(distrtypes.ModuleName)) + + // System under test + err = s.App.ProtoRevKeeper.AfterEpochEnd(s.Ctx, "day", 1) + + expectedDevProfit := sdk.Coins{} + expectedOsmoBurn := sdk.Coins{} + arbProfitsBaseDenoms := sdk.Coins{} + arbProfitsNonBaseDenoms := sdk.Coins{} + + // Split the profits into base and non base denoms + for _, coin := range tc.arbProfits { + isBaseDenom := false + for _, baseDenom := range baseDenoms { + if coin.Denom == baseDenom.Denom { + isBaseDenom = true + break + } + } + if isBaseDenom { + arbProfitsBaseDenoms = append(arbProfitsBaseDenoms, coin) + } else { + arbProfitsNonBaseDenoms = append(arbProfitsNonBaseDenoms, coin) + } + } + profitSplit := types.ProfitSplitPhase1 + for _, arbProfit := range arbProfitsBaseDenoms { + devProfitAmount := arbProfit.Amount.MulRaw(profitSplit).QuoRaw(100) + expectedDevProfit = append(expectedDevProfit, sdk.NewCoin(arbProfit.Denom, devProfitAmount)) + } + + // Get the developer account balance + devAccountBalance := s.App.BankKeeper.GetAllBalances(s.Ctx, devAccount) + s.Require().Equal(expectedDevProfit, devAccountBalance) + + // Get the burn address balance + burnAddressBalance := s.App.BankKeeper.GetAllBalances(s.Ctx, types.DefaultNullAddress) + if arbProfitsBaseDenoms.AmountOf(types.OsmosisDenomination).IsPositive() { + expectedOsmoBurn = sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, arbProfitsBaseDenoms.AmountOf(types.OsmosisDenomination).Sub(expectedDevProfit.AmountOf(types.OsmosisDenomination)))) + s.Require().Equal(expectedOsmoBurn, burnAddressBalance) + } else { + s.Require().Equal(sdk.Coins{}, burnAddressBalance) + } + + // Get the community pool balance + communityPoolBalancePost := s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(distrtypes.ModuleName)) + actualCommunityPool := communityPoolBalancePost.Sub(communityPoolBalancePre...) + expectedCommunityPool := arbProfitsBaseDenoms.Sub(expectedDevProfit...).Sub(expectedOsmoBurn...) + s.Require().Equal(expectedCommunityPool, actualCommunityPool) + + // The protorev module account should only contain the non base denoms if there are any + protorevModuleAccount := s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(types.ModuleName)) + s.Require().Equal(arbProfitsNonBaseDenoms, protorevModuleAccount) + }) + } +} diff --git a/x/protorev/keeper/rebalance.go b/x/protorev/keeper/rebalance.go index 97ab2b27479..c7a83f48e6f 100644 --- a/x/protorev/keeper/rebalance.go +++ b/x/protorev/keeper/rebalance.go @@ -378,11 +378,6 @@ func (k Keeper) ExecuteTrade(ctx sdk.Context, route poolmanagertypes.SwapAmountI return err } - // Distribute the arbitrage profit. - if err := k.DistributeProfit(ctx, sdk.NewCoin(inputCoin.Denom, profit)); err != nil { - ctx.Logger().Error("failed to distribute arbitrage profit: " + err.Error()) - } - // Create and emit the backrun event and add it to the context EmitBackrunEvent(ctx, pool, inputCoin, profit, tokenOutAmount, remainingTxPoolPoints, remainingBlockPoolPoints) diff --git a/x/protorev/keeper/rebalance_test.go b/x/protorev/keeper/rebalance_test.go index abd049e0a9e..0902cd7429a 100644 --- a/x/protorev/keeper/rebalance_test.go +++ b/x/protorev/keeper/rebalance_test.go @@ -3,6 +3,8 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" + "github.com/osmosis-labs/osmosis/osmomath" "github.com/osmosis-labs/osmosis/v23/app/apptesting" "github.com/osmosis-labs/osmosis/v23/x/gamm/pool-models/stableswap" @@ -494,8 +496,14 @@ func (s *KeeperTestSuite) TestExecuteTrade() { txPoolPointsRemaining := uint64(100) blockPoolPointsRemaining := uint64(100) + initialDeveloperAccBalance := s.App.AppKeepers.BankKeeper.GetBalance(s.Ctx, devAccount, test.arbDenom) + initialCommunityPoolBalance := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(distrtypes.ModuleName)) + initialBurnAccBalance := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, types.DefaultNullAddress) + + cacheCtx, write := s.Ctx.CacheContext() + err := s.App.ProtoRevKeeper.ExecuteTrade( - s.Ctx, + cacheCtx, test.param.route, test.param.inputCoin, pool, @@ -504,6 +512,7 @@ func (s *KeeperTestSuite) TestExecuteTrade() { ) if test.expectPass { + write() s.Require().NoError(err) // Check the protorev statistics @@ -523,9 +532,46 @@ func (s *KeeperTestSuite) TestExecuteTrade() { s.Require().NoError(err) s.Require().Equal(test.expectedNumOfTrades, totalNumberOfTrades) - // Check the dev account was paid the correct amount + // Check the dev account was not paid anything, as epoch has not happened yet + developerAccBalanceAfterTrade := s.App.AppKeepers.BankKeeper.GetBalance(s.Ctx, devAccount, test.arbDenom) + s.Require().Equal(initialDeveloperAccBalance, developerAccBalanceAfterTrade) + + // Check that the burn address was not paid anything + burnAccBalance := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, types.DefaultNullAddress) + s.Require().Equal(initialBurnAccBalance, burnAccBalance) + + // Check that the community pool was not paid anything + communityPoolAddress := s.App.AccountKeeper.GetModuleAddress(distrtypes.ModuleName) + communityPoolBalance := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, communityPoolAddress) + s.Require().Equal(initialCommunityPoolBalance, communityPoolBalance) + + // Get the protorev module account balance + protorevModuleAcc := s.App.AccountKeeper.GetModuleAddress(types.ModuleName) + remainingProtorevAccBal := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, protorevModuleAcc) + + // Run the epoch hook + s.App.ProtoRevKeeper.AfterEpochEnd(s.Ctx, "day", 1) + + // Check the dev account was paid the correct amount after epoch developerAccBalance := s.App.AppKeepers.BankKeeper.GetBalance(s.Ctx, devAccount, test.arbDenom) s.Require().Equal(test.param.expectedProfit.MulRaw(types.ProfitSplitPhase1).QuoRaw(100), developerAccBalance.Amount) + remainingProtorevAccBal = remainingProtorevAccBal.Sub(developerAccBalance) + + // If the arb denom is osmo, check that the remaining profit was sent to the burn address + if test.arbDenom == types.OsmosisDenomination { + burnAccBalance := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, types.DefaultNullAddress) + s.Require().Equal(remainingProtorevAccBal, burnAccBalance) + remainingProtorevAccBal = remainingProtorevAccBal.Sub(burnAccBalance...) + } else { + // If the arb denom is not osmo, check that the burn address is the same as before + burnAccBalance := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, types.DefaultNullAddress) + s.Require().Equal(initialBurnAccBalance, burnAccBalance) + } + + // Check that the community pool was paid the correct amount after epoch + communityPoolBalance = s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, communityPoolAddress) + actualCommunityPoolBalance := communityPoolBalance.Sub(initialCommunityPoolBalance...) + s.Require().Equal(remainingProtorevAccBal, actualCommunityPoolBalance) } else { s.Require().Error(err) diff --git a/x/protorev/types/expected_keepers.go b/x/protorev/types/expected_keepers.go index 0ed1e9ad7e7..071d8a3db07 100644 --- a/x/protorev/types/expected_keepers.go +++ b/x/protorev/types/expected_keepers.go @@ -21,6 +21,8 @@ type BankKeeper interface { SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error BurnCoins(ctx sdk.Context, name 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 } // GAMMKeeper defines the Gamm contract that must be fulfilled when