Refactoring balance threshold filter

6 files changed, 255 insertions, 74 deletions

diff --git a/contracts/extensions/contracts/BalanceThresholdFilter/BalanceThresholdFilter.sol b/contracts/extensions/contracts/BalanceThresholdFilter/BalanceThresholdFilter.sol
index ea248793f..16cacd461 100644
--- a/contracts/extensions/contracts/BalanceThresholdFilter/BalanceThresholdFilter.sol
+++ b/contracts/extensions/contracts/BalanceThresholdFilter/BalanceThresholdFilter.sol
@@ -17,7 +17,6 @@
 */
 
 pragma solidity 0.4.24;
-pragma experimental ABIEncoderV2;
 
 import "@0x/contracts-interfaces/contracts/protocol/Exchange/IExchange.sol";
 import "./interfaces/IThresholdAsset.sol";
diff --git a/contracts/extensions/contracts/BalanceThresholdFilter/MixinBalanceThresholdFilterCore.sol b/contracts/extensions/contracts/BalanceThresholdFilter/MixinBalanceThresholdFilterCore.sol
index 8d15fe6c8..a8947751a 100644
--- a/contracts/extensions/contracts/BalanceThresholdFilter/MixinBalanceThresholdFilterCore.sol
+++ b/contracts/extensions/contracts/BalanceThresholdFilter/MixinBalanceThresholdFilterCore.sol
@@ -62,8 +62,19 @@ contract MixinBalanceThresholdFilterCore is
     ) 
         external
     {
-        // Validate addresses.
-        validateBalanceThresholdsOrRevert(signerAddress);
+        // Get accounts whose balances must be validated
+        address[] memory addressesToValidate = getAddressesToValidate(signerAddress);
+
+        // Validate account balances
+        uint256 balanceThreshold = BALANCE_THRESHOLD;
+        IThresholdAsset thresholdAsset = THRESHOLD_ASSET;
+        for(uint256 i = 0; i < addressesToValidate.length; ++i) {
+            uint256 addressBalance = thresholdAsset.balanceOf(addressesToValidate[i]);
+            if (addressBalance < balanceThreshold) {
+                revert("AT_LEAST_ONE_ADDRESS_DOES_NOT_MEET_BALANCE_THRESHOLD");
+            }
+        }
+        emit ValidatedAddresses(addressesToValidate);
         
         // All addresses are valid. Execute fillOrder.
         EXCHANGE.executeTransaction(
@@ -74,20 +85,15 @@ contract MixinBalanceThresholdFilterCore is
         );
     }
 
-    
-
-    /// @dev Validates addresses meet the balance threshold specified by `BALANCE_THRESHOLD`
-    ///      for the asset `THRESHOLD_ASSET`. If one address does not meet the thresold
-    ///      then this function will revert. Which addresses are validated depends on
-    ///      which Exchange function is to be called (defined by `signedExchangeTransaction` above).
-    ///      No parameters are taken as this function reads arguments directly from calldata, to save gas.
-    ///      If all addresses are valid then this function emits a ValidatedAddresses event, listing all
-    ///      of the addresses whose balance thresholds it checked.
-    function validateBalanceThresholdsOrRevert(address signerAddress)
-        internal
+    /// @dev Constructs an array of addresses to be validated.
+    ///      Addresses depend on which Exchange function is to be called
+    ///      (defined by `signedExchangeTransaction` above).
+    /// @param signerAddress Address of transaction signer.
+    /// @return addressesToValidate Array of addresses to validate.
+    function getAddressesToValidate(address signerAddress)
+        internal pure
+        returns (address[] memory addressesToValidate)
     {
-        // Extract addresses to validate from Exchange calldata
-        address[] memory addressesToValidate = new address[](0);
         bytes4 exchangeFunctionSelector = bytes4(exchangeCalldataload(0));
         if(
             exchangeFunctionSelector == batchFillOrdersSelector         || 
@@ -99,21 +105,21 @@ contract MixinBalanceThresholdFilterCore is
             exchangeFunctionSelector == marketSellOrdersNoThrowSelector
         ) {
             addressesToValidate = loadMakerAddressesFromOrderArray(0);
-            recordAddressToValidate(signerAddress, addressesToValidate);
+            addressesToValidate = addressesToValidate.append(signerAddress);
         } else if(
             exchangeFunctionSelector == fillOrderSelector           ||
             exchangeFunctionSelector == fillOrderNoThrowSelector    ||
             exchangeFunctionSelector == fillOrKillOrderSelector
         ) {
             address makerAddress = loadMakerAddressFromOrder(0);
-            recordAddressToValidate(makerAddress, addressesToValidate);
-            recordAddressToValidate(signerAddress, addressesToValidate);
+            addressesToValidate = addressesToValidate.append(makerAddress);
+            addressesToValidate = addressesToValidate.append(signerAddress);
         } else if(exchangeFunctionSelector == matchOrdersSelector) {
-            address leftOrderAddress = loadMakerAddressFromOrder(0);
-            recordAddressToValidate(leftOrderAddress, addressesToValidate);
-            address rightOrderAddress = loadMakerAddressFromOrder(1);
-            recordAddressToValidate(rightOrderAddress, addressesToValidate);
-            recordAddressToValidate(signerAddress, addressesToValidate);
+            address leftMakerAddress = loadMakerAddressFromOrder(0);
+            addressesToValidate = addressesToValidate.append(leftMakerAddress);
+            address rightMakerAddress = loadMakerAddressFromOrder(1);
+            addressesToValidate = addressesToValidate.append(rightMakerAddress);
+            addressesToValidate = addressesToValidate.append(signerAddress);
         } else if(
             exchangeFunctionSelector != cancelOrderSelector         && 
             exchangeFunctionSelector != batchCancelOrdersSelector   &&
@@ -121,16 +127,5 @@ contract MixinBalanceThresholdFilterCore is
         ) {
             revert("INVALID_OR_BLOCKED_EXCHANGE_SELECTOR");
         }
-
-        // Validate account balances
-        uint256 balanceThreshold = BALANCE_THRESHOLD;
-        IThresholdAsset thresholdAsset = THRESHOLD_ASSET;
-        for(uint i = 0; i < addressesToValidate.length; ++i) {
-            uint256 addressBalance = thresholdAsset.balanceOf(addressesToValidate[i]);
-            if (addressBalance < balanceThreshold) {
-                revert("AT_LEAST_ONE_ADDRESS_DOES_NOT_MEET_BALANCE_THRESHOLD");
-            }
-        }
-        emit ValidatedAddresses(addressesToValidate);
     }
 }
