add hooks test

x/protorev/keeper/developer_fees.go

@@ -35,21 +35,21 @@ func (k Keeper) DistributeProfit(ctx sdk.Context, arbProfits sdk.Coins) error {
 		profitSplit = types.ProfitSplitPhase3
 	}
 
+	// Calculate the developer fee from all arb profits
 	for _, arbProfit := range arbProfits {
-		// Calculate the developer fee
 		devProfitAmount := arbProfit.Amount.MulRaw(profitSplit).QuoRaw(100)
 		devProfit = append(devProfit, sdk.NewCoin(arbProfit.Denom, devProfitAmount))
 	}
 
-	// Calculate the remaining profit
-	remainingProfit = arbProfits.Sub(devProfit...)
-
 	// Send the developer profit to the developer account
 	if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, developerAccount, devProfit); err != nil {
 		return err
 	}
 
-	// If the osmo denom is part of total remaining profits, burn the osmo by sending to the null address iff the profit is denominated in osmo.
+	// 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(
@@ -63,9 +63,10 @@ func (k Keeper) DistributeProfit(ctx sdk.Context, arbProfits sdk.Coins) error {
 		}
 	}
 
+	// Remove the burned OSMO from the remaining arb profits
 	remainingProfit = remainingProfit.Sub(arbProfitsOsmoCoin)
 
-	// Send all remaining profit to the community pool.
+	// Send all remaining arb profits to the community pool
 	return k.distributionKeeper.FundCommunityPool(
 		ctx,
 		remainingProfit,

x/protorev/keeper/hooks.go

@@ -2,7 +2,6 @@ package keeper
 
 import (
 	"errors"
-	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
@@ -248,7 +247,7 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN
 }
 
 func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error {
-	// Get the current profit
+	// Get the current arb profits (only in base denoms to prevent spam vector)
 	profit, err := k.CurrentBaseDenomProfits(ctx)
 	if err != nil {
 		return err
@@ -257,7 +256,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb
 	// Distribute profits to developer account, community pool, and burn osmo
 	err = k.DistributeProfit(ctx, profit)
 	if err != nil {
-		return fmt.Errorf("failed to send developer fee: %w", err)
+		return err
 	}
 
 	return 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,113 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() {
 		})
 	}
 }
+
+func (s *KeeperTestSuite) TestAfterEpochEnd() {
+	tests := []struct {
+		name       string
+		arbProfits sdk.Coins
+		expectPass bool
+	}{
+		{
+			name:       "osmo denom only",
+			arbProfits: sdk.NewCoins(sdk.NewCoin("uosmo", osmomath.NewInt(100000000))),
+			expectPass: true,
+		},
+		{
+			name: "osmo denom and another base denom",
+			arbProfits: sdk.NewCoins(sdk.NewCoin("uosmo", osmomath.NewInt(100000000)),
+				sdk.NewCoin("juno", osmomath.NewInt(100000000))),
+			expectPass: true,
+		},
+		{
+			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))),
+			expectPass: true,
+		},
+		{
+			name:       "no profits",
+			arbProfits: sdk.Coins{},
+			expectPass: true,
+		},
+	}
+
+	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))
+
+			err = s.App.ProtoRevKeeper.AfterEpochEnd(s.Ctx, "day", 1)
+			if tc.expectPass {
+				expectedDevProfit := sdk.Coins{}
+				expectedOsmoBurn := sdk.Coins{}
+				arbProfitsBaseDenoms := sdk.Coins{}
+				arbProfitsNonBaseDenoms := sdk.Coins{}
+				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)
+			}
+		})
+	}
+}