[TRA-417] Allocate market mapper revshare for appropriate markets (#1773

)

Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>

protocol/app/app.go

@@ -1029,6 +1029,7 @@ func New(
 		app.BankKeeper,
 		app.PerpetualsKeeper,
 		app.BlockTimeKeeper,
+		app.RevShareKeeper,
 		app.IndexerEventManager,
 	)
 	subaccountsModule := subaccountsmodule.NewAppModule(

protocol/testutil/keeper/clob.go

@@ -138,6 +138,7 @@ func NewClobKeepersTestContextWithUninitializedMemStore(
 			bankKeeper,
 			ks.PerpetualsKeeper,
 			ks.BlockTimeKeeper,
+			revShareKeeper,
 			indexerEventsTransientStoreKey,
 			true,
 		)

protocol/testutil/keeper/prices.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"testing"
 
+	revsharetypes "github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"
+
 	"github.com/cosmos/gogoproto/proto"
 
 	storetypes "cosmossdk.io/store/types"
@@ -115,6 +117,13 @@ func CreateTestMarkets(t testing.TB, ctx sdk.Context, k *keeper.Keeper) {
 			},
 		})
 		require.NoError(t, err)
+
+		// update all markets to not have revenue share
+		k.RevShareKeeper.SetMarketMapperRevShareDetails(
+			ctx,
+			uint32(i),
+			revsharetypes.MarketMapperRevShareDetails{ExpirationTs: 0},
+		)
 	}
 }
 

protocol/testutil/keeper/sending.go

@@ -92,6 +92,7 @@ func SendingKeepersWithSubaccountsKeeper(t testing.TB, saKeeper types.Subaccount
 				ks.BankKeeper,
 				ks.PerpetualsKeeper,
 				blockTimeKeeper,
+				revShareKeeper,
 				transientStoreKey,
 				true,
 			)

protocol/testutil/keeper/subaccounts.go

