Remove TxOrigin signature type, modify whitelist to use Validator signature type

7 files changed, 35 insertions, 35 deletions

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
@@ -41,10 +41,6 @@ contract LibBytes {
         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,
diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts
index fe6df2f75..294e3aae9 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -218,6 +218,10 @@ describe('Exchange transactions', () => {
                 txDefaults,
                 exchange.address,
             );
+            const isApproved = true;
+            await exchange.approveSignatureValidator.sendTransactionAsync(whitelist.address, isApproved, {
+                from: takerAddress,
+            });
             const defaultOrderParams = {
                 ...constants.STATIC_ORDER_PARAMS,
                 senderAddress: whitelist.address,
diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts
index ac654e037..c85cee5e3 100644
--- a/packages/contracts/test/libraries/lib_bytes.ts
+++ b/packages/contracts/test/libraries/lib_bytes.ts
@@ -62,12 +62,11 @@ describe('LibBytes', () => {
 
     describe('deepCopyBytes', () => {
         const byteArrayLongerThan32BytesLen = (byteArrayLongerThan32Bytes.length - 2) / 2;
-        it('should throw if length of copy is 0', async () => {
+        it('should return a byte array of length 0 if len is 0', async () => {
             const index = new BigNumber(0);
             const len = new BigNumber(0);
-            return expect(
-                libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len),
-            ).to.be.rejectedWith(constants.REVERT);
+            const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
+            expect(copy).to.equal(constants.NULL_BYTES);
         });
 
         it('should throw if start index + length to copy is greater than length of byte array', async () => {