Retroactive pr (backport #3033) (#3034)

Co-authored-by: Justin Barnett <61020572+jusbar23@users.noreply.github.com>

Author: mergify[bot]

protocol/x/clob/keeper/orders.go

@@ -187,13 +187,6 @@ func (k Keeper) PlaceShortTermOrder(
 	order.OrderId.MustBeShortTermOrder()
 	orderLabels := order.GetOrderLabels()
 
-	if _, err := k.revshareKeeper.GetOrderRouterRevShare(
-		ctx,
-		order.GetOrderRouterAddress(),
-	); err != nil {
-		return 0, 0, err
-	}
-
 	defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), metrics.PlaceOrder, metrics.Latency)
 	defer func() {
 		telemetry.IncrCounterWithLabels(

protocol/x/clob/keeper/orders_test.go

@@ -31,7 +31,6 @@ import (
 	"github.com/dydxprotocol/v4-chain/protocol/x/perpetuals"
 	perptypes "github.com/dydxprotocol/v4-chain/protocol/x/perpetuals/types"
 	"github.com/dydxprotocol/v4-chain/protocol/x/prices"
-	revsharetypes "github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"
 	rewardtypes "github.com/dydxprotocol/v4-chain/protocol/x/rewards/types"
 	statstypes "github.com/dydxprotocol/v4-chain/protocol/x/stats/types"
 	satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
@@ -365,7 +364,7 @@ func TestPlaceShortTermOrder(t *testing.T) {
 				constants.BtcUsd_SmallMarginRequirement.Params.Id: big.NewInt(100_000_000),
 			},
 		},
-		"Cannot open an order with an invalid order router address": {
+		"Can open an order with an invalid order router address": {
 			perpetuals: []perptypes.Perpetual{
 				constants.BtcUsd_100PercentMarginRequirement,
 			},
@@ -388,7 +387,12 @@ func TestPlaceShortTermOrder(t *testing.T) {
 				OrderRouterAddress: constants.CarlAccAddress.String(),
 			},
 
-			expectedErr: revsharetypes.ErrOrderRouterRevShareNotFound,
+			expectedOrderStatus: types.Success,
+			expectedFilledSize:  0,
+			expectedOpenInterests: map[uint32]*big.Int{
+				// unchanged, no match happened
+				constants.BtcUsd_100PercentMarginRequirement.Params.Id: big.NewInt(100_000_000),
+			},
 		},
 		`New order should be undercollateralized when matching when previous fills make it undercollateralized when using
 				maker orders subticks, but would be collateralized if using taker order subticks`: {
@@ -1207,7 +1211,7 @@ func TestPerformStatefulOrderValidation(t *testing.T) {
 			},
 			expectedErr: "must be a multiple of the ClobPair's StepBaseQuantums",
 		},
-		"Fails if Order router address is not found": {
+		"Still succeeds if Order router address is not found": {
 			order: types.Order{
 				OrderId: types.OrderId{
 					ClientId:     0,
@@ -1220,7 +1224,6 @@ func TestPerformStatefulOrderValidation(t *testing.T) {
 				GoodTilOneof:       &types.Order_GoodTilBlock{GoodTilBlock: blockHeight + 5},
 				OrderRouterAddress: constants.AliceAccAddress.String(),
 			},
-			expectedErr: "order router rev share not found",
 		},
 		"Fails if GoodTilBlock is in the past": {
 			order: types.Order{

protocol/x/revshare/keeper/revshare.go

@@ -222,11 +222,10 @@ func (k Keeper) GetAllRevShares(
 	var orderRouterRevShares []types.RevShare
 	netFeesSubRevenueShare := new(big.Int).Set(netFees)
 	// No affiliate fees shared, so we can generate order router rev shares
-	if affiliateRevShares == nil {
-		orderRouterRevShares, err = k.getOrderRouterRevShares(ctx, fill, takerFees, makerFees)
-		if err != nil {
-			return types.RevSharesForFill{}, err
-		}
+	// In the case that the taker has an affiliate fee and the maker does not, then no order router fees are generated
+	// for the maker or the taker
+	if len(affiliateRevShares) == 0 {
+		orderRouterRevShares = k.getOrderRouterRevShares(ctx, fill, takerFees, makerFees)
 		for _, revShare := range orderRouterRevShares {
 			netFeesSubRevenueShare.Sub(netFeesSubRevenueShare, revShare.QuoteQuantums)
 		}
@@ -325,58 +324,54 @@ func (k Keeper) getOrderRouterRevShares(
 	fill clobtypes.FillForProcess,
 	takerFees *big.Int,
 	makerFees *big.Int,
-) ([]types.RevShare, error) {
+) []types.RevShare {
 	if fill.TakerOrderRouterAddr == "" && fill.MakerOrderRouterAddr == "" {
-		return nil, nil
+		return []types.RevShare{}
 	}
 
 	orderRouterRevShares := []types.RevShare{}
 	takerOrderRouterRevSharePpm, err := k.GetOrderRouterRevShare(ctx, fill.TakerOrderRouterAddr)
 	if err != nil {
-		// This should never happen
-		k.Logger(ctx).Error("order router rev share not found for taker: " + fill.TakerOrderRouterAddr)
-		return nil, err
-	}
-
-	if fill.TakerOrderRouterAddr != "" {
-		// Orders can have 2 rev share ids, we need to calculate each side separately
-		// This is taker ppm * min(taker, taker - maker_rebate)
-		takerFeesSide := lib.BigMin(takerFees, new(big.Int).Add(takerFees, makerFees))
-		takerRevShare := lib.BigMulPpm(lib.BigU(takerOrderRouterRevSharePpm), takerFeesSide, false)
-		orderRouterRevShares = append(orderRouterRevShares, types.RevShare{
-			Recipient:         fill.TakerOrderRouterAddr,
-			RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_TAKER_FEE,
-			RevShareType:      types.REV_SHARE_TYPE_ORDER_ROUTER,
-			QuoteQuantums:     takerRevShare,
-			RevSharePpm:       takerOrderRouterRevSharePpm,
-		})
+		k.Logger(ctx).Error("order router rev share invalid for taker: " + fill.TakerOrderRouterAddr)
+	} else {
+		if fill.TakerOrderRouterAddr != "" {
+			// Orders can have 2 rev share ids, we need to calculate each side separately
+			// This is taker ppm * min(taker, taker - maker_rebate)
+			takerFeesSide := lib.BigMin(takerFees, new(big.Int).Add(takerFees, makerFees))
+			takerRevShare := lib.BigMulPpm(lib.BigU(takerOrderRouterRevSharePpm), takerFeesSide, false)
+			orderRouterRevShares = append(orderRouterRevShares, types.RevShare{
+				Recipient:         fill.TakerOrderRouterAddr,
+				RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_TAKER_FEE,
+				RevShareType:      types.REV_SHARE_TYPE_ORDER_ROUTER,
+				QuoteQuantums:     takerRevShare,
+				RevSharePpm:       takerOrderRouterRevSharePpm,
+			})
+		}
 	}
 
 	makerOrderRouterRevSharePpm, err := k.GetOrderRouterRevShare(ctx, fill.MakerOrderRouterAddr)
 	if err != nil {
-		// This should never happen
-		k.Logger(ctx).Error("order router rev share not found for maker: " + fill.MakerOrderRouterAddr)
-		return nil, err
-	}
-
-	if fill.MakerOrderRouterAddr != "" {
-		// maker ppm * max(0, maker)
-		makerFeeSide := lib.BigMax(lib.BigI(0), makerFees)
-		makerRevShare := lib.BigMulPpm(makerFeeSide,
-			lib.BigU(makerOrderRouterRevSharePpm),
-			false,
-		)
-
-		orderRouterRevShares = append(orderRouterRevShares, types.RevShare{
-			Recipient:         fill.MakerOrderRouterAddr,
-			RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_MAKER_FEE,
-			RevShareType:      types.REV_SHARE_TYPE_ORDER_ROUTER,
-			QuoteQuantums:     makerRevShare,
-			RevSharePpm:       makerOrderRouterRevSharePpm,
-		})
+		k.Logger(ctx).Error("order router rev share invalid for maker: " + fill.MakerOrderRouterAddr)
+	} else {
+		if fill.MakerOrderRouterAddr != "" {
+			// maker ppm * max(0, maker)
+			makerFeeSide := lib.BigMax(lib.BigI(0), makerFees)
+			makerRevShare := lib.BigMulPpm(makerFeeSide,
+				lib.BigU(makerOrderRouterRevSharePpm),
+				false,
+			)
+
+			orderRouterRevShares = append(orderRouterRevShares, types.RevShare{
+				Recipient:         fill.MakerOrderRouterAddr,
+				RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_MAKER_FEE,
+				RevShareType:      types.REV_SHARE_TYPE_ORDER_ROUTER,
+				QuoteQuantums:     makerRevShare,
+				RevSharePpm:       makerOrderRouterRevSharePpm,
+			})
+		}
 	}
 
-	return orderRouterRevShares, nil
+	return orderRouterRevShares
 }
 
 func (k Keeper) getUnconditionalRevShares(

protocol/x/revshare/keeper/revshare_test.go

@@ -1029,6 +1029,74 @@ func TestKeeper_GetAllRevShares_Valid(t *testing.T) {
 				require.NoError(t, err)
 			},
 		},
+		{
+			name: "Rev share populates order router rev share even if one is missing",
+			expectedRevSharesForFill: types.RevSharesForFill{
+				AllRevShares: []types.RevShare{
+					{
+						Recipient:         constants.DaveAccAddress.String(),
+						RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_MAKER_FEE,
+						RevShareType:      types.REV_SHARE_TYPE_ORDER_ROUTER,
+						QuoteQuantums:     big.NewInt(400_000),
+						RevSharePpm:       200_000, // 20%
+					},
+					{
+						Recipient:         constants.AliceAccAddress.String(),
+						RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE,
+						RevShareType:      types.REV_SHARE_TYPE_MARKET_MAPPER,
+						QuoteQuantums:     big.NewInt(1_160_000),
+						RevSharePpm:       100_000, // 10%
+					},
+				},
+				AffiliateRevShare: nil,
+				FeeSourceToQuoteQuantums: map[types.RevShareFeeSource]*big.Int{
+					types.REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE: big.NewInt(1_160_000),
+					types.REV_SHARE_FEE_SOURCE_TAKER_FEE:            big.NewInt(0),
+					types.REV_SHARE_FEE_SOURCE_MAKER_FEE:            big.NewInt(400_000),
+				},
+				FeeSourceToRevSharePpm: map[types.RevShareFeeSource]uint32{
+					types.REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE: 100_000, // 10%
+					types.REV_SHARE_FEE_SOURCE_TAKER_FEE:            0,       // 10%
+					types.REV_SHARE_FEE_SOURCE_MAKER_FEE:            200_000, // 20%
+				},
+			},
+			fill: clobtypes.FillForProcess{
+				TakerAddr:                         constants.AliceAccAddress.String(),
+				TakerFeeQuoteQuantums:             big.NewInt(10_000_000),
+				MakerAddr:                         constants.BobAccAddress.String(),
+				MakerFeeQuoteQuantums:             big.NewInt(2_000_000),
+				FillQuoteQuantums:                 big.NewInt(100_000_000_000),
+				ProductId:                         marketId,
+				MonthlyRollingTakerVolumeQuantums: 1_000_000_000_000,
+				TakerOrderRouterAddr:              constants.CarlAccAddress.String(),
+				MakerOrderRouterAddr:              constants.DaveAccAddress.String(),
+			},
+			setup: func(tApp *testapp.TestApp, ctx sdk.Context, keeper *keeper.Keeper,
+				affiliatesKeeper *affiliateskeeper.Keeper) {
+				err := keeper.SetMarketMapperRevenueShareParams(ctx, types.MarketMapperRevenueShareParams{
+					Address:         constants.AliceAccAddress.String(),
+					RevenueSharePpm: 100_000, // 10%
+					ValidDays:       1,
+				})
+				require.NoError(t, err)
+
+				err = affiliatesKeeper.UpdateAffiliateTiers(ctx, affiliatetypes.DefaultAffiliateTiers)
+				require.NoError(t, err)
+
+				err = keeper.SetOrderRouterRevShare(ctx, constants.DaveAccAddress.String(), 200_000) // 20%
+				require.NoError(t, err)
+
+				err = affiliatesKeeper.SetAffiliateWhitelist(ctx, affiliatetypes.AffiliateWhitelist{
+					Tiers: []affiliatetypes.AffiliateWhitelist_Tier{
+						{
+							Addresses:        []string{constants.BobAccAddress.String()},
+							TakerFeeSharePpm: 250_000, // 25%
+						},
+					},
+				})
+				require.NoError(t, err)
+			},
+		},
 		{
 			name: "Rev share populates order router rev share if affiliates is empty",
 			expectedRevSharesForFill: types.RevSharesForFill{