Skip to content

Commit

Permalink
[DEC-2148] Convert stateful order rate limits to be per account inste…
Browse files Browse the repository at this point in the history 
…ad of per subaccount. (dydxprotocol#556)

* [DEC-2148] Convert stateful order rate limits to be per account instead of per subaccount.
  • Loading branch information
@lcwik
lcwik committed Oct 11, 2023
1 parent 1ef0a25 commit a68b735
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 131 deletions.
8 changes: 4 additions & 4 deletions indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/block_rate_limit_config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ export interface BlockRateLimitConfiguration {
*/
maxShortTermOrdersPerNBlocks: MaxPerNBlocksRateLimit[];
/**
* How many stateful order attempts (successful and failed) are allowed for a
* subaccount per N blocks. Note that the rate limits are applied
* How many stateful order attempts (successful and failed) are allowed for
* an account per N blocks. Note that the rate limits are applied
* in an AND fashion such that an order placement must pass all rate limit
* configurations.
*
Expand All @@ -37,8 +37,8 @@ export interface BlockRateLimitConfigurationSDKType {
*/
max_short_term_orders_per_n_blocks: MaxPerNBlocksRateLimitSDKType[];
/**
* How many stateful order attempts (successful and failed) are allowed for a
* subaccount per N blocks. Note that the rate limits are applied
* How many stateful order attempts (successful and failed) are allowed for
* an account per N blocks. Note that the rate limits are applied
* in an AND fashion such that an order placement must pass all rate limit
* configurations.
*
Expand Down
4 changes: 2 additions & 2 deletions proto/dydxprotocol/clob/block_rate_limit_config.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ message BlockRateLimitConfiguration {
repeated MaxPerNBlocksRateLimit max_short_term_orders_per_n_blocks = 1
[ (gogoproto.nullable) = false ];

// How many stateful order attempts (successful and failed) are allowed for a
// subaccount per N blocks. Note that the rate limits are applied
// How many stateful order attempts (successful and failed) are allowed for
// an account per N blocks. Note that the rate limits are applied
// in an AND fashion such that an order placement must pass all rate limit
// configurations.
//
Expand Down
1 change: 0 additions & 1 deletion protocol/lib/metrics/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ const (
OrderSide = "order_side"
PlaceOrder = "place_order"
PlaceOrderAccounts = "place_order_accounts"
PlaceOrderSubaccounts = "place_order_subaccounts"
PlaceStatefulOrder = "place_stateful_order"
ProcessMatches = "process_matches"
ProcessOperations = "process_operations"
Expand Down
129 changes: 31 additions & 98 deletions protocol/x/clob/e2e/rate_limit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
blockRateLimitConifg clobtypes.BlockRateLimitConfiguration
firstMsg sdktypes.Msg
secondMsg sdktypes.Msg
expectedRateLimit bool
}{
"Short term orders with same subaccounts": {
blockRateLimitConifg: clobtypes.BlockRateLimitConfiguration{
Expand All @@ -32,9 +31,8 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
},
},
},
firstMsg: &PlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20,
secondMsg: &PlaceOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB20,
expectedRateLimit: true,
firstMsg: &PlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20,
secondMsg: &PlaceOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTB20,
},
"Short term orders with different subaccounts": {
blockRateLimitConifg: clobtypes.BlockRateLimitConfiguration{
Expand All @@ -45,9 +43,8 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
},
},
},
firstMsg: &PlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20,
secondMsg: &PlaceOrder_Alice_Num1_Id0_Clob0_Buy5_Price10_GTB20,
expectedRateLimit: true,
firstMsg: &PlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB20,
secondMsg: &PlaceOrder_Alice_Num1_Id0_Clob0_Buy5_Price10_GTB20,
},
"Stateful orders with same subaccounts": {
blockRateLimitConifg: clobtypes.BlockRateLimitConfiguration{
Expand All @@ -58,9 +55,8 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
},
},
},
firstMsg: &LongTermPlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5,
secondMsg: &LongTermPlaceOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT5,
expectedRateLimit: true,
firstMsg: &LongTermPlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5,
secondMsg: &LongTermPlaceOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT5,
},
"Stateful orders with different subaccounts": {
blockRateLimitConifg: clobtypes.BlockRateLimitConfiguration{
Expand All @@ -71,9 +67,8 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
},
},
},
firstMsg: &LongTermPlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5,
secondMsg: &LongTermPlaceOrder_Alice_Num1_Id0_Clob0_Buy5_Price10_GTBT5,
expectedRateLimit: false,
firstMsg: &LongTermPlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5,
secondMsg: &LongTermPlaceOrder_Alice_Num1_Id0_Clob0_Buy5_Price10_GTBT5,
},
"Short term order cancellations with same subaccounts": {
blockRateLimitConifg: clobtypes.BlockRateLimitConfiguration{
Expand All @@ -84,9 +79,8 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
},
},
},
firstMsg: &CancelOrder_Alice_Num0_Id0_Clob1_GTB5,
secondMsg: &CancelOrder_Alice_Num0_Id0_Clob0_GTB20,
expectedRateLimit: true,
firstMsg: &CancelOrder_Alice_Num0_Id0_Clob1_GTB5,
secondMsg: &CancelOrder_Alice_Num0_Id0_Clob0_GTB20,
},
"Short term order cancellations with different subaccounts": {
blockRateLimitConifg: clobtypes.BlockRateLimitConfiguration{
Expand All @@ -97,9 +91,8 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
},
},
},
firstMsg: &CancelOrder_Alice_Num0_Id0_Clob1_GTB5,
secondMsg: &CancelOrder_Alice_Num1_Id0_Clob0_GTB20,
expectedRateLimit: true,
firstMsg: &CancelOrder_Alice_Num0_Id0_Clob1_GTB5,
secondMsg: &CancelOrder_Alice_Num1_Id0_Clob0_GTB20,
},
}

Expand Down Expand Up @@ -148,32 +141,28 @@ func TestRateLimitingOrders_RateLimitsAreEnforced(t *testing.T) {
)
// Rate limit is 1 over two block, second attempt should be blocked.
resp = tApp.CheckTx(secondCheckTx)
if tc.expectedRateLimit {
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Equal(t, clobtypes.ErrBlockRateLimitExceeded.ABCICode(), resp.Code)
require.Contains(t, resp.Log, "Rate of 2 exceeds configured block rate limit")
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Equal(t, clobtypes.ErrBlockRateLimitExceeded.ABCICode(), resp.Code)
require.Contains(t, resp.Log, "Rate of 2 exceeds configured block rate limit")

// Rate limit of 1 over two blocks should still apply, total should be 3 now (2 in block 2, 1 in block 3).
tApp.AdvanceToBlock(3, testapp.AdvanceToBlockOptions{})
resp = tApp.CheckTx(secondCheckTx)
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Equal(t, clobtypes.ErrBlockRateLimitExceeded.ABCICode(), resp.Code)
require.Contains(t, resp.Log, "Rate of 3 exceeds configured block rate limit")
// Rate limit of 1 over two blocks should still apply, total should be 3 now (2 in block 2, 1 in block 3).
tApp.AdvanceToBlock(3, testapp.AdvanceToBlockOptions{})
resp = tApp.CheckTx(secondCheckTx)
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Equal(t, clobtypes.ErrBlockRateLimitExceeded.ABCICode(), resp.Code)
require.Contains(t, resp.Log, "Rate of 3 exceeds configured block rate limit")

// Rate limit of 1 over two blocks should still apply, total should be 2 now (1 in block 3, 1 in block 4).
tApp.AdvanceToBlock(4, testapp.AdvanceToBlockOptions{})
resp = tApp.CheckTx(secondCheckTx)
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Equal(t, clobtypes.ErrBlockRateLimitExceeded.ABCICode(), resp.Code)
require.Contains(t, resp.Log, "Rate of 2 exceeds configured block rate limit")
// Rate limit of 1 over two blocks should still apply, total should be 2 now (1 in block 3, 1 in block 4).
tApp.AdvanceToBlock(4, testapp.AdvanceToBlockOptions{})
resp = tApp.CheckTx(secondCheckTx)
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Equal(t, clobtypes.ErrBlockRateLimitExceeded.ABCICode(), resp.Code)
require.Contains(t, resp.Log, "Rate of 2 exceeds configured block rate limit")

// Advancing two blocks should make the total count 0 now and the msg should be accepted.
tApp.AdvanceToBlock(6, testapp.AdvanceToBlockOptions{})
resp = tApp.CheckTx(secondCheckTx)
require.Conditionf(t, resp.IsOK, "Expected CheckTx to succeed. Response: %+v", resp)
} else {
require.Conditionf(t, resp.IsOK, "Expected CheckTx to succeed. Response: %+v", resp)
}
// Advancing two blocks should make the total count 0 now and the msg should be accepted.
tApp.AdvanceToBlock(6, testapp.AdvanceToBlockOptions{})
resp = tApp.CheckTx(secondCheckTx)
require.Conditionf(t, resp.IsOK, "Expected CheckTx to succeed. Response: %+v", resp)
})
}
}
Expand Down Expand Up @@ -438,62 +427,6 @@ func TestStatefulOrderPlacement_Deduplication(t *testing.T) {
}
}

func TestRateLimitingOrders_StatefulOrderRateLimitsAreAcrossMarkets(t *testing.T) {
tApp := testapp.NewTestAppBuilder().WithGenesisDocFn(func() (genesis types.GenesisDoc) {
genesis = testapp.DefaultGenesis()
testapp.UpdateGenesisDocWithAppStateForModule(
&genesis,
func(genesisState *clobtypes.GenesisState) {
genesisState.BlockRateLimitConfig = clobtypes.BlockRateLimitConfiguration{
MaxStatefulOrdersPerNBlocks: []clobtypes.MaxPerNBlocksRateLimit{
{
NumBlocks: 2,
Limit: 1,
},
},
}
},
)
return genesis
}).WithTesting(t).Build()
ctx := tApp.InitChain()

firstMarketCheckTx := testapp.MustMakeCheckTx(
ctx,
tApp.App,
testapp.MustMakeCheckTxOptions{
AccAddressForSigning: testtx.MustGetOnlySignerAddress(
&LongTermPlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5),
},
&LongTermPlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5,
)

// Second order should not be allowed in 2nd block and allowed in 4th block.
secondMarketCheckTx := testapp.MustMakeCheckTx(
ctx,
tApp.App,
testapp.MustMakeCheckTxOptions{
AccAddressForSigning: testtx.MustGetOnlySignerAddress(
&LongTermPlaceOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT5),
AccSequenceNumberForSigning: 2,
},
&LongTermPlaceOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT5,
)

tApp.AdvanceToBlock(2, testapp.AdvanceToBlockOptions{})
// First order should be allowed and second should be rejected.
require.True(t, tApp.CheckTx(firstMarketCheckTx).IsOK())
resp := tApp.CheckTx(secondMarketCheckTx)
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Equal(t, clobtypes.ErrBlockRateLimitExceeded.ABCICode(), resp.Code)
require.Contains(t, resp.Log, "Rate of 2 exceeds configured block rate limit")

// Retrying in the 4th block should succeed since the rate limits should have been pruned.
tApp.AdvanceToBlock(4, testapp.AdvanceToBlockOptions{})
resp = tApp.CheckTx(secondMarketCheckTx)
require.Conditionf(t, resp.IsOK, "Expected CheckTx to succeed. Response: %+v", resp)
}

func TestRateLimitingOrders_StatefulOrdersDuringDeliverTxAreNotRateLimited(t *testing.T) {
tApp := testapp.NewTestAppBuilder().WithGenesisDocFn(func() (genesis types.GenesisDoc) {
genesis = testapp.DefaultGenesis()
Expand Down
33 changes: 9 additions & 24 deletions protocol/x/clob/rate_limit/order_rate_limiter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/dydxprotocol/v4-chain/protocol/lib"
"github.com/dydxprotocol/v4-chain/protocol/lib/metrics"
"github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
)

// A RateLimiter which rate limits types.MsgPlaceOrder.
Expand All @@ -15,19 +14,17 @@ import (
// CheckTx.
type placeOrderRateLimiter struct {
checkStateShortTermOrderRateLimiter RateLimiter[string]
checkStateStatefulOrderRateLimiter RateLimiter[satypes.SubaccountId]
checkStateStatefulOrderRateLimiter RateLimiter[string]
// The set of rate limited accounts is only stored for telemetry purposes.
rateLimitedAccounts map[string]bool
// The set of rate limited subaccounts is only stored for telemetry purposes.
rateLimitedSubaccounts map[satypes.SubaccountId]bool
}

var _ RateLimiter[*types.MsgPlaceOrder] = (*placeOrderRateLimiter)(nil)

// NewPlaceOrderRateLimiter returns a RateLimiter which rate limits types.MsgPlaceOrder based upon the provided
// types.BlockRateLimitConfiguration. The rate limiter currently supports limiting based upon:
// - how many short term orders per subaccount (by using satypes.SubaccountId).
// - how many stateful order per subaccount (by using satypes.SubaccountId).
// - how many short term orders per account (by using string).
// - how many stateful order per account (by using string).
//
// The rate limiting must only be used during `CheckTx` because the rate limiting information is not recovered
// on application restart preventing it from being deterministic during `DeliverTx`.
Expand All @@ -48,8 +45,7 @@ func NewPlaceOrderRateLimiter(config types.BlockRateLimitConfiguration) RateLimi
}

r := placeOrderRateLimiter{
rateLimitedAccounts: make(map[string]bool, 0),
rateLimitedSubaccounts: make(map[satypes.SubaccountId]bool, 0),
rateLimitedAccounts: make(map[string]bool, 0),
}
if len(config.MaxShortTermOrdersPerNBlocks) == 0 {
r.checkStateShortTermOrderRateLimiter = NewNoOpRateLimiter[string]()
Expand All @@ -66,15 +62,15 @@ func NewPlaceOrderRateLimiter(config types.BlockRateLimitConfiguration) RateLimi
)
}
if len(config.MaxStatefulOrdersPerNBlocks) == 0 {
r.checkStateStatefulOrderRateLimiter = NewNoOpRateLimiter[satypes.SubaccountId]()
r.checkStateStatefulOrderRateLimiter = NewNoOpRateLimiter[string]()
} else if len(config.MaxStatefulOrdersPerNBlocks) == 1 &&
config.MaxStatefulOrdersPerNBlocks[0].NumBlocks == 1 {
r.checkStateStatefulOrderRateLimiter = NewSingleBlockRateLimiter[satypes.SubaccountId](
r.checkStateStatefulOrderRateLimiter = NewSingleBlockRateLimiter[string](
"MaxStatefulOrdersPerNBlocks",
config.MaxStatefulOrdersPerNBlocks[0],
)
} else {
r.checkStateStatefulOrderRateLimiter = NewMultiBlockRateLimiter[satypes.SubaccountId](
r.checkStateStatefulOrderRateLimiter = NewMultiBlockRateLimiter[string](
"MaxStatefulOrdersPerNBlocks",
config.MaxStatefulOrdersPerNBlocks,
)
Expand All @@ -93,7 +89,7 @@ func (r *placeOrderRateLimiter) RateLimit(ctx sdk.Context, msg *types.MsgPlaceOr
)
} else {
msg.Order.MustBeStatefulOrder()
err = r.checkStateStatefulOrderRateLimiter.RateLimit(ctx, msg.Order.OrderId.SubaccountId)
err = r.checkStateStatefulOrderRateLimiter.RateLimit(ctx, msg.Order.OrderId.SubaccountId.Owner)
}

if err != nil {
Expand All @@ -103,7 +99,6 @@ func (r *placeOrderRateLimiter) RateLimit(ctx sdk.Context, msg *types.MsgPlaceOr
msg.Order.GetOrderLabels(),
)
r.rateLimitedAccounts[msg.Order.OrderId.SubaccountId.Owner] = true
r.rateLimitedSubaccounts[msg.Order.OrderId.SubaccountId] = true
}
return err
}
Expand All @@ -116,22 +111,12 @@ func (r *placeOrderRateLimiter) PruneRateLimits(ctx sdk.Context) {
metrics.PlaceOrderAccounts,
metrics.Count,
)
telemetry.IncrCounter(
float32(len(r.rateLimitedSubaccounts)),
types.ModuleName,
metrics.RateLimit,
metrics.PlaceOrderSubaccounts,
metrics.Count,
)
// Note that this method for clearing the map is optimized by the go compiler significantly
// and will leave the relative size of the map the same so that it doesn't need to be resized
// often.
for key := range r.rateLimitedAccounts {
delete(r.rateLimitedAccounts, key)
}
for key := range r.rateLimitedSubaccounts {
delete(r.rateLimitedSubaccounts, key)
}
r.checkStateShortTermOrderRateLimiter.PruneRateLimits(ctx)
r.checkStateStatefulOrderRateLimiter.PruneRateLimits(ctx)
}
Expand All @@ -149,7 +134,7 @@ var _ RateLimiter[*types.MsgCancelOrder] = (*cancelOrderRateLimiter)(nil)

// NewCancelOrderRateLimiter returns a RateLimiter which rate limits types.MsgCancelOrder based upon the provided
// types.BlockRateLimitConfiguration. The rate limiter currently supports limiting based upon:
// - how many short term order cancellations per subaccount (by using satypes.SubaccountId).
// - how many short term order cancellations per account (by using string).
//
// The rate limiting must only be used during `CheckTx` because the rate limiting information is not recovered
// on application restart preventing it from being deterministic during `DeliverTx`.
Expand Down
4 changes: 2 additions & 2 deletions protocol/x/clob/types/block_rate_limit_config.pb.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a68b735

Please sign in to comment.