feat: Burning ProtoRev Profit (#7509)

* init

* nit

* send to community pool

* change log entry

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

CHANGELOG.md

@@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#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
 * [#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
 * [#7562](https://github.com/osmosis-labs/osmosis/pull/7562) Speedup Protorev estimation logic by removing unnecessary taker fee simulations.
 
 ## v23.0.0

app/keepers/keepers.go

@@ -391,6 +391,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 		appKeepers.EpochsKeeper,
 		appKeepers.PoolManagerKeeper,
 		appKeepers.ConcentratedLiquidityKeeper,
+		appKeepers.DistrKeeper,
 	)
 	appKeepers.ProtoRevKeeper = &protorevKeeper
 	appKeepers.PoolManagerKeeper.SetProtorevKeeper(appKeepers.ProtoRevKeeper)

x/protorev/keeper/developer_fees.go

@@ -7,8 +7,9 @@ import (
 	"github.com/osmosis-labs/osmosis/v23/x/protorev/types"
 )
 
-// SendDeveloperFee sends the developer fee from the module account to the developer account
-func (k Keeper) SendDeveloperFee(ctx sdk.Context, arbProfit sdk.Coin) error {
+// 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 {
 	// Developer account must be set in order to be able to withdraw developer fees
 	developerAccount, err := k.GetDeveloperAccount(ctx)
 	if err != nil {
@@ -38,5 +39,21 @@ func (k Keeper) SendDeveloperFee(ctx sdk.Context, arbProfit sdk.Coin) error {
 		return err
 	}
 
-	return nil
+	// 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(
+			ctx,
+			types.ModuleName,
+			types.DefaultNullAddress,
+			sdk.NewCoins(remainingProfit),
+		)
+	}
+
+	// Otherwise distribute the remaining profit to the community pool.
+	return k.distributionKeeper.FundCommunityPool(
+		ctx,
+		sdk.NewCoins(remainingProfit),
+		k.accountKeeper.GetModuleAddress(types.ModuleName),
+	)
 }

x/protorev/keeper/developer_fees_test.go

@@ -3,82 +3,160 @@ package keeper_test
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	distributiontypes "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/protorev/types"
 )
 
-// TestSendDeveloperFee tests the SendDeveloperFee function
-func (suite *KeeperTestSuite) TestSendDeveloperFee() {
+var (
+	usdcDenom = "usdc"
+	arbProfit = osmomath.NewInt(1000)
+)
+
+func (suite *KeeperTestSuite) TestDistributeProfit() {
 	cases := []struct {
 		description       string
 		alterState        func()
+		denom             string
 		expectedErr       bool
 		expectedDevProfit sdk.Coin
+		expectedBurnBal   sdk.Coin
+		expectedCommBal   sdk.Coin
 	}{
 		{
 			description:       "Send with unset developer account",
 			alterState:        func() {},
+			denom:             types.OsmosisDenomination,
 			expectedErr:       true,
 			expectedDevProfit: sdk.Coin{},
+			expectedBurnBal:   sdk.Coin{},
+			expectedCommBal:   sdk.Coin{},
 		},
 		{
 			description: "Send with set developer account in first phase",
 			alterState: func() {
 				account := apptesting.CreateRandomAccounts(1)[0]
 				suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)
 
-				err := suite.pseudoExecuteTrade(types.OsmosisDenomination, osmomath.NewInt(1000), 100)
+				err := suite.pseudoExecuteTrade(types.OsmosisDenomination, arbProfit, 100)
 				suite.Require().NoError(err)
 			},
+			denom:             types.OsmosisDenomination,
 			expectedErr:       false,
-			expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(20)),
+			expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(200)),
+			expectedBurnBal:   sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(800)),
+			expectedCommBal:   sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(0)),
 		},
 		{
 			description: "Send with set developer account in second phase",
 			alterState: func() {
 				account := apptesting.CreateRandomAccounts(1)[0]
 				suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)
 
-				err := suite.pseudoExecuteTrade(types.OsmosisDenomination, osmomath.NewInt(1000), 500)
+				err := suite.pseudoExecuteTrade(types.OsmosisDenomination, arbProfit, 500)
 				suite.Require().NoError(err)
 			},
+			denom:             types.OsmosisDenomination,
 			expectedErr:       false,
