Abstract signature to opaque bytearray

7 files changed, 148 insertions, 161 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol
index 5c513341b..e46b0437f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol
@@ -17,15 +17,16 @@
 */
 
 pragma solidity ^0.4.19;
+pragma experimental ABIEncoderV2;
 
 import "./MixinExchangeCore.sol";
-import "./MixinSignatureValidatorEcrecover.sol";
+import "./MixinSignatureValidator.sol";
 import "./MixinSettlementProxy.sol";
 import "./MixinWrapperFunctions.sol";
 
 contract Exchange is
     MixinExchangeCore,
-    MixinSignatureValidatorEcrecover,
+    MixinSignatureValidator,
     MixinSettlementProxy,
     MixinWrapperFunctions
 {
@@ -37,7 +38,7 @@ contract Exchange is
     )
         public
         MixinExchangeCore()
-        MixinSignatureValidatorEcrecover()
+        MixinSignatureValidator()
         MixinSettlementProxy(_tokenTransferProxy, _zrxToken)
         MixinWrapperFunctions()
     {
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index c61006374..083491a7a 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -73,19 +73,15 @@ contract MixinExchangeCore is
     /// @param orderAddresses Array of order's maker, taker, makerToken, takerToken, and feeRecipient.
     /// @param orderValues Array of order's makerTokenAmount, takerTokenAmount, makerFee, takerFee, expirationTimestampInSec, and salt.
     /// @param takerTokenFillAmount Desired amount of takerToken to fill.
-    /// @param v ECDSA signature parameter v.
-    /// @param r ECDSA signature parameters r.
-    /// @param s ECDSA signature parameters s.
+    /// @param signature Proof of signing order by maker.
     /// @return Total amount of takerToken filled in trade.
     function fillOrder(
-          address[5] orderAddresses,
-          uint256[6] orderValues,
-          uint256 takerTokenFillAmount,
-          uint8 v,
-          bytes32 r,
-          bytes32 s)
-          public
-          returns (uint256 takerTokenFilledAmount)
+        address[5] orderAddresses,
+        uint[6] orderValues,
+        uint takerTokenFillAmount,
+        bytes signature)
+        public
+        returns (uint256 takerTokenFilledAmount)
     {
         Order memory order = Order({
             maker: orderAddresses[0],
@@ -100,29 +96,37 @@ contract MixinExchangeCore is
             expirationTimestampInSec: orderValues[4],
             orderHash: getOrderHash(orderAddresses, orderValues)
         });
-
-        require(order.taker == address(0) || order.taker == msg.sender);
-        require(order.makerTokenAmount > 0 && order.takerTokenAmount > 0 && takerTokenFillAmount > 0);
+        
+        // Validate maker
+        require(order.makerTokenAmount > 0);
+        require(order.takerTokenAmount > 0);
         require(isValidSignature(
-            order.maker,
             order.orderHash,
-            v,
-            r,
-            s
+            order.maker,
+            signature
         ));
+        
+        // Validate taker
+        if (order.taker != address(0)) {
+            require(order.taker == msg.sender);
+        }
+        require(takerTokenFillAmount > 0);
 
+        // Validate order expiration
         if (block.timestamp >= order.expirationTimestampInSec) {
             LogError(uint8(Errors.ORDER_EXPIRED), order.orderHash);
             return 0;
         }
-
+        
+        // Validate order availability
         uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(order.orderHash));
         takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenAmount);
         if (takerTokenFilledAmount == 0) {
             LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), order.orderHash);
             return 0;
         }