@@ -3,6 +3,8 @@ package keeper
 import (
 	"testing"
 
+	revsharekeeper "github.com/dydxprotocol/v4-chain/protocol/x/revshare/keeper"
+
 	"github.com/cosmos/gogoproto/proto"
 
 	dbm "github.com/cosmos/cosmos-db"
@@ -27,10 +29,7 @@ import (
 	"github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
 )
 
-func SubaccountsKeepers(
-	t testing.TB,
-	msgSenderEnabled bool,
-) (
+func SubaccountsKeepers(t testing.TB, msgSenderEnabled bool) (
 	ctx sdk.Context,
 	keeper *keeper.Keeper,
 	pricesKeeper *priceskeeper.Keeper,
@@ -39,6 +38,7 @@ func SubaccountsKeepers(
 	bankKeeper *bankkeeper.BaseKeeper,
 	assetsKeeper *asskeeper.Keeper,
 	blocktimeKeeper *blocktimekeeper.Keeper,
+	revShareKeeper *revsharekeeper.Keeper,
 	storeKey storetypes.StoreKey,
 ) {
 	var mockTimeProvider *mocks.TimeProvider
@@ -50,7 +50,7 @@ func SubaccountsKeepers(
 		transientStoreKey storetypes.StoreKey,
 	) []GenesisInitializer {
 		// Define necessary keepers here for unit tests
-		revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc)
+		revShareKeeper, _, _ = createRevShareKeeper(stateStore, db, cdc)
 		pricesKeeper, _, _, mockTimeProvider = createPricesKeeper(
 			stateStore,
 			db,
@@ -74,17 +74,27 @@ func SubaccountsKeepers(
 			bankKeeper,
 			perpetualsKeeper,
 			blocktimeKeeper,
+			revShareKeeper,
 			transientStoreKey,
 			msgSenderEnabled,
 		)
 
-		return []GenesisInitializer{pricesKeeper, perpetualsKeeper, assetsKeeper, keeper}
+		return []GenesisInitializer{pricesKeeper, perpetualsKeeper, assetsKeeper, revShareKeeper, keeper}
 	})
 
 	// Mock time provider response for market creation.
 	mockTimeProvider.On("Now").Return(constants.TimeT)
 
-	return ctx, keeper, pricesKeeper, perpetualsKeeper, accountKeeper, bankKeeper, assetsKeeper, blocktimeKeeper, storeKey
+	return ctx,
+		keeper,
+		pricesKeeper,
+		perpetualsKeeper,
+		accountKeeper,
+		bankKeeper,
+		assetsKeeper,
+		blocktimeKeeper,
+		revShareKeeper,
+		storeKey
 }
 
 func createSubaccountsKeeper(
@@ -95,6 +105,7 @@ func createSubaccountsKeeper(
 	bk types.BankKeeper,
 	pk *perpskeeper.Keeper,
 	btk *blocktimekeeper.Keeper,
+	rsk *revsharekeeper.Keeper,
 	transientStoreKey storetypes.StoreKey,
 	msgSenderEnabled bool,
 ) (*keeper.Keeper, storetypes.StoreKey) {
@@ -113,6 +124,7 @@ func createSubaccountsKeeper(
 		bk,
 		pk,
 		btk,
+		rsk,
 		mockIndexerEventsManager,
 	)
 

protocol/x/clob/keeper/process_single_match.go

@@ -427,13 +427,17 @@ func (k Keeper) persistMatchedOrders(
 		)
 	}
 
+	// TODO: get perpetual from perpetualId once and pass it to the functions that need the full
+	// perpetual object. This will reduce the number of times we need to get the perpetual from the
+	// keeper.
+
 	if err := k.subaccountsKeeper.TransferInsuranceFundPayments(ctx, insuranceFundDelta, perpetualId); err != nil {
 		return takerUpdateResult, makerUpdateResult, err
 	}
 
-	// Transfer the fee amount from subacounts module to fee collector module account.
+	// Distribute the fee amount from subacounts module to fee collector and rev share accounts
 	bigTotalFeeQuoteQuantums := new(big.Int).Add(bigTakerFeeQuoteQuantums, bigMakerFeeQuoteQuantums)
-	if err := k.subaccountsKeeper.TransferFeesToFeeCollectorModule(
+	if err := k.subaccountsKeeper.DistributeFees(
 		ctx,
 		assettypes.AssetUsdc.Id,
 		bigTotalFeeQuoteQuantums,

protocol/x/clob/types/expected_keepers.go

@@ -63,7 +63,6 @@ type SubaccountsKeeper interface {
 		perpetualId uint32,
 		blockHeight uint32,
 	) error
-	TransferFeesToFeeCollectorModule(ctx sdk.Context, assetId uint32, amount *big.Int, perpetualId uint32) error
 	TransferInsuranceFundPayments(
 		ctx sdk.Context,
 		amount *big.Int,
@@ -73,6 +72,12 @@ type SubaccountsKeeper interface {
 		ctx sdk.Context,
 		perpetualId uint32,
 	) (sdk.AccAddress, error)
+	DistributeFees(
+		ctx sdk.Context,
+		assetId uint32,
+		quantums *big.Int,
+		perpetualId uint32,
+	) error
 }
 
 type AssetsKeeper interface {

protocol/x/prices/keeper/keeper.go

@@ -27,7 +27,7 @@ type (
 		authorities                    map[string]struct{}
 		currencyPairIDCache            *CurrencyPairIDCache
 		currencyPairIdCacheInitialized *atomic.Bool
-		revShareKeeper                 types.RevShareKeeper
+		RevShareKeeper                 types.RevShareKeeper
 	}
 )
 
@@ -52,7 +52,7 @@ func NewKeeper(
 		authorities:                    lib.UniqueSliceToSet(authorities),
 		currencyPairIDCache:            NewCurrencyPairIDCache(),
 		currencyPairIdCacheInitialized: &atomic.Bool{}, // Initialized to false
-		revShareKeeper:                 revShareKeeper,
+		RevShareKeeper:                 revShareKeeper,
 	}
 }
 

protocol/x/prices/keeper/market.go

@@ -90,7 +90,7 @@ func (k Keeper) CreateMarket(
 	metrics.SetMarketPairForTelemetry(marketParam.Id, marketParam.Pair)
 
 	// create a new market rev share
-	k.revShareKeeper.CreateNewMarketRevShare(ctx, marketParam.Id)
+	k.RevShareKeeper.CreateNewMarketRevShare(ctx, marketParam.Id)
 
 	return marketParam, nil
 }

protocol/x/prices/types/expected_keepers.go

@@ -3,6 +3,8 @@ package types
 import (
 	"context"
 
+	"github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -19,4 +21,9 @@ type BankKeeper interface {
 // RevShareKeeper defines the expected revshare keeper used for simulations.
 type RevShareKeeper interface {
 	CreateNewMarketRevShare(ctx sdk.Context, marketId uint32)
+	SetMarketMapperRevShareDetails(
+		ctx sdk.Context,
+		marketId uint32,
+		params types.MarketMapperRevShareDetails,
+	)
 }

protocol/x/revshare/keeper/grpc_query_marketmapper_revshare_details_test.go

@@ -40,5 +40,5 @@ func TestQueryMarketMapperRevShareDetailsFailure(t *testing.T) {
 			MarketId: 42,
 		},
 	)
-	require.ErrorContains(t, err, "MarketMapperRevShareDetails not found for marketId: 42")
+	require.ErrorIs(t, err, types.ErrMarketMapperRevShareDetailsNotFound)
 }

protocol/x/revshare/keeper/revshare.go

@@ -1,8 +1,6 @@
 package keeper
 
 import (
-	"fmt"
-
 	"cosmossdk.io/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
@@ -58,7 +56,7 @@ func (k Keeper) GetMarketMapperRevShareDetails(
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte(types.MarketMapperRevSharePrefix))
 	b := store.Get(lib.Uint32ToKey(marketId))
 	if b == nil {
-		return params, fmt.Errorf("MarketMapperRevShareDetails not found for marketId: %d", marketId)
+		return params, types.ErrMarketMapperRevShareDetailsNotFound
 	}
 	k.cdc.MustUnmarshal(b, &params)
 	return params, nil
@@ -76,3 +74,30 @@ func (k Keeper) CreateNewMarketRevShare(ctx sdk.Context, marketId uint32) {
 	}
 	k.SetMarketMapperRevShareDetails(ctx, marketId, details)
 }
+
+func (k Keeper) GetMarketMapperRevenueShareForMarket(ctx sdk.Context, marketId uint32) (
+	address sdk.AccAddress,
+	revenueSharePpm uint32,
+	err error,
+) {
+	// get the revenue share details for the market
+	revShareDetails, err := k.GetMarketMapperRevShareDetails(ctx, marketId)
+	if err != nil {
+		return nil, 0, err
+	}
+
+	// check if the rev share details are expired
+	if revShareDetails.ExpirationTs < uint64(ctx.BlockTime().Unix()) {
+		return nil, 0, nil
+	}
+
+	// Get revenue share params
+	revShareParams := k.GetMarketMapperRevenueShareParams(ctx)
+
+	revShareAddr, err := sdk.AccAddressFromBech32(revShareParams.Address)
+	if err != nil {
+		return nil, 0, err
+	}
+
+	return revShareAddr, revShareParams.RevenueSharePpm, nil
+}

protocol/x/revshare/keeper/revshare_test.go

@@ -2,6 +2,9 @@ package keeper_test
 
 import (
 	"testing"
+	"time"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
 
@@ -43,7 +46,7 @@ func TestGetMarketMapperRevShareDetailsFailure(t *testing.T) {
 
 	// Get the rev share details for non-existent market
 	_, err := k.GetMarketMapperRevShareDetails(ctx, 42)
-	require.ErrorContains(t, err, "MarketMapperRevShareDetails not found for marketId: 42")
+	require.ErrorIs(t, err, types.ErrMarketMapperRevShareDetailsNotFound)
 }
 
 func TestCreateNewMarketRevShare(t *testing.T) {
@@ -75,3 +78,92 @@ func TestCreateNewMarketRevShare(t *testing.T) {
 	expectedExpirationTs := ctx.BlockTime().Unix() + 240*24*60*60
 	require.Equal(t, details.ExpirationTs, uint64(expectedExpirationTs))
 }
+
+func TestGetMarketMapperRevenueShareForMarket(t *testing.T) {
+	tests := map[string]struct {
+		revShareParams     types.MarketMapperRevenueShareParams
+		marketId           uint32
+		expirationDelta    int64
+		setRevShareDetails bool
+
+		// expected
+		expectedMarketMapperAddr sdk.AccAddress
+		expectedRevenueSharePpm  uint32
+		expectedErr              error
+	}{
+		"valid market": {
+			revShareParams: types.MarketMapperRevenueShareParams{
+				Address:         constants.AliceAccAddress.String(),
+				RevenueSharePpm: 100_000, // 10%
+				ValidDays:       0,
+			},
+			marketId:           42,
+			expirationDelta:    10,
+			setRevShareDetails: true,
+
+			expectedMarketMapperAddr: constants.AliceAccAddress,
+			expectedRevenueSharePpm:  100_000,
+		},
+		"invalid market": {
+			revShareParams: types.MarketMapperRevenueShareParams{
+				Address:         constants.AliceAccAddress.String(),
+				RevenueSharePpm: 100_000, // 10%
+				ValidDays:       0,
+			},
+			marketId:           42,
+			setRevShareDetails: false,
+
+			expectedErr: types.ErrMarketMapperRevShareDetailsNotFound,
+		},
+		// TODO (TRA-455): investigate why tApp blocktime doesn't translate to ctx.BlockTime()
+		//"expired market rev share": {
+		//	revShareParams: types.MarketMapperRevenueShareParams{
+		//		Address:         constants.AliceAccAddress.String(),
+		//		RevenueSharePpm: 100_000, // 10%
+		//		ValidDays:       0,
+		//	},
+		//	marketId:           42,
+		//	expirationDelta:    -10,
+		//	setRevShareDetails: true,
+		//
+		//	expectedMarketMapperAddr: nil,
+		//	expectedRevenueSharePpm:  0,
+		//},
+	}
+
+	for name, tc := range tests {
+		t.Run(
+			name, func(t *testing.T) {
+				tApp := testapp.NewTestAppBuilder(t).Build()
+				ctx := tApp.InitChain()
+				k := tApp.App.RevShareKeeper
+				tApp.AdvanceToBlock(
+					2, testapp.AdvanceToBlockOptions{BlockTime: time.Now()},
+				)
+
+				// Set base rev share params
+				err := k.SetMarketMapperRevenueShareParams(ctx, tc.revShareParams)
+				require.NoError(t, err)
+
+				// Set market rev share details
+				if tc.setRevShareDetails {
+					k.SetMarketMapperRevShareDetails(
+						ctx, tc.marketId, types.MarketMapperRevShareDetails{
+							ExpirationTs: uint64(ctx.BlockTime().Unix() + tc.expirationDelta),
+						},
+					)
+				}
+
+				// Get the revenue share for the market
+				marketMapperAddr, revenueSharePpm, err := k.GetMarketMapperRevenueShareForMarket(ctx, tc.marketId)
+				if tc.expectedErr != nil {
+					require.ErrorIs(t, err, tc.expectedErr)
+				} else {
+					require.NoError(t, err)
+					require.Equal(t, tc.expectedMarketMapperAddr, marketMapperAddr)
+					require.Equal(t, tc.expectedRevenueSharePpm, revenueSharePpm)
+				}
+			},
+		)
+	}
+}

protocol/x/revshare/types/errors.go

@@ -14,4 +14,10 @@ var (
 		2,
 		"invalid revenue share ppm",
 	)
+
+	ErrMarketMapperRevShareDetailsNotFound = errorsmod.Register(
+		ModuleName,
+		3,
+		"MarketMapperRevShareDetails not found for marketId",
+	)
 )