diff --git a/contracts/extensions/contracts/BalanceThresholdFilter/MixinExchangeCalldata.sol b/contracts/extensions/contracts/BalanceThresholdFilter/MixinExchangeCalldata.sol
new file mode 100644
index 000000000..12f601dea
--- /dev/null
+++ b/contracts/extensions/contracts/BalanceThresholdFilter/MixinExchangeCalldata.sol
@@ -0,0 +1,102 @@
+    
+    /*
+
+  Copyright 2018 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/LICENSE2.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.4.24;
+
+import "./mixins/MExchangeCalldata.sol";
+import "@0x/contracts-libs/contracts/libs/LibAddressArray.sol";
+
+
+contract MixinExchangeCalldata is 
+    MExchangeCalldata
+{
+
+    using LibAddressArray for address[];
+
+    /// @dev Emulates the `calldataload` opcode on the embedded Exchange calldata,
+    ///      which is accessed through `signedExchangeTransaction`.
+    /// @param offset  Offset into the Exchange calldata.
+    /// @return value  Corresponding 32 byte value stored at `offset`.
+    function exchangeCalldataload(uint256 offset)
+        internal pure
+        returns (bytes32 value)
+    {
+        assembly {
+            // Pointer to exchange transaction
+            // 0x04 for calldata selector
+            // 0x40 to access `signedExchangeTransaction`, which is the third parameter
+            let exchangeTxPtr := calldataload(0x44)
+
+            // Offset into Exchange calldata
+            // We compute this by adding 0x24 to the `exchangeTxPtr` computed above.
+            // 0x04 for calldata selector
+            // 0x20 for length field of `signedExchangeTransaction`
+            let exchangeCalldataOffset := add(exchangeTxPtr, add(0x24, offset))
+            value := calldataload(exchangeCalldataOffset)
+        }
+    }
+
+    /// @dev Convenience function that skips the 4 byte selector when loading
+    ///      from the embedded Exchange calldata.
+    /// @param offset  Offset into the Exchange calldata (minus the 4 byte selector)
+    /// @return value  Corresponding 32 byte value stored at `offset` + 4.
+    function loadExchangeData(uint256 offset)
+        internal pure
+        returns (bytes32 value)
+    {
+        value = exchangeCalldataload(offset + 4);
+    }
+
+    /// @dev Extracts the maker address from an order stored in the Exchange calldata
+    ///      (which is embedded in `signedExchangeTransaction`).
+    /// @param orderParamIndex  Index of the order in the Exchange function's signature.
+    /// @return makerAddress The extracted maker address.
+    function loadMakerAddressFromOrder(uint256 orderParamIndex)
+        internal pure
+        returns (address makerAddress)
+    {
+        uint256 orderOffsetInBytes = orderParamIndex * 32;
+        uint256 orderPtr = uint256(loadExchangeData(orderOffsetInBytes));
+        makerAddress = address(loadExchangeData(orderPtr));
+        return makerAddress;
+    }
+
+    /// @dev Extracts the maker addresses from an array of orders stored in the Exchange calldata
+    ///      (which is embedded in `signedExchangeTransaction`), and records them in
+    ///      the running list of addresses to validate.
+    /// @param orderArrayParamIndex  Index of the order array in the Exchange function's signature
+    /// @return makerAddresses The extracted maker addresses.
+    function loadMakerAddressesFromOrderArray(uint256 orderArrayParamIndex)
+        internal pure
+        returns (address[] makerAddresses)
+    {
+        uint256 orderArrayOffsetInBytes = orderArrayParamIndex * 32;
+        uint256 orderArrayPtr = uint256(loadExchangeData(orderArrayOffsetInBytes));
+        uint256 orderArrayLength = uint256(loadExchangeData(orderArrayPtr));
+        uint256 orderArrayLengthInBytes = orderArrayLength * 32;
+        uint256 orderArrayElementPtr = orderArrayPtr + 32;
+        uint256 orderArrayElementEndPtr = orderArrayElementPtr + orderArrayLengthInBytes;
+        for(uint orderPtrOffset = orderArrayElementPtr; orderPtrOffset < orderArrayElementEndPtr; orderPtrOffset += 32) {
+            uint256 orderPtr = uint256(loadExchangeData(orderPtrOffset));
+            address makerAddress = address(loadExchangeData(orderPtr + orderArrayElementPtr));
+            makerAddresses = makerAddresses.append(makerAddress);
+        }
+        return makerAddresses;
+    }
+}
diff --git a/contracts/extensions/contracts/BalanceThresholdFilter/interfaces/IBalanceThresholdFilterCore.sol b/contracts/extensions/contracts/BalanceThresholdFilter/interfaces/IBalanceThresholdFilterCore.sol
new file mode 100644
index 000000000..37e607be1
--- /dev/null
+++ b/contracts/extensions/contracts/BalanceThresholdFilter/interfaces/IBalanceThresholdFilterCore.sol
@@ -0,0 +1,56 @@
+
+/*
+
+  Copyright 2018 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.4.24;
+
+
+
+contract IBalanceThresholdFilterCore {
+    
+    /// @dev Executes an Exchange transaction iff the maker and taker meet 
+    ///      the hold at least `BALANCE_THRESHOLD` of the asset `THRESHOLD_ASSET` OR 
+    ///      the exchange function is a cancellation.
+    ///      Supported Exchange functions:
+    ///         - batchFillOrders
+    ///         - batchFillOrdersNoThrow
+    ///         - batchFillOrKillOrders
+    ///         - fillOrder
+    ///         - fillOrderNoThrow
+    ///         - fillOrKillOrder
+    ///         - marketBuyOrders
+    ///         - marketBuyOrdersNoThrow
+    ///         - marketSellOrders
+    ///         - marketSellOrdersNoThrow
+    ///         - matchOrders
+    ///         - cancelOrder
+    ///         - batchCancelOrders
+    ///         - cancelOrdersUpTo
+    ///     Trying to call any other exchange function will throw.
+    /// @param salt Arbitrary number to ensure uniqueness of transaction hash.
+    /// @param signerAddress Address of transaction signer.
+    /// @param signedExchangeTransaction AbiV2 encoded calldata.
+    /// @param signature Proof of signer transaction by signer.
+    function executeTransaction(
+        uint256 salt,
+        address signerAddress,
+        bytes signedExchangeTransaction,
+        bytes signature
+    ) 
+        external;
+}
\ No newline at end of file
diff --git a/contracts/extensions/contracts/BalanceThresholdFilter/mixins/MBalanceThresholdFilterCore.sol b/contracts/extensions/contracts/BalanceThresholdFilter/mixins/MBalanceThresholdFilterCore.sol
index 6aaa729fb..b8b67e6ee 100644
--- a/contracts/extensions/contracts/BalanceThresholdFilter/mixins/MBalanceThresholdFilterCore.sol
+++ b/contracts/extensions/contracts/BalanceThresholdFilter/mixins/MBalanceThresholdFilterCore.sol
@@ -20,9 +20,12 @@ pragma solidity 0.4.24;
 
 import "@0x/contracts-interfaces/contracts/protocol/Exchange/IExchange.sol";
 import "../interfaces/IThresholdAsset.sol";
+import "../interfaces/IBalanceThresholdFilterCore.sol";
 
 
-contract MBalanceThresholdFilterCore {
+contract MBalanceThresholdFilterCore is
+    IBalanceThresholdFilterCore
+{
 
     // Points to 0x exchange contract
     // solhint-disable var-name-mixedcase
@@ -40,43 +43,12 @@ contract MBalanceThresholdFilterCore {
         address[] addresses
     );
 
-    /// @dev Executes an Exchange transaction iff the maker and taker meet 
-    ///      the hold at least `BALANCE_THRESHOLD` of the asset `THRESHOLD_ASSET` OR 
-    ///      the exchange function is a cancellation.
-    ///      Supported Exchange functions:
-    ///         - batchFillOrders
-    ///         - batchFillOrdersNoThrow
-    ///         - batchFillOrKillOrders
-    ///         - fillOrder
-    ///         - fillOrderNoThrow
-    ///         - fillOrKillOrder
-    ///         - marketBuyOrders
-    ///         - marketBuyOrdersNoThrow
-    ///         - marketSellOrders
-    ///         - marketSellOrdersNoThrow
-    ///         - matchOrders
-    ///         - cancelOrder
-    ///         - batchCancelOrders
-    ///         - cancelOrdersUpTo
-    ///     Trying to call any other exchange function will throw.
-    /// @param salt Arbitrary number to ensure uniqueness of transaction hash.
+    /// @dev Constructs an array of addresses to be validated.
+    ///      Addresses depend on which Exchange function is to be called
+    ///      (defined by `signedExchangeTransaction` above).
     /// @param signerAddress Address of transaction signer.
-    /// @param signedExchangeTransaction AbiV2 encoded calldata.
-    /// @param signature Proof of signer transaction by signer.
-    function executeTransaction(
-        uint256 salt,
-        address signerAddress,
-        bytes signedExchangeTransaction,
-        bytes signature
-    ) 
-        external;
-
-    /// @dev Validates addresses meet the balance threshold specified by `BALANCE_THRESHOLD`
-    ///      for the asset `THRESHOLD_ASSET`. If one address does not meet the thresold
-    ///      then this function will revert. Which addresses are validated depends on
-    ///      which Exchange function is to be called (defined by `signedExchangeTransaction` above).
-    ///      No parameters are taken as this function reads arguments directly from calldata, to save gas.
-    ///      If all addresses are valid then this function emits a ValidatedAddresses event, listing all
-    ///      of the addresses whose balance thresholds it checked.
-    function validateBalanceThresholdsOrRevert(address signerAddress) internal;
+    /// @return addressesToValidate Array of addresses to validate.
+    function getAddressesToValidate(address signerAddress)
+        internal pure
+        returns (address[] memory addressesToValidate);
 }
diff --git a/contracts/extensions/contracts/BalanceThresholdFilter/mixins/MExchangeCalldata.sol b/contracts/extensions/contracts/BalanceThresholdFilter/mixins/MExchangeCalldata.sol
new file mode 100644
index 000000000..8e0414c17
--- /dev/null
+++ b/contracts/extensions/contracts/BalanceThresholdFilter/mixins/MExchangeCalldata.sol
@@ -0,0 +1,57 @@
+    
+    /*
+
+  Copyright 2018 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/LICENSE2.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.4.24;
+
+
+contract MExchangeCalldata {
+
+    /// @dev Emulates the `calldataload` opcode on the embedded Exchange calldata,
+    ///      which is accessed through `signedExchangeTransaction`.
+    /// @param offset  Offset into the Exchange calldata.
+    /// @return value  Corresponding 32 byte value stored at `offset`.
+    function exchangeCalldataload(uint256 offset)
+        internal pure
+        returns (bytes32 value);
+
+    /// @dev Convenience function that skips the 4 byte selector when loading
+    ///      from the embedded Exchange calldata.
+    /// @param offset  Offset into the Exchange calldata (minus the 4 byte selector)
+    /// @return value  Corresponding 32 byte value stored at `offset` + 4.
+    function loadExchangeData(uint256 offset)
+        internal pure
+        returns (bytes32 value);
+
+    /// @dev Extracts the maker address from an order stored in the Exchange calldata
+    ///      (which is embedded in `signedExchangeTransaction`).
+    /// @param orderParamIndex  Index of the order in the Exchange function's signature.
+    /// @return makerAddress The extracted maker address.
+    function loadMakerAddressFromOrder(uint256 orderParamIndex)
+        internal pure
+        returns (address makerAddress);
+
+    /// @dev Extracts the maker addresses from an array of orders stored in the Exchange calldata
+    ///      (which is embedded in `signedExchangeTransaction`), and records them in
+    ///      the running list of addresses to validate.
+    /// @param orderArrayParamIndex  Index of the order array in the Exchange function's signature
+    /// @return makerAddresses The extracted maker addresses.
+    function loadMakerAddressesFromOrderArray(uint256 orderArrayParamIndex)
+        internal pure
+        returns (address[] makerAddresses);
+}