Remove partial cancels

6 files changed, 31 insertions, 72 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index acbed1ad7..a0603ec78 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -41,7 +41,7 @@ contract MixinExchangeCore is
 {
     // Mappings of orderHash => amounts of takerTokenAmount filled or cancelled.
     mapping (bytes32 => uint256) public filled;
-    mapping (bytes32 => uint256) public cancelled;
+    mapping (bytes32 => bool) public cancelled;
 
     // Mapping of makerAddress => lowest salt an order can have in order to be fillable
     // Orders with a salt less than their maker's epoch are considered cancelled
@@ -65,8 +65,6 @@ contract MixinExchangeCore is
         address indexed feeRecipientAddress,
         address makerTokenAddress,
         address takerTokenAddress,
-        uint256 makerTokenCancelledAmount,
-        uint256 takerTokenCancelledAmount,
         bytes32 indexed orderHash
     );
 
@@ -94,9 +92,15 @@ contract MixinExchangeCore is
         // Compute the order hash
         bytes32 orderHash = getOrderHash(order);
 
+        // Check if order has been cancelled
+        if (cancelled[orderHash]) {
+            LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash);
+            return 0;
+        }
+
         // Validate order and maker only if first time seen
         // TODO: Read filled and cancelled only once
-        if (filled[orderHash] == 0 && cancelled[orderHash] == 0) {
+        if (filled[orderHash] == 0) {
             require(order.makerTokenAmount > 0);
             require(order.takerTokenAmount > 0);
             require(isValidSignature(orderHash, order.makerAddress, signature));
@@ -115,14 +119,14 @@ contract MixinExchangeCore is
         }
 
         // Validate order availability
-        uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(orderHash));
-        takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenAmount);
-        if (takerTokenFilledAmount == 0) {
-            LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash);
+        uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, filled[orderHash]);
+        if (remainingTakerTokenAmount == 0) {
+            LogError(uint8(Errors.ORDER_FULLY_FILLED), orderHash);
             return 0;
         }
 
         // Validate fill order rounding
+        takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenAmount);
         if (isRoundingError(takerTokenFilledAmount, order.takerTokenAmount, order.makerTokenAmount)) {
             LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash);
             return 0;
@@ -157,15 +161,12 @@ contract MixinExchangeCore is
         return takerTokenFilledAmount;
     }
 
-    /// @dev Cancels the input order.
+    /// @dev After calling, the order can not be filled anymore.
     /// @param order Order struct containing order specifications.
-    /// @param takerTokenCancelAmount Desired amount of takerToken to cancel in order.
-    /// @return Amount of takerToken cancelled.
-    function cancelOrder(
-        Order order,
-        uint256 takerTokenCancelAmount)
+    /// @return True if the order state changed to cancelled. False if the transaction was already cancelled or expired.
+    function cancelOrder(Order order)
         public
-        returns (uint256 takerTokenCancelledAmount)
+        returns (bool)
     {
         // Compute the order hash
         bytes32 orderHash = getOrderHash(order);
@@ -173,33 +174,28 @@ contract MixinExchangeCore is
         // Validate the order
         require(order.makerTokenAmount > 0);
         require(order.takerTokenAmount > 0);
-        require(takerTokenCancelAmount > 0);
         require(order.makerAddress == msg.sender);
 
         if (block.timestamp >= order.expirationTimeSeconds) {
             LogError(uint8(Errors.ORDER_EXPIRED), orderHash);
-            return 0;
+            return false;
         }
 
-        uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(orderHash));
-        takerTokenCancelledAmount = min256(takerTokenCancelAmount, remainingTakerTokenAmount);
-        if (takerTokenCancelledAmount == 0) {
+        if (cancelled[orderHash]) {
             LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash);
-            return 0;
+            return false;
         }
 
-        cancelled[orderHash] = safeAdd(cancelled[orderHash], takerTokenCancelledAmount);
+        cancelled[orderHash] = true;
 
         LogCancel(
             order.makerAddress,
             order.feeRecipientAddress,
             order.makerTokenAddress,
             order.takerTokenAddress,
-            getPartialAmount(takerTokenCancelledAmount, order.takerTokenAmount, order.makerTokenAmount),
-            takerTokenCancelledAmount,
             orderHash
         );
-        return takerTokenCancelledAmount;
+        return true;
     }
 
     /// @param salt Orders created with a salt less or equal to this value will be cancelled.
@@ -233,15 +229,4 @@ contract MixinExchangeCore is
         isError = errPercentageTimes1000000 > 1000;
         return isError;
     }
