[CLOB-914] Ensure that the rate limiting is only occurring during `Ch…

…eckTx` and `ReCheckTx`. (dydxprotocol#488)

* [CLOB-914] Ensure that the rate limiting is only occurring during `CheckTx` and `ReCheckTx`.

protocol/x/clob/keeper/rate_limit.go

@@ -7,7 +7,13 @@ import (
 )
 
 // RateLimitCancelOrder passes order cancellations with valid clob pairs to `cancelOrderRateLimiter`.
+// The rate limiting is only performed during `CheckTx` and `ReCheckTx`.
 func (k *Keeper) RateLimitCancelOrder(ctx sdk.Context, msg *types.MsgCancelOrder) error {
+	// Only rate limit during `CheckTx` and `ReCheckTx`.
+	if lib.IsDeliverTxMode(ctx) {
+		return nil
+	}
+
 	_, found := k.GetClobPair(ctx, types.ClobPairId(msg.OrderId.GetClobPairId()))
 	// If the clob pair isn't found then we expect order cancellation validation to fail the order cancellation as
 	// being invalid.
@@ -30,7 +36,13 @@ func (k *Keeper) RateLimitCancelOrder(ctx sdk.Context, msg *types.MsgCancelOrder
 }
 
 // RateLimitPlaceOrder passes orders with valid clob pairs to `placeOrderRateLimiter`.
+// The rate limiting is only performed during `CheckTx` and `ReCheckTx`.
 func (k *Keeper) RateLimitPlaceOrder(ctx sdk.Context, msg *types.MsgPlaceOrder) error {
+	// Only rate limit during `CheckTx` and `ReCheckTx`.
+	if lib.IsDeliverTxMode(ctx) {
+		return nil
+	}
+
 	_, found := k.GetClobPair(ctx, msg.Order.GetClobPairId())
 	// If the clob pair isn't found then we expect order validation to fail the order as being invalid.
 	if !found {

protocol/x/clob/keeper/rate_limit_test.go

@@ -0,0 +1,43 @@
+package keeper_test
+
+import (
+	testApp "github.com/dydxprotocol/v4-chain/protocol/testutil/app"
+	"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
+	clobtypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func TestRateLimitPlaceOrderIsNoopOutsideOfCheckTxAndReCheckTx(t *testing.T) {
+	tApp := testApp.NewTestAppBuilder().WithTesting(t).Build()
+	checkTxCtx := tApp.AdvanceToBlock(21, testApp.AdvanceToBlockOptions{})
+	deliverTxCtx := checkTxCtx.WithIsCheckTx(false).WithIsReCheckTx(false)
+	msg := clobtypes.NewMsgPlaceOrder(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price5_GTB20)
+
+	// We expect an error and that the GTB is out of bounds.
+	require.Error(
+		t,
+		tApp.App.ClobKeeper.RateLimitPlaceOrder(checkTxCtx, msg),
+		"GoodTilBlock 20 is less than the current blockHeight 22",
+	)
+
+	// We don't expect any checks from occurring.
+	require.Nil(t, tApp.App.ClobKeeper.RateLimitPlaceOrder(deliverTxCtx, msg))
+}
+
+func TestRateLimitCancelOrderIsNoopOutsideOfCheckTxAndReCheckTx(t *testing.T) {
+	tApp := testApp.NewTestAppBuilder().WithTesting(t).Build()
+	checkTxCtx := tApp.AdvanceToBlock(21, testApp.AdvanceToBlockOptions{})
+	deliverTxCtx := checkTxCtx.WithIsCheckTx(false).WithIsReCheckTx(false)
+	msg := clobtypes.NewMsgCancelOrderShortTerm(constants.Order_Alice_Num0_Id0_Clob0_Buy5_Price5_GTB20.OrderId, 20)
+
+	// We expect an error and that the GTB is out of bounds.
+	require.Error(
+		t,
+		tApp.App.ClobKeeper.RateLimitCancelOrder(checkTxCtx, msg),
+		"GoodTilBlock 20 is less than the current blockHeight 22",
+	)
+
+	// We don't expect any checks from occurring.
+	require.Nil(t, tApp.App.ClobKeeper.RateLimitCancelOrder(deliverTxCtx, msg))
+}

protocol/x/clob/rate_limit/order_rate_limiter.go

@@ -29,7 +29,8 @@ var _ RateLimiter[*types.MsgPlaceOrder] = (*placeOrderRateLimiter)(nil)
 //   - how many short term orders per subaccount (by using satypes.SubaccountId).
 //   - how many stateful order per subaccount (by using satypes.SubaccountId).
 //
-// The rate limiting keeps track of orders only placed during CheckTx.
+// 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`.
 //
 // Depending upon the provided types.BlockRateLimitConfiguration, the returned RateLimiter may rely on:
 //   - `ctx.BlockHeight()` in RateLimit to track which block the rate limit should apply to.
@@ -83,10 +84,7 @@ func NewPlaceOrderRateLimiter(config types.BlockRateLimitConfiguration) RateLimi
 }
 
 func (r *placeOrderRateLimiter) RateLimit(ctx sdk.Context, msg *types.MsgPlaceOrder) (err error) {
-	// Only perform rate limiting in CheckTx.
-	if lib.IsDeliverTxMode(ctx) {
-		return nil
-	}
+	lib.AssertCheckTxMode(ctx)
 
 	if msg.Order.IsShortTermOrder() {
 		err = r.checkStateShortTermOrderRateLimiter.RateLimit(
@@ -153,7 +151,8 @@ var _ RateLimiter[*types.MsgCancelOrder] = (*cancelOrderRateLimiter)(nil)
 // types.BlockRateLimitConfiguration. The rate limiter currently supports limiting based upon:
 //   - how many short term order cancellations per subaccount (by using satypes.SubaccountId).
 //
-// The rate limiting keeps track of order cancellations placed during CheckTx.
+// 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`.
 //
 // Depending upon the provided types.BlockRateLimitConfiguration, the returned RateLimiter may rely on:
 //   - `ctx.BlockHeight()` in RateLimit to track which block the rate limit should apply to.
@@ -190,10 +189,7 @@ func NewCancelOrderRateLimiter(config types.BlockRateLimitConfiguration) RateLim
 }
 
 func (r *cancelOrderRateLimiter) RateLimit(ctx sdk.Context, msg *types.MsgCancelOrder) (err error) {
-	// Only perform rate limiting in CheckTx.
-	if lib.IsDeliverTxMode(ctx) {
-		return nil
-	}
+	lib.AssertCheckTxMode(ctx)
 
 	if msg.OrderId.IsShortTermOrder() {
 		err = r.checkStateShortTermRateLimiter.RateLimit(