@0x:contracts-exchange Addressed lingering review comments

contracts/exchange/contracts/src/MixinExchangeCore.sol

@@ -61,7 +61,9 @@ contract MixinExchangeCore is
     /// @param targetOrderEpoch Orders created with a salt less or equal to this value will be cancelled.
     function cancelOrdersUpTo(uint256 targetOrderEpoch)
         external
+        payable
         nonReentrant
+        refundFinalBalance
     {
         address makerAddress = _getCurrentContextAddress();
         // If this function is called via `executeTransaction`, we only update the orderEpoch for the makerAddress/msg.sender combination.
@@ -119,7 +121,9 @@ contract MixinExchangeCore is
     /// @param order Order to cancel. Order must be OrderStatus.FILLABLE.
     function cancelOrder(LibOrder.Order memory order)
         public
+        payable
         nonReentrant
+        refundFinalBalance
     {
         _cancelOrder(order);
     }

contracts/exchange/contracts/src/MixinMatchOrders.sol

@@ -1,16 +1,20 @@
 /*
+
   Copyright 2019 ZeroEx Intl.
+
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
   You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
-*/
 
+*/
 pragma solidity ^0.5.9;
 pragma experimental ABIEncoderV2;
 
@@ -502,18 +506,20 @@ contract MixinMatchOrders is
             // a stack variable does not need to be allocated for the call.
             uint256 valuePaid = 0;
 
+            // Since the `BALANCE` opcode costs 400 gas, we choose to calculate this value by hand rather than calling it twice.
+            uint256 balance = address(this).balance;
+
             // Pay the left order's protocol fee.
-            if (address(this).balance >= protocolFee) {
+            if (balance >= protocolFee) {
                 valuePaid = protocolFee;
             }
             IStaking(feeCollector).payProtocolFee.value(valuePaid)(leftOrder.makerAddress, takerAddress, protocolFee);
 
-            // Clear the value paid for the next calculation.
-            valuePaid = 0;
-
             // Pay the right order's protocol fee.
-            if (address(this).balance >= protocolFee) {
+            if (balance - valuePaid >= protocolFee) {
                 valuePaid = protocolFee;
+            } else {
+                valuePaid = 0;
             }
             IStaking(feeCollector).payProtocolFee.value(valuePaid)(rightOrder.makerAddress, takerAddress, protocolFee);
         } else {

contracts/exchange/contracts/src/MixinSignatureValidator.sol

@@ -59,7 +59,9 @@ contract MixinSignatureValidator is
     /// @param hash Any 32-byte hash.
     function preSign(bytes32 hash)
         external
+        payable
         nonReentrant
+        refundFinalBalance
     {
         address signerAddress = _getCurrentContextAddress();
         preSigned[hash][signerAddress] = true;
@@ -74,7 +76,9 @@ contract MixinSignatureValidator is
         bool approval
     )
         external
+        payable
         nonReentrant
+        refundFinalBalance
     {
         address signerAddress = _getCurrentContextAddress();
         allowedValidators[signerAddress][validatorAddress] = approval;

contracts/exchange/contracts/src/MixinWrapperFunctions.sol

@@ -331,7 +331,9 @@ contract MixinWrapperFunctions is
     /// @param orders Array of order specifications.
     function batchCancelOrders(LibOrder.Order[] memory orders)
         public
+        payable
         nonReentrant
+        refundFinalBalance
     {
         uint256 ordersLength = orders.length;
         for (uint256 i = 0; i != ordersLength; i++) {

contracts/exchange/contracts/src/interfaces/IExchangeCore.sol

@@ -64,7 +64,8 @@ contract IExchangeCore {
     ///      and senderAddress equal to msg.sender (or null address if msg.sender == makerAddress).
     /// @param targetOrderEpoch Orders created with a salt less or equal to this value will be cancelled.
     function cancelOrdersUpTo(uint256 targetOrderEpoch)
-        external;
+        external
+        payable;
 
     /// @dev Fills the input order.
     /// @param order Order struct containing order specifications.
@@ -83,7 +84,8 @@ contract IExchangeCore {
     /// @dev After calling, the order can not be filled anymore.
     /// @param order Order struct containing order specifications.
     function cancelOrder(LibOrder.Order memory order)
-        public;
+        public
+        payable;
 
     /// @dev Gets information about an order: status, hash, and amount filled.
     /// @param order Order to gather information on.

contracts/exchange/contracts/src/interfaces/ISignatureValidator.sol

@@ -48,7 +48,8 @@ contract ISignatureValidator {
     ///      After presigning a hash, the preSign signature type will become valid for that hash and signer.
     /// @param hash Any 32-byte hash.
     function preSign(bytes32 hash)
-        external;
+        external
+        payable;
 
     /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf.
     /// @param validatorAddress Address of Validator contract.
@@ -57,7 +58,8 @@ contract ISignatureValidator {
         address validatorAddress,
         bool approval
     )
-        external;
+        external
+        payable;
 
     /// @dev Verifies that a hash has been signed by the given signer.
     /// @param hash Any 32-byte hash.

contracts/exchange/contracts/src/interfaces/IWrapperFunctions.sol

@@ -155,7 +155,8 @@ contract IWrapperFunctions {
     /// @dev Synchronously cancels multiple orders in a single transaction.
     /// @param orders Array of order specifications.
     function batchCancelOrders(LibOrder.Order[] memory orders)
-        public;
+        public
+        payable;
 
     /// @dev Fetches information for all passed in orders
     /// @param orders Array of order specifications.

contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts

@@ -48,13 +48,6 @@ export class FillOrderWrapper {
         takerAddress: string,
         opts: { takerAssetFillAmount?: BigNumber } = {},
         initBalanceStore: BalanceStore,
-        // stakingOpts: {
-        //     gasPrice: BigNumber;
-        //     messageValue: BigNumber;
-        //     protocolFeeMultiplier: BigNumber;
-        //     stakingAddress: string;
-        //     wethAddress: string;
-        // },
     ): [FillResults, FillEventArgs, BalanceStore] {
         const balanceStore = LocalBalanceStore.create(initBalanceStore);
         const takerAssetFillAmount =

contracts/exchange/test/internal.ts

@@ -182,9 +182,6 @@ blockchainTests('Exchange core internal functions', env => {
                 gasPrice,
             );
             const expectedFilledState = orderTakerAssetFilledAmount.plus(takerAssetFillAmount);
-            // const opts = isProtocolFeePaidInEth
-            //    ? { value: fillResults.protocolFeePaid }
-            //    : { value: constants.ZERO_AMOUNT };
             // CAll `testUpdateFilledState()`, which will set the `filled`
             // state for this order to `orderTakerAssetFilledAmount` before
             // calling `_updateFilledState()`.

contracts/exchange/test/transactions.ts

@@ -1043,7 +1043,6 @@ blockchainTests.resets('Exchange transactions', env => {
                         cancelTransaction.salt,
                         cancelTransaction.expirationTimeSeconds,
                         cancelTransaction.signature,
-                        // { from: makerAddress },
                         { from: makerAddress },
                     );