address comment

dydxprotocol · Sep 24, 2024 · 098ad3d · 098ad3d
1 parent 962942e
commit 098ad3d
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 15 deletions.
diff --git a/protocol/x/clob/keeper/process_single_match.go b/protocol/x/clob/keeper/process_single_match.go
@@ -14,6 +14,7 @@ import (
 	"github.com/dydxprotocol/v4-chain/protocol/lib/metrics"
 	assettypes "github.com/dydxprotocol/v4-chain/protocol/x/assets/types"
 	"github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
+	revsharetypes "github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"
 	satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
 	gometrics "github.com/hashicorp/go-metrics"
 )
@@ -464,7 +465,10 @@ func (k Keeper) persistMatchedOrders(
 	// Distribute the fee amount from subacounts module to fee collector and rev share accounts
 	bigTotalFeeQuoteQuantums := new(big.Int).Add(bigTakerFeeQuoteQuantums, bigMakerFeeQuoteQuantums)
 	revSharesForFill, err := k.revshareKeeper.GetAllRevShares(ctx, fillForProcess, affiliatesWhitelistMap)
-
+	if err != nil {
+		revSharesForFill = revsharetypes.RevSharesForFill{}
+		log.ErrorLog(ctx, "error getting rev shares for fill", "error", err)
+	}
 	if revSharesForFill.AffiliateRevShare != nil {
 		affiliateRevSharesQuoteQuantums = revSharesForFill.AffiliateRevShare.QuoteQuantums
 	}

diff --git a/protocol/x/revshare/keeper/revshare.go b/protocol/x/revshare/keeper/revshare.go
@@ -6,7 +6,6 @@ import (
 	"cosmossdk.io/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
-	"github.com/dydxprotocol/v4-chain/protocol/lib/log"
 	affiliatetypes "github.com/dydxprotocol/v4-chain/protocol/x/affiliates/types"
 	clobtypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
 	"github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"
@@ -173,24 +172,20 @@ func (k Keeper) GetAllRevShares(
 
 	affiliateRevShares, affiliateFeesShared, err := k.getAffiliateRevShares(ctx, fill, affiliatesWhitelistMap)
 	if err != nil {
-		log.ErrorLogWithError(ctx, "error getting affiliate rev shares", err)
-		return types.RevSharesForFill{}, nil
+		return types.RevSharesForFill{}, err
 	}
 	netFeesSubAffiliateFeesShared := new(big.Int).Sub(netFees, affiliateFeesShared)
 	if netFeesSubAffiliateFeesShared.Sign() <= 0 {
-		log.ErrorLog(ctx, "net fees sub affiliate fees shared is less than or equal to 0")
-		return types.RevSharesForFill{}, nil
+		return types.RevSharesForFill{}, err
 	}
 
 	unconditionalRevShares, err := k.getUnconditionalRevShares(ctx, netFeesSubAffiliateFeesShared)
 	if err != nil {
-		log.ErrorLogWithError(ctx, "error getting unconditional rev shares", err)
-		return types.RevSharesForFill{}, nil
+		return types.RevSharesForFill{}, err
 	}
 	marketMapperRevShares, err := k.getMarketMapperRevShare(ctx, fill.MarketId, netFeesSubAffiliateFeesShared)
 	if err != nil {
-		log.ErrorLogWithError(ctx, "error getting market mapper rev shares", err)
-		return types.RevSharesForFill{}, nil
+		return types.RevSharesForFill{}, err
 	}
 
 	revShares = append(revShares, affiliateRevShares...)
@@ -215,8 +210,7 @@ func (k Keeper) GetAllRevShares(
 	}
 	//check total fees shared is less than or equal to net fees
 	if totalFeesShared.Cmp(netFees) > 0 {
-		log.ErrorLog(ctx, "total fees shared exceeds net fees")
-		return types.RevSharesForFill{}, nil
+		return types.RevSharesForFill{}, types.ErrTotalFeesSharedExceedsNetFees
 	}
 
 	return types.RevSharesForFill{

diff --git a/protocol/x/revshare/keeper/revshare_test.go b/protocol/x/revshare/keeper/revshare_test.go
@@ -826,6 +826,7 @@ func TestKeeper_GetAllRevShares_Invalid(t *testing.T) {
 		revenueSharePpmNetFees            uint32
 		revenueSharePpmTakerFees          uint32
 		monthlyRollingTakerVolumeQuantums uint64
+		expectedError                     error
 		setup                             func(tApp *testapp.TestApp, ctx sdk.Context,
 			keeper *keeper.Keeper, affiliatesKeeper *affiliateskeeper.Keeper)
 	}{
@@ -834,6 +835,7 @@ func TestKeeper_GetAllRevShares_Invalid(t *testing.T) {
 			revenueSharePpmNetFees:            950_000,           // 95%,
 			revenueSharePpmTakerFees:          150_000,           // 15%
 			monthlyRollingTakerVolumeQuantums: 1_000_000_000_000, // 1 million USDC
+			expectedError:                     types.ErrTotalFeesSharedExceedsNetFees,
 			setup: func(tApp *testapp.TestApp, ctx sdk.Context, keeper *keeper.Keeper,
 				affiliatesKeeper *affiliateskeeper.Keeper) {
 				err := keeper.SetMarketMapperRevenueShareParams(ctx, types.MarketMapperRevenueShareParams{
@@ -863,6 +865,7 @@ func TestKeeper_GetAllRevShares_Invalid(t *testing.T) {
 			revenueSharePpmNetFees:            1_150_000,         // 115%,
 			revenueSharePpmTakerFees:          0,                 // 0%
 			monthlyRollingTakerVolumeQuantums: 1_000_000_000_000, // 1 million USDC
+			expectedError:                     types.ErrTotalFeesSharedExceedsNetFees,
 			setup: func(tApp *testapp.TestApp, ctx sdk.Context, keeper *keeper.Keeper,
 				affiliatesKeeper *affiliateskeeper.Keeper) {
 				err := keeper.SetMarketMapperRevenueShareParams(ctx, types.MarketMapperRevenueShareParams{
@@ -907,10 +910,9 @@ func TestKeeper_GetAllRevShares_Invalid(t *testing.T) {
 
 			keeper.CreateNewMarketRevShare(ctx, marketId)
 
-			revSharesForFill, err := keeper.GetAllRevShares(ctx, fill, map[string]uint32{})
+			_, err := keeper.GetAllRevShares(ctx, fill, map[string]uint32{})
 
-			require.NoError(t, err)
-			require.Equal(t, types.RevSharesForFill{}, revSharesForFill)
+			require.ErrorIs(t, err, tc.expectedError)
 		})
 	}
 }