[CT-514] Clob MsgBatchCancel functionality (dydxprotocol#1110)

* wip implementation * use new cometbft * Revert "use new cometbft" This reverts commit e5b8a03. * go mod tidy * basic e2e test * more msgBatchCancels in code * repeated fixed32 -> uint32 * remove debug prints * update cometbft replace go.mod sha * one more debug print * typo * regen indexer protos * update comment on proto * proto comment changes * extract stateful validation into own fn * pr format comments * clean up test file * new return type with success and failure Signed-off-by: Eric <eric.warehime@gmail.com>
skip-mev · Mar 12, 2024 · 929f09e · 929f09e
1 parent 41de83e
commit 929f09e
Show file tree

Hide file tree

Showing 19 changed files with 599 additions and 81 deletions.
diff --git a/indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/tx.ts b/indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/tx.ts
@@ -167,7 +167,11 @@ export interface MsgBatchCancelSDKType {
 export interface OrderBatch {
   /** The Clob Pair ID all orders in this order batch belong to. */
   clobPairId: number;
-  /** List of client ids in this order batch. */
+  /**
+   * List of client ids in this order batch.
+   * Note that this is serialized as a uint32 instead of a fixed32 to
+   * avoid issues when decoding repeated packed fixed32.
+   */
 
   clientIds: number[];
 }
@@ -180,7 +184,11 @@ export interface OrderBatch {
 export interface OrderBatchSDKType {
   /** The Clob Pair ID all orders in this order batch belong to. */
   clob_pair_id: number;
-  /** List of client ids in this order batch. */
+  /**
+   * List of client ids in this order batch.
+   * Note that this is serialized as a uint32 instead of a fixed32 to
+   * avoid issues when decoding repeated packed fixed32.
+   */
 
   client_ids: number[];
 }
@@ -802,7 +810,7 @@ export const OrderBatch = {
     writer.uint32(18).fork();
 
     for (const v of message.clientIds) {
-      writer.fixed32(v);
+      writer.uint32(v);
     }
 
     writer.ldelim();
@@ -827,10 +835,10 @@ export const OrderBatch = {
             const end2 = reader.uint32() + reader.pos;
 
             while (reader.pos < end2) {
-              message.clientIds.push(reader.fixed32());
+              message.clientIds.push(reader.uint32());
             }
           } else {
-            message.clientIds.push(reader.fixed32());
+            message.clientIds.push(reader.uint32());
           }
 
           break;

diff --git a/proto/dydxprotocol/clob/tx.proto b/proto/dydxprotocol/clob/tx.proto
@@ -123,7 +123,9 @@ message OrderBatch {
   // The Clob Pair ID all orders in this order batch belong to.
   uint32 clob_pair_id = 1;
   // List of client ids in this order batch.
-  repeated fixed32 client_ids = 2;
+  // Note that this is serialized as a uint32 instead of a fixed32 to
+  // avoid issues when decoding repeated packed fixed32.
+  repeated uint32 client_ids = 2;
 }
 
 // MsgBatchCancelResponse is a response type used for batch canceling orders.

diff --git a/protocol/app/module/interface_registry.go b/protocol/app/module/interface_registry.go
@@ -1,8 +1,9 @@
 package module
 
 import (
-	"cosmossdk.io/x/tx/signing"
 	"fmt"
+
+	"cosmossdk.io/x/tx/signing"
 	addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
 	"github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -76,6 +77,9 @@ func NewInterfaceRegistry(addrPrefix string, valAddrPrefix string) (types.Interf
 			// https://github.com/cosmos/cosmos-sdk/issues/18722 is fixed, replace this with the cosmos.msg.v1.signing
 			// annotation on the protos.
 			CustomGetSigners: map[protoreflect.FullName]signing.GetSignersFunc{
+				"dydxprotocol.clob.MsgBatchCancel": getLegacyMsgSignerFn(
+					[]string{"subaccount_id", "owner"},
+				),
 				"dydxprotocol.clob.MsgCancelOrder": getLegacyMsgSignerFn(
 					[]string{"order_id", "subaccount_id", "owner"},
 				),

diff --git a/protocol/app/process/other_msgs_test.go b/protocol/app/process/other_msgs_test.go
@@ -1,9 +1,10 @@
 package process_test
 
 import (
-	errorsmod "cosmossdk.io/errors"
 	"testing"
 
+	errorsmod "cosmossdk.io/errors"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/dydxprotocol/v4-chain/protocol/app/process"
 	"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
@@ -87,6 +88,13 @@ func TestDecodeOtherMsgsTx(t *testing.T) {
 				"Msg type *types.MsgCancelOrder is not allowed in OtherTxs",
 			),
 		},
+		"Error: batch cancel order is not allowed": {
+			txBytes: constants.Msg_BatchCancel_TxBtyes,
+			expectedErr: errorsmod.Wrap(
+				process.ErrUnexpectedMsgType,
+				"Msg type *types.MsgBatchCancel is not allowed in OtherTxs",
+			),
+		},
 		"Valid: single msg": {
 			txBytes:      constants.Msg_Send_TxBytes,
 			expectedMsgs: []sdk.Msg{constants.Msg_Send},

diff --git a/protocol/app/process/utils_disallow_msgs.go b/protocol/app/process/utils_disallow_msgs.go
@@ -18,6 +18,8 @@ func IsDisallowClobOrderMsgInOtherTxs(targetMsg sdk.Msg) bool {
 		order := msg.GetOrder()
 		orderId := order.GetOrderId()
 		return !orderId.IsStatefulOrder() // not stateful -> returns true -> disallow
+	case *clobtypes.MsgBatchCancel:
+		return true
 	}
 	return false
 }
diff --git a/protocol/app/process/utils_disallow_msgs_test.go b/protocol/app/process/utils_disallow_msgs_test.go
@@ -25,7 +25,7 @@ func TestIsDisallowClobOrderMsgInOtherTxs(t *testing.T) {
 	for _, msg := range allMsgSamples {
 		result := process.IsDisallowClobOrderMsgInOtherTxs(msg)
 		switch msg.(type) {
-		case *clobtypes.MsgCancelOrder, *clobtypes.MsgPlaceOrder:
+		case *clobtypes.MsgCancelOrder, *clobtypes.MsgPlaceOrder, *clobtypes.MsgBatchCancel:
 			// The sample msgs are short-term orders, so we expect these to be disallowed.
 			require.True(t, result) // true -> disallow
 		default:

diff --git a/protocol/go.mod b/protocol/go.mod
@@ -434,7 +434,7 @@ replace (
 	// Use dYdX fork of Cosmos SDK/store
 	cosmossdk.io/store => github.com/dydxprotocol/cosmos-sdk/store v1.0.3-0.20240227194839-f4e166bc1057
 	// Use dYdX fork of CometBFT
-	github.com/cometbft/cometbft => github.com/dydxprotocol/cometbft v0.38.6-0.20240220185844-e704122c8540
+	github.com/cometbft/cometbft => github.com/dydxprotocol/cometbft v0.38.6-0.20240229050000-3b085f30b462
 	// Use dYdX fork of Cosmos SDK
 	github.com/cosmos/cosmos-sdk => github.com/dydxprotocol/cosmos-sdk v0.50.5-0.20240227194839-f4e166bc1057
 )

diff --git a/protocol/go.sum b/protocol/go.sum
@@ -525,8 +525,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp
 github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
 github.com/dvsekhvalnov/jose2go v1.6.0 h1:Y9gnSnP4qEI0+/uQkHvFXeD2PLPJeXEL+ySMEA2EjTY=
 github.com/dvsekhvalnov/jose2go v1.6.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU=
-github.com/dydxprotocol/cometbft v0.38.6-0.20240220185844-e704122c8540 h1:pkYQbAdOAAoZBSId9kLupCgZHj8YvA9LzM31fVYpjlw=
-github.com/dydxprotocol/cometbft v0.38.6-0.20240220185844-e704122c8540/go.mod h1:REQN+ObgfYxi39TcYR/Hv95C9bPxY3sYJCvghryj7vY=
+github.com/dydxprotocol/cometbft v0.38.6-0.20240229050000-3b085f30b462 h1:ufl8wDrax5K/33NMX/2P54iJAb5LlHJA8CYIcdfeR+o=
+github.com/dydxprotocol/cometbft v0.38.6-0.20240229050000-3b085f30b462/go.mod h1:REQN+ObgfYxi39TcYR/Hv95C9bPxY3sYJCvghryj7vY=
 github.com/dydxprotocol/cosmos-sdk v0.50.5-0.20240227194839-f4e166bc1057 h1:5Aq2oldr7T5+PEo6HYWA2cTcD4tY5ZM6755pKxhOvNg=
 github.com/dydxprotocol/cosmos-sdk v0.50.5-0.20240227194839-f4e166bc1057/go.mod h1:I2uLntIWztPOXW0abkUb3SRphofiLL5DzrvHObyJVkM=
 github.com/dydxprotocol/cosmos-sdk/store v1.0.3-0.20240227194839-f4e166bc1057 h1:rY2cckHMrpk3+9ggVqp3Zsvfn7AwEjjazvyp4HK3lgw=

diff --git a/protocol/lib/log/constants.go b/protocol/lib/log/constants.go
@@ -46,9 +46,10 @@ const (
 	// Module tag values are prefixed with `x/`
 	Clob = "x/clob"
 
-	CheckTx   = "check_tx"
-	RecheckTx = "recheck_tx"
-	DeliverTx = "deliver_tx"
+	CheckTx        = "check_tx"
+	RecheckTx      = "recheck_tx"
+	DeliverTx      = "deliver_tx"
+	MsgBatchCancel = "msg_batch_cancel"
 )
 
 // Special tag values that should be PascalCased (i.e function names)

diff --git a/protocol/mocks/ClobKeeper.go b/protocol/mocks/ClobKeeper.go
diff --git a/protocol/testutil/app/app.go b/protocol/testutil/app/app.go
@@ -1320,7 +1320,7 @@ func launchValidatorInDir(
 
 // MustMakeCheckTxsWithClobMsg creates one signed RequestCheckTx for each msg passed in.
 // The messsage must use one of the hard-coded well known subaccount owners otherwise this will panic.
-func MustMakeCheckTxsWithClobMsg[T clobtypes.MsgPlaceOrder | clobtypes.MsgCancelOrder](
+func MustMakeCheckTxsWithClobMsg[T clobtypes.MsgPlaceOrder | clobtypes.MsgCancelOrder | clobtypes.MsgBatchCancel](
 	ctx sdk.Context,
 	app *app.App,
 	messages ...T,
@@ -1336,6 +1336,9 @@ func MustMakeCheckTxsWithClobMsg[T clobtypes.MsgPlaceOrder | clobtypes.MsgCancel
 		case clobtypes.MsgCancelOrder:
 			signerAddress = v.OrderId.SubaccountId.Owner
 			m = &v
+		case clobtypes.MsgBatchCancel:
+			signerAddress = v.SubaccountId.Owner
+			m = &v
 		default:
 			panic(fmt.Errorf("MustMakeCheckTxsWithClobMsg: Unknown message type %T", msg))
 		}

diff --git a/protocol/testutil/constants/messages.go b/protocol/testutil/constants/messages.go
@@ -21,6 +21,9 @@ func init() {
 	_ = TestTxBuilder.SetMsgs(Msg_CancelOrder)
 	Msg_CancelOrder_TxBtyes, _ = TestEncodingCfg.TxConfig.TxEncoder()(TestTxBuilder.GetTx())
 
+	_ = TestTxBuilder.SetMsgs(Msg_BatchCancel)
+	Msg_BatchCancel_TxBtyes, _ = TestEncodingCfg.TxConfig.TxEncoder()(TestTxBuilder.GetTx())
+
 	_ = TestTxBuilder.SetMsgs(Msg_Send)
 	Msg_Send_TxBytes, _ = TestEncodingCfg.TxConfig.TxEncoder()(TestTxBuilder.GetTx())
 
@@ -54,6 +57,18 @@ var (
 	}
 	Msg_PlaceOrder_TxBtyes []byte
 
+	Msg_BatchCancel = &clobtypes.MsgBatchCancel{
+		SubaccountId: Alice_Num0,
+		ShortTermCancels: []clobtypes.OrderBatch{
+			{
+				ClobPairId: 0,
+				ClientIds:  []uint32{0},
+			},
+		},
+		GoodTilBlock: 5,
+	}
+	Msg_BatchCancel_TxBtyes []byte
+
 	Msg_PlaceOrder_LongTerm = &clobtypes.MsgPlaceOrder{
 		Order: LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15,
 	}

diff --git a/protocol/testutil/encoding/utils.go b/protocol/testutil/encoding/utils.go
@@ -1,9 +1,10 @@
 package encoding
 
 import (
-	"github.com/dydxprotocol/v4-chain/protocol/testutil/ante"
 	"testing"
 
+	"github.com/dydxprotocol/v4-chain/protocol/testutil/ante"
+
 	feegrantmodule "cosmossdk.io/x/feegrant/module"
 	"cosmossdk.io/x/upgrade"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
@@ -73,6 +74,7 @@ func GetTestEncodingCfg() testutil.TestEncodingConfig {
 		&clobtypes.MsgProposedOperations{},
 		&clobtypes.MsgPlaceOrder{},
 		&clobtypes.MsgCancelOrder{},
+		&clobtypes.MsgBatchCancel{},
 
 		// Perpetuals.
 		&perpetualtypes.MsgAddPremiumVotes{},

diff --git a/protocol/x/clob/ante/clob.go b/protocol/x/clob/ante/clob.go
@@ -130,6 +130,38 @@ func (cd ClobDecorator) AnteHandle(
 				log.Error, err,
 			)
 		}
+	case *types.MsgBatchCancel:
+		// MsgBatchCancel currently only processes short-term cancels right now.
+		// No need to process short term orders on `ReCheckTx`.
+		if ctx.IsReCheckTx() {
+			return next(ctx, tx, simulate)
+		}
+
+		ctx = log.AddPersistentTagsToLogger(
+			ctx,
+			log.Handler, log.MsgBatchCancel,
+		)
+
+		success, failures, err := cd.clobKeeper.BatchCancelShortTermOrder(
+			ctx,
+			msg,
+		)
+		// If there are no successful cancellations and no validation errors,
+		// return an error indicating no cancels have succeeded.
+		if len(success) == 0 && err == nil {
+			err = errorsmod.Wrapf(
+				types.ErrBatchCancelFailed,
+				"No successful cancellations. Failures: %+v",
+				failures,
+			)
+		}
+
+		log.DebugLog(
+			ctx,
+			"Received new batch cancellation",
+			log.Tx, cometbftlog.NewLazySprintf("%X", tmhash.Sum(ctx.TxBytes())),
+			log.Error, err,
+		)
 	}
 	if err != nil {
 		return ctx, err
@@ -139,15 +171,15 @@ func (cd ClobDecorator) AnteHandle(
 }
 
 // IsSingleClobMsgTx returns `true` if the supplied `tx` consist of a single clob message
-// (`MsgPlaceOrder` or `MsgCancelOrder`). If `msgs` consist of multiple clob messages,
-// or a mix of on-chain and clob messages, an error is returned.
+// (`MsgPlaceOrder` or `MsgCancelOrder` or `MsgBatchCancel`). If `msgs` consist of multiple
+// clob messages, or a mix of on-chain and clob messages, an error is returned.
 func IsSingleClobMsgTx(tx sdk.Tx) (bool, error) {
 	msgs := tx.GetMsgs()
 	var hasMessage = false
 
 	for _, msg := range msgs {
 		switch msg.(type) {
-		case *types.MsgCancelOrder, *types.MsgPlaceOrder:
+		case *types.MsgCancelOrder, *types.MsgPlaceOrder, *types.MsgBatchCancel:
 			hasMessage = true
 		}
 
@@ -164,16 +196,16 @@ func IsSingleClobMsgTx(tx sdk.Tx) (bool, error) {
 	if numMsgs > 1 {
 		return false, errorsmod.Wrap(
 			sdkerrors.ErrInvalidRequest,
-			"a transaction containing MsgCancelOrder or MsgPlaceOrder may not contain more than one message",
+			"a transaction containing MsgCancelOrder or MsgPlaceOrder or MsgBatchCancel may not contain more than one message",
 		)
 	}
 
 	return true, nil
 }
 
 // IsShortTermClobMsgTx returns `true` if the supplied `tx` consist of a single clob message
-// (`MsgPlaceOrder` or `MsgCancelOrder`) which references a Short-Term Order. If `msgs` consist of multiple
-// clob messages, or a mix of on-chain and clob messages, an error is returned.
+// (`MsgPlaceOrder` or `MsgCancelOrder` or `MsgBatchCancel`) which references a Short-Term Order.
+// If `msgs` consist of multiple clob messages, or a mix of on-chain and clob messages, an error is returned.
 func IsShortTermClobMsgTx(ctx sdk.Context, tx sdk.Tx) (bool, error) {
 	msgs := tx.GetMsgs()
 
@@ -193,6 +225,11 @@ func IsShortTermClobMsgTx(ctx sdk.Context, tx sdk.Tx) (bool, error) {
 					isShortTermOrder = true
 				}
 			}
+		case *types.MsgBatchCancel:
+			{
+				// MsgBatchCancel processes only short term orders for now.
+				isShortTermOrder = true
+			}
 		}
 
 		if isShortTermOrder {