Return all amounts traded and fees paid

7 files changed, 115 insertions, 106 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol
index d173199b3..de3989a96 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol
@@ -23,7 +23,7 @@ contract ISigner {
 
     function isValidSignature(
         bytes32 hash,
-        bytes signature)
+        bytes memory signature)
         public view
         returns (bool isValid);
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index 375848ed1..aefffe465 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -89,15 +89,21 @@ contract MixinExchangeCore is
         uint256 takerSellAmount,
         bytes memory signature)
         public
-        returns (uint256 takerAmountSold)
+        returns (FillResults memory fillResults)
     {
         // Compute the order hash
         bytes32 orderHash = getOrderHash(order);
 
-        // Check if order has been cancelled
+        // Check if order has been cancelled by salt value
+        if (order.salt < makerEpoch[order.makerAddress]) {
+            emit ExchangeError(uint8(Errors.ORDER_CANCELLED), orderHash);
+            return fillResults;
+        }
+
+        // Check if order has been cancelled by orderHash
         if (cancelled[orderHash]) {
             LogError(uint8(Errors.ORDER_CANCELLED), orderHash);
-            return 0;
+            return fillResults;
         }
 
         // Validate order and maker only if first time seen
@@ -114,35 +120,30 @@ contract MixinExchangeCore is
         // Validate order expiration
         if (block.timestamp >= order.expirationTimeSeconds) {
             LogError(uint8(Errors.ORDER_EXPIRED), orderHash);
-            return 0;
+            return fillResults;
         }
 
         // Validate order availability
         uint256 remainingMakerBuyAmount = safeSub(order.makerBuyAmount, filled[orderHash]);
         if (remainingMakerBuyAmount == 0) {
             LogError(uint8(Errors.ORDER_FULLY_FILLED), orderHash);
-            return 0;
+            return fillResults;
         }
 
         // Validate fill order rounding
-        takerAmountSold = min256(takerSellAmount, remainingMakerBuyAmount);
-        if (isRoundingError(takerAmountSold, order.makerBuyAmount, order.makerSellAmount)) {
+        fillResults.takerAmountSold = min256(takerSellAmount, remainingMakerBuyAmount);
+        if (isRoundingError(fillResults.takerAmountSold, order.makerBuyAmount, order.makerSellAmount)) {
             LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash);
-            return 0;
-        }
-
-        // Validate order is not cancelled
-        if (order.salt < makerEpoch[order.makerAddress]) {
-            LogError(uint8(Errors.ORDER_CANCELLED), orderHash);
-            return 0;
+            fillResults.takerAmountSold = 0;
+            return fillResults;
         }
 
         // Update state
-        filled[orderHash] = safeAdd(filled[orderHash], takerAmountSold);
+        filled[orderHash] = safeAdd(filled[orderHash], fillResults.takerAmountSold);
 
         // Settle order
-        var (makerAmountSold, makerFeePaid, takerFeePaid) =
-            settleOrder(order, msg.sender, takerAmountSold);
+        (fillResults.makerAmountSold, fillResults.makerFeePaid, fillResults.takerFeePaid) =
+            settleOrder(order, msg.sender, fillResults.takerAmountSold);
 
         // Log order
         LogFill(
@@ -151,13 +152,13 @@ contract MixinExchangeCore is
             order.feeRecipientAddress,
             order.makerTokenAddress,
             order.takerTokenAddress,
-            makerAmountSold,
-            takerAmountSold,
-            makerFeePaid,
-            takerFeePaid,
+            fillResults.makerAmountSold,
+            fillResults.takerAmountSold,
+            fillResults.makerFeePaid,
+            fillResults.takerFeePaid,
             orderHash
         );
-        return takerAmountSold;
+        return fillResults;
     }
 
     /// @dev After calling, the order can not be filled anymore.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 49b6949e2..d8bb25fca 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -39,7 +39,7 @@ contract MixinSignatureValidator is
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes memory signature)
         public view
         returns (bool isValid)
     {
@@ -145,7 +145,7 @@ contract MixinSignatureValidator is
         revert();
     }
 
-    function get32(bytes b, uint256 index)
+    function get32(bytes memory b, uint256 index)
         private pure
         returns (bytes32 result)
     {
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
index 030743348..34bd77092 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
@@ -38,14 +38,15 @@ contract MixinWrapperFunctions is
         uint256 takerSellAmount,
         bytes memory signature)
         public