-
-    /// @dev Calculates the sum of values already filled and cancelled for a given order.
-    /// @param orderHash The Keccak-256 hash of the given order.
-    /// @return Sum of values already filled and cancelled.
-    function getUnavailableTakerTokenAmount(bytes32 orderHash)
-        public view
-        returns (uint256 unavailableTakerTokenAmount)
-    {
-        unavailableTakerTokenAmount = safeAdd(filled[orderHash], cancelled[orderHash]);
-        return unavailableTakerTokenAmount;
-    }
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
index 358649aa1..39f4784cb 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
@@ -262,17 +262,11 @@ contract MixinWrapperFunctions is
 
     /// @dev Synchronously cancels multiple orders in a single transaction.
     /// @param orders Array of orders.
-    /// @param takerTokenCancelAmounts Array of desired amounts of takerToken to cancel in orders.
-    function batchCancelOrders(
-        Order[] orders,
-        uint256[] takerTokenCancelAmounts)
+    function batchCancelOrders(Order[] orders)
         public
     {
         for (uint256 i = 0; i < orders.length; i++) {
-            cancelOrder(
-                orders[i],
-                takerTokenCancelAmounts[i]
-            );
+            cancelOrder(orders[i]);
         }
     }
     
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
index 6768e2376..a495434e6 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -30,11 +30,9 @@ contract MExchangeCore is LibOrder {
         public
         returns (uint256 takerTokenFilledAmount);
 
-    function cancelOrder(
-        Order order,
-        uint256 takerTokenCancelAmount)
+    function cancelOrder(Order order)
         public
-        returns (uint256 takerTokenCancelledAmount);
+        returns (bool);
 
  function cancelOrdersUpTo(uint256 salt)
         external;
diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts
index 3f56fd52f..bbe9a1baf 100644
--- a/packages/contracts/src/utils/exchange_wrapper.ts
+++ b/packages/contracts/src/utils/exchange_wrapper.ts
@@ -34,17 +34,9 @@ export class ExchangeWrapper {
         const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash);
         return tx;
     }
-    public async cancelOrderAsync(
-        signedOrder: SignedOrder,
-        from: string,
-        opts: { takerTokenCancelAmount?: BigNumber } = {},
-    ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = orderUtils.createCancel(signedOrder, opts.takerTokenCancelAmount);
-        const txHash = await this._exchange.cancelOrder.sendTransactionAsync(
-            params.order,
-            params.takerTokenCancelAmount,
-            { from },
-        );
+    public async cancelOrderAsync(signedOrder: SignedOrder, from: string): Promise<TransactionReceiptWithDecodedLogs> {
+        const params = orderUtils.createCancel(signedOrder);
+        const txHash = await this._exchange.cancelOrder.sendTransactionAsync(params.order, { from });
         const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash);
         return tx;
     }
@@ -156,14 +148,9 @@ export class ExchangeWrapper {
     public async batchCancelOrdersAsync(
         orders: SignedOrder[],
         from: string,
-        opts: { takerTokenCancelAmounts?: BigNumber[] } = {},
     ): Promise<TransactionReceiptWithDecodedLogs> {
-        const params = formatters.createBatchCancel(orders, opts.takerTokenCancelAmounts);
-        const txHash = await this._exchange.batchCancelOrders.sendTransactionAsync(
-            params.orders,
-            params.takerTokenCancelAmounts,
-            { from },
-        );
+        const params = formatters.createBatchCancel(orders);
+        const txHash = await this._exchange.batchCancelOrders.sendTransactionAsync(params.orders, { from });
         const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash);
         return tx;
     }
diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts
index 4f6116f06..2b261f967 100644
--- a/packages/contracts/src/utils/formatters.ts
+++ b/packages/contracts/src/utils/formatters.ts
@@ -55,10 +55,9 @@ export const formatters = {
         });
         return marketFillOrders;
     },
-    createBatchCancel(signedOrders: SignedOrder[], takerTokenCancelAmounts: BigNumber[] = []) {
+    createBatchCancel(signedOrders: SignedOrder[]) {
         const batchCancel: BatchCancelOrders = {
             orders: [],
-            takerTokenCancelAmounts,
         };
         _.forEach(signedOrders, signedOrder => {
             batchCancel.orders.push({
@@ -74,9 +73,6 @@ export const formatters = {
                 expirationTimeSeconds: signedOrder.expirationTimeSeconds,
                 salt: signedOrder.salt,
             });
-            if (takerTokenCancelAmounts.length < signedOrders.length) {
-                batchCancel.takerTokenCancelAmounts.push(signedOrder.takerTokenAmount);
-            }
         });
         return batchCancel;
     },
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index 06fe23824..460d0a58a 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -25,7 +25,6 @@ export interface MarketFillOrders {
 
 export interface BatchCancelOrders {
     orders: OrderStruct[];
-    takerTokenCancelAmounts: BigNumber[];
 }
 
 export interface CancelOrdersBefore {