check canceling order independently

consenlabs · Jul 1, 2022 · 04f5fc1 · 04f5fc1
1 parent 3ebb1ef
commit 04f5fc1
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 7 deletions.
diff --git 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
@@ -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);
     }