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

Jump to bottom

[OTE-817] update trading rewards with new logic #2300

Merged
merged 13 commits into from
Sep 24, 2024
Merged

[OTE-817] update trading rewards with new logic #2300

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
213977b
update trading rewards
affanv14 Sep 19, 2024
368f359
add comment
affanv14 Sep 19, 2024
9e422bb
lint
affanv14 Sep 19, 2024
8750e0d
address comments
affanv14 Sep 19, 2024
f9d89c7
update logic
affanv14 Sep 23, 2024
61a4542
update comment
affanv14 Sep 23, 2024
972f274
lint
affanv14 Sep 23, 2024
3deb52e
update to quote quantums
affanv14 Sep 23, 2024
2184b6a
address comments
affanv14 Sep 23, 2024
581f861
update e2e tests
affanv14 Sep 23, 2024
6fada43
update comment
affanv14 Sep 23, 2024
15ba5e6
Update protocol/x/rewards/keeper/keeper.go
teddyding Sep 24, 2024
f4d0f93
lint
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
5 changes: 3 additions & 2 deletions protocol/x/rewards/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@ func (k Keeper) GetRewardShare(
// * fill_quote_quantum - max_possible_taker_fee_rev_share) * (1 - F)
// are added to reward share score.
// max_possible_taker_fee_rev_share is 0 when taker trailing volume is > MaxReferee30dVolumeForAffiliateShareQuantums,
// since taker trailing volume is only affiliate at the moment, and they don’t generate affiliate rev share.
// since taker_fee_share is only affiliate at the moment, and they don’t generate affiliate rev share.
// When taker volume ≤ MaxReferee30dVolumeForAffiliateShareQuantums,
// max_possible_taker_fee_rev_share = max_vip_affiliate_share * taker_fee
// regardless of if the taker has an affiliate or not.

func (k Keeper) AddRewardSharesForFill(
ctx sdk.Context,
Expand All @@ -152,7 +153,7 @@ func (k Keeper) AddRewardSharesForFill(

// taker revshare is not returned if taker volume is greater than Max30dTakerVolumeQuantums
Copy link
Contributor

Choose a reason for hiding this comment

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

We aren't using Max30dRefereeVolumeQuantums below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We dont need to - since no taker revshares are returned over 50 million - the if condition is never hit for value > 0

Copy link
Contributor

@teddyding teddyding Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The new trading rewards logic is below:

If taker volume is <50M (equal statement: they would generate affiliate rev share if referred), then the max possible affiliate rev share is deducted from their trading rewards score.

The following implementation doesn't work correctly for a non-referred user under 50M volume

if value, ok := revSharesForFill.FeeSourceToRevSharePpm[revsharetypes.REV_SHARE_FEE_SOURCE_TAKER_FEE]; ok &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we remove FeeSourceToRevSharePpm[revsharetypes.REV_SHARE_FEE_SOURCE_TAKER_FEE from the if condition and only rely on fill.MonthlyRollingTakerVolumeQuantums < revsharetypes.MaxReferee30dVolumeForAffiliateShareQuantums only?

Copy link
Contributor Author

@affanv14 affanv14 Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

my b - didnt realize this when i was working through it fast 😓

value > 0 {
value > 0 && fill.MonthlyRollingTakerVolumeQuantums < revsharetypes.MaxReferee30dVolumeForAffiliateShareQuantums {
totalTakerFeeRevShareQuantums = lib.BigMulPpm(fill.TakerFeeQuoteQuantums,
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming of variable is inaccurate, should be max_possible_taker_fee_rev_share

lib.BigU(affiliatetypes.AffiliatesRevSharePpmCap),
false,
Expand Down
61 changes: 61 additions & 0 deletions protocol/x/rewards/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,67 @@ func TestAddRewardSharesForFill(t *testing.T) {
Weight: dtypes.NewInt(900_000), // 1 * (1 - 0.1)
},
},
"positive maker + taker fees reduced by maker rebate, taker + net fee revshare,rolling taker volume > 50 mil": {
prevTakerRewardShare: nil,
prevMakerRewardShare: nil,
fill: clobtypes.FillForProcess{
TakerAddr: takerAddress,
TakerFeeQuoteQuantums: big.NewInt(2_000_000),
MakerAddr: makerAddress,
MakerFeeQuoteQuantums: big.NewInt(1_000_000),
FillQuoteQuantums: big.NewInt(800_000_000),
ProductId: uint32(1),
MarketId: uint32(1),
MonthlyRollingTakerVolumeQuantums: 60_000_000_000_000,
},
revSharesForFill: revsharetypes.RevSharesForFill{
AllRevShares: []revsharetypes.RevShare{
{
Recipient: constants.AliceAccAddress.String(),
RevShareFeeSource: revsharetypes.REV_SHARE_FEE_SOURCE_NET_FEE,
RevShareType: revsharetypes.REV_SHARE_TYPE_UNCONDITIONAL,
QuoteQuantums: big.NewInt(200_000),
RevSharePpm: 100_000, // 10%
},
{
Recipient: takerAddress,
RevShareFeeSource: revsharetypes.REV_SHARE_FEE_SOURCE_TAKER_FEE,
RevShareType: revsharetypes.REV_SHARE_TYPE_AFFILIATE,
QuoteQuantums: big.NewInt(200_000),
RevSharePpm: 100_000, // 10%
},
},
FeeSourceToQuoteQuantums: map[revsharetypes.RevShareFeeSource]*big.Int{
revsharetypes.REV_SHARE_FEE_SOURCE_NET_FEE: big.NewInt(200_000),
revsharetypes.REV_SHARE_FEE_SOURCE_TAKER_FEE: big.NewInt(200_000),
},
FeeSourceToRevSharePpm: map[revsharetypes.RevShareFeeSource]uint32{
revsharetypes.REV_SHARE_FEE_SOURCE_NET_FEE: 100_000, // 10%
revsharetypes.REV_SHARE_FEE_SOURCE_TAKER_FEE: 100_000, // 10%
},
AffiliateRevShare: &revsharetypes.RevShare{
Recipient: takerAddress,
RevShareFeeSource: revsharetypes.REV_SHARE_FEE_SOURCE_TAKER_FEE,
RevShareType: revsharetypes.REV_SHARE_TYPE_AFFILIATE,
QuoteQuantums: big.NewInt(200_000),
RevSharePpm: 100_000, // 10%
},
},
feeTiers: []*feetierstypes.PerpetualFeeTier{
{
MakerFeePpm: -1_000, // -0.1%
TakerFeePpm: 2_000, // 0.2%
},
},
expectedTakerShare: types.RewardShare{
Address: takerAddress,
Weight: dtypes.NewInt(1_080_000), // (2 - 0.1% * 800 - 0) * (1 - 0.1)
},
expectedMakerShare: types.RewardShare{
Address: makerAddress,
Weight: dtypes.NewInt(900_000), // 1 * (1 - 0.1)
},
},
}

// Run tests.
Expand Down
Loading