+        returns (FillResults memory fillResults)
     {
-        require(
-            fillOrder(
-                order,
-                takerSellAmount,
-                signature
-            ) == takerSellAmount
+        fillResults = fillOrder(
+            order,
+            takerSellAmount,
+            signature
         );
+        require(fillResults.takerAmountSold == takerSellAmount);
+        return fillResults;
     }
 
     /// @dev Fills an order with specified parameters and ECDSA signature.
@@ -53,13 +54,13 @@ contract MixinWrapperFunctions is
     /// @param order Order struct containing order specifications.
     /// @param takerSellAmount Desired amount of takerToken to fill.
     /// @param signature Maker's signature of the order.
-    /// @return Total amount of takerToken filled in trade.
+    /// @return Amounts filled and fees paid by maker and taker.
     function fillOrderNoThrow(
         Order memory order,
         uint256 takerSellAmount,
         bytes memory signature)
         public
-        returns (uint256 takerAmountSold)
+        returns (FillResults memory fillResults)
     {
         // We need to call MExchangeCore.fillOrder using a delegatecall in
         // assembly so that we can intercept a call that throws. For this, we
@@ -133,18 +134,24 @@ contract MixinWrapperFunctions is
                 start,                       // pointer to start of input
                 add(452, sigLenWithPadding), // input length is 420 + signature length + padding length
                 start,                       // write output over input
-                32                           // output size is 32 bytes
+                128                          // output size is 128 bytes
             )
             switch success
             case 0 {
-                takerAmountSold := 0
+                mstore(fillResults, 0)
+                mstore(add(fillResults, 32), 0)
+                mstore(add(fillResults, 64), 0)
+                mstore(add(fillResults, 96), 0)
             }
             case 1 {
-                takerAmountSold := mload(start)
+                mstore(fillResults, mload(start))
+                mstore(add(fillResults, 32), mload(add(start, 32)))
+                mstore(add(fillResults, 64), mload(add(start, 64)))
+                mstore(add(fillResults, 96), mload(add(start, 96)))
             }
 
         }
-        return takerAmountSold;
+        return fillResults;
     }
 
     /// @dev Synchronously executes multiple calls of fillOrder in a single transaction.
@@ -214,24 +221,25 @@ contract MixinWrapperFunctions is
         uint256 takerSellAmount,
         bytes[] memory signatures)
         public
-        returns (uint256 takerAmountSold)
+        returns (FillResults memory fillResults)
     {
         for (uint256 i = 0; i < orders.length; i++) {
             require(orders[i].takerTokenAddress == orders[0].takerTokenAddress);
-            uint256 remainingTakerSellAmount = safeSub(takerSellAmount, takerAmountSold);
-            takerAmountSold = safeAdd(
-                takerAmountSold,
-                fillOrder(
-                    orders[i],
-                    remainingTakerSellAmount,
-                    signatures[i]
-                )
+            uint256 remainingTakerSellAmount = safeSub(takerSellAmount, fillResults.takerAmountSold);
+            FillResults memory currentFillResults = fillOrder(
+                orders[i],
+                remainingTakerSellAmount,
+                signatures[i]
             );
-            if (takerAmountSold == takerSellAmount) {
+            fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold);
+            fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold);
+            fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid);
+            fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid);
+            if (fillResults.takerAmountSold == takerSellAmount) {
                 break;
             }
         }
-        return takerAmountSold;
+        return fillResults;
     }
 
     /// @dev Synchronously executes multiple calls of fillOrderNoThrow in a single transaction until total amount is sold by taker.
@@ -245,24 +253,25 @@ contract MixinWrapperFunctions is
         uint256 takerSellAmount,
         bytes[] memory signatures)
         public
-        returns (uint256 takerAmountSold)
+        returns (FillResults memory fillResults)
     {
         for (uint256 i = 0; i < orders.length; i++) {
             require(orders[i].takerTokenAddress == orders[0].takerTokenAddress);
-            uint256 remainingTakerSellAmount = safeSub(takerSellAmount, takerAmountSold);
-            takerAmountSold = safeAdd(
-                takerAmountSold,
-                fillOrderNoThrow(
-                    orders[i],
-                    remainingTakerSellAmount,
-                    signatures[i]
-                )
+            uint256 remainingTakerSellAmount = safeSub(takerSellAmount, fillResults.takerAmountSold);
+            FillResults memory currentFillResults = fillOrderNoThrow(
+                orders[i],
+                remainingTakerSellAmount,
+                signatures[i]
             );
-            if (takerAmountSold == takerSellAmount) {
+            fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold);
+            fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold);
+            fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid);
+            fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid);
+            if (fillResults.takerAmountSold == takerSellAmount) {
                 break;
             }
         }
-        return takerAmountSold;
+        return fillResults;
     }
 
     /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker.
@@ -275,34 +284,30 @@ contract MixinWrapperFunctions is
         uint256 takerBuyAmount,
         bytes[] memory signatures)
         public
