perf(protorev): send rewards to dev acc once per epoch (#7564)

* send rewards to dev acc once per epoch

* changelog

* spell fix

* merge

* Generated protofile changes

* fix test

* test and remove early return

* wording

* remove need for storing last epochs profits

* remove unused key

* add hooks test

* test clarify

---------

Co-authored-by: github-actions <[email protected]>

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.

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),
 	)
 }

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

x/protorev/keeper/hooks.go

@@ -8,14 +8,16 @@ 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 {
 	k Keeper
 }
 
 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
+}

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)
+		})
+	}
+}

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)