Ensure orders cannot be cancelled when they are in a pending state (#32)

* Ensure orders cannot be cancelled when they are in a pending state

* Fix maker mock deploy hook

.github/workflows/deploy-mm-mock-heroku.yml

@@ -1,9 +1,9 @@
 name: Maker Mock - Deploy to Heroku
 
 on:
-  push
-#     branches:
-#       - main
+  push:
+    branches:
+      - main
 
 jobs:
   build-and-deploy:
@@ -19,7 +19,7 @@ jobs:
           HEROKU_API_KEY: ${{ secrets.HEROKU_API_KEY }}
 
       - name: Build and Push Docker Image
-        run: |                    
+        run: |
           docker build -t registry.heroku.com/order-book-maker-mock/worker -f cmd/maker-mock.Dockerfile .
           docker push registry.heroku.com/order-book-maker-mock/worker
         env:

models/errors.go

@@ -13,6 +13,7 @@ var ErrOrderNotOpen = errors.New("order must be status open to perform this acti
 var ErrTransactionFailed = errors.New("transaction failed")
 var ErrInsufficientLiquity = errors.New("not enough liquidity in book to satisfy amountIn")
 var ErrAuctionInvalid = errors.New("orders in the auction can not fill any longer")
+var ErrOrderPending = errors.New("order is pending")
 
 // store generic errors
 var ErrValAlreadyInSet = errors.New("the value is already a memeber of the set")

service/auction.go

@@ -30,7 +30,7 @@ func validateOrderFrag(frag models.OrderFrag, order *models.Order) bool {
 	if order.Status != models.STATUS_OPEN {
 		return false
 	}
-	// order.size - (Order.filled + prder.pending) >= frag.size
+	// order.size - (Order.filled + Order.pending) >= frag.size
 	return order.GetAvailableSize().GreaterThanOrEqual(frag.Size)
 }
 
@@ -39,7 +39,7 @@ func validatePendingFrag(frag models.OrderFrag, order *models.Order) bool {
 	if order.Status != models.STATUS_OPEN {
 		return false
 	}
-	// order.Size pending should be greater or equal to orderFrag: (Order.sizePending + prder.pending) >= frag.size
+	// order.Size pending should be greater or equal to orderFrag: (Order.sizePending + Order.pending) >= frag.size
 	return order.SizePending.GreaterThanOrEqual(frag.Size)
 }
 

service/cancel_order.go

@@ -7,6 +7,7 @@ import (
 	"github.com/orbs-network/order-book/models"
 	"github.com/orbs-network/order-book/utils/logger"
 	"github.com/orbs-network/order-book/utils/logger/logctx"
+	"github.com/shopspring/decimal"
 )
 
 type CancelOrderInput struct {
@@ -42,6 +43,11 @@ func (s *Service) CancelOrder(ctx context.Context, input CancelOrderInput) (canc
 		return nil, models.ErrOrderNotFound
 	}
 
+	if order.SizePending.GreaterThan(decimal.Zero) {
+		logctx.Warn(ctx, "cancelling order not possible when order is pending", logger.String("orderId", order.Id.String()), logger.String("sizePending", order.SizePending.String()))
+		return nil, models.ErrOrderPending
+	}
+
 	if order.Status != models.STATUS_OPEN {
 		logctx.Warn(ctx, "trying to cancel order that is not open", logger.String("orderId", order.Id.String()), logger.String("status", order.Status.String()))
 		return nil, models.ErrOrderNotOpen
@@ -58,6 +64,6 @@ func (s *Service) CancelOrder(ctx context.Context, input CancelOrderInput) (canc
 		return nil, err
 	}
 
-	logctx.Info(ctx, "order removed", logger.String("orderId", order.Id.String()))
+	logctx.Info(ctx, "order removed", logger.String("orderId", order.Id.String()), logger.String("userId", order.UserId.String()), logger.String("size", order.Size.String()), logger.String("sizeFilled", order.SizeFilled.String()), logger.String("sizePending", order.SizePending.String()))
 	return &order.Id, nil
 }

service/cancel_order_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/orbs-network/order-book/mocks"
 	"github.com/orbs-network/order-book/models"
 	"github.com/orbs-network/order-book/service"
+	"github.com/shopspring/decimal"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -30,8 +31,8 @@ func TestService_CancelOrder(t *testing.T) {
 		{name: "unexpected error when finding order by orderId - returns error", isClientOId: false, err: assert.AnError, expectedOrderId: nil, expectedErr: assert.AnError},
 		{name: "unexpected error when finding order by clientOId - returns error", isClientOId: true, err: assert.AnError, expectedOrderId: nil, expectedErr: assert.AnError},
 		{name: "order not found - returns `ErrOrderNotFound` error", isClientOId: false, order: nil, err: nil, expectedOrderId: nil, expectedErr: models.ErrOrderNotFound},
-		// {name: "trying to cancel order that is not open - returns `ErrOrderNotOpen` error", isClientOId: false, order: &models.Order{Status: models.STATUS_PENDING}, err: nil, expectedOrderId: nil, expectedErr: models.ErrOrderNotOpen},
 		{name: "user trying to cancel another user's order - returns `ErrUnauthorized` error", isClientOId: false, order: &models.Order{UserId: uuid.MustParse("00000000-0000-0000-0000-000000000009"), Status: models.STATUS_OPEN}, expectedOrderId: nil, expectedErr: models.ErrUnauthorized},
+		{name: "cancelling order not possible when order is pending", isClientOId: false, order: &models.Order{UserId: userId, SizePending: decimal.NewFromFloat(254), SizeFilled: decimal.NewFromFloat(32323.32)}, expectedOrderId: nil, expectedErr: models.ErrOrderPending},
 		{name: "unexpected error when removing order - returns error", isClientOId: false, order: order, err: assert.AnError, expectedOrderId: nil, expectedErr: assert.AnError},
 		{name: "order removed successfully by orderId - returns cancelled orderId", isClientOId: false, order: order, err: nil, expectedOrderId: &orderId, expectedErr: nil},
 		{name: "order removed successfully by clientOId - returns cancelled orderId", isClientOId: true, order: order, err: nil, expectedOrderId: &orderId, expectedErr: nil},

transport/rest/cancel_order.go

@@ -111,6 +111,12 @@ func (h *Handler) handleCancelOrder(input hInput) {
 		return
 	}
 
+	if err == models.ErrOrderPending {
+		logctx.Warn(input.ctx, "cancelling order not possible when order is pending", logger.String("id", input.id.String()))
+		http.Error(input.w, "Cannot cancel order due to pending fill", http.StatusConflict)
+		return
+	}
+
 	if err != nil {
 		logctx.Error(input.ctx, "failed to cancel order", logger.Error(err))
 		http.Error(input.w, "Error cancelling order. Try again later", http.StatusInternalServerError)

transport/rest/cancel_order_test.go

@@ -80,6 +80,13 @@ func TestHandler_CancelOrderByOrderId(t *testing.T) {
 			http.StatusInternalServerError,
 			"Error cancelling order. Try again later\n",
 		},
+		{
+			"cannot currently cancel due to pending fill",
+			&mocks.MockOrderBookService{Error: models.ErrOrderPending},
+			fmt.Sprintf("/order/%s", orderId.String()),
+			http.StatusConflict,
+			"Cannot cancel order due to pending fill\n",
+		},
 		{
 			"successful cancel",
 			&mocks.MockOrderBookService{Order: &models.Order{Id: orderId}},