-
+        
+        // Validate fill order rounding
         if (isRoundingError(takerTokenFilledAmount, order.takerTokenAmount, order.makerTokenAmount)) {
             LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), order.orderHash);
             return 0;
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
new file mode 100644
index 000000000..919dbe312
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -0,0 +1,77 @@
+/*
+
+  Copyright 2017 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.19;
+
+import "./mixins/MSignatureValidator.sol";
+
+/// @dev Provides MSignatureValidator
+contract MixinSignatureValidator is
+    MSignatureValidator
+{
+    enum SignatureType {
+        Invalid,
+        Ecrecover
+    }
+  
+    function isValidSignature(
+        bytes32 hash,
+        address signer,
+        bytes signature)
+        public view
+        returns (bool isValid)
+    {
+        require(signature.length >= 1);
+        
+        // Select signature type
+        SignatureType signatureType = SignatureType(uint8(signature[0]));
+        if (signatureType != SignatureType.Ecrecover) {
+            valid = false;
+            return;
+        }
+        
+        // Verify using ecrecover
+        require(signature.length == 66);
+        uint8 v = uint8(signature[1]);
+        bytes32 r = get32(signature, 2);
+        bytes32 s = get32(signature, 34);
+        address recovered = ecrecover(
+            keccak256("\x19Ethereum Signed Message:\n32", hash),
+            v,
+            r,
+            s
+        );
+        isValid = signer == recovered;
+    }
+    
+    function get32(bytes b, uint256 index)
+        private pure
+        returns (bytes32 result)
+    {
+        require(b.length >= index + 32);
+        
+        // Arrays are prefixed by a 256 bit length parameter
+        index += 32;
+        
+        // Read the bytes32 from array memory
+        assembly {
+            result := mload(add(b, index))
+        }
+    }
+
+}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol
deleted file mode 100644
index 7c9155921..000000000
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
-
-  Copyright 2017 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.19;
-
-import "./mixins/MSignatureValidator.sol";
-
-/// @dev Provides MSignatureValidator
-contract MixinSignatureValidatorEcrecover is
-    MSignatureValidator
-{
-    function isValidSignature(
-        address signer,
-        bytes32 hash,
-        uint8 v,
-        bytes32 r,
-        bytes32 s)
-        public
-        constant
-        returns (bool isValid)
-    {
-        isValid = signer == ecrecover(
-            keccak256("\x19Ethereum Signed Message:\n32", hash),
-            v,
-            r,
-            s
-        );
-        return isValid;
-    }
-}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
index e09d93851..81a82eeec 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
@@ -17,6 +17,7 @@
 */
 
 pragma solidity ^0.4.19;
+pragma experimental ABIEncoderV2;
 
 import './mixins/MExchangeCore.sol';
 import "../../utils/SafeMath/SafeMath.sol";
@@ -27,29 +28,18 @@ contract MixinWrapperFunctions is
     SafeMath
 {
   
-    /// @dev Fills an order with specified parameters and ECDSA signature. Throws if specified amount not filled entirely.
-    /// @param orderAddresses Array of order's maker, taker, makerToken, takerToken, and feeRecipient.
-    /// @param orderValues Array of order's makerTokenAmount, takerTokenAmount, makerFee, takerFee, expirationTimestampInSec, and salt.
-    /// @param takerTokenFillAmount Desired amount of takerToken to fill.
-    /// @param v ECDSA signature parameter v.
-    /// @param r ECDSA signature parameters r.
-    /// @param s ECDSA signature parameters s.
     function fillOrKillOrder(
         address[5] orderAddresses,
-        uint256[6] orderValues,
-        uint256 takerTokenFillAmount,
-        uint8 v,
-        bytes32 r,
-        bytes32 s)
+        uint[6] orderValues,
+        uint takerTokenFillAmount,
+        bytes signature)
         public
     {
         require(fillOrder(
             orderAddresses,
             orderValues,
             takerTokenFillAmount,
-            v,
-            r,
-            s
+            signature
         ) == takerTokenFillAmount);
     }
 
@@ -57,18 +47,13 @@ contract MixinWrapperFunctions is
     /// @param orderAddresses Array of order's maker, taker, makerToken, takerToken, and feeRecipient.
     /// @param orderValues Array of order's makerTokenAmount, takerTokenAmount, makerFee, takerFee, expirationTimestampInSec, and salt.
     /// @param takerTokenFillAmount Desired amount of takerToken to fill.
