From a5a7217c8f24fe98275fd9927ca8c5e5f140c6f3 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 9 May 2018 16:27:50 -0700
Subject: Add deepCopyBytes method to LibBytes

---
 .../current/test/TestLibBytes/TestLibBytes.sol     | 17 +++++++++
 .../contracts/current/utils/LibBytes/LibBytes.sol  | 41 ++++++++++++++++++++++
 2 files changed, 58 insertions(+)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
index 5a6801262..b3ed1f620 100644
--- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
+++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
@@ -25,6 +25,23 @@ contract TestLibBytes is
     LibBytes
 {
 
+    /// @dev Performs a deep copy of a section of a byte array.
+    /// @param b Byte array that will be sliced.
+    /// @param startIndex Index of first byte to copy.
+    /// @param endIndex Index of last byte to copy + 1.
+    /// @return A deep copy of b from startIndex to endIndex.
+    function publicDeepCopyBytes(
+        bytes memory b,
+        uint256 startIndex,
+        uint256 endIndex)
+        public
+        pure
+        returns (bytes memory copy)
+    {
+        copy = deepCopyBytes(b, startIndex, endIndex);
+        return copy;
+    }
+
     /// @dev Tests equality of two byte arrays.
     /// @param lhs First byte array to compare.
     /// @param rhs Second byte array to compare.
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index 975565773..527f7a7a9 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -21,9 +21,50 @@ pragma solidity ^0.4.24;
 contract LibBytes {
 
     // Revert reasons
+    string constant GT_ZERO_LENGTH_REQUIRED = "Length must be greater than 0.";
     string constant GTE_4_LENGTH_REQUIRED = "Length must be greater than or equal to 4.";
     string constant GTE_20_LENGTH_REQUIRED = "Length must be greater than or equal to 20.";
     string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32.";
+    string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds.";
+
+    /// @dev Performs a deep copy of a section of a byte array.
+    /// @param b Byte array that will be copied.
+    /// @param index Index of first byte to copy.
+    /// @param len Number of bytes to copy starting from index.
+    /// @return A deep copy `len` bytes of b starting from index.
+    function deepCopyBytes(
+        bytes memory b,
+        uint256 index,
+        uint256 len)
+        internal
+        pure
+        returns (bytes memory)
+    {
+        require(
+            len > 0,
+            GT_ZERO_LENGTH_REQUIRED
+        );
+        require(
+            b.length >= index + len,
+            INDEX_OUT_OF_BOUNDS
+        );
+
+        bytes memory copy = new bytes(len);
+
+        // Arrays are prefixed by a 256 bit length parameter
+        index += 32;
+
+        assembly {
+            // Start storing onto copy after length field
+            let storeStartIndex := add(copy, 32)
+            // Start loading from b at index
+            let startLoadIndex := add(b, index)
+            for {let i := 0} lt(i, len) {i := add(i, 32)} {
+                mstore(add(storeStartIndex, i), mload(add(startLoadIndex, i)))
+            }
+        }
+        return copy;
+    }
 
     /// @dev Tests equality of two byte arrays.
     /// @param lhs First byte array to compare.
-- 
cgit v1.2.3


From b587f076fe02a21274c91d35b6a18a7ba86e3b58 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 9 May 2018 16:42:25 -0700
Subject: Add Validator signature type

---
 .../protocol/Exchange/MixinSignatureValidator.sol  | 34 +++++++++++++++++++--
 .../protocol/Exchange/interfaces/IValidator.sol    | 35 ++++++++++++++++++++++
 .../Exchange/mixins/MSignatureValidator.sol        |  1 +
 3 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index f7fcd36b6..2a54da90f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -20,6 +20,7 @@ pragma solidity ^0.4.24;
 
 import "./mixins/MSignatureValidator.sol";
 import "./interfaces/ISigner.sol";
+import "./interfaces/IValidator.sol";
 import "./libs/LibExchangeErrors.sol";
 import "../../utils/LibBytes/LibBytes.sol";
 
@@ -169,9 +170,38 @@ contract MixinSignatureValidator is
             isValid = signer == recovered;
             return isValid;
 
-        // Signature verified by signer contract
+        // Signature verified by signer contract.
+        // If used with an order, the maker of the order is the signer contract.
         } else if (signatureType == SignatureType.Contract) {
-            isValid = ISigner(signer).isValidSignature(hash, signature);
+            // Pass in signature without signature type.
+            bytes memory signatureWithoutType = deepCopyBytes(
+                signature,
+                1,
+                signature.length - 1
+            );
+            isValid = ISigner(signer).isValidSignature(hash, signatureWithoutType);
+            return isValid;
+
+        // Signature verified by validator contract.
+        // If used with an order, the maker of the order can still be an EOA.
+        // A signature using this type should be encoded as:
+        // | Offset | Length | Contents                        |
+        // | 0x00   | 1      | Signature type is always "\x07" |
+        // | 0x01   | 20     | Address of validator contract   |
+        // | 0x15   | **     | Signature to validate           |
+        } else if (signatureType == SignatureType.Validator) {
+            address validator = readAddress(signature, 1);
+            // Pass in signature without type or validator address.
+            bytes memory signatureWithoutTypeOrAddress = deepCopyBytes(
+                signature,
+                21,
+                signature.length - 21
+            );
+            isValid = IValidator(validator).isValidSignature(
+                hash,
+                signer,
+                signatureWithoutTypeOrAddress
+            );
             return isValid;
 
         // Signer signed hash previously using the preSign function
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
new file mode 100644
index 000000000..aed725066
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
@@ -0,0 +1,35 @@
+/*
+
+  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.23;
+
+contract IValidator {
+
+    /// @dev Verifies that a signature is valid.
+    /// @param hash Message hash that is signed.
+    /// @param signer Address that should have signed the given hash.
+    /// @param signature Proof of signing.
+    /// @return Validity of order signature.
+    function isValidSignature(
+        bytes32 hash,
+        address signer,
+        bytes signature)
+        external
+        view
+        returns (bool isValid);
+}
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 3658e7c6f..083fa89d7 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -32,6 +32,7 @@ contract MSignatureValidator is
         EIP712,
         Trezor,
         Contract,
+        Validator,
         PreSigned
     }
 
-- 
cgit v1.2.3


From 0789c6a3d893eb1b1d9636a2d10e8d495b0aa13b Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Sun, 20 May 2018 20:33:41 -0700
Subject: Add approveValidator function

---
 .../protocol/Exchange/MixinSignatureValidator.sol       | 17 ++++++++++++++++-
 .../protocol/Exchange/mixins/MSignatureValidator.sol    |  2 +-
 2 files changed, 17 insertions(+), 2 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 2a54da90f..76de961bc 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -33,6 +33,9 @@ contract MixinSignatureValidator is
     // Mapping of hash => signer => signed
     mapping(bytes32 => mapping(address => bool)) preSigned;
 
+    // Mapping of signer => validator => approved
+    mapping(address => mapping (address => bool)) allowedValidators;
+
     /// @dev Approves a hash on-chain using any valid signature type.
     ///      After presigning a hash, the preSign signature type will become valid for that hash and signer.
     /// @param signer Address that should have signed the given hash.
@@ -50,6 +53,15 @@ contract MixinSignatureValidator is
         preSigned[hash][signer] = true;
     }
 
+    /// @dev Approves a Validator contract to verify signatures on signer's behalf.
+    /// @param validator Address of Validator contract.
+    /// @param approval Approval or disapproval of  Validator contract.
+    function approveSignatureValidator(address validator, bool approval)
+        external
+    {
+        allowedValidators[msg.sender][validator] = approval;
+    }
+
     /// @dev Verifies that a hash has been signed by the given signer.
     /// @param hash Any 32 byte hash.
     /// @param signer Address that should have signed the given hash.
@@ -172,7 +184,7 @@ contract MixinSignatureValidator is
 
         // Signature verified by signer contract.
         // If used with an order, the maker of the order is the signer contract.
-        } else if (signatureType == SignatureType.Contract) {
+        } else if (signatureType == SignatureType.Signer) {
             // Pass in signature without signature type.
             bytes memory signatureWithoutType = deepCopyBytes(
                 signature,
@@ -191,6 +203,9 @@ contract MixinSignatureValidator is
         // | 0x15   | **     | Signature to validate           |
         } else if (signatureType == SignatureType.Validator) {
             address validator = readAddress(signature, 1);
+            if (!allowedValidators[signer][validator]) {
+                return false;
+            }
             // Pass in signature without type or validator address.
             bytes memory signatureWithoutTypeOrAddress = deepCopyBytes(
                 signature,
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 083fa89d7..33424ff84 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -31,7 +31,7 @@ contract MSignatureValidator is
         Ecrecover,
         EIP712,
         Trezor,
-        Contract,
+        Signer,
         Validator,
         PreSigned
     }
-- 
cgit v1.2.3


From 3eb05b45053748f227bc984458580a33596800d1 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Thu, 26 Apr 2018 12:09:48 -0700
Subject: Add TxOrigin signature type and rearrange order of types

---
 .../protocol/Exchange/MixinSignatureValidator.sol  | 113 +++++++++++----------
 .../Exchange/mixins/MSignatureValidator.sol        |  19 ++--
 packages/contracts/src/utils/types.ts              |   8 +-
 3 files changed, 77 insertions(+), 63 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 76de961bc..1022b1fe7 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -83,13 +83,13 @@ contract MixinSignatureValidator is
         );
         SignatureType signatureType = SignatureType(uint8(signature[0]));
 
-        // Variables are not scoped in Solidity
+        // Variables are not scoped in Solidity.
         uint8 v;
         bytes32 r;
         bytes32 s;
         address recovered;
 
-        // Always illegal signature
+        // 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
@@ -98,7 +98,7 @@ contract MixinSignatureValidator is
             // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051
             revert("Illegal signature type.");
 
-        // Always invalid signature
+        // 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.
@@ -110,20 +110,14 @@ contract MixinSignatureValidator is
             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,
-                INVALID_SIGNATURE_LENGTH
-            );
-            isValid = signer == msg.sender;
+        // 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;
 
         // Signed using web3.eth_sign
@@ -144,42 +138,29 @@ contract MixinSignatureValidator is
             isValid = signer == recovered;
             return isValid;
 
-        // Signature using EIP712
-        } else if (signatureType == SignatureType.EIP712) {
-            require(
-                signature.length == 66,
-                INVALID_SIGNATURE_LENGTH
-            );
-            v = uint8(signature[1]);
-            r = readBytes32(signature, 2);
-            s = readBytes32(signature, 34);
-            recovered = ecrecover(hash, v, r, s);
-            isValid = signer == recovered;
+        // Implicitly signed by signer of Ethereum transaction.
+        // This is useful when initiating a call from a contract that might
+        // otherwise require multiple signatures.
+        // Example: Contract A calls Exchange.fillOrder using `executeTransaction`.
+        // This would normally require the user to sign both an Ethereum transaction
+        // and a 0x transaction. By using the TxOrigin signature type, the signature
+        // for the Ethereum transaction will encompass both signatures.
+        } else if (signatureType == SignatureType.TxOrigin) {
+            require(signature.length == 1);
+            isValid = signer == tx.origin;
             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,
-                INVALID_SIGNATURE_LENGTH
-            );
-            v = uint8(signature[1]);
-            r = readBytes32(signature, 2);
-            s = readBytes32(signature, 34);
-            recovered = ecrecover(
-                keccak256("\x19Ethereum Signed Message:\n\x41", hash),
-                v,
-                r,
-                s
-            );
-            isValid = signer == recovered;
+        // 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;
 
         // Signature verified by signer contract.
@@ -219,6 +200,36 @@ contract MixinSignatureValidator is
             );
             return isValid;
 
+        // Signer signed hash previously using the preSign function.
+        } else if (signatureType == SignatureType.PreSigned) {
+            isValid = preSigned[hash][signer];
+            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,
+                INVALID_SIGNATURE_LENGTH
+            );
+            v = uint8(signature[1]);
+            r = readBytes32(signature, 2);
+            s = readBytes32(signature, 34);
+            recovered = ecrecover(
+                keccak256("\x19Ethereum Signed Message:\n\x41", hash),
+                v,
+                r,
+                s
+            );
+            isValid = signer == recovered;
+            return isValid;
+
         // Signer signed hash previously using the preSign function
         } else if (signatureType == SignatureType.PreSigned) {
             isValid = preSigned[hash][signer];
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 33424ff84..a13f4524b 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -25,15 +25,16 @@ contract MSignatureValidator is
 {
     // Allowed signature types.
     enum SignatureType {
-        Illegal,  // Default value
-        Invalid,
-        Caller,
-        Ecrecover,
-        EIP712,
-        Trezor,
-        Signer,
-        Validator,
-        PreSigned
+        Illegal,    // 0x00, default value
+        Invalid,    // 0x01
+        EIP712,     // 0x02
+        Ecrecover,  // 0x03
+        TxOrigin,   // 0x04
+        Caller,     // 0x05
+        Signer,     // 0x06
+        Validator,  // 0x07
+        PreSigned,  // 0x08
+        Trezor      // 0x09
     }
 
     /// @dev Verifies that a signature is valid.
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index 90f90ec27..cd2d128f1 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -116,11 +116,13 @@ export enum ContractName {
 export enum SignatureType {
     Illegal,
     Invalid,
-    Caller,
-    Ecrecover,
     EIP712,
-    Trezor,
+    Ecrecover,
+    TxOrigin,
+    Caller,
     Contract,
+    PreSigned,
+    Trezor,
 }
 
 export interface SignedTransaction {
-- 
cgit v1.2.3


From 87d36f06fd7116afb62b65cdbc013d93a61d1969 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Thu, 26 Apr 2018 12:10:04 -0700
Subject: Add sample whitelist contract

---
 .../contracts/current/test/Whitelist/Whitelist.sol | 58 ++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
new file mode 100644
index 000000000..02158485e
--- /dev/null
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -0,0 +1,58 @@
+/*
+
+  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.21;
+pragma experimental ABIEncoderV2;
+
+import "../../protocol/Exchange/mixins/MTransactions.sol";
+import "../../protocol/Exchange/LibOrder.sol";
+import "../../utils/Ownable/Ownable.sol";
+
+contract Whitelist is Ownable {
+
+    mapping (address => bool) public isWhitelisted;
+    MTransactions EXCHANGE;
+
+    bytes txOriginSignatureType = new bytes(1);
+
+    function Whitelist(address _exchange)
+        public
+    {
+        EXCHANGE = MTransactions(_exchange);
+        txOriginSignatureType[0] = 0x04;
+    }
+
+    function updateWhitelistStatus(address target, bool isApproved)
+        external
+        onlyOwner
+    {
+        isWhitelisted[target] = isApproved;
+    }
+
+    function fillOrderIfWhitelisted(
+        LibOrder.Order memory order,
+        uint256 takerAssetFillAmount,
+        uint256 salt,
+        bytes memory signature)
+        public
+    {
+        require(isWhitelisted[msg.sender]);
+        bytes memory data = abi.encode(order, takerAssetFillAmount, signature);
+        EXCHANGE.executeTransaction(salt, msg.sender, data, txOriginSignatureType);
+    }
+}
\ No newline at end of file
-- 
cgit v1.2.3


From d6be6f79ce944258193d01fa3a23c5210e17ed49 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Thu, 26 Apr 2018 12:55:16 -0700
Subject: Add example whitelist contract and minimum tests

---
 .../contracts/current/test/Whitelist/Whitelist.sol | 40 +++++++++++++++++-----
 packages/contracts/src/utils/types.ts              |  1 +
 2 files changed, 32 insertions(+), 9 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 02158485e..6955b6d96 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -19,22 +19,24 @@
 pragma solidity ^0.4.21;
 pragma experimental ABIEncoderV2;
 
-import "../../protocol/Exchange/mixins/MTransactions.sol";
+import "../../protocol/Exchange/Exchange.sol";
 import "../../protocol/Exchange/LibOrder.sol";
 import "../../utils/Ownable/Ownable.sol";
 
 contract Whitelist is Ownable {
 
     mapping (address => bool) public isWhitelisted;
-    MTransactions EXCHANGE;
+    Exchange EXCHANGE;
 
-    bytes txOriginSignatureType = new bytes(1);
+    bytes txOriginSignature = new bytes(1);
+    bytes4 fillOrderFunctionSelector;
 
     function Whitelist(address _exchange)
         public
     {
-        EXCHANGE = MTransactions(_exchange);
-        txOriginSignatureType[0] = 0x04;
+        EXCHANGE = Exchange(_exchange);
+        txOriginSignature[0] = 0x04;
+        fillOrderFunctionSelector = EXCHANGE.fillOrder.selector;
     }
 
     function updateWhitelistStatus(address target, bool isApproved)
@@ -48,11 +50,31 @@ contract Whitelist is Ownable {
         LibOrder.Order memory order,
         uint256 takerAssetFillAmount,
         uint256 salt,
-        bytes memory signature)
+        bytes memory orderSignature)
         public
     {
-        require(isWhitelisted[msg.sender]);
-        bytes memory data = abi.encode(order, takerAssetFillAmount, signature);
-        EXCHANGE.executeTransaction(salt, msg.sender, data, txOriginSignatureType);
+        address takerAddress = msg.sender;
+    
+        // This contract must be the entry point for the transaction.
+        require(takerAddress == tx.origin);
+
+        // Check if sender is on the whitelist.
+        require(isWhitelisted[takerAddress]);
+
+        // Encode arguments into byte array.
+        bytes memory data = abi.encodeWithSelector(
+            fillOrderFunctionSelector,
+            order,
+            takerAssetFillAmount,
+            orderSignature
+        );
+
+        // Call `fillOrder`via `executeTransaction`.
+        EXCHANGE.executeTransaction(
+            salt,
+            takerAddress,
+            data,
+            txOriginSignature
+        );
     }
 }
\ No newline at end of file
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index cd2d128f1..1eeffc70e 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -111,6 +111,7 @@ export enum ContractName {
     DummyERC721Token = 'DummyERC721Token',
     TestLibBytes = 'TestLibBytes',
     Authorizable = 'Authorizable',
+    Whitelist = 'Whitelist',
 }
 
 export enum SignatureType {
-- 
cgit v1.2.3


From 34ab53173daad302e0d13cbf5369116b373d72d3 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 9 May 2018 13:51:26 -0700
Subject: Fix build

---
 .../protocol/Exchange/MixinSignatureValidator.sol     | 19 ++++++++++++++-----
 .../Exchange/interfaces/IWrapperFunctions.sol         | 14 +++++---------
 .../contracts/current/test/Whitelist/Whitelist.sol    | 10 +++++-----
 3 files changed, 24 insertions(+), 19 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 1022b1fe7..1fb87c148 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -112,10 +112,13 @@ contract MixinSignatureValidator is
 
         // Signature using EIP712
         } else if (signatureType == SignatureType.EIP712) {
-            require(signature.length == 66);
+            require(
+                signature.length == 66,
+                INVALID_SIGNATURE_LENGTH
+            );
             v = uint8(signature[1]);
-            r = get32(signature, 2);
-            s = get32(signature, 34);
+            r = readBytes32(signature, 2);
+            s = readBytes32(signature, 34);
             recovered = ecrecover(hash, v, r, s);
             isValid = signer == recovered;
             return isValid;
@@ -146,7 +149,10 @@ contract MixinSignatureValidator is
         // and a 0x transaction. By using the TxOrigin signature type, the signature
         // for the Ethereum transaction will encompass both signatures.
         } else if (signatureType == SignatureType.TxOrigin) {
-            require(signature.length == 1);
+            require(
+                signature.length == 1,
+                INVALID_SIGNATURE_LENGTH
+            );
             isValid = signer == tx.origin;
             return isValid;
 
@@ -159,7 +165,10 @@ contract MixinSignatureValidator is
         // `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);
+            require(
+                signature.length == 1,
+                INVALID_SIGNATURE_LENGTH
+            );
             isValid = signer == msg.sender;
             return isValid;
 
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol
index 1eb1233ed..8682b394a 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol
@@ -19,27 +19,23 @@
 pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
-import "./libs/LibOrder.sol";
-import "./libs/LibFillResults.sol";
+import "../libs/LibOrder.sol";
+import "../libs/LibFillResults.sol";
 
 contract IWrapperFunctions is
-    LibBytes,
-    LibMath,
     LibOrder,
-    LibFillResults,
-    LibExchangeErrors,
-    MExchangeCore
+    LibFillResults
 {
     /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
     /// @param order LibOrder.Order struct containing order specifications.
     /// @param takerAssetFillAmount Desired amount of takerAsset to sell.
     /// @param signature Proof that order has been created by maker.
     function fillOrKillOrder(
-        LibOrder.LibOrder.Order memory order,
+        LibOrder.Order memory order,
         uint256 takerAssetFillAmount,
         bytes memory signature)
         public
-        returns (LibFillResults.LibFillResults.FillResults memory fillResults);
+        returns (LibFillResults.FillResults memory fillResults);
 
     /// @dev Fills an order with specified parameters and ECDSA signature.
     ///      Returns false if the transaction would otherwise revert.
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 6955b6d96..c53856602 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -16,17 +16,17 @@
 
 */
 
-pragma solidity ^0.4.21;
+pragma solidity ^0.4.23;
 pragma experimental ABIEncoderV2;
 
-import "../../protocol/Exchange/Exchange.sol";
-import "../../protocol/Exchange/LibOrder.sol";
+import "../../protocol/Exchange/interfaces/IExchange.sol";
+import "../../protocol/Exchange/libs/LibOrder.sol";
 import "../../utils/Ownable/Ownable.sol";
 
 contract Whitelist is Ownable {
 
     mapping (address => bool) public isWhitelisted;
-    Exchange EXCHANGE;
+    IExchange EXCHANGE;
 
     bytes txOriginSignature = new bytes(1);
     bytes4 fillOrderFunctionSelector;
@@ -34,7 +34,7 @@ contract Whitelist is Ownable {
     function Whitelist(address _exchange)
         public
     {
-        EXCHANGE = Exchange(_exchange);
+        EXCHANGE = IExchange(_exchange);
         txOriginSignature[0] = 0x04;
         fillOrderFunctionSelector = EXCHANGE.fillOrder.selector;
     }
-- 
cgit v1.2.3


From 4b71c65aea44bba34429e288c5411a090dd78071 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 9 May 2018 14:41:44 -0700
Subject: Update Whitelist contract with comments, also require maker to be
 whitelisted

---
 .../protocol/Exchange/MixinTransactions.sol        |  5 ++-
 .../protocol/Exchange/libs/LibExchangeErrors.sol   |  1 +
 .../contracts/current/test/Whitelist/Whitelist.sol | 47 +++++++++++++++++-----
 packages/contracts/src/utils/artifacts.ts          |  2 +
 4 files changed, 43 insertions(+), 12 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index f93a80705..d28df11d8 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -47,7 +47,10 @@ contract MixinTransactions is
         external
     {
         // Prevent reentrancy
-        require(currentContextAddress == address(0));
+        require(
+            currentContextAddress == address(0),
+            REENTRANCY_NOT_ALLOWED
+        );
 
         // Calculate transaction hash
         bytes32 transactionHash = keccak256(
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
index 4712ee36c..7d67e5080 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
@@ -32,6 +32,7 @@ contract LibExchangeErrors {
     string constant INVALID_ORDER_MAKER_ASSET_AMOUNT = "Invalid order maker asset amount: expected a non-zero value.";
 
     // Transaction revert reasons
+    string constant REENTRANCY_NOT_ALLOWED = "`executeTransaction` is not allowed to call itself recursively.";
     string constant DUPLICATE_TRANSACTION_HASH = "Transaction has already been executed.";
     string constant TRANSACTION_EXECUTION_FAILED = "Transaction execution failed.";
 
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index c53856602..4a2e9a931 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -23,22 +23,30 @@ import "../../protocol/Exchange/interfaces/IExchange.sol";
 import "../../protocol/Exchange/libs/LibOrder.sol";
 import "../../utils/Ownable/Ownable.sol";
 
-contract Whitelist is Ownable {
+contract Whitelist is 
+    Ownable
+{
+    // Revert reasons
+    string constant ADDRESS_NOT_WHITELISTED = "Address not whitelisted.";
 
+    // Mapping of address => whitelist status.
     mapping (address => bool) public isWhitelisted;
+
+    // Exchange contract.
     IExchange EXCHANGE;
 
-    bytes txOriginSignature = new bytes(1);
-    bytes4 fillOrderFunctionSelector;
+    // TxOrigin signature type is the 5th value in enum SignatureType and has a length of 1.
+    bytes constant TX_ORIGIN_SIGNATURE = "\x04";
 
-    function Whitelist(address _exchange)
+    constructor (address _exchange)
         public
     {
         EXCHANGE = IExchange(_exchange);
-        txOriginSignature[0] = 0x04;
-        fillOrderFunctionSelector = EXCHANGE.fillOrder.selector;
     }
 
+    /// @dev Adds or removes an address from the whitelist.
+    /// @param target Address to add or remove from whitelist.
+    /// @param isApproved Whitelist status to assign to address.
     function updateWhitelistStatus(address target, bool isApproved)
         external
         onlyOwner
@@ -46,6 +54,14 @@ contract Whitelist is Ownable {
         isWhitelisted[target] = isApproved;
     }
 
+    /// @dev Fills an order using `msg.sender` as the taker.
+    ///      The transaction will revert if both the maker and taker are not whitelisted.
+    ///      Orders should specify this contract as the `senderAddress` in order to gaurantee
+    ///      that both maker and taker have been whitelisted.
+    /// @param order Order struct containing order specifications.
+    /// @param takerAssetFillAmount Desired amount of takerAsset to sell.
+    /// @param salt Arbitrary value to gaurantee uniqueness of 0x transaction hash.
+    /// @param orderSignature Proof that order has been created by maker.
     function fillOrderIfWhitelisted(
         LibOrder.Order memory order,
         uint256 takerAssetFillAmount,
@@ -58,23 +74,32 @@ contract Whitelist is Ownable {
         // This contract must be the entry point for the transaction.
         require(takerAddress == tx.origin);
 
-        // Check if sender is on the whitelist.
-        require(isWhitelisted[takerAddress]);
+        // Check if maker is on the whitelist
+        require(
+            isWhitelisted[order.makerAddress],
+            ADDRESS_NOT_WHITELISTED
+        );
+
+        // Check if taker is on the whitelist.
+        require(
+            isWhitelisted[takerAddress],
+            ADDRESS_NOT_WHITELISTED
+        );
 
         // Encode arguments into byte array.
         bytes memory data = abi.encodeWithSelector(
-            fillOrderFunctionSelector,
+            EXCHANGE.fillOrder.selector,
             order,
             takerAssetFillAmount,
             orderSignature
         );
 
-        // Call `fillOrder`via `executeTransaction`.
+        // Call `fillOrder` via `executeTransaction`.
         EXCHANGE.executeTransaction(
             salt,
             takerAddress,
             data,
-            txOriginSignature
+            TX_ORIGIN_SIGNATURE
         );
     }
 }
\ No newline at end of file
diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts
index fe74ea072..357c66a0a 100644
--- a/packages/contracts/src/utils/artifacts.ts
+++ b/packages/contracts/src/utils/artifacts.ts
@@ -15,6 +15,7 @@ import * as TestLibs from '../artifacts/TestLibs.json';
 import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json';
 import * as TokenRegistry from '../artifacts/TokenRegistry.json';
 import * as EtherToken from '../artifacts/WETH9.json';
+import * as Whitelist from '../artifacts/Whitelist.json';
 import * as ZRX from '../artifacts/ZRXToken.json';
 
 export const artifacts = {
@@ -33,5 +34,6 @@ export const artifacts = {
     TestLibs: (TestLibs as any) as ContractArtifact,
     TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact,
     TokenRegistry: (TokenRegistry as any) as ContractArtifact,
+    Whitelist: (Whitelist as any) as ContractArtifact,
     ZRX: (ZRX as any) as ContractArtifact,
 };
-- 
cgit v1.2.3


From 6d462fc961da2ba4d5502ad6b71654c1715550d9 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Mon, 21 May 2018 10:04:17 -0700
Subject: Remove TxOrigin signature type, modify whitelist to use Validator
 signature type

---
 .../protocol/Exchange/MixinSignatureValidator.sol  | 21 +++------------------
 .../Exchange/mixins/MSignatureValidator.sol        | 11 +++++------
 .../contracts/current/test/Whitelist/Whitelist.sol | 22 +++++++++++++++++++---
 .../contracts/current/utils/LibBytes/LibBytes.sol  |  4 ----
 packages/contracts/src/utils/constants.ts          |  1 +
 5 files changed, 28 insertions(+), 31 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 1fb87c148..423f306cb 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -31,10 +31,10 @@ contract MixinSignatureValidator is
 {
 
     // Mapping of hash => signer => signed
-    mapping(bytes32 => mapping(address => bool)) preSigned;
+    mapping (bytes32 => mapping (address => bool)) preSigned;
 
     // Mapping of signer => validator => approved
-    mapping(address => mapping (address => bool)) allowedValidators;
+    mapping (address => mapping (address => bool)) allowedValidators;
 
     /// @dev Approves a hash on-chain using any valid signature type.
     ///      After presigning a hash, the preSign signature type will become valid for that hash and signer.
@@ -141,21 +141,6 @@ contract MixinSignatureValidator is
             isValid = signer == recovered;
             return isValid;
 
-        // Implicitly signed by signer of Ethereum transaction.
-        // This is useful when initiating a call from a contract that might
-        // otherwise require multiple signatures.
-        // Example: Contract A calls Exchange.fillOrder using `executeTransaction`.
-        // This would normally require the user to sign both an Ethereum transaction
-        // and a 0x transaction. By using the TxOrigin signature type, the signature
-        // for the Ethereum transaction will encompass both signatures.
-        } else if (signatureType == SignatureType.TxOrigin) {
-            require(
-                signature.length == 1,
-                INVALID_SIGNATURE_LENGTH
-            );
-            isValid = signer == tx.origin;
-            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.
@@ -188,7 +173,7 @@ contract MixinSignatureValidator is
         // If used with an order, the maker of the order can still be an EOA.
         // A signature using this type should be encoded as:
         // | Offset | Length | Contents                        |
-        // | 0x00   | 1      | Signature type is always "\x07" |
+        // | 0x00   | 1      | Signature type is always "\x06" |
         // | 0x01   | 20     | Address of validator contract   |
         // | 0x15   | **     | Signature to validate           |
         } else if (signatureType == SignatureType.Validator) {
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 a13f4524b..57600f508 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -29,12 +29,11 @@ contract MSignatureValidator is
         Invalid,    // 0x01
         EIP712,     // 0x02
         Ecrecover,  // 0x03
-        TxOrigin,   // 0x04
-        Caller,     // 0x05
-        Signer,     // 0x06
-        Validator,  // 0x07
-        PreSigned,  // 0x08
-        Trezor      // 0x09
+        Caller,     // 0x04
+        Signer,     // 0x05
+        Validator,  // 0x06
+        PreSigned,  // 0x07
+        Trezor      // 0x08
     }
 
     /// @dev Verifies that a signature is valid.
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 4a2e9a931..3bd3179e5 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -35,13 +35,14 @@ contract Whitelist is
     // Exchange contract.
     IExchange EXCHANGE;
 
-    // TxOrigin signature type is the 5th value in enum SignatureType and has a length of 1.
-    bytes constant TX_ORIGIN_SIGNATURE = "\x04";
+    byte constant VALIDATOR_SIGNATURE_BYTE = "\x06";
+    bytes TX_ORIGIN_SIGNATURE;
 
     constructor (address _exchange)
         public
     {
         EXCHANGE = IExchange(_exchange);
+        TX_ORIGIN_SIGNATURE = abi.encodePacked(VALIDATOR_SIGNATURE_BYTE, address(this));
     }
 
     /// @dev Adds or removes an address from the whitelist.
@@ -102,4 +103,19 @@ contract Whitelist is
             TX_ORIGIN_SIGNATURE
         );
     }
-}
\ No newline at end of file
+
+    /// @dev Verifies signer is same as signer of current Ethereum transaction.
+    /// @param signer Address that should have signed the given hash.
+    /// @param signature Proof of signing.
+    /// @return Validity of order signature.
+    function isValidSignature(
+        bytes32 hash,
+        address signer,
+        bytes signature)
+        external
+        view
+        returns (bool isValid)
+    {
+        return signer == tx.origin;
+    }
+}
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index 527f7a7a9..5c4fdbb23 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -40,10 +40,6 @@ contract LibBytes {
         pure
         returns (bytes memory)
     {
-        require(
-            len > 0,
-            GT_ZERO_LENGTH_REQUIRED
-        );
         require(
             b.length >= index + len,
             INDEX_OUT_OF_BOUNDS
diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts
index 7a0e26a48..9b0b92545 100644
--- a/packages/contracts/src/utils/constants.ts
+++ b/packages/contracts/src/utils/constants.ts
@@ -28,6 +28,7 @@ export const constants = {
     DUMMY_TOKEN_SYMBOL: '',
     DUMMY_TOKEN_DECIMALS: new BigNumber(18),
     DUMMY_TOKEN_TOTAL_SUPPLY: new BigNumber(0),
+    NULL_BYTES: '0x',
     NUM_DUMMY_ERC20_TO_DEPLOY: 3,
     NUM_DUMMY_ERC721_TO_DEPLOY: 1,
     NUM_ERC721_TOKENS_TO_MINT: 2,
-- 
cgit v1.2.3


From 822e319efea9c862702ddf589eb2344ff02e5bc5 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Mon, 21 May 2018 15:59:58 -0700
Subject: Use last byte of signature as signature type

---
 .../protocol/Exchange/MixinSignatureValidator.sol  | 58 +++++++++-------------
 .../current/test/TestLibBytes/TestLibBytes.sol     | 31 +++++++-----
 .../contracts/current/test/Whitelist/Whitelist.sol |  2 +-
 .../contracts/current/utils/LibBytes/LibBytes.sol  | 54 +++++++++++---------
 packages/contracts/src/utils/signing_utils.ts      |  4 +-
 5 files changed, 75 insertions(+), 74 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 423f306cb..6c018683e 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -81,7 +81,9 @@ contract MixinSignatureValidator is
             signature.length >= 1,
             INVALID_SIGNATURE_LENGTH
         );
-        SignatureType signatureType = SignatureType(uint8(signature[0]));
+
+        // Pop last byte off of 
+        SignatureType signatureType = SignatureType(uint8(popByte(signature)));
 
         // Variables are not scoped in Solidity.
         uint8 v;
@@ -104,7 +106,7 @@ contract MixinSignatureValidator is
         // a correctly formatted but incorrect signature.
         } else if (signatureType == SignatureType.Invalid) {
             require(
-                signature.length == 1,
+                signature.length == 0,
                 INVALID_SIGNATURE_LENGTH
             );
             isValid = false;
@@ -113,12 +115,12 @@ contract MixinSignatureValidator is
         // Signature using EIP712
         } else if (signatureType == SignatureType.EIP712) {
             require(
-                signature.length == 66,
+                signature.length == 65,
                 INVALID_SIGNATURE_LENGTH
             );
-            v = uint8(signature[1]);
-            r = readBytes32(signature, 2);
-            s = readBytes32(signature, 34);
+            v = uint8(signature[0]);
+            r = readBytes32(signature, 1);
+            s = readBytes32(signature, 33);
             recovered = ecrecover(hash, v, r, s);
             isValid = signer == recovered;
             return isValid;
@@ -126,12 +128,12 @@ contract MixinSignatureValidator is
         // Signed using web3.eth_sign
         } else if (signatureType == SignatureType.Ecrecover) {
             require(
-                signature.length == 66,
+                signature.length == 65,
                 INVALID_SIGNATURE_LENGTH
             );
-            v = uint8(signature[1]);
-            r = readBytes32(signature, 2);
-            s = readBytes32(signature, 34);
+            v = uint8(signature[0]);
+            r = readBytes32(signature, 1);
+            s = readBytes32(signature, 33);
             recovered = ecrecover(
                 keccak256("\x19Ethereum Signed Message:\n32", hash),
                 v,
@@ -151,7 +153,7 @@ contract MixinSignatureValidator is
         // submit using `Caller`. Having `Caller` allows this flexibility.
         } else if (signatureType == SignatureType.Caller) {
             require(
-                signature.length == 1,
+                signature.length == 0,
                 INVALID_SIGNATURE_LENGTH
             );
             isValid = signer == msg.sender;
@@ -160,37 +162,25 @@ contract MixinSignatureValidator is
         // Signature verified by signer contract.
         // If used with an order, the maker of the order is the signer contract.
         } else if (signatureType == SignatureType.Signer) {
-            // Pass in signature without signature type.
-            bytes memory signatureWithoutType = deepCopyBytes(
-                signature,
-                1,
-                signature.length - 1
-            );
-            isValid = ISigner(signer).isValidSignature(hash, signatureWithoutType);
+            isValid = ISigner(signer).isValidSignature(hash, signature);
             return isValid;
 
         // Signature verified by validator contract.
         // If used with an order, the maker of the order can still be an EOA.
         // A signature using this type should be encoded as:
-        // | Offset | Length | Contents                        |
-        // | 0x00   | 1      | Signature type is always "\x06" |
-        // | 0x01   | 20     | Address of validator contract   |
-        // | 0x15   | **     | Signature to validate           |
+        // | Offset   | Length | Contents                        |
+        // | 0x00     | x      | Signature to validate           |
+        // | 0x00 + x | 20     | Address of validator contract   |
+        // | 0x14 + x | 1      | Signature type is always "\x06" |
         } else if (signatureType == SignatureType.Validator) {
-            address validator = readAddress(signature, 1);
+            address validator = popAddress(signature);
             if (!allowedValidators[signer][validator]) {
                 return false;
             }
-            // Pass in signature without type or validator address.
-            bytes memory signatureWithoutTypeOrAddress = deepCopyBytes(
-                signature,
-                21,
-                signature.length - 21
-            );
             isValid = IValidator(validator).isValidSignature(
                 hash,
                 signer,
-                signatureWithoutTypeOrAddress
+                signature
             );
             return isValid;
 
@@ -209,12 +199,12 @@ contract MixinSignatureValidator is
         // https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36
         } else if (signatureType == SignatureType.Trezor) {
             require(
-                signature.length == 66,
+                signature.length == 65,
                 INVALID_SIGNATURE_LENGTH
             );
-            v = uint8(signature[1]);
-            r = readBytes32(signature, 2);
-            s = readBytes32(signature, 34);
+            v = uint8(signature[0]);
+            r = readBytes32(signature, 1);
+            s = readBytes32(signature, 33);
             recovered = ecrecover(
                 keccak256("\x19Ethereum Signed Message:\n\x41", hash),
                 v,
diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
index b3ed1f620..cd5206eb8 100644
--- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
+++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
@@ -25,21 +25,26 @@ contract TestLibBytes is
     LibBytes
 {
 
-    /// @dev Performs a deep copy of a section of a byte array.
-    /// @param b Byte array that will be sliced.
-    /// @param startIndex Index of first byte to copy.
-    /// @param endIndex Index of last byte to copy + 1.
-    /// @return A deep copy of b from startIndex to endIndex.
-    function publicDeepCopyBytes(
-        bytes memory b,
-        uint256 startIndex,
-        uint256 endIndex)
+    /// @dev Pops the last byte off of a byte array by modifying its length.
+    /// @param b Byte array that will be modified.
+    /// @return The byte that was popped off.
+    function publicPopByte(bytes memory b)
         public
-        pure
-        returns (bytes memory copy)
+        returns (bytes memory, byte result)
+    {
+        result = popByte(b);
+        return (b, result);
+    }
+
+    /// @dev Pops the last 20 bytes off of a byte array by modifying its length.
+    /// @param b Byte array that will be modified.
+    /// @return The 20 byte address that was popped off.
+    function publicPopAddress(bytes memory b)
+        public
+        returns (bytes memory, address result)
     {
-        copy = deepCopyBytes(b, startIndex, endIndex);
-        return copy;
+        result = popAddress(b);
+        return (b, result);
     }
 
     /// @dev Tests equality of two byte arrays.
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 3bd3179e5..11576fd89 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -75,7 +75,7 @@ contract Whitelist is
         // This contract must be the entry point for the transaction.
         require(takerAddress == tx.origin);
 
-        // Check if maker is on the whitelist
+        // Check if maker is on the whitelist.
         require(
             isWhitelisted[order.makerAddress],
             ADDRESS_NOT_WHITELISTED
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index 5c4fdbb23..f0a622fe4 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -27,39 +27,45 @@ contract LibBytes {
     string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32.";
     string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds.";
 
-    /// @dev Performs a deep copy of a section of a byte array.
-    /// @param b Byte array that will be copied.
-    /// @param index Index of first byte to copy.
-    /// @param len Number of bytes to copy starting from index.
-    /// @return A deep copy `len` bytes of b starting from index.
-    function deepCopyBytes(
-        bytes memory b,
-        uint256 index,
-        uint256 len)
+    /// @dev Pops the last byte off of a byte array by modifying its length.
+    /// @param b Byte array that will be modified.
+    /// @return The byte that was popped off.
+    function popByte(bytes memory b)
         internal
-        pure
-        returns (bytes memory)
+        returns (byte result)
     {
         require(
-            b.length >= index + len,
-            INDEX_OUT_OF_BOUNDS
+            b.length > 0,
+            GT_ZERO_LENGTH_REQUIRED
         );
 
-        bytes memory copy = new bytes(len);
+        result = b[b.length - 1];
+        assembly {
+            let newLen := sub(mload(b), 1)
+            mstore(b, newLen)
+        }
+        result;
+    }
 
-        // Arrays are prefixed by a 256 bit length parameter
-        index += 32;
+    /// @dev Pops the last 20 bytes off of a byte array by modifying its length.
+    /// @param b Byte array that will be modified.
+    /// @return The 20 byte address that was popped off.
+    function popAddress(bytes memory b)
+        internal
+        returns (address result)
+    {
+        require(
+            b.length >= 20,
+            GTE_20_LENGTH_REQUIRED
+        );
+
+        result = readAddress(b, b.length - 20);
 
         assembly {
-            // Start storing onto copy after length field
-            let storeStartIndex := add(copy, 32)
-            // Start loading from b at index
-            let startLoadIndex := add(b, index)
-            for {let i := 0} lt(i, len) {i := add(i, 32)} {
-                mstore(add(storeStartIndex, i), mload(add(startLoadIndex, i)))
-            }
+            let newLen := sub(mload(b), 20)
+            mstore(b, newLen)
         }
-        return copy;
+        return result;
     }
 
     /// @dev Tests equality of two byte arrays.
diff --git a/packages/contracts/src/utils/signing_utils.ts b/packages/contracts/src/utils/signing_utils.ts
index 61ab1f138..4c36c8310 100644
--- a/packages/contracts/src/utils/signing_utils.ts
+++ b/packages/contracts/src/utils/signing_utils.ts
@@ -8,19 +8,19 @@ export const signingUtils = {
             const prefixedMessage = ethUtil.hashPersonalMessage(message);
             const ecSignature = ethUtil.ecsign(prefixedMessage, privateKey);
             const signature = Buffer.concat([
-                ethUtil.toBuffer(signatureType),
                 ethUtil.toBuffer(ecSignature.v),
                 ecSignature.r,
                 ecSignature.s,
+                ethUtil.toBuffer(signatureType),
             ]);
             return signature;
         } else if (signatureType === SignatureType.EIP712) {
             const ecSignature = ethUtil.ecsign(message, privateKey);
             const signature = Buffer.concat([
-                ethUtil.toBuffer(signatureType),
                 ethUtil.toBuffer(ecSignature.v),
                 ecSignature.r,
                 ecSignature.s,
+                ethUtil.toBuffer(signatureType),
             ]);
             return signature;
         } else {
-- 
cgit v1.2.3


From fc5c598f8f6cc9c17407c8078101fa07356aa257 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Tue, 22 May 2018 09:58:08 -0700
Subject: Fix Exchange interface

---
 .../current/protocol/Exchange/interfaces/IExchange.sol       | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol
index fc428e9c0..9f21c18d7 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchange.sol
@@ -20,17 +20,15 @@ pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
 import "./IExchangeCore.sol";
-import "./IMatchOrders";
-import "./ISettlement";
-import "./ISignatureValidator";
-import "./ITransactions";
-import "./IAssetProxyDispatcher";
-import "./IWrapperFunctions";
+import "./IMatchOrders.sol";
+import "./ISignatureValidator.sol";
+import "./ITransactions.sol";
+import "./IAssetProxyDispatcher.sol";
+import "./IWrapperFunctions.sol";
 
 contract IExchange is
     IExchangeCore,
     IMatchOrders,
-    ISettlement,
     ISignatureValidator,
     ITransactions,
     IAssetProxyDispatcher,
-- 
cgit v1.2.3


From ecdd0ce9f2378a1ac271b9eb5661a6d37d6edf41 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Tue, 22 May 2018 16:57:02 -0700
Subject: Update Whitelist

---
 .../protocol/Exchange/MixinSignatureValidator.sol  | 18 ++++++++----
 .../protocol/Exchange/MixinTransactions.sol        |  3 +-
 .../current/test/TestLibBytes/TestLibBytes.sol     |  2 +-
 .../contracts/current/test/Whitelist/Whitelist.sol | 21 ++++++++-----
 .../contracts/current/utils/LibBytes/LibBytes.sol  | 34 ++++++++++++++++------
 5 files changed, 54 insertions(+), 24 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 6c018683e..8ef961a81 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -43,7 +43,8 @@ contract MixinSignatureValidator is
     function preSign(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes signature
+    )
         external
     {
         require(
@@ -56,7 +57,10 @@ contract MixinSignatureValidator is
     /// @dev Approves a Validator contract to verify signatures on signer's behalf.
     /// @param validator Address of Validator contract.
     /// @param approval Approval or disapproval of  Validator contract.
-    function approveSignatureValidator(address validator, bool approval)
+    function approveSignatureValidator(
+        address validator,
+        bool approval
+    )
         external
     {
         allowedValidators[msg.sender][validator] = approval;
@@ -70,19 +74,19 @@ contract MixinSignatureValidator is
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes memory signature)
+        bytes memory signature
+    )
         internal
         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,
             INVALID_SIGNATURE_LENGTH
         );
 
-        // Pop last byte off of 
+        // Pop last byte off of signature byte array.
         SignatureType signatureType = SignatureType(uint8(popByte(signature)));
 
         // Variables are not scoped in Solidity.
@@ -90,7 +94,7 @@ contract MixinSignatureValidator is
         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
@@ -173,7 +177,9 @@ contract MixinSignatureValidator is
         // | 0x00 + x | 20     | Address of validator contract   |
         // | 0x14 + x | 1      | Signature type is always "\x06" |
         } else if (signatureType == SignatureType.Validator) {
+            // Pop last 20 bytes off of signature byte array.
             address validator = popAddress(signature);
+            // Ensure signer has approved validator.
             if (!allowedValidators[signer][validator]) {
                 return false;
             }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index d28df11d8..12bba1ef2 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -43,7 +43,8 @@ contract MixinTransactions is
         uint256 salt,
         address signer,
         bytes data,
-        bytes signature)
+        bytes signature
+    )
         external
     {
         // Prevent reentrancy
diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
index cd5206eb8..b835e70d0 100644
--- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
+++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
@@ -30,7 +30,7 @@ contract TestLibBytes is
     /// @return The byte that was popped off.
     function publicPopByte(bytes memory b)
         public
-        returns (bytes memory, byte result)
+        returns (bytes memory, bytes1 result)
     {
         result = popByte(b);
         return (b, result);
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 11576fd89..034222915 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -27,7 +27,9 @@ contract Whitelist is
     Ownable
 {
     // Revert reasons
-    string constant ADDRESS_NOT_WHITELISTED = "Address not whitelisted.";
+    string constant MAKER_NOT_WHITELISTED = "Maker address not whitelisted.";
+    string constant TAKER_NOT_WHITELISTED = "Taker address not whitelisted.";
+    string constant INVALID_SENDER = "Sender must equal transaction origin.";
 
     // Mapping of address => whitelist status.
     mapping (address => bool) public isWhitelisted;
@@ -42,7 +44,7 @@ contract Whitelist is
         public
     {
         EXCHANGE = IExchange(_exchange);
-        TX_ORIGIN_SIGNATURE = abi.encodePacked(VALIDATOR_SIGNATURE_BYTE, address(this));
+        TX_ORIGIN_SIGNATURE = abi.encodePacked(address(this), VALIDATOR_SIGNATURE_BYTE);
     }
 
     /// @dev Adds or removes an address from the whitelist.
@@ -67,24 +69,28 @@ contract Whitelist is
         LibOrder.Order memory order,
         uint256 takerAssetFillAmount,
         uint256 salt,
-        bytes memory orderSignature)
+        bytes memory orderSignature
+    )
         public
     {
         address takerAddress = msg.sender;
     
         // This contract must be the entry point for the transaction.
-        require(takerAddress == tx.origin);
+        require(
+            takerAddress == tx.origin,
+            INVALID_SENDER
+        );
 
         // Check if maker is on the whitelist.
         require(
             isWhitelisted[order.makerAddress],
-            ADDRESS_NOT_WHITELISTED
+            MAKER_NOT_WHITELISTED
         );
 
         // Check if taker is on the whitelist.
         require(
             isWhitelisted[takerAddress],
-            ADDRESS_NOT_WHITELISTED
+            TAKER_NOT_WHITELISTED
         );
 
         // Encode arguments into byte array.
@@ -111,7 +117,8 @@ contract Whitelist is
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes signature
+    )
         external
         view
         returns (bool isValid)
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index f0a622fe4..df2221c93 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -32,19 +32,23 @@ contract LibBytes {
     /// @return The byte that was popped off.
     function popByte(bytes memory b)
         internal
-        returns (byte result)
+        pure
+        returns (bytes1 result)
     {
         require(
             b.length > 0,
             GT_ZERO_LENGTH_REQUIRED
         );
 
+        // Store last byte.
         result = b[b.length - 1];
+        
         assembly {
+            // Decrement length of byte array.
             let newLen := sub(mload(b), 1)
             mstore(b, newLen)
         }
-        result;
+        return result;
     }
 
     /// @dev Pops the last 20 bytes off of a byte array by modifying its length.
@@ -52,6 +56,7 @@ contract LibBytes {
     /// @return The 20 byte address that was popped off.
     function popAddress(bytes memory b)
         internal
+        pure
         returns (address result)
     {
         require(
@@ -59,9 +64,11 @@ contract LibBytes {
             GTE_20_LENGTH_REQUIRED
         );
 
+        // Store last 20 bytes.
         result = readAddress(b, b.length - 20);
 
         assembly {
+            // Subtract 20 from byte array length.
             let newLen := sub(mload(b), 20)
             mstore(b, newLen)
         }
@@ -72,7 +79,10 @@ contract LibBytes {
     /// @param lhs First byte array to compare.
     /// @param rhs Second byte array to compare.
     /// @return True if arrays are the same. False otherwise.
-    function areBytesEqual(bytes memory lhs, bytes memory rhs)
+    function areBytesEqual(
+        bytes memory lhs,
+        bytes memory rhs
+    )
         internal
         pure
         returns (bool equal)
@@ -106,7 +116,8 @@ contract LibBytes {
     /// @return address from byte array.
     function readAddress(
         bytes memory b,
-        uint256 index)
+        uint256 index
+    )
         internal
         pure
         returns (address result)
@@ -138,7 +149,8 @@ contract LibBytes {
     function writeAddress(
         bytes memory b,
         uint256 index,
-        address input)
+        address input
+    )
         internal
         pure
     {
@@ -175,7 +187,8 @@ contract LibBytes {
     /// @return bytes32 value from byte array.
     function readBytes32(
         bytes memory b,
-        uint256 index)
+        uint256 index
+    )
         internal
         pure
         returns (bytes32 result)
@@ -202,7 +215,8 @@ contract LibBytes {
     function writeBytes32(
         bytes memory b,
         uint256 index,
-        bytes32 input)
+        bytes32 input
+    )
         internal
         pure
     {
@@ -226,7 +240,8 @@ contract LibBytes {
     /// @return uint256 value from byte array.
     function readUint256(
         bytes memory b,
-        uint256 index)
+        uint256 index
+    )
         internal
         pure
         returns (uint256 result)
@@ -241,7 +256,8 @@ contract LibBytes {
     function writeUint256(
         bytes memory b,
         uint256 index,
-        uint256 input)
+        uint256 input
+    )
         internal
         pure
     {
-- 
cgit v1.2.3


From e5b7e29113fb7c87f41e492abb1fd81247f0db46 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 23 May 2018 10:08:06 -0700
Subject: Add signer to txHash, allow approveValidator to be used with
 executeTransaction

---
 .../current/protocol/Exchange/MixinSignatureValidator.sol        | 7 +++++--
 .../contracts/current/protocol/Exchange/MixinTransactions.sol    | 1 +
 packages/contracts/src/utils/transaction_factory.ts              | 9 ++++-----
 3 files changed, 10 insertions(+), 7 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 8ef961a81..e2d4808c3 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -19,6 +19,7 @@
 pragma solidity ^0.4.24;
 
 import "./mixins/MSignatureValidator.sol";
+import "./mixins/MTransactions.sol";
 import "./interfaces/ISigner.sol";
 import "./interfaces/IValidator.sol";
 import "./libs/LibExchangeErrors.sol";
@@ -27,7 +28,8 @@ import "../../utils/LibBytes/LibBytes.sol";
 contract MixinSignatureValidator is
     LibBytes,
     LibExchangeErrors,
-    MSignatureValidator
+    MSignatureValidator,
+    MTransactions
 {
 
     // Mapping of hash => signer => signed
@@ -63,7 +65,8 @@ contract MixinSignatureValidator is
     )
         external
     {
-        allowedValidators[msg.sender][validator] = approval;
+        address signer = getCurrentContextAddress();
+        allowedValidators[signer][validator] = approval;
     }
 
     /// @dev Verifies that a hash has been signed by the given signer.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index 12bba1ef2..d153dfa5c 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -56,6 +56,7 @@ contract MixinTransactions is
         // Calculate transaction hash
         bytes32 transactionHash = keccak256(
             address(this),
+            signer,
             salt,
             data
         );
diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts
index 941bff96d..65cdb3f89 100644
--- a/packages/contracts/src/utils/transaction_factory.ts
+++ b/packages/contracts/src/utils/transaction_factory.ts
@@ -7,26 +7,25 @@ import { signingUtils } from './signing_utils';
 import { SignatureType, SignedTransaction } from './types';
 
 export class TransactionFactory {
-    private _signer: string;
+    private _signerBuff: Buffer;
     private _exchangeAddress: string;
     private _privateKey: Buffer;
     constructor(privateKey: Buffer, exchangeAddress: string) {
         this._privateKey = privateKey;
         this._exchangeAddress = exchangeAddress;
-        const signerBuff = ethUtil.privateToAddress(this._privateKey);
-        this._signer = `0x${signerBuff.toString('hex')}`;
+        this._signerBuff = ethUtil.privateToAddress(this._privateKey);
     }
     public newSignedTransaction(
         data: string,
         signatureType: SignatureType = SignatureType.Ecrecover,
     ): SignedTransaction {
         const salt = generatePseudoRandomSalt();
-        const txHash = crypto.solSHA3([this._exchangeAddress, salt, ethUtil.toBuffer(data)]);
+        const txHash = crypto.solSHA3([this._exchangeAddress, this._signerBuff, salt, ethUtil.toBuffer(data)]);
         const signature = signingUtils.signMessage(txHash, this._privateKey, signatureType);
         const signedTx = {
             exchangeAddress: this._exchangeAddress,
             salt,
-            signer: this._signer,
+            signer: `0x${this._signerBuff.toString('hex')}`,
             data,
             signature: `0x${signature.toString('hex')}`,
         };
-- 
cgit v1.2.3


From 6050a59e4a190cf8f8d42e390b435a7184b8f718 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 23 May 2018 15:20:47 -0700
Subject: Make AssetProxyId last byte of assetData

---
 .../contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 10 ++++++----
 .../current/protocol/AssetProxy/ERC721Proxy.sol          | 12 +++++++-----
 .../current/protocol/AssetProxy/MixinAssetProxy.sol      |  6 ++++--
 .../current/protocol/AssetProxy/MixinAuthorizable.sol    |  5 ++++-
 .../protocol/AssetProxy/interfaces/IAssetProxy.sol       |  6 ++++--
 .../protocol/AssetProxy/interfaces/IAuthorizable.sol     |  5 ++++-
 .../current/protocol/AssetProxy/mixins/MAssetProxy.sol   |  3 ++-
 .../current/protocol/AssetProxy/mixins/MAuthorizable.sol |  2 +-
 .../protocol/Exchange/MixinAssetProxyDispatcher.sol      | 12 ++++++++----
 .../protocol/Exchange/MixinSignatureValidator.sol        |  2 +-
 .../protocol/Exchange/interfaces/ISignatureValidator.sol | 12 +++++++++++-
 .../current/protocol/Exchange/interfaces/ISigner.sol     |  3 ++-
 .../current/protocol/Exchange/interfaces/IValidator.sol  |  3 ++-
 .../protocol/Exchange/mixins/MAssetProxyDispatcher.sol   |  3 ++-
 .../protocol/Exchange/mixins/MSignatureValidator.sol     |  3 ++-
 .../TestSignatureValidator/TestSignatureValidator.sol    |  3 ++-
 .../src/contracts/current/test/Whitelist/Whitelist.sol   |  5 ++++-
 packages/contracts/src/utils/asset_proxy_utils.ts        | 16 ++++++++--------
 18 files changed, 74 insertions(+), 37 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index ee0c66fdc..7c25c9770 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -47,21 +47,23 @@ contract ERC20Proxy is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal
     {
         // Data must be intended for this proxy.
+        uint256 length = assetMetadata.length;
         require(
-            uint8(assetMetadata[0]) == PROXY_ID,
+            uint8(assetMetadata[length - 1]) == PROXY_ID,
             PROXY_ID_MISMATCH
         );
 
         // Decode metadata.
         require(
-            assetMetadata.length == 21,
+            length == 21,
             INVALID_METADATA_LENGTH
         );
-        address token = readAddress(assetMetadata, 1);
+        address token = readAddress(assetMetadata, 0);
 
         // Transfer tokens.
         bool success = IERC20Token(token).transferFrom(from, to, amount);
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index 94aab9139..59045f73a 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -47,12 +47,14 @@ contract ERC721Proxy is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal
     {
         // Data must be intended for this proxy.
+        uint256 length = assetMetadata.length;
         require(
-            uint8(assetMetadata[0]) == PROXY_ID,
+            uint8(assetMetadata[length - 1]) == PROXY_ID,
             PROXY_ID_MISMATCH
         );
 
@@ -64,11 +66,11 @@ contract ERC721Proxy is
 
         // Decode metadata
         require(
-            assetMetadata.length == 53,
+            length == 53,
             INVALID_METADATA_LENGTH
         );
-        address token = readAddress(assetMetadata, 1);
-        uint256 tokenId = readUint256(assetMetadata, 21);
+        address token = readAddress(assetMetadata, 0);
+        uint256 tokenId = readUint256(assetMetadata, 20);
 
         // Transfer token.
         // Either succeeds or throws.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
index 4ec31304f..4bd533148 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
@@ -36,7 +36,8 @@ contract MixinAssetProxy is
         bytes assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         external
         onlyAuthorized
     {
@@ -57,7 +58,8 @@ contract MixinAssetProxy is
         bytes[] memory assetMetadata,
         address[] memory from,
         address[] memory to,
-        uint256[] memory amounts)
+        uint256[] memory amounts
+    )
         public
         onlyAuthorized
     {
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
index 0bbd3b318..4aaeb66d1 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
@@ -87,7 +87,10 @@ contract MixinAuthorizable is
     /// @dev Removes authorizion of an address.
     /// @param target Address to remove authorization from.
     /// @param index Index of target in authorities array.
-    function removeAuthorizedAddressAtIndex(address target, uint256 index)
+    function removeAuthorizedAddressAtIndex(
+        address target,
+        uint256 index
+    )
         external
     {
         require(
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
index 8b30dfabb..7e1848889 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
@@ -34,7 +34,8 @@ contract IAssetProxy is
         bytes assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         external;
     
     /// @dev Makes multiple transfers of assets. Either succeeds or throws.
@@ -46,7 +47,8 @@ contract IAssetProxy is
         bytes[] memory assetMetadata,
         address[] memory from,
         address[] memory to,
-        uint256[] memory amounts)
+        uint256[] memory amounts
+    )
         public;
 
     /// @dev Gets the proxy id associated with the proxy address.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
index d6fe03898..cedd1744c 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAuthorizable.sol
@@ -45,6 +45,9 @@ contract IAuthorizable is
     /// @dev Removes authorizion of an address.
     /// @param target Address to remove authorization from.
     /// @param index Index of target in authorities array.
-    function removeAuthorizedAddressAtIndex(address target, uint256 index)
+    function removeAuthorizedAddressAtIndex(
+        address target,
+        uint256 index
+    )
         external;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol
index 3800bf04c..de9d65a53 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol
@@ -34,6 +34,7 @@ contract MAssetProxy is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
index cdf60bdee..6f35bd7ec 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAuthorizable.sol
@@ -38,5 +38,5 @@ contract MAuthorizable is
     );
 
     /// @dev Only authorized addresses can invoke functions with this modifier.
-    modifier onlyAuthorized { _; }
+    modifier onlyAuthorized { revert(); _; }
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
index 3b38d1f37..d996f933c 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -39,7 +39,8 @@ contract MixinAssetProxyDispatcher is
     function registerAssetProxy(
         uint8 assetProxyId,
         address newAssetProxy,
-        address oldAssetProxy)
+        address oldAssetProxy
+    )
         external
         onlyOwner
     {
@@ -86,17 +87,20 @@ contract MixinAssetProxyDispatcher is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal
     {
         // Do nothing if no amount should be transferred.
         if (amount > 0) {
+
             // Lookup asset proxy
+            uint256 length = assetMetadata.length;
             require(
-                assetMetadata.length >= 1,
+                length > 0,
                 GT_ZERO_LENGTH_REQUIRED
             );
-            uint8 assetProxyId = uint8(assetMetadata[0]);
+            uint8 assetProxyId = uint8(assetMetadata[length - 1]);
             IAssetProxy assetProxy = assetProxies[assetProxyId];
 
             // transferFrom will either succeed or throw.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index e2d4808c3..96a1f578a 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -105,7 +105,7 @@ contract MixinSignatureValidator is
         // also the initialization value for the enum type.
         if (signatureType == SignatureType.Illegal) {
             // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051
-            revert("Illegal signature type.");
+            revert(ILLEGAL_SIGNATURE_TYPE);
 
         // Always invalid signature.
         // Like Illegal, this is always implicitly available and therefore
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
index 65ff45f7b..72f15a2a4 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
@@ -27,6 +27,16 @@ contract ISignatureValidator {
     function preSign(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes signature
+    )
+        external;
+    
+    /// @dev Approves a Validator contract to verify signatures on signer's behalf.
+    /// @param validator Address of Validator contract.
+    /// @param approval Approval or disapproval of  Validator contract.
+    function approveSignatureValidator(
+        address validator,
+        bool approval
+    )
         external;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
index 53c41d331..0186ad490 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
@@ -26,7 +26,8 @@ contract ISigner {
     /// @return Validity of order signature.
     function isValidSignature(
         bytes32 hash,
-        bytes signature)
+        bytes signature
+    )
         external
         view
         returns (bool isValid);
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
index aed725066..3e5ccc190 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
@@ -28,7 +28,8 @@ contract IValidator {
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes signature)
+        bytes signature
+    )
         external
         view
         returns (bool isValid);
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
index ccc960d6e..87c5f6361 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
@@ -41,6 +41,7 @@ contract MAssetProxyDispatcher is
         bytes memory assetMetadata,
         address from,
         address to,
-        uint256 amount)
+        uint256 amount
+    )
         internal;
 }
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 57600f508..dc08c6d27 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -44,7 +44,8 @@ contract MSignatureValidator is
     function isValidSignature(
         bytes32 hash,
         address signer,
-        bytes memory signature)
+        bytes memory signature
+    )
         internal
         view
         returns (bool isValid);
diff --git a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
index 15d9ca189..e2bc75b37 100644
--- a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
@@ -26,7 +26,8 @@ contract TestSignatureValidator is MixinSignatureValidator {
     function publicIsValidSignature(
         bytes32 hash,
         address signer,
-        bytes memory signature)
+        bytes memory signature
+    )
         public
         view
         returns (bool isValid)
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 034222915..8b9763c65 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -50,7 +50,10 @@ contract Whitelist is
     /// @dev Adds or removes an address from the whitelist.
     /// @param target Address to add or remove from whitelist.
     /// @param isApproved Whitelist status to assign to address.
-    function updateWhitelistStatus(address target, bool isApproved)
+    function updateWhitelistStatus(
+        address target,
+        bool isApproved
+    )
         external
         onlyOwner
     {
diff --git a/packages/contracts/src/utils/asset_proxy_utils.ts b/packages/contracts/src/utils/asset_proxy_utils.ts
index c042da5d0..a17d4cdfa 100644
--- a/packages/contracts/src/utils/asset_proxy_utils.ts
+++ b/packages/contracts/src/utils/asset_proxy_utils.ts
@@ -39,7 +39,7 @@ export const assetProxyUtils = {
     encodeERC20ProxyData(tokenAddress: string): string {
         const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC20);
         const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress);
-        const encodedMetadata = Buffer.concat([encodedAssetProxyId, encodedAddress]);
+        const encodedMetadata = Buffer.concat([encodedAddress, encodedAssetProxyId]);
         const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata);
         return encodedMetadataHex;
     },
@@ -52,7 +52,7 @@ export const assetProxyUtils = {
                 }`,
             );
         }
-        const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1);
+        const encodedAssetProxyId = encodedProxyMetadata.slice(-1);
         const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
         if (assetProxyId !== AssetProxyId.ERC20) {
             throw new Error(
@@ -61,7 +61,7 @@ export const assetProxyUtils = {
                 }), but got ${assetProxyId}`,
             );
         }
-        const encodedTokenAddress = encodedProxyMetadata.slice(1, 21);
+        const encodedTokenAddress = encodedProxyMetadata.slice(0, 20);
         const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress);
         const erc20ProxyData = {
             assetProxyId,
@@ -73,7 +73,7 @@ export const assetProxyUtils = {
         const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC721);
         const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress);
         const encodedTokenId = assetProxyUtils.encodeUint256(tokenId);
-        const encodedMetadata = Buffer.concat([encodedAssetProxyId, encodedAddress, encodedTokenId]);
+        const encodedMetadata = Buffer.concat([encodedAddress, encodedTokenId, encodedAssetProxyId]);
         const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata);
         return encodedMetadataHex;
     },
@@ -86,7 +86,7 @@ export const assetProxyUtils = {
                 }`,
             );
         }
-        const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1);
+        const encodedAssetProxyId = encodedProxyMetadata.slice(-1);
         const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
         if (assetProxyId !== AssetProxyId.ERC721) {
             throw new Error(
@@ -95,9 +95,9 @@ export const assetProxyUtils = {
                 }), but got ${assetProxyId}`,
             );
         }
-        const encodedTokenAddress = encodedProxyMetadata.slice(1, 21);
+        const encodedTokenAddress = encodedProxyMetadata.slice(0, 20);
         const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress);
-        const encodedTokenId = encodedProxyMetadata.slice(21, 53);
+        const encodedTokenId = encodedProxyMetadata.slice(20, 52);
         const tokenId = assetProxyUtils.decodeUint256(encodedTokenId);
         const erc721ProxyData = {
             assetProxyId,
@@ -115,7 +115,7 @@ export const assetProxyUtils = {
                 }`,
             );
         }
-        const encodedAssetProxyId = encodedProxyMetadata.slice(0, 1);
+        const encodedAssetProxyId = encodedProxyMetadata.slice(-1);
         const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
         return assetProxyId;
     },
-- 
cgit v1.2.3


From 9f93d8f53380bf650702c653fea13b3e7ed5f8f7 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Thu, 24 May 2018 10:24:32 -0700
Subject: Fix formatting and tests

---
 .../contracts/current/protocol/Exchange/MixinSignatureValidator.sol | 4 +---
 .../src/contracts/current/test/TestLibBytes/TestLibBytes.sol        | 2 ++
 .../current/test/TestSignatureValidator/TestSignatureValidator.sol  | 6 +++++-
 .../contracts/src/contracts/current/test/Whitelist/Whitelist.sol    | 2 ++
 4 files changed, 10 insertions(+), 4 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 96a1f578a..4b0b1d02d 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -104,7 +104,6 @@ contract MixinSignatureValidator is
         // it an explicit option. This aids testing and analysis. It is
         // also the initialization value for the enum type.
         if (signatureType == SignatureType.Illegal) {
-            // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051
             revert(ILLEGAL_SIGNATURE_TYPE);
 
         // Always invalid signature.
@@ -234,7 +233,6 @@ contract MixinSignatureValidator is
         // that we currently support. In this case returning false
         // may lead the caller to incorrectly believe that the
         // signature was invalid.)
-        // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051
-        revert("Unsupported signature type.");
+        revert(UNSUPPORTED_SIGNATURE_TYPE);
     }
 }
diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
index b835e70d0..69554605d 100644
--- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
+++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
@@ -30,6 +30,7 @@ contract TestLibBytes is
     /// @return The byte that was popped off.
     function publicPopByte(bytes memory b)
         public
+        pure
         returns (bytes memory, bytes1 result)
     {
         result = popByte(b);
@@ -41,6 +42,7 @@ contract TestLibBytes is
     /// @return The 20 byte address that was popped off.
     function publicPopAddress(bytes memory b)
         public
+        pure
         returns (bytes memory, address result)
     {
         result = popAddress(b);
diff --git a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
index e2bc75b37..0f84678cf 100644
--- a/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/test/TestSignatureValidator/TestSignatureValidator.sol
@@ -20,8 +20,12 @@ pragma solidity ^0.4.24;
 pragma experimental ABIEncoderV2;
 
 import "../../protocol/Exchange/MixinSignatureValidator.sol";
+import "../../protocol/Exchange/MixinTransactions.sol";
 
-contract TestSignatureValidator is MixinSignatureValidator {
+contract TestSignatureValidator is
+    MixinSignatureValidator,
+    MixinTransactions
+{
 
     function publicIsValidSignature(
         bytes32 hash,
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 8b9763c65..0594e2767 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -114,6 +114,8 @@ contract Whitelist is
     }
 
     /// @dev Verifies signer is same as signer of current Ethereum transaction.
+    ///      NOTE: This function can currently be used to validate signatures coming from outside of this contract.
+    ///      Extra safety checks can be added for a production contract.
     /// @param signer Address that should have signed the given hash.
     /// @param signature Proof of signing.
     /// @return Validity of order signature.
-- 
cgit v1.2.3


From 101e9be7b928493efe96c6a3c05641c68e30a02e Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Fri, 25 May 2018 15:04:44 -0700
Subject: Change names of signature types

---
 .../current/protocol/Exchange/MixinSignatureValidator.sol         | 8 ++++----
 .../current/protocol/Exchange/mixins/MSignatureValidator.sol      | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 4b0b1d02d..fc61112eb 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -132,7 +132,7 @@ contract MixinSignatureValidator is
             return isValid;
 
         // Signed using web3.eth_sign
-        } else if (signatureType == SignatureType.Ecrecover) {
+        } else if (signatureType == SignatureType.EthSign) {
             require(
                 signature.length == 65,
                 INVALID_SIGNATURE_LENGTH
@@ -165,9 +165,9 @@ contract MixinSignatureValidator is
             isValid = signer == msg.sender;
             return isValid;
 
-        // Signature verified by signer contract.
-        // If used with an order, the maker of the order is the signer contract.
-        } else if (signatureType == SignatureType.Signer) {
+        // Signature verified by wallet contract.
+        // If used with an order, the maker of the order is the wallet contract.
+        } else if (signatureType == SignatureType.Wallet) {
             isValid = ISigner(signer).isValidSignature(hash, signature);
             return isValid;
 
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 dc08c6d27..7eed453ff 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -28,9 +28,9 @@ contract MSignatureValidator is
         Illegal,    // 0x00, default value
         Invalid,    // 0x01
         EIP712,     // 0x02
-        Ecrecover,  // 0x03
+        EthSign,    // 0x03
         Caller,     // 0x04
-        Signer,     // 0x05
+        Wallet,     // 0x05
         Validator,  // 0x06
         PreSigned,  // 0x07
         Trezor      // 0x08
-- 
cgit v1.2.3


From d625b65a095381c3fa8dd3192ee2115ee739cc07 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Tue, 29 May 2018 09:03:23 -0700
Subject: Make preSigned and allowedValidators mappings public

---
 .../contracts/current/protocol/Exchange/MixinSignatureValidator.sol   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index fc61112eb..7f46766d6 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -33,10 +33,10 @@ contract MixinSignatureValidator is
 {
 
     // Mapping of hash => signer => signed
-    mapping (bytes32 => mapping (address => bool)) preSigned;
+    mapping (bytes32 => mapping (address => bool)) public preSigned;
 
     // Mapping of signer => validator => approved
-    mapping (address => mapping (address => bool)) allowedValidators;
+    mapping (address => mapping (address => bool)) public allowedValidators;
 
     /// @dev Approves a hash on-chain using any valid signature type.
     ///      After presigning a hash, the preSign signature type will become valid for that hash and signer.
-- 
cgit v1.2.3


From 8f2fd9b603f74aa8a44dc0e8c3b845d497f9b279 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Tue, 29 May 2018 11:22:57 -0700
Subject: Rename Signer to Wallet, rename GAS_ESTIMATE to GAS_LIMIT

---
 .../protocol/Exchange/MixinSignatureValidator.sol  |  4 +--
 .../protocol/Exchange/interfaces/ISigner.sol       | 34 ----------------------
 .../protocol/Exchange/interfaces/IWallet.sol       | 34 ++++++++++++++++++++++
 packages/contracts/src/utils/web3_wrapper.ts       |  2 +-
 4 files changed, 37 insertions(+), 37 deletions(-)
 delete mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
 create mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWallet.sol

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 7f46766d6..7f4e20b5b 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -20,7 +20,7 @@ pragma solidity ^0.4.24;
 
 import "./mixins/MSignatureValidator.sol";
 import "./mixins/MTransactions.sol";
-import "./interfaces/ISigner.sol";
+import "./interfaces/IWallet.sol";
 import "./interfaces/IValidator.sol";
 import "./libs/LibExchangeErrors.sol";
 import "../../utils/LibBytes/LibBytes.sol";
@@ -168,7 +168,7 @@ contract MixinSignatureValidator is
         // Signature verified by wallet contract.
         // If used with an order, the maker of the order is the wallet contract.
         } else if (signatureType == SignatureType.Wallet) {
-            isValid = ISigner(signer).isValidSignature(hash, signature);
+            isValid = IWallet(signer).isValidSignature(hash, signature);
             return isValid;
 
         // Signature verified by validator contract.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
deleted file mode 100644
index 0186ad490..000000000
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISigner.sol
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
-
-  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 ISigner {
-
-    /// @dev Verifies that a signature is valid.
-    /// @param hash Message hash that is signed.
-    /// @param signature Proof of signing.
-    /// @return Validity of order signature.
-    function isValidSignature(
-        bytes32 hash,
-        bytes signature
-    )
-        external
-        view
-        returns (bool isValid);
-}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWallet.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWallet.sol
new file mode 100644
index 000000000..c86a2c057
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWallet.sol
@@ -0,0 +1,34 @@
+/*
+
+  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 IWallet {
+
+    /// @dev Verifies that a signature is valid.
+    /// @param hash Message hash that is signed.
+    /// @param signature Proof of signing.
+    /// @return Validity of order signature.
+    function isValidSignature(
+        bytes32 hash,
+        bytes signature
+    )
+        external
+        view
+        returns (bool isValid);
+}
diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts
index 02595506b..4b8512222 100644
--- a/packages/contracts/src/utils/web3_wrapper.ts
+++ b/packages/contracts/src/utils/web3_wrapper.ts
@@ -7,7 +7,7 @@ import { coverage } from './coverage';
 
 export const txDefaults = {
     from: devConstants.TESTRPC_FIRST_ADDRESS,
-    gas: devConstants.GAS_ESTIMATE,
+    gas: devConstants.GAS_LIMIT,
 };
 const providerConfigs = { shouldUseInProcessGanache: true };
 export const provider = web3Factory.getRpcProvider(providerConfigs);
-- 
cgit v1.2.3


From 79e7c44884f81f12733d555314c54d4c912f0e88 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 30 May 2018 17:52:37 -0700
Subject: Check length before accessing indices, add awaitTransactionSuccess
 where needed, and rename function

---
 .../src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol   | 10 ++++++----
 .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol  | 10 ++++++----
 .../current/protocol/Exchange/MixinSignatureValidator.sol      |  4 ++--
 .../protocol/Exchange/interfaces/ISignatureValidator.sol       |  4 ++--
 4 files changed, 16 insertions(+), 12 deletions(-)

(limited to 'packages/contracts/src')

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index 7c25c9770..79824fbbb 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -53,16 +53,18 @@ contract ERC20Proxy is
     {
         // Data must be intended for this proxy.
         uint256 length = assetMetadata.length;
+
+        require(
+            length == 21,
+            INVALID_METADATA_LENGTH
+        );
+
         require(
             uint8(assetMetadata[length - 1]) == PROXY_ID,
             PROXY_ID_MISMATCH
         );
 
         // Decode metadata.
-        require(
-            length == 21,
-            INVALID_METADATA_LENGTH
-        );
         address token = readAddress(assetMetadata, 0);
 
         // Transfer tokens.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index 59045f73a..ace3570bc 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -53,6 +53,12 @@ contract ERC721Proxy is
     {
         // Data must be intended for this proxy.
         uint256 length = assetMetadata.length;
+
+        require(
+            length == 53,
+            INVALID_METADATA_LENGTH
+        );
+
         require(
             uint8(assetMetadata[length - 1]) == PROXY_ID,
             PROXY_ID_MISMATCH
@@ -65,10 +71,6 @@ contract ERC721Proxy is
         );
 
         // Decode metadata
-        require(
-            length == 53,
-            INVALID_METADATA_LENGTH
-        );
         address token = readAddress(assetMetadata, 0);
         uint256 tokenId = readUint256(assetMetadata, 20);
 
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 7f4e20b5b..d40974e5f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -56,10 +56,10 @@ contract MixinSignatureValidator is
         preSigned[hash][signer] = true;
     }
 
-    /// @dev Approves a Validator contract to verify signatures on signer's behalf.
+    /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf.
     /// @param validator Address of Validator contract.
     /// @param approval Approval or disapproval of  Validator contract.
-    function approveSignatureValidator(
+    function setSignatureValidatorApproval(
         address validator,
         bool approval
     )
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
index 72f15a2a4..26e360c91 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
@@ -31,10 +31,10 @@ contract ISignatureValidator {
     )
         external;
     
-    /// @dev Approves a Validator contract to verify signatures on signer's behalf.
+    /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf.
     /// @param validator Address of Validator contract.
     /// @param approval Approval or disapproval of  Validator contract.
-    function approveSignatureValidator(
+    function setSignatureValidatorApproval(
         address validator,
         bool approval
     )
-- 
cgit v1.2.3