diff --git a/indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/block_rate_limit_config.ts b/indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/block_rate_limit_config.ts index 14cfe637e6..4db5bf31a3 100644 --- a/indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/block_rate_limit_config.ts +++ b/indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/block_rate_limit_config.ts @@ -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. * @@ -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. * diff --git a/proto/dydxprotocol/clob/block_rate_limit_config.proto b/proto/dydxprotocol/clob/block_rate_limit_config.proto index d323e69749..8f212132e7 100644 --- a/proto/dydxprotocol/clob/block_rate_limit_config.proto +++ b/proto/dydxprotocol/clob/block_rate_limit_config.proto @@ -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. // diff --git a/protocol/lib/metrics/constants.go b/protocol/lib/metrics/constants.go index cd59441af5..190dfce2c5 100644 --- a/protocol/lib/metrics/constants.go +++ b/protocol/lib/metrics/constants.go @@ -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" diff --git a/protocol/x/clob/e2e/rate_limit_test.go b/protocol/x/clob/e2e/rate_limit_test.go index 93d7d94389..a117eedd8e 100644 --- a/protocol/x/clob/e2e/rate_limit_test.go +++ b/protocol/x/clob/e2e/rate_limit_test.go @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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, }, } @@ -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) }) } } @@ -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() diff --git a/protocol/x/clob/rate_limit/order_rate_limiter.go b/protocol/x/clob/rate_limit/order_rate_limiter.go index bfd3578b1b..19e3d03631 100644 --- a/protocol/x/clob/rate_limit/order_rate_limiter.go +++ b/protocol/x/clob/rate_limit/order_rate_limiter.go @@ -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. @@ -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`. @@ -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]() @@ -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, ) @@ -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 { @@ -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 } @@ -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) } @@ -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`. diff --git a/protocol/x/clob/types/block_rate_limit_config.pb.go b/protocol/x/clob/types/block_rate_limit_config.pb.go index f1a3b936e5..b1f0be069e 100644 --- a/protocol/x/clob/types/block_rate_limit_config.pb.go +++ b/protocol/x/clob/types/block_rate_limit_config.pb.go @@ -32,8 +32,8 @@ type BlockRateLimitConfiguration struct { // // Specifying 0 values disables this rate limit. MaxShortTermOrdersPerNBlocks []MaxPerNBlocksRateLimit `protobuf:"bytes,1,rep,name=max_short_term_orders_per_n_blocks,json=maxShortTermOrdersPerNBlocks,proto3" json:"max_short_term_orders_per_n_blocks"` - // 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. //