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-788] Update revshare safety #2284

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
+282 −261
Open

[OTE-788] Update revshare safety #2284

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ad7a78d
update validate revshare safety
affanv14 Sep 18, 2024
193df7c
update comment
affanv14 Sep 18, 2024
af30e36
lint
affanv14 Sep 18, 2024
d29f906
updaet
affanv14 Sep 18, 2024
12d634e
update
affanv14 Sep 18, 2024
4c842a6
add test
affanv14 Sep 18, 2024
d1a58d0
fix
affanv14 Sep 19, 2024
ed5be06
changes
affanv14 Sep 20, 2024
5b50b2f
temp
affanv14 Sep 20, 2024
21bb82b
address error
affanv14 Sep 23, 2024
11f651d
Merge remote-tracking branch 'origin/main' into affan/revshare-safety
affanv14 Sep 23, 2024
6111a9e
address comments
affanv14 Sep 23, 2024
f8e2a77
nit
affanv14 Sep 23, 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
46 changes: 25 additions & 21 deletions protocol/app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,26 +949,43 @@ func New(
)
affiliatesModule := affiliatesmodule.NewAppModule(appCodec, app.AffiliatesKeeper)

app.MarketMapKeeper = *marketmapmodulekeeper.NewKeeper(
runtime.NewKVStoreService(keys[marketmapmoduletypes.StoreKey]),
appCodec,
authtypes.NewModuleAddress(govtypes.ModuleName),
)

marketmapModule := marketmapmodule.NewAppModule(appCodec, &app.MarketMapKeeper)

app.FeeTiersKeeper = feetiersmodulekeeper.NewKeeper(
appCodec,
app.StatsKeeper,
app.AffiliatesKeeper,
keys[feetiersmoduletypes.StoreKey],
// set the governance and delaymsg module accounts as the authority for conducting upgrades
[]string{
lib.GovModuleAddress.String(),
delaymsgmoduletypes.ModuleAddress.String(),
},
)
feeTiersModule := feetiersmodule.NewAppModule(appCodec, app.FeeTiersKeeper)

app.AffiliatesKeeper.SetFeetiersKeeper(app.FeeTiersKeeper)

app.RevShareKeeper = *revsharemodulekeeper.NewKeeper(
appCodec,
keys[revsharemoduletypes.StoreKey],
[]string{
lib.GovModuleAddress.String(),
},
app.AffiliatesKeeper,
*app.FeeTiersKeeper,
)
revShareModule := revsharemodule.NewAppModule(appCodec, app.RevShareKeeper)

// Set the revshare keeper in the affiliates keeper.
app.AffiliatesKeeper.SetRevShareKeeper(app.RevShareKeeper)

app.MarketMapKeeper = *marketmapmodulekeeper.NewKeeper(
runtime.NewKVStoreService(keys[marketmapmoduletypes.StoreKey]),
appCodec,
authtypes.NewModuleAddress(govtypes.ModuleName),
)

marketmapModule := marketmapmodule.NewAppModule(appCodec, &app.MarketMapKeeper)
app.FeeTiersKeeper.SetRevShareKeeper(app.RevShareKeeper)

