Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OTE-816] Update Revshare logic for affiliates #2298

Merged
merged 14 commits into from
Sep 24, 2024
+245 −342
Merged

[OTE-816] Update Revshare logic for affiliates #2298

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
809a0ce
update revshare logic
affanv14 Sep 19, 2024
b8d0f0d
Merge remote-tracking branch 'origin/main' into affan/update-revshare
affanv14 Sep 19, 2024
5c2fa13
fix test
affanv14 Sep 19, 2024
2060d21
update comment
affanv14 Sep 19, 2024
3f21cf6
rename
affanv14 Sep 19, 2024
f162535
address comments
affanv14 Sep 23, 2024
71e434a
changes
affanv14 Sep 23, 2024
ab729a3
remove unused param
affanv14 Sep 23, 2024
5fcdffa
update
affanv14 Sep 23, 2024
e69bbb7
Merge remote-tracking branch 'origin/main' into affan/update-revshare
affanv14 Sep 23, 2024
962942e
nits
affanv14 Sep 23, 2024
75b30fc
address comment
affanv14 Sep 24, 2024
53b26b2
add error
affanv14 Sep 24, 2024
e98790b
Merge remote-tracking branch 'origin/main' into affan/update-revshare
affanv14 Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions protocol/x/revshare/keeper/revshare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package keeper
import (
"math/big"

errorsmod "cosmossdk.io/errors"
"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"
Expand Down Expand Up @@ -163,27 +163,35 @@ func (k Keeper) GetAllRevShares(
feeSourceToRevSharePpm := make(map[types.RevShareFeeSource]uint32)
feeSourceToQuoteQuantums[types.REV_SHARE_FEE_SOURCE_TAKER_FEE] = big.NewInt(0)
feeSourceToRevSharePpm[types.REV_SHARE_FEE_SOURCE_TAKER_FEE] = 0
feeSourceToQuoteQuantums[types.REV_SHARE_FEE_SOURCE_NET_FEE] = big.NewInt(0)
feeSourceToRevSharePpm[types.REV_SHARE_FEE_SOURCE_NET_FEE] = 0
feeSourceToQuoteQuantums[types.REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE] = big.NewInt(0)
feeSourceToRevSharePpm[types.REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE] = 0

totalFeesShared := big.NewInt(0)
takerFees := fill.TakerFeeQuoteQuantums
makerFees := fill.MakerFeeQuoteQuantums
Copy link
Contributor

@teddyding teddyding Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also change REV_SHARE_FEE_SOURCE_NET_FEE to REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE to disambiguate

NET_FEE could be understood as taker fee minus rebates

netFees := big.NewInt(0).Add(takerFees, makerFees)

affiliateRevShares, err := k.getAffiliateRevShares(ctx, fill, affiliatesWhitelistMap)
affiliateRevShares, affiliateFeesShared, err := k.getAffiliateRevShares(ctx, fill, affiliatesWhitelistMap)
if err != nil {
return types.RevSharesForFill{}, err
log.ErrorLogWithError(ctx, "error getting affiliate rev shares", err)
return types.RevSharesForFill{}, nil
}

unconditionalRevShares, err := k.getUnconditionalRevShares(ctx, netFees)
netFeesSubAffiliateFeesShared := new(big.Int).Sub(netFees, affiliateFeesShared)
unconditionalRevShares, err := k.getUnconditionalRevShares(ctx, netFeesSubAffiliateFeesShared)
teddyding marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return types.RevSharesForFill{}, err
log.ErrorLogWithError(ctx, "error getting unconditional rev shares", err)
return types.RevSharesForFill{}, nil
}

if netFeesSubAffiliateFeesShared.Sign() <= 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: move to immediately below line 129

log.ErrorLog(ctx, "net fees sub affiliate fees shared is less than or equal to 0")
return types.RevSharesForFill{}, nil
}

marketMapperRevShares, err := k.getMarketMapperRevShare(ctx, fill.MarketId, netFees)
marketMapperRevShares, err := k.getMarketMapperRevShare(ctx, fill.MarketId, netFeesSubAffiliateFeesShared)
if err != nil {
return types.RevSharesForFill{}, err
log.ErrorLogWithError(ctx, "error getting market mapper rev shares", err)
return types.RevSharesForFill{}, nil
}