-			expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(10)),
+			expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(100)),
+			expectedBurnBal:   sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(900)),
+			expectedCommBal:   sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(0)),
 		},
 		{
 			description: "Send with set developer account in third (final) phase",
 			alterState: func() {
 				account := apptesting.CreateRandomAccounts(1)[0]
 				suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)
 
-				err := suite.pseudoExecuteTrade(types.OsmosisDenomination, osmomath.NewInt(1000), 1000)
+				err := suite.pseudoExecuteTrade(types.OsmosisDenomination, arbProfit, 1000)
 				suite.Require().NoError(err)
 			},
+			denom:             types.OsmosisDenomination,
 			expectedErr:       false,
-			expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(5)),
+			expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(50)),
+			expectedBurnBal:   sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(950)),
+			expectedCommBal:   sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(0)),
+		},
+		{
+			description: "Send with set developer account in first phase with non-osmo profit",
+			alterState: func() {
+				account := apptesting.CreateRandomAccounts(1)[0]
+				suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)
+
+				err := suite.pseudoExecuteTrade(usdcDenom, arbProfit, 100)
+				suite.Require().NoError(err)
+			},
+			denom:             usdcDenom,
+			expectedErr:       false,
+			expectedDevProfit: sdk.NewCoin(usdcDenom, osmomath.NewInt(200)),
+			expectedBurnBal:   sdk.NewCoin(usdcDenom, osmomath.NewInt(0)),
+			expectedCommBal:   sdk.NewCoin(usdcDenom, osmomath.NewInt(800)),
+		},
+		{
+			description: "Send with set developer account in second phase with non-osmo profit",
+			alterState: func() {
+				account := apptesting.CreateRandomAccounts(1)[0]
+				suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)
+
+				err := suite.pseudoExecuteTrade(usdcDenom, arbProfit, 500)
+				suite.Require().NoError(err)
+			},
+			denom:             usdcDenom,
+			expectedErr:       false,
+			expectedDevProfit: sdk.NewCoin(usdcDenom, osmomath.NewInt(100)),
+			expectedBurnBal:   sdk.NewCoin(usdcDenom, osmomath.NewInt(0)),
+			expectedCommBal:   sdk.NewCoin(usdcDenom, osmomath.NewInt(900)),
+		},
+		{
+			description: "Send with set developer account in third (final) phase with non-osmo profit",
+			alterState: func() {
+				account := apptesting.CreateRandomAccounts(1)[0]
+				suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)
+
+				err := suite.pseudoExecuteTrade(usdcDenom, arbProfit, 1000)
+				suite.Require().NoError(err)
+			},
+			denom:             usdcDenom,
+			expectedErr:       false,
+			expectedDevProfit: sdk.NewCoin(usdcDenom, osmomath.NewInt(50)),
+			expectedBurnBal:   sdk.NewCoin(usdcDenom, osmomath.NewInt(0)),
+			expectedCommBal:   sdk.NewCoin(usdcDenom, osmomath.NewInt(950)),
 		},
 	}
 
 	for _, tc := range cases {
 		suite.Run(tc.description, func() {
 			suite.SetupTest()
+
+			commAccount := suite.App.AppKeepers.AccountKeeper.GetModuleAddress(distributiontypes.ModuleName)
+			commBalanceBefore := suite.App.AppKeepers.BankKeeper.GetBalance(suite.Ctx, commAccount, tc.denom)
+
 			tc.alterState()
 
-			err := suite.App.ProtoRevKeeper.SendDeveloperFee(suite.Ctx, sdk.NewCoin(types.OsmosisDenomination, osmomath.NewInt(100)))
+			err := suite.App.ProtoRevKeeper.DistributeProfit(suite.Ctx, sdk.NewCoin(tc.denom, arbProfit))
 			if tc.expectedErr {
 				suite.Require().Error(err)
+				return
 			} else {
 				suite.Require().NoError(err)
 			}
 
+			// Validate the developer account balance.
 			developerAccount, err := suite.App.ProtoRevKeeper.GetDeveloperAccount(suite.Ctx)
-			if !tc.expectedErr {
-				developerFee := suite.App.AppKeepers.BankKeeper.GetBalance(suite.Ctx, developerAccount, types.OsmosisDenomination)
-				suite.Require().Equal(tc.expectedDevProfit, developerFee)
-			} else {
-				suite.Require().Error(err)
-			}
+			suite.Require().NoError(err)
+			developerFee := suite.App.AppKeepers.BankKeeper.GetBalance(suite.Ctx, developerAccount, tc.denom)
+			suite.Require().True(tc.expectedDevProfit.Equal(developerFee))
+
+			// Validate the module community pool balance.
+			commBalanceAfter := suite.App.AppKeepers.BankKeeper.GetBalance(suite.Ctx, commAccount, tc.denom)
+			diff := commBalanceAfter.Sub(commBalanceBefore)
+			suite.Require().True(tc.expectedCommBal.Equal(diff))
+
+			// Validate the burn balance.
+			burnBal := suite.App.AppKeepers.BankKeeper.GetBalance(suite.Ctx, types.DefaultNullAddress, tc.denom)
+			suite.Require().True(tc.expectedBurnBal.Equal(burnBal))
 		})
 	}
 }