app.PricesKeeper = *pricesmodulekeeper.NewKeeper(
appCodec,
Expand Down Expand Up @@ -1041,19 +1058,6 @@ func New(
)
perpetualsModule := perpetualsmodule.NewAppModule(appCodec, app.PerpetualsKeeper)

app.FeeTiersKeeper = feetiersmodulekeeper.NewKeeper(
appCodec,
app.StatsKeeper,
app.AffiliatesKeeper,
keys[feetiersmoduletypes.StoreKey],
// set the governance and delaymsg module accounts as the authority for conducting upgrades
[]string{
lib.GovModuleAddress.String(),
delaymsgmoduletypes.ModuleAddress.String(),
},
)
feeTiersModule := feetiersmodule.NewAppModule(appCodec, app.FeeTiersKeeper)

app.VestKeeper = *vestmodulekeeper.NewKeeper(
appCodec,
keys[vestmoduletypes.StoreKey],
Expand Down
4 changes: 3 additions & 1 deletion protocol/testutil/keeper/assets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ func AssetsKeepers(
stakingKeeper,
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, msgSenderEnabled)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
vaultKeeper, _ := createVaultKeeper(stateStore, db, cdc, transientStoreKey)
feetiersKeeper, _ := createFeeTiersKeeper(stateStore, statsKeeper, vaultKeeper, affiliatesKeeper, db, cdc)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feetiersKeeper)
marketMapKeeper, _ := createMarketMapKeeper(stateStore, db, cdc)
pricesKeeper, _, _, mockTimeProvider = createPricesKeeper(
stateStore,
Expand Down
33 changes: 18 additions & 15 deletions protocol/testutil/keeper/clob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,24 @@ func NewClobKeepersTestContextWithUninitializedMemStore(
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, ks.StatsKeeper,
indexerEventsTransientStoreKey, true)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
ks.VaultKeeper, _ = createVaultKeeper(
stateStore,
db,
cdc,
indexerEventsTransientStoreKey,
)
ks.FeeTiersKeeper, _ = createFeeTiersKeeper(
stateStore,
ks.StatsKeeper,
ks.VaultKeeper,
affiliatesKeeper,
db,
cdc,
)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, ks.FeeTiersKeeper)
ks.FeeTiersKeeper.SetRevShareKeeper(revShareKeeper)
affiliatesKeeper.SetRevShareKeeper(revShareKeeper)
affiliatesKeeper.SetFeetiersKeeper(ks.FeeTiersKeeper)
ks.MarketMapKeeper, _ = createMarketMapKeeper(stateStore, db, cdc)
ks.PricesKeeper, _, _, mockTimeProvider = createPricesKeeper(
stateStore,
Expand Down Expand Up @@ -136,20 +153,6 @@ func NewClobKeepersTestContextWithUninitializedMemStore(
true,
)
ks.BlockTimeKeeper, _ = createBlockTimeKeeper(stateStore, db, cdc)
ks.VaultKeeper, _ = createVaultKeeper(
stateStore,
db,
cdc,
indexerEventsTransientStoreKey,
)
ks.FeeTiersKeeper, _ = createFeeTiersKeeper(
stateStore,
ks.StatsKeeper,
ks.VaultKeeper,
affiliatesKeeper,
db,
cdc,
)
ks.RewardsKeeper, _ = createRewardsKeeper(
stateStore,
ks.AssetsKeeper,
Expand Down
30 changes: 15 additions & 15 deletions protocol/testutil/keeper/listing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,21 @@ func ListingKeepers(
stakingKeeper,
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, true)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
vaultKeeper, _ := createVaultKeeper(
stateStore,
db,
cdc,
transientStoreKey,
)
feeTiersKeeper, _ := createFeeTiersKeeper(
stateStore,
statsKeeper,
vaultKeeper,
affiliatesKeeper,
db,
cdc,
)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feeTiersKeeper)
marketMapKeeper, _ = createMarketMapKeeper(stateStore, db, cdc)
pricesKeeper, _, _, mockTimeProvider = createPricesKeeper(
stateStore,
Expand Down Expand Up @@ -102,20 +116,6 @@ func ListingKeepers(
)

blockTimeKeeper, _ := createBlockTimeKeeper(stateStore, db, cdc)
vaultKeeper, _ := createVaultKeeper(
stateStore,
db,
cdc,
transientStoreKey,
)
feeTiersKeeper, _ := createFeeTiersKeeper(
stateStore,
statsKeeper,
vaultKeeper,
affiliatesKeeper,
db,
cdc,
)
rewardsKeeper, _ := createRewardsKeeper(
stateStore,
assetsKeeper,
Expand Down
4 changes: 3 additions & 1 deletion protocol/testutil/keeper/perpetuals.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ func PerpetualsKeepersWithClobHelpers(
stakingKeeper,
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, true)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
vaultKeeper, _ := createVaultKeeper(stateStore, db, cdc, transientStoreKey)
feetiersKeeper, _ := createFeeTiersKeeper(stateStore, statsKeeper, vaultKeeper, affiliatesKeeper, db, cdc)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feetiersKeeper)
pc.MarketMapKeeper, _ = createMarketMapKeeper(stateStore, db, cdc)
pc.PricesKeeper, _, pc.IndexPriceCache, pc.MockTimeProvider = createPricesKeeper(
stateStore,
Expand Down
4 changes: 3 additions & 1 deletion protocol/testutil/keeper/prices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ func PricesKeepers(t testing.TB) (
stakingKeeper,
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, true)
revShareKeeper, _, _ = createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
vaultKeeper, _ := createVaultKeeper(stateStore, db, cdc, transientStoreKey)
feetiersKeeper, _ := createFeeTiersKeeper(stateStore, statsKeeper, vaultKeeper, affiliatesKeeper, db, cdc)
revShareKeeper, _, _ = createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feetiersKeeper)
marketMapKeeper, _ = createMarketMapKeeper(stateStore, db, cdc)
// Define necessary keepers here for unit tests
keeper, storeKey, indexPriceCache, mockTimeProvider =
Expand Down
7 changes: 6 additions & 1 deletion protocol/testutil/keeper/revshare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/dydxprotocol/v4-chain/protocol/lib"
affiliateskeeper "github.com/dydxprotocol/v4-chain/protocol/x/affiliates/keeper"
feetierskeeper "github.com/dydxprotocol/v4-chain/protocol/x/feetiers/keeper"
"github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"

storetypes "cosmossdk.io/store/types"
Expand Down Expand Up @@ -54,8 +55,10 @@ func RevShareKeepers(t testing.TB) (
stakingKeeper,
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, true)
vaultKeeper, _ := createVaultKeeper(stateStore, db, cdc, transientStoreKey)
feetiersKeeper, _ := createFeeTiersKeeper(stateStore, statsKeeper, vaultKeeper, affiliatesKeeper, db, cdc)
keeper, storeKey, mockTimeProvider =
createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feetiersKeeper)

return []GenesisInitializer{keeper}
},
Expand All @@ -69,6 +72,7 @@ func createRevShareKeeper(
db *dbm.MemDB,
cdc *codec.ProtoCodec,
affiliatesKeeper *affiliateskeeper.Keeper,
feetiersKeeper *feetierskeeper.Keeper,
) (
*keeper.Keeper,
storetypes.StoreKey,
Expand All @@ -83,6 +87,7 @@ func createRevShareKeeper(
lib.GovModuleAddress.String(),
},
*affiliatesKeeper,
*feetiersKeeper,
)

return k, storeKey, mockTimeProvider
Expand Down
24 changes: 12 additions & 12 deletions protocol/testutil/keeper/rewards.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,6 @@ func RewardsKeepers(
stakingKeeper,
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, true)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
marketMapKeeper, _ := createMarketMapKeeper(stateStore, db, cdc)
pricesKeeper, _, _, _ = createPricesKeeper(stateStore, db, cdc, transientStoreKey, revShareKeeper, marketMapKeeper)
// Mock time provider response for market creation.
assetsKeeper, _ = createAssetsKeeper(
stateStore,
db,
cdc,
pricesKeeper,
transientStoreKey,
true,
)
vaultKeeper, _ := createVaultKeeper(
stateStore,
db,
Expand All @@ -92,6 +80,18 @@ func RewardsKeepers(
db,
cdc,
)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feetiersKeeper)
marketMapKeeper, _ := createMarketMapKeeper(stateStore, db, cdc)
pricesKeeper, _, _, _ = createPricesKeeper(stateStore, db, cdc, transientStoreKey, revShareKeeper, marketMapKeeper)
// Mock time provider response for market creation.
assetsKeeper, _ = createAssetsKeeper(
stateStore,
db,
cdc,
pricesKeeper,
transientStoreKey,
true,
)
rewardsKeeper, storeKey = createRewardsKeeper(
stateStore,
assetsKeeper,
Expand Down
4 changes: 3 additions & 1 deletion protocol/testutil/keeper/sending.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ func SendingKeepersWithSubaccountsKeeper(t testing.TB, saKeeper types.Subaccount
stakingKeeper,
)
affiliatesKeeper, _ := createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, true)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
vaultKeeper, _ := createVaultKeeper(stateStore, db, cdc, transientStoreKey)
feetiersKeeper, _ := createFeeTiersKeeper(stateStore, statsKeeper, vaultKeeper, affiliatesKeeper, db, cdc)
revShareKeeper, _, _ := createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feetiersKeeper)
marketMapKeeper, _ := createMarketMapKeeper(stateStore, db, cdc)
ks.PricesKeeper, _, _, mockTimeProvider = createPricesKeeper(
stateStore,
Expand Down
4 changes: 3 additions & 1 deletion protocol/testutil/keeper/subaccounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ func SubaccountsKeepers(t testing.TB, msgSenderEnabled bool) (
stakingKeeper,
)
affiliatesKeeper, _ = createAffiliatesKeeper(stateStore, db, cdc, statsKeeper, transientStoreKey, true)
revShareKeeper, _, _ = createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper)
vaultKeeper, _ := createVaultKeeper(stateStore, db, cdc, transientStoreKey)
feetiersKeeper, _ := createFeeTiersKeeper(stateStore, statsKeeper, vaultKeeper, affiliatesKeeper, db, cdc)
revShareKeeper, _, _ = createRevShareKeeper(stateStore, db, cdc, affiliatesKeeper, feetiersKeeper)
affiliatesKeeper.SetRevShareKeeper(revShareKeeper)
marketMapKeeper, _ := createMarketMapKeeper(stateStore, db, cdc)
pricesKeeper, _, _, mockTimeProvider = createPricesKeeper(
Expand Down
5 changes: 5 additions & 0 deletions protocol/x/affiliates/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type (
authorities map[string]struct{}
statsKeeper types.StatsKeeper
revShareKeeper types.RevShareKeeper
feetiersKeeper types.FeetiersKeeper
indexerEventManager indexer_manager.IndexerEventManager
}
)
Expand Down Expand Up @@ -255,6 +256,10 @@ func (k *Keeper) SetRevShareKeeper(revShareKeeper types.RevShareKeeper) {
k.revShareKeeper = revShareKeeper
}