revShares = append(revShares, affiliateRevShares...)
Expand All @@ -208,8 +216,8 @@ func (k Keeper) GetAllRevShares(
}
//check total fees shared is less than or equal to net fees
if totalFeesShared.Cmp(netFees) > 0 {
return types.RevSharesForFill{}, errorsmod.Wrap(
types.ErrTotalFeesSharedExceedsNetFees, "total fees shared exceeds net fees")
log.ErrorLog(ctx, "total fees shared exceeds net fees")
return types.RevSharesForFill{}, nil
}

return types.RevSharesForFill{
Expand All @@ -224,20 +232,20 @@ func (k Keeper) getAffiliateRevShares(
ctx sdk.Context,
fill clobtypes.FillForProcess,
affiliatesWhitelistMap map[string]uint32,
) ([]types.RevShare, error) {
) ([]types.RevShare, *big.Int, error) {
takerAddr := fill.TakerAddr
takerFee := fill.TakerFeeQuoteQuantums
if fill.MonthlyRollingTakerVolumeQuantums >= types.Max30dRefereeVolumeQuantums {
return nil, nil
return nil, big.NewInt(0), nil
}

takerAffiliateAddr, feeSharePpm, exists, err := k.affiliatesKeeper.GetTakerFeeShare(
ctx, takerAddr, affiliatesWhitelistMap)
if err != nil {
return nil, err
return nil, big.NewInt(0), err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return nil for affiliateFeesShared when an error occurs

In the error case at line 237, consider returning nil for affiliateFeesShared instead of big.NewInt(0). This makes it clear that the value is invalid due to the error, aligning with common Go practices.

Apply this diff to adjust the return value:

-return nil, big.NewInt(0), err
+return nil, nil, err
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return nil, big.NewInt(0), err
return nil, nil, err

}
if !exists {
return nil, nil
return nil, big.NewInt(0), nil
}
feesShared := lib.BigMulPpm(takerFee, lib.BigU(feeSharePpm), false)
return []types.RevShare{
Expand All @@ -248,23 +256,23 @@ func (k Keeper) getAffiliateRevShares(
QuoteQuantums: feesShared,
RevSharePpm: feeSharePpm,
},
}, nil
}, feesShared, nil
}

func (k Keeper) getUnconditionalRevShares(
ctx sdk.Context,
netFees *big.Int,
netFeesSubAffiliateFeesShared *big.Int,
) ([]types.RevShare, error) {
revShares := []types.RevShare{}
unconditionalRevShareConfig, err := k.GetUnconditionalRevShareConfigParams(ctx)
if err != nil {
return nil, err
}
for _, revShare := range unconditionalRevShareConfig.Configs {
feeShared := lib.BigMulPpm(netFees, lib.BigU(revShare.SharePpm), false)
feeShared := lib.BigMulPpm(netFeesSubAffiliateFeesShared, lib.BigU(revShare.SharePpm), false)
revShare := types.RevShare{
Recipient: revShare.Address,
RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_NET_FEE,
RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE,
RevShareType: types.REV_SHARE_TYPE_UNCONDITIONAL,
QuoteQuantums: feeShared,
RevSharePpm: revShare.SharePpm,
Expand All @@ -277,7 +285,7 @@ func (k Keeper) getUnconditionalRevShares(
func (k Keeper) getMarketMapperRevShare(
ctx sdk.Context,
marketId uint32,
netFees *big.Int,
netFeesSubAffiliateFeesShared *big.Int,
) ([]types.RevShare, error) {
revShares := []types.RevShare{}
marketMapperRevshareAddress, revenueSharePpm, err := k.GetMarketMapperRevenueShareForMarket(ctx, marketId)
Expand All @@ -288,10 +296,10 @@ func (k Keeper) getMarketMapperRevShare(
return nil, nil
}

marketMapperRevshareAmount := lib.BigMulPpm(netFees, lib.BigU(revenueSharePpm), false)
marketMapperRevshareAmount := lib.BigMulPpm(netFeesSubAffiliateFeesShared, lib.BigU(revenueSharePpm), false)
revShares = append(revShares, types.RevShare{
Recipient: marketMapperRevshareAddress.String(),
RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_NET_FEE,
RevShareFeeSource: types.REV_SHARE_FEE_SOURCE_NET_PROTOCOL_REVENUE,
RevShareType: types.REV_SHARE_TYPE_MARKET_MAPPER,
QuoteQuantums: marketMapperRevshareAmount,
RevSharePpm: revenueSharePpm,
Expand Down
Loading
Loading