x/protorev/keeper/keeper.go

@@ -25,6 +25,7 @@ type (
 		epochKeeper                 types.EpochKeeper
 		poolmanagerKeeper           types.PoolManagerKeeper
 		concentratedLiquidityKeeper types.ConcentratedLiquidityKeeper
+		distributionKeeper          types.DistributionKeeper
 	}
 )
 
@@ -38,6 +39,7 @@ func NewKeeper(
 	epochKeeper types.EpochKeeper,
 	poolmanagerKeeper types.PoolManagerKeeper,
 	concentratedLiquidityKeeper types.ConcentratedLiquidityKeeper,
+	distributionKeeper types.DistributionKeeper,
 ) Keeper {
 	// set KeyTable if it has not already been set
 	if !ps.HasKeyTable() {
@@ -54,6 +56,7 @@ func NewKeeper(
 		epochKeeper:                 epochKeeper,
 		poolmanagerKeeper:           poolmanagerKeeper,
 		concentratedLiquidityKeeper: concentratedLiquidityKeeper,
+		distributionKeeper:          distributionKeeper,
 	}
 }
 

x/protorev/keeper/rebalance.go

@@ -378,9 +378,9 @@ func (k Keeper) ExecuteTrade(ctx sdk.Context, route poolmanagertypes.SwapAmountI
 		return err
 	}
 
-	// Send the developer fee to the developer address
-	if err := k.SendDeveloperFee(ctx, sdk.NewCoin(inputCoin.Denom, profit)); err != nil {
-		ctx.Logger().Error("failed to send developer fee: " + err.Error())
+	// 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

x/protorev/types/expected_keepers.go

@@ -81,3 +81,9 @@ type ConcentratedLiquidityKeeper interface {
 		maxTicksCrossed uint64,
 	) (maxTokenIn, resultingTokenOut sdk.Coin, err error)
 }
+
+// DistributionKeeper defines the distribution contract that must be fulfilled when
+// creating a x/protorev keeper.
+type DistributionKeeper interface {
+	FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
+}

x/protorev/types/genesis_test.go

@@ -3,6 +3,7 @@ package types_test
 import (
 	"testing"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
 
 	"github.com/osmosis-labs/osmosis/v23/x/protorev/types"
@@ -33,3 +34,9 @@ func TestGenesisStateValidate(t *testing.T) {
 		})
 	}
 }
+
+func TestNullAccount(t *testing.T) {
+	strAddress, err := sdk.AccAddressFromBech32("osmo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqmcn030")
+	require.NoError(t, err)
+	require.True(t, types.DefaultNullAddress.Equals(strAddress))
+}

x/protorev/types/params.go

@@ -15,6 +15,10 @@ var (
 	// All the settings manager's controls have limits, so it can't lead to a chain halt, excess processing time or prevention of swaps.
 	DefaultAdminAccount = "osmo17nv67dvc7f8yr00rhgxd688gcn9t9wvhn783z4"
 
+	// DefaultNullAddress is the default value for the null account. ProtoRev arbitrage profits denominated in OSMO are sent to this account
+	// as of V24. This address is equivalent to osmo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqmcn030.
+	DefaultNullAddress = sdk.AccAddress([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})
+
 	ParamStoreKeyEnableModule = []byte("EnableProtoRevModule")
 	ParamStoreKeyAdminAccount = []byte("AdminAccount")
 )