func (k *Keeper) SetFeetiersKeeper(feetiersKeeper types.FeetiersKeeper) {
k.feetiersKeeper = feetiersKeeper
}

func (k Keeper) GetIndexerEventManager() indexer_manager.IndexerEventManager {
return k.indexerEventManager
}
Expand Down
5 changes: 4 additions & 1 deletion protocol/x/affiliates/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ func (k msgServer) UpdateAffiliateTiers(ctx context.Context,
return nil, err
}
marketMapperRevShareParams := k.revShareKeeper.GetMarketMapperRevenueShareParams(sdkCtx)
lowestTakerFee := k.feetiersKeeper.GetAffiliateRefereeLowestTakerFee(sdkCtx)
lowestMakerFee := k.feetiersKeeper.GetLowestMakerFee(sdkCtx)

if !k.revShareKeeper.ValidateRevShareSafety(msg.Tiers, unconditionalRevShareConfig, marketMapperRevShareParams) {
if !k.revShareKeeper.ValidateRevShareSafety(sdkCtx, msg.Tiers,
unconditionalRevShareConfig, marketMapperRevShareParams, lowestTakerFee, lowestMakerFee) {
return nil, errorsmod.Wrapf(
types.ErrRevShareSafetyViolation,
"rev share safety violation",
Expand Down
8 changes: 8 additions & 0 deletions protocol/x/affiliates/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@ type RevShareKeeper interface {
ctx sdk.Context,
) revsharetypes.MarketMapperRevenueShareParams
ValidateRevShareSafety(
ctx sdk.Context,
affiliateTiers AffiliateTiers,
unconditionalRevShareConfig revsharetypes.UnconditionalRevShareConfig,
marketMapperRevShareParams revsharetypes.MarketMapperRevenueShareParams,
lowestTakerFee int32,
lowestMakerFee int32,
) bool
}

type FeetiersKeeper interface {
GetAffiliateRefereeLowestTakerFee(ctx sdk.Context) int32
GetLowestMakerFee(ctx sdk.Context) int32
Comment on lines +15 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Incomplete Removal of RevShareKeeper Detected

The search results indicate that RevShareKeeper is still present in multiple areas of the codebase, including interfaces, keeper implementations, and test files. This suggests that the replacement with FeetiersKeeper is not fully implemented.

Action Items:

  • Complete Refactoring: Ensure that all instances of RevShareKeeper are replaced with FeetiersKeeper across the entire codebase.
  • Update Dependencies: Verify that all modules and tests are updated to use FeetiersKeeper instead of RevShareKeeper.
  • Review Safety Checks: Confirm that any safety checks previously handled by RevShareKeeper are appropriately managed by the new FeetiersKeeper.
Analysis chain

New FeetiersKeeper interface introduced

The new FeetiersKeeper interface has been added, replacing the previous RevShareKeeper. This change aligns with the PR objective of updating revenue share safety.

A few points to consider:

  1. The new methods focus on retrieving the lowest fees, which is a shift from the previous revenue sharing approach.
  2. Ensure that all places where RevShareKeeper was used have been updated to use FeetiersKeeper instead.
  3. The removal of RevShareKeeper methods like ValidateRevShareSafety suggests that the safety checks might have been moved elsewhere or implemented differently.

To ensure that all usages of the old RevShareKeeper have been properly updated, please run the following script:

This script will help identify any places where the old RevShareKeeper might still be in use and confirm that FeetiersKeeper is being used appropriately.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usages of RevShareKeeper in the codebase

# Search for RevShareKeeper usages
echo "Searching for RevShareKeeper usages:"
rg --type go "RevShareKeeper"

# Search for the old methods that were in RevShareKeeper
echo "Searching for old RevShareKeeper methods:"
rg --type go "GetUnconditionalRevShareConfigParams|GetMarketMapperRevenueShareParams|ValidateRevShareSafety"

# Search for new FeetiersKeeper usages
echo "Searching for FeetiersKeeper usages:"
rg --type go "FeetiersKeeper"

Length of output: 8519

}
22 changes: 22 additions & 0 deletions protocol/x/feetiers/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/dydxprotocol/v4-chain/protocol/lib"
"github.com/dydxprotocol/v4-chain/protocol/x/feetiers/types"
revsharetypes "github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"
)

type (
Expand All @@ -21,6 +22,7 @@ type (
storeKey storetypes.StoreKey
authorities map[string]struct{}
affiliatesKeeper types.AffiliatesKeeper
revShareKeeper types.RevShareKeeper
}
)

Expand Down Expand Up @@ -131,3 +133,23 @@ func (k Keeper) GetLowestMakerFee(ctx sdk.Context) int32 {

return lowestMakerFee
}

// GetAffiliateRefxereeLowestTakerFee returns the lowest taker fee
// for volume under Max30dRefereeVolumeQuantums.
func (k Keeper) GetAffiliateRefereeLowestTakerFee(ctx sdk.Context) int32 {
feeParams := k.GetPerpetualFeeParams(ctx)
for _, tier := range feeParams.Tiers {
// assumes tiers are ordered by absolute volume requirement
if tier.AbsoluteVolumeRequirement < revsharetypes.Max30dRefereeVolumeQuantums {
return tier.TakerFeePpm
} else {
break
}
}

return feeParams.Tiers[types.RefereeStartingFeeTier].TakerFeePpm
}

func (k *Keeper) SetRevShareKeeper(revShareKeeper types.RevShareKeeper) {
k.revShareKeeper = revShareKeeper
}
Loading
Loading