-    /// @param v ECDSA signature parameter v.
-    /// @param r ECDSA signature parameters r.
-    /// @param s ECDSA signature parameters s.
     /// @return Success if the transaction did not revert.
     /// @return Total amount of takerToken filled in trade.
     function fillOrderNoThrow(
         address[5] orderAddresses,
         uint256[6] orderValues,
         uint256 takerTokenFillAmount,
-        uint8 v,
-        bytes32 r,
-        bytes32 s)
+        bytes signature)
         public
         returns (bool success, uint256 takerTokenFilledAmount)
     {
@@ -91,9 +76,16 @@ contract MixinWrapperFunctions is
             mstore(add(x, 292), add(orderValues, 160))     // expirationTimestampInSec
             mstore(add(x, 324), add(orderValues, 192))     // salt
             mstore(add(x, 356), takerTokenFillAmount)
-            mstore(add(x, 388), v)
-            mstore(add(x, 420), r)
-            mstore(add(x, 452), s)
+            for {
+                let src := signature
+                let dst := add(x, 388)
+                let end := add(add(src, mload(signature)), 32)
+            } lt(src, end) { 
+                src := add(src, 32)
+                dst := add(dst, 32)
+            } {
+                mstore(dst, mload(src))
+            }
 
             success := delegatecall(
                 gas,      // TODO: don't send all gas, save some for returning is case of throw
@@ -113,16 +105,11 @@ contract MixinWrapperFunctions is
     /// @param orderAddresses Array of address arrays containing individual order addresses.
     /// @param orderValues Array of uint256 arrays containing individual order values.
     /// @param takerTokenFillAmounts Array of desired amounts of takerToken to fill in orders.
-    /// @param v Array ECDSA signature v parameters.
-    /// @param r Array of ECDSA signature r parameters.
-    /// @param s Array of ECDSA signature s parameters.
     function batchFillOrders(
         address[5][] orderAddresses,
         uint256[6][] orderValues,
         uint256[] takerTokenFillAmounts,
-        uint8[] v,
-        bytes32[] r,
-        bytes32[] s)
+        bytes[] signatures)
         external
     {
         for (uint256 i = 0; i < orderAddresses.length; i++) {
@@ -130,9 +117,7 @@ contract MixinWrapperFunctions is
                 orderAddresses[i],
                 orderValues[i],
                 takerTokenFillAmounts[i],
-                v[i],
-                r[i],
-                s[i]
+                signatures[i]
             );
         }
     }
@@ -141,54 +126,38 @@ contract MixinWrapperFunctions is
     /// @param orderAddresses Array of address arrays containing individual order addresses.
     /// @param orderValues Array of uint256 arrays containing individual order values.
     /// @param takerTokenFillAmounts Array of desired amounts of takerToken to fill in orders.
-    /// @param v Array ECDSA signature v parameters.
-    /// @param r Array of ECDSA signature r parameters.
-    /// @param s Array of ECDSA signature s parameters.
     function batchFillOrKillOrders(
         address[5][] orderAddresses,
-        uint256[6][] orderValues,
-        uint256[] takerTokenFillAmounts,
-        uint8[] v,
-        bytes32[] r,
-        bytes32[] s)
-        external
+        uint[6][] orderValues,
+        uint[] takerTokenFillAmounts,
+        bytes[] signatures)
+        public /// Compiler crash when set to external
     {
         for (uint256 i = 0; i < orderAddresses.length; i++) {
             fillOrKillOrder(
                 orderAddresses[i],
                 orderValues[i],
                 takerTokenFillAmounts[i],
-                v[i],
-                r[i],
-                s[i]
+                signatures[i]
             );
         }
     }
 
-    /// @dev Synchronously executes multiple calls of fillOrderNoThrow in a single transaction.
-    /// @param orderAddresses Array of address arrays containing individual order addresses.
-    /// @param orderValues Array of uint256 arrays containing individual order values.
-    /// @param takerTokenFillAmounts Array of desired amounts of takerToken to fill in orders.
-    /// @param v Array ECDSA signature v parameters.
-    /// @param r Array of ECDSA signature r parameters.
-    /// @param s Array of ECDSA signature s parameters.
-    function batchFillOrdersNoThrow(
+
+    function fillOrdersUpTo(
         address[5][] orderAddresses,
-        uint256[6][] orderValues,
-        uint256[] takerTokenFillAmounts,
-        uint8[] v,
-        bytes32[] r,
-        bytes32[] s)
-        external
+        uint[6][] orderValues,
+        uint[] takerTokenFillAmounts,
+        bytes[] signatures)
+        public /// Stack to deep when set to external
+        returns (uint)
     {
         for (uint256 i = 0; i < orderAddresses.length; i++) {
             fillOrderNoThrow(
                 orderAddresses[i],
                 orderValues[i],
                 takerTokenFillAmounts[i],
-                v[i],
-                r[i],
-                s[i]
+                signatures[i]
             );
         }
     }
