From 04f5fc1c95b09853a483c67b5dd5e561e78a3bad Mon Sep 17 00:00:00 2001
From: Charles Jhong <charlesjhongc@gmail.com>
Date: Fri, 1 Jul 2022 14:30:32 +0800
Subject: [PATCH] check canceling order independently

---
 contracts/LimitOrder.sol                    | 9 +++++----
 contracts/test/forkMainnet/LimitOrder.t.sol | 7 ++++---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/contracts/LimitOrder.sol b/contracts/LimitOrder.sol
index 64c48303..c0f4899a 100644
--- a/contracts/LimitOrder.sol
+++ b/contracts/LimitOrder.sol
@@ -485,19 +485,20 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc
      * Cancel limit order
      */
     function cancelLimitOrder(LimitOrderLibEIP712.Order calldata _order, bytes calldata _cancelOrderMakerSig) external override onlyUserProxy nonReentrant {
+        require(_order.expiry > uint64(block.timestamp), "LimitOrder: Order is expired");
+        bytes32 orderHash = getEIP712Hash(LimitOrderLibEIP712._getOrderStructHash(_order));
+        bool isCancelled = LibOrderStorage.getStorage().orderHashToCancelled[orderHash];
+        require(!isCancelled, "LimitOrder: Order is cancelled already");
         {
             LimitOrderLibEIP712.Order memory cancelledOrder = _order;
             cancelledOrder.takerTokenAmount = 0;
 
             bytes32 cancelledOrderHash = getEIP712Hash(LimitOrderLibEIP712._getOrderStructHash(cancelledOrder));
-            // validate order and check cancel signature
-            _validateOrder(cancelledOrder, cancelledOrderHash, _cancelOrderMakerSig);
+            require(isValidSignature(_order.maker, cancelledOrderHash, bytes(""), _cancelOrderMakerSig), "LimitOrder: Cancel request is not signed by maker");
         }
 
         // Set cancelled state to storage
-        bytes32 orderHash = getEIP712Hash(LimitOrderLibEIP712._getOrderStructHash(_order));
         LibOrderStorage.getStorage().orderHashToCancelled[orderHash] = true;
-
         emit OrderCancelled(orderHash, _order.maker);
     }
 
diff --git a/contracts/test/forkMainnet/LimitOrder.t.sol b/contracts/test/forkMainnet/LimitOrder.t.sol
index 64dbb0c7..1676d931 100644
--- a/contracts/test/forkMainnet/LimitOrder.t.sol
+++ b/contracts/test/forkMainnet/LimitOrder.t.sol
@@ -1107,7 +1107,7 @@ contract LimitOrderTest is StrategySharedSetup {
         zeroOrder.takerTokenAmount = 0;
 
         bytes memory cancelPayload = _genCancelLimitOrderPayload(DEFAULT_ORDER, _signOrder(userPrivateKey, zeroOrder, SignatureValidator.SignatureType.EIP712));
-        vm.expectRevert("LimitOrder: Order is not signed by maker");
+        vm.expectRevert("LimitOrder: Cancel request is not signed by maker");
         vm.prank(user, user); // Only EOA
         userProxy.toLimitOrder(cancelPayload);
     }
@@ -1116,7 +1116,7 @@ contract LimitOrderTest is StrategySharedSetup {
         LimitOrderLibEIP712.Order memory expiredOrder = DEFAULT_ORDER;
         expiredOrder.expiry = 0;
 
-        bytes memory payload = _genCancelLimitOrderPayload(DEFAULT_ORDER, _signOrder(userPrivateKey, expiredOrder, SignatureValidator.SignatureType.EIP712));
+        bytes memory payload = _genCancelLimitOrderPayload(expiredOrder, _signOrder(userPrivateKey, expiredOrder, SignatureValidator.SignatureType.EIP712));
         vm.expectRevert("LimitOrder: Order is expired");
         vm.prank(user, user); // Only EOA
         userProxy.toLimitOrder(payload);
@@ -1129,7 +1129,8 @@ contract LimitOrderTest is StrategySharedSetup {
         bytes memory payload = _genCancelLimitOrderPayload(DEFAULT_ORDER, _signOrder(makerPrivateKey, zeroOrder, SignatureValidator.SignatureType.EIP712));
         vm.prank(user, user); // Only EOA
         userProxy.toLimitOrder(payload);
-        vm.expectRevert("LimitOrder: Order is cancelled");
+        vm.expectRevert("LimitOrder: Order is cancelled already");
+        vm.prank(user, user); // Only EOA
         userProxy.toLimitOrder(payload);
     }