-        returns (uint256 takerAmountBought)
+        returns (FillResults memory fillResults)
     {
         for (uint256 i = 0; i < orders.length; i++) {
             require(orders[i].makerTokenAddress == orders[0].makerTokenAddress);
-            uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, takerAmountBought);
-            uint256 takerSellAmount = getPartialAmount(
+            uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, fillResults.makerAmountSold);
+            uint256 remainingTakerSellAmount = getPartialAmount(
                 orders[i].makerBuyAmount,
                 orders[i].makerSellAmount,
                 remainingTakerBuyAmount
             );
-            uint256 takerAmountSold = fillOrder(
-                    orders[i],
-                    takerSellAmount,
-                    signatures[i]
-            );
-            takerAmountBought = safeAdd(
-                takerAmountBought,
-                getPartialAmount(
-                    orders[i].makerSellAmount,
-                    orders[i].makerBuyAmount,
-                    takerAmountSold
-                )
+            FillResults memory currentFillResults = fillOrder(
+                orders[i],
+                remainingTakerSellAmount,
+                signatures[i]
             );
-            if (takerAmountBought == takerBuyAmount) {
+            fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold);
+            fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold);
+            fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid);
+            fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid);
+            if (fillResults.makerAmountSold == takerBuyAmount) {
                 break;
             }
         }
-        return takerAmountBought;
+        return fillResults;
     }
 
     /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker.
@@ -316,34 +321,30 @@ contract MixinWrapperFunctions is
         uint256 takerBuyAmount,
         bytes[] memory signatures)
         public
-        returns (uint256 takerAmountBought)
+        returns (FillResults memory fillResults)
     {
         for (uint256 i = 0; i < orders.length; i++) {
             require(orders[i].makerTokenAddress == orders[0].makerTokenAddress);
-            uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, takerAmountBought);
-            uint256 takerSellAmount = getPartialAmount(
+            uint256 remainingTakerBuyAmount = safeSub(takerBuyAmount, fillResults.makerAmountSold);
+            uint256 remainingTakerSellAmount = getPartialAmount(
                 orders[i].makerBuyAmount,
                 orders[i].makerSellAmount,
                 remainingTakerBuyAmount
             );
-            uint256 takerAmountSold = fillOrderNoThrow(
-                    orders[i],
-                    takerSellAmount,
-                    signatures[i]
-            );
-            takerAmountBought = safeAdd(
-                takerAmountBought,
-                getPartialAmount(
-                    orders[i].makerSellAmount,
-                    orders[i].makerBuyAmount,
-                    takerAmountSold
-                )
+            FillResults memory currentFillResults = fillOrderNoThrow(
+                orders[i],
+                remainingTakerSellAmount,
+                signatures[i]
             );
-            if (takerAmountBought == takerBuyAmount) {
+            fillResults.makerAmountSold = safeAdd(fillResults.makerAmountSold, currentFillResults.makerAmountSold);
+            fillResults.takerAmountSold = safeAdd(fillResults.takerAmountSold, currentFillResults.takerAmountSold);
+            fillResults.makerFeePaid = safeAdd(fillResults.makerFeePaid, currentFillResults.makerFeePaid);
+            fillResults.takerFeePaid = safeAdd(fillResults.takerFeePaid, currentFillResults.takerFeePaid);
+            if (fillResults.makerAmountSold == takerBuyAmount) {
                 break;
             }
         }
-        return takerAmountBought;
+        return fillResults;
     }
 
     /// @dev Synchronously cancels multiple orders in a single transaction.
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 a495434e6..968fec195 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -23,17 +23,24 @@ import "../LibOrder.sol";
 
 contract MExchangeCore is LibOrder {
 
+    struct FillResults {
+        uint256 makerAmountSold;
+        uint256 takerAmountSold;
+        uint256 makerFeePaid;
+        uint256 takerFeePaid;
+    }
+
     function fillOrder(
-        Order order,
-        uint256 takerTokenFillAmount,
-        bytes signature)
+        Order memory order,
+        uint256 takerSellAmount,
+        bytes memory signature)
         public
-        returns (uint256 takerTokenFilledAmount);
+        returns (FillResults memory fillResults);
 
-    function cancelOrder(Order order)
+    function cancelOrder(Order memory order)
         public
         returns (bool);
 
- function cancelOrdersUpTo(uint256 salt)
+    function cancelOrdersUpTo(uint256 salt)
         external;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
index f960e4198..c3e332a87 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
@@ -24,14 +24,14 @@ import "../LibOrder.sol";
 contract MSettlement is LibOrder {
 
     function settleOrder(
-        Order order,
-        address taker,
-        uint256 takerTokenFilledAmount)
+        Order memory order,
+        address takerAddress,
+        uint256 takerAmountSold)
         internal
         returns (
-            uint256 makerTokenFilledAmount,
-            uint256 makerFeeAmountPaid,
-            uint256 takerFeeAmountPaid
+            uint256 makerAmountSold,
+            uint256 makerFeePaid,
+            uint256 takerFeePaid
         );
 
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
index fa75830b4..3f020e0a4 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -29,7 +29,7 @@ contract MSignatureValidator {
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes memory signature)
         public view
         returns (bool isValid);
 }