@@ -197,17 +166,12 @@ contract MixinWrapperFunctions is
     /// @param orderAddresses Array of address arrays containing individual order addresses.
     /// @param orderValues Array of uint256 arrays containing individual order values.
     /// @param takerTokenFillAmount Desired total amount of takerToken to fill in orders.
-    /// @param v Array ECDSA signature v parameters.
-    /// @param r Array of ECDSA signature r parameters.
-    /// @param s Array of ECDSA signature s parameters.
     /// @return Total amount of takerTokenFillAmount filled in orders.
     function marketFillOrders(
         address[5][] orderAddresses,
         uint256[6][] orderValues,
         uint256 takerTokenFillAmount,
-        uint8[] v,
-        bytes32[] r,
-        bytes32[] s)
+        bytes[] signatures)
         external
         returns (uint256 totalTakerTokenFilledAmount)
     {
@@ -217,9 +181,7 @@ contract MixinWrapperFunctions is
                 orderAddresses[i],
                 orderValues[i],
                 safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount),
-                v[i],
-                r[i],
-                s[i]
+                signatures[i]
             ));
             if (totalTakerTokenFilledAmount == takerTokenFillAmount) break;
         }
@@ -230,17 +192,12 @@ contract MixinWrapperFunctions is
     /// @param orderAddresses Array of address arrays containing individual order addresses.
     /// @param orderValues Array of uint256 arrays containing individual order values.
     /// @param takerTokenFillAmount Desired total amount of takerToken to fill in orders.
-    /// @param v Array ECDSA signature v parameters.
-    /// @param r Array of ECDSA signature r parameters.
-    /// @param s Array of ECDSA signature s parameters.
     /// @return Total amount of takerTokenFillAmount filled in orders.
     function marketFillOrdersNoThrow(
         address[5][] orderAddresses,
         uint256[6][] orderValues,
         uint256 takerTokenFillAmount,
-        uint8[] v,
-        bytes32[] r,
-        bytes32[] s)
+        bytes[] signatures)
         external
         returns (uint256 totalTakerTokenFilledAmount)
     {
@@ -250,9 +207,7 @@ contract MixinWrapperFunctions is
                 orderAddresses[i],
                 orderValues[i],
                 safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount),
-                v[i],
-                r[i],
-                s[i]
+                signatures[i]
             );
             totalTakerTokenFilledAmount = safeAdd(totalTakerTokenFilledAmount, takerTokenFilledAmount);
             if (totalTakerTokenFilledAmount == takerTokenFillAmount) break;
@@ -278,4 +233,5 @@ contract MixinWrapperFunctions is
             );
         }
     }
+    
 }
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 3977c3bbd..b7901e16b 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -24,9 +24,7 @@ contract MExchangeCore {
           address[5] orderAddresses,
           uint256[6] orderValues,
           uint256 takerTokenFillAmount,
-          uint8 v,
-          bytes32 r,
-          bytes32 s)
+          bytes signature)
           public
           returns (uint256 takerTokenFilledAmount);
 
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 b4ca4bb48..2d21a6c86 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -20,19 +20,15 @@ pragma solidity ^0.4.19;
 
 contract MSignatureValidator {
 
-    /// @dev Verifies that an order signature is valid.
-    /// @param signer address of signer.
-    /// @param hash Signed Keccak-256 hash.
-    /// @param v ECDSA signature parameter v.
-    /// @param r ECDSA signature parameters r.
-    /// @param s ECDSA signature parameters s.
+    /// @dev Verifies that a signature is valid.
+    /// @param digest Message digest that is signed.
+    /// @param signer Address of signer.
+    /// @param signature Proof of signing.
     /// @return Validity of order signature.
     function isValidSignature(
+        bytes32 digest,
         address signer,
-        bytes32 hash,
-        uint8 v,
-        bytes32 r,
-        bytes32 s)
+        bytes signature)
         public view
         returns (bool isValid);
 }