Signature generalisation

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()
     {

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol → packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol

@@ -18,28 +18,11 @@
 
 pragma solidity ^0.4.19;
 
-import "./mixins/MSignatureValidator.sol";
-
-/// @dev Provides MSignatureValidator
-contract MixinSignatureValidatorEcrecover is
-    MSignatureValidator
-{
+contract ISigner {
+
     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;
-    }
+        bytes signature)
+        public view
+        returns (bool isValid);
 }

packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol

@@ -19,7 +19,22 @@
 pragma solidity ^0.4.19;
 
 contract LibOrder {
-
+
+    bytes32 constant orderSchemaHash = keccak256(
+        "address exchangeContractAddress",
+        "address makerAddress",
+        "address takerAddress",
+        "address makerTokenAddress",
+        "address takerTokenAddress",
+        "address feeRecipientAddress",
+        "uint256 makerTokenAmount",
+        "uint256 takerTokenAmount",
+        "uint256 makerFeeAmount",
+        "uint256 takerFeeAmount",
+        "uint256 expirationTimestamp",
+        "uint256 salt"
+    );
+
     struct Order {
         address maker;
         address taker;

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,40 @@ contract MixinExchangeCore is
             expirationTimestampInSec: orderValues[4],
             orderHash: getOrderHash(orderAddresses, orderValues)
         });
+
+        // Validate order and maker only if first time seen
+        // TODO: Read filled and cancelled only once
+        if (filled[order.orderHash] == 0 && cancelled[order.orderHash] == 0) {
+            require(order.makerTokenAmount > 0);
+            require(order.takerTokenAmount > 0);
+            require(isValidSignature(
+                keccak256(orderSchemaHash, order.orderHash),
+                order.maker,
+                signature
+            ));
+        }
+
+        // Validate taker
+        if (order.taker != address(0)) {
+            require(order.taker == msg.sender);
+        }
+        require(takerTokenFillAmount > 0);
 
-        require(order.taker == address(0) || order.taker == msg.sender);
-        require(order.makerTokenAmount > 0 && order.takerTokenAmount > 0 && takerTokenFillAmount > 0);
-        require(isValidSignature(
-            order.maker,
-            order.orderHash,
-            v,
-            r,
-            s
-        ));
-
+        // 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;
@@ -177,8 +184,10 @@ contract MixinExchangeCore is
             orderHash: getOrderHash(orderAddresses, orderValues)
         });
 
+        require(order.makerTokenAmount > 0);
+        require(order.takerTokenAmount > 0);
+        require(takerTokenCancelAmount > 0);
         require(order.maker == msg.sender);
-        require(order.makerTokenAmount > 0 && order.takerTokenAmount > 0 && takerTokenCancelAmount > 0);
 
         if (block.timestamp >= order.expirationTimestampInSec) {
             LogError(uint8(Errors.ORDER_EXPIRED), order.orderHash);

packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol

@@ -0,0 +1,163 @@
+/*
+
+  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";
+import "./ISigner.sol";
+
+/// @dev Provides MSignatureValidator
+contract MixinSignatureValidator is
+    MSignatureValidator
+{
+    enum SignatureType {
+        Illegal, // Default value
+        Invalid,
+        Caller,
+        Ecrecover,
+        EIP712,
+        Trezor,
+        Contract
+    }
+
+    function isValidSignature(
+        bytes32 hash,
+        address signer,
+        bytes signature)
+        public view
+        returns (bool isValid)
+    {
+        // TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.)
+
+        require(signature.length >= 1);
+        SignatureType signatureType = SignatureType(uint8(signature[0]));
+
+        // Variables are not scoped in Solidity
+        uint8 v;
+        bytes32 r;
+        bytes32 s;
+        address recovered;
+
+        // Always illegal signature
+        // This is always an implicit option since a signer can create a
+        // signature array with invalid type or length. We may as well make
+        // it an explicit option. This aids testing and analysis. It is
+        // also the initialization value for the enum type.
+        if (signatureType == SignatureType.Illegal) {
+            revert();
+
+        // Always invalid signature
+        // Like Illegal, this is always implicitly available and therefore
+        // offered explicitly. It can be implicitly created by providing
+        // a correctly formatted but incorrect signature.
+        } else if (signatureType == SignatureType.Invalid) {
+            require(signature.length == 1);
+            isValid = false;
+            return isValid;
+
+        // Implicitly signed by caller
+        // The signer has initiated the call. In the case of non-contract
+        // accounts it means the transaction itself was signed.
+        // Example: let's say for a particular operation three signatures
+        // A, B and C are required. To submit the transaction, A and B can
+        // give a signature to C, who can then submit the transaction using
+        // `Caller` for his own signature. Or A and C can sign and B can
+        // submit using `Caller`. Having `Caller` allows this flexibility.
+        } else if (signatureType == SignatureType.Caller) {
+            require(signature.length == 1);
+            isValid = signer == msg.sender;
+            return isValid;
+
+        // Signed using web3.eth_sign
+        } else if (signatureType == SignatureType.Ecrecover) {
+            require(signature.length == 66);
+            v = uint8(signature[1]);
+            r = get32(signature, 2);
+            s = get32(signature, 34);
+            recovered = ecrecover(
+                keccak256("\x19Ethereum Signed Message:\n32", hash),
+                v,
+                r,
+                s
+            );
+            isValid = signer == recovered;
+            return isValid;
+
+        // Signature using EIP712
+        } else if (signatureType == SignatureType.EIP712) {
+            require(signature.length == 66);
+            v = uint8(signature[1]);
+            r = get32(signature, 2);
+            s = get32(signature, 34);
+            recovered = ecrecover(hash, v, r, s);
+            isValid = signer == recovered;
+            return isValid;
+
+        // Signature from Trezor hardware wallet
+        // It differs from web3.eth_sign in the encoding of message length
+        // (Bitcoin varint encoding vs ascii-decimal, the latter is not
+        // self-terminating which leads to ambiguities).
+        // See also:
+        // https://en.bitcoin.it/wiki/Protocol_documentation#Variable_length_integer
+        // https://github.com/trezor/trezor-mcu/blob/master/firmware/ethereum.c#L602
+        // https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36
+        } else if (signatureType == SignatureType.Trezor) {
+            require(signature.length == 66);
+            v = uint8(signature[1]);
+            r = get32(signature, 2);
+            s = get32(signature, 34);
+            recovered = ecrecover(
+                keccak256("\x19Ethereum Signed Message:\n\x41", hash),
+                v,
+                r,
+                s
+            );
+            isValid = signer == recovered;
+            return isValid;
+
+        // Signature verified by signer contract
+        } else if (signatureType == SignatureType.Contract) {
+            isValid = ISigner(signer).isValidSignature(hash, signature);
+            return isValid;
+        }
+
+        // Anything else is illegal (We do not return false because
+        // the signature may actually be valid, just not in a format
+        // that we currently support. In this case returning false
+        // may lead the caller to incorrectly believe that the
+        // signature was invalid.)
+        revert();
+    }
+
+    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))
+        }
+        return result;
+    }
+
+}