Twenty new tests for MixinSignatureValidator. Fixed handling of unsupported types. Fixed trezor prefix.

9 files changed, 530 insertions, 27 deletions

diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index a5cfa8761..fa7ede817 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -36,6 +36,8 @@
         "TestLibMem",
         "TestLibs",
         "TestSignatureValidator",
+        "TestValidator",
+        "TestWallet",
         "TokenRegistry",
         "Whitelist",
         "WETH9",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index cf3f6f01d..9e7dad674 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -34,7 +34,7 @@
     },
     "config": {
         "abis":
-            "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+            "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
     },
     "repository": {
         "type": "git",
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 8ad15aaff..4a2beff57 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -33,7 +33,7 @@ contract MixinSignatureValidator is
 {
     // Personal message headers
     string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32";
-    string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x41";
+    string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20";
 
     // Mapping of hash => signer => signed
     mapping (bytes32 => mapping (address => bool)) public preSigned;
@@ -92,8 +92,15 @@ contract MixinSignatureValidator is
             LENGTH_GREATER_THAN_0_REQUIRED
         );
 
+        // Ensure signature is supported
+        uint8 signatureTypeRaw = uint8(popLastByte(signature));
+        require(
+            signatureTypeRaw < uint8(SignatureType.NSignatureTypes),
+            SIGNATURE_UNSUPPORTED
+        );
+
         // Pop last byte off of signature byte array.
-        SignatureType signatureType = SignatureType(uint8(popLastByte(signature)));
+        SignatureType signatureType = SignatureType(signatureTypeRaw);
 
         // Variables are not scoped in Solidity.
         uint8 v;
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 5e286e43a..0cab95193 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -25,14 +25,15 @@ contract MSignatureValidator is
 {
     // Allowed signature types.
     enum SignatureType {
-        Illegal,    // 0x00, default value
-        Invalid,    // 0x01
-        EIP712,     // 0x02
-        EthSign,    // 0x03
-        Caller,     // 0x04
-        Wallet,     // 0x05
-        Validator,  // 0x06
-        PreSigned,  // 0x07
-        Trezor      // 0x08
+        Illegal,        // 0x00, default value
+        Invalid,        // 0x01
+        EIP712,         // 0x02
+        EthSign,        // 0x03
+        Caller,         // 0x04
+        Wallet,         // 0x05
+        Validator,      // 0x06
+        PreSigned,      // 0x07
+        Trezor,         // 0x08
+        NSignatureTypes // 0x09, always leave at end.
     }
 }
diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol
new file mode 100644
index 000000000..ba661f89d
--- /dev/null
+++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol
@@ -0,0 +1,52 @@
+/*
+
+  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;
+
+import "../../protocol/Exchange/interfaces/IValidator.sol";
+
+contract TestValidator is 
+    IValidator
+{
+
+    // The single valid signer for this wallet.
+    address validSigner;
+
+    /// @dev constructs a new `TestValidator` with a single valid signer.
+    /// @param _validSigner The sole signer for this wallet.
+    constructor (address _validSigner) public {
+        validSigner = _validSigner;
+    }
+
+     /// @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)
+    {
+        return (signer == validSigner);
+    }
+}
diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol
new file mode 100644
index 000000000..5baba1d98
--- /dev/null
+++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol
@@ -0,0 +1,65 @@
+/*
+
+  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;
+
+import "../../protocol/Exchange/interfaces/IWallet.sol";
+import "../../utils/LibBytes/LibBytes.sol";
+
+contract TestWallet is 
+    IWallet,
+    LibBytes
+{
+
+    string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED";
+
+    // The owner of this wallet.
+    address walletOwner;
+
+    /// @dev constructs a new `TestWallet` with a single owner.
+    /// @param _walletOwner The owner of this wallet.
+    constructor (address _walletOwner) public {
+        walletOwner = _walletOwner;
+    }
+
+    /// @dev Validates an EIP712 signature.
+    ///      The signer must match the signer of this wallet.
+    /// @param hash Message hash that is signed.
+    /// @param eip721Signature Proof of signing.
+    /// @return Validity of order signature.
+    function isValidSignature(
+        bytes32 hash,
+        bytes eip721Signature
+    )
+        external
+        view
+        returns (bool isValid)
+    {
+        require(
+            eip721Signature.length == 65,
+            LENGTH_65_REQUIRED
+        );
+
+        uint8 v = uint8(eip721Signature[0]);
+        bytes32 r = readBytes32(eip721Signature, 1);
+        bytes32 s = readBytes32(eip721Signature, 33);
+        address recoveredAddress = ecrecover(hash, v, r, s);
+        isValid = walletOwner == recoveredAddress;
+        return isValid;
+    }
+}
diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts
index 7ba467708..4375d87c6 100644
--- a/packages/contracts/src/utils/artifacts.ts
+++ b/packages/contracts/src/utils/artifacts.ts
@@ -17,6 +17,8 @@ import * as TestLibBytes from '../artifacts/TestLibBytes.json';
 import * as TestLibMem from '../artifacts/TestLibMem.json';
 import * as TestLibs from '../artifacts/TestLibs.json';
 import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json';
+import * as TestValidator from '../artifacts/TestValidator.json';
+import * as TestWallet from '../artifacts/TestWallet.json';
 import * as TokenRegistry from '../artifacts/TokenRegistry.json';
 import * as EtherToken from '../artifacts/WETH9.json';
 import * as Whitelist from '../artifacts/Whitelist.json';
@@ -41,6 +43,8 @@ export const artifacts = {
     TestLibMem: (TestLibMem as any) as ContractArtifact,
     TestLibs: (TestLibs as any) as ContractArtifact,
     TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact,
+    TestValidator: (TestValidator as any) as ContractArtifact,
+    TestWallet: (TestWallet as any) as ContractArtifact,
     TokenRegistry: (TokenRegistry as any) as ContractArtifact,
     Whitelist: (Whitelist as any) as ContractArtifact,
     ZRX: (ZRX as any) as ContractArtifact,
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index bb8c12088..a6d0c2a88 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -100,6 +100,7 @@ export enum ContractName {
     DummyERC721Receiver = 'DummyERC721Receiver',
     DummyERC721Token = 'DummyERC721Token',
     TestLibBytes = 'TestLibBytes',
+    TestWallet = 'TestWallet',
     Authorizable = 'Authorizable',
     Whitelist = 'Whitelist',
 }
diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts
index c39fd6ee4..8f796361f 100644
--- a/packages/contracts/test/exchange/signature_validator.ts
+++ b/packages/contracts/test/exchange/signature_validator.ts
@@ -1,12 +1,16 @@
 import { BlockchainLifecycle } from '@0xproject/dev-utils';
-import { assetProxyUtils, orderHashUtils } from '@0xproject/order-utils';
-import { SignedOrder } from '@0xproject/types';
+import { addSignedMessagePrefix, assetProxyUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils';
+import { SignatureType, SignedOrder } from '@0xproject/types';
 import * as chai from 'chai';
 import ethUtil = require('ethereumjs-util');
+import * as _ from 'lodash';
 
 import { TestSignatureValidatorContract } from '../../src/generated_contract_wrappers/test_signature_validator';
+import { TestValidatorContract } from '../../src/generated_contract_wrappers/test_validator';
+import { TestWalletContract } from '../../src/generated_contract_wrappers/test_wallet';
 import { addressUtils } from '../../src/utils/address_utils';
 import { artifacts } from '../../src/utils/artifacts';
+import { expectRevertOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions';
 import { chaiSetup } from '../../src/utils/chai_setup';
 import { constants } from '../../src/utils/constants';
 import { OrderFactory } from '../../src/utils/order_factory';
@@ -21,6 +25,12 @@ describe('MixinSignatureValidator', () => {
     let signedOrder: SignedOrder;
     let orderFactory: OrderFactory;
     let signatureValidator: TestSignatureValidatorContract;
+    let testWallet: TestWalletContract;
+    let testValidator: TestValidatorContract;
+    let signerAddress: string;
+    let signerPrivateKey: Buffer;
+    let notSignerAddress: string;
+    let notSignerPrivateKey: Buffer;
 
     before(async () => {
         await blockchainLifecycle.startAsync();
@@ -31,11 +41,31 @@ describe('MixinSignatureValidator', () => {
     before(async () => {
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
         const makerAddress = accounts[0];
+        signerAddress = makerAddress;
+        notSignerAddress = accounts[1];
         signatureValidator = await TestSignatureValidatorContract.deployFrom0xArtifactAsync(
             artifacts.TestSignatureValidator,
             provider,
             txDefaults,
         );
+        testWallet = await TestWalletContract.deployFrom0xArtifactAsync(
+            artifacts.TestWallet,
+            provider,
+            txDefaults,
+            signerAddress,
+        );
+        testValidator = await TestValidatorContract.deployFrom0xArtifactAsync(
+            artifacts.TestValidator,
+            provider,
+            txDefaults,
+            signerAddress,
+        );
+        await web3Wrapper.awaitTransactionSuccessAsync(
+            await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, {
+                from: signerAddress,
+            }),
+            constants.AWAIT_TRANSACTION_MINED_MS,
+        );
 
         const defaultOrderParams = {
             ...constants.STATIC_ORDER_PARAMS,
@@ -45,8 +75,9 @@ describe('MixinSignatureValidator', () => {
             makerAssetData: assetProxyUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()),
             takerAssetData: assetProxyUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()),
         };
-        const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)];
-        orderFactory = new OrderFactory(privateKey, defaultOrderParams);
+        signerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)];
+        notSignerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(notSignerAddress)];
+        orderFactory = new OrderFactory(signerPrivateKey, defaultOrderParams);
     });
 
     beforeEach(async () => {
@@ -61,29 +92,369 @@ describe('MixinSignatureValidator', () => {
             signedOrder = orderFactory.newSignedOrder();
         });
 
-        it('should return true with a valid signature', async () => {
+        it('should revert with an empty signature', async () => {
+            const emptySignature = '0x';
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                signatureValidator.publicIsValidSignature.callAsync(
+                    orderHashHex,
+                    signedOrder.makerAddress,
+                    emptySignature,
+                ),
+            );
+        });
+
+        it('should revert with an unsupported signature type', async () => {
+            const unsupportedSignatureType = SignatureType.NSignatureTypes;
+            const unsupportedSignatureHex = `0x${unsupportedSignatureType}`;
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                signatureValidator.publicIsValidSignature.callAsync(
+                    orderHashHex,
+                    signedOrder.makerAddress,
+                    unsupportedSignatureHex,
+                ),
+            );
+        });
+
+        it('should revert when SignatureType=Illegal', async () => {
+            const unsupportedSignatureHex = `0x${SignatureType.Illegal}`;
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                signatureValidator.publicIsValidSignature.callAsync(
+                    orderHashHex,
+                    signedOrder.makerAddress,
+                    unsupportedSignatureHex,
+                ),
+            );
+        });
+
+        it('should return false when SignatureType=Invalid and signature has a length of zero', async () => {
+            const signatureHex = `0x${SignatureType.Invalid}`;
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signedOrder.makerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.false();
+        });
+
+        it('should revert when SignatureType=Invalid and signature length is non-zero', async () => {
+            const fillerData = ethUtil.toBuffer('0xdeadbeef');
+            const signatureType = ethUtil.toBuffer(`0x${SignatureType.Invalid}`);
+            const signatureBuffer = Buffer.concat([fillerData, signatureType]);
+            const signatureHex = ethUtil.bufferToHex(signatureBuffer);
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            return expectRevertOrAlwaysFailingTransactionAsync(
+                signatureValidator.publicIsValidSignature.callAsync(
+                    orderHashHex,
+                    signedOrder.makerAddress,
+                    signatureHex,
+                ),
+            );
+        });
+
+        it('should return true when SignatureType=EIP712 and signature is valid', async () => {
+            // Create EIP712 signature
             const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
+            const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey);
+            // Create 0x signature from EIP712 signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.EIP712}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.true();
+        });
+
+        it('should return true when SignatureType=EIP712 and signature is valid', async () => {
+            // Create EIP712 signature
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
+            const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey);
+            // Create 0x signature from EIP712 signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.EIP712}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                notSignerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.false();
+        });
+
+        it('should return true when SignatureType=EthSign and signature is valid', async () => {
+            // Create EthSign signature
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign);
+            const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex);
+            const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey);
+            // Create 0x signature from EthSign signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.EthSign}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.true();
+        });
+
+        it('should return true when SignatureType=EthSign and signature is invalid', async () => {
+            // Create EthSign signature
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign);
+            const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex);
+            const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey);
+            // Create 0x signature from EthSign signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.EthSign}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature.
+            // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress`
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                notSignerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.false();
+        });
+
+        it('should return true when SignatureType=Caller and signer is caller', async () => {
+            const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signerAddress,
+                signatureHex,
+                { from: signerAddress },
+            );
+            expect(isValidSignature).to.be.true();
+        });
+
+        it('should return false when SignatureType=Caller and signer is not caller', async () => {
+            const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signerAddress,
+                signatureHex,
+                { from: notSignerAddress },
+            );
+            expect(isValidSignature).to.be.false();
+        });
+
+        it('should return true when SignatureType=Wallet and signature is valid', async () => {
+            // Create EIP712 signature
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
+            const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey);
+            // Create 0x signature from EIP712 signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.Wallet}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                testWallet.address,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.true();
+        });
+
+        it('should return false when SignatureType=Wallet and signature is invalid', async () => {
+            // Create EIP712 signature using a private key that does not belong to the wallet owner.
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
+            const notWalletOwnerPrivateKey = notSignerPrivateKey;
+            const ecSignature = ethUtil.ecsign(orderHashBuffer, notWalletOwnerPrivateKey);
+            // Create 0x signature from EIP712 signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.Wallet}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                testWallet.address,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.false();
+        });
+
+        it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => {
+            const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`);
+            const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`);
+            const signature = Buffer.concat([validatorAddress, signatureType]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.true();
+        });
+
+        it('should return false when SignatureType=Validator, signature is invalid and validator is approved', async () => {
+            const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`);
+            const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`);
+            const signature = Buffer.concat([validatorAddress, signatureType]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                notSignerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.false();
+        });
+
+        it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => {
+            // Set approval of signature validator to false
+            await web3Wrapper.awaitTransactionSuccessAsync(
+                await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(
+                    testValidator.address,
+                    false,
+                    { from: signerAddress },
+                ),
+                constants.AWAIT_TRANSACTION_MINED_MS,
+            );
+            // Validate signature
+            const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`);
+            const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`);
+            const signature = Buffer.concat([validatorAddress, signatureType]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.false();
+            // Set approval of signature validator back to true
+            await web3Wrapper.awaitTransactionSuccessAsync(
+                await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(
+                    testValidator.address,
+                    true,
+                    { from: signerAddress },
+                ),
+                constants.AWAIT_TRANSACTION_MINED_MS,
+            );
+        });
+
+        it('should return true when SignatureType=Trezor and signature is valid', async () => {
+            // Create Trezor signature
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor);
+            const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex);
+            const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey);
+            // Create 0x signature from Trezor signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.Trezor}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                signerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.true();
+        });
+
+        it('should return true when SignatureType=Trezor and signature is invalid', async () => {
+            // Create Trezor signature
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor);
+            const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex);
+            const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey);
+            // Create 0x signature from Trezor signature
+            const signature = Buffer.concat([
+                ethUtil.toBuffer(ecSignature.v),
+                ecSignature.r,
+                ecSignature.s,
+                ethUtil.toBuffer(`0x${SignatureType.Trezor}`),
+            ]);
+            const signatureHex = ethUtil.bufferToHex(signature);
+            // Validate signature.
+            // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress`
+            const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
+                orderHashHex,
+                notSignerAddress,
+                signatureHex,
+            );
+            expect(isValidSignature).to.be.false();
+        });
+
+        it('should return true when SignatureType=Presigned and signer has presigned hash', async () => {
+            // Presign hash
+            const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+            await web3Wrapper.awaitTransactionSuccessAsync(
+                await signatureValidator.preSign.sendTransactionAsync(
+                    orderHashHex,
+                    signedOrder.makerAddress,
+                    signedOrder.signature,
+                ),
+                constants.AWAIT_TRANSACTION_MINED_MS,
+            );
+            // Validate presigned signature
+            const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`);
+            const signatureHex = ethUtil.bufferToHex(signature);
             const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
                 orderHashHex,
                 signedOrder.makerAddress,
-                signedOrder.signature,
+                signatureHex,
             );
             expect(isValidSignature).to.be.true();
         });
 
-        it('should return false with an invalid signature', async () => {
-            const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4));
-            const invalidR = ethUtil.sha3('invalidR');
-            const invalidS = ethUtil.sha3('invalidS');
-            const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`);
-            const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]);
-            const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`;
-            signedOrder.signature = invalidSigHex;
+        it('should return false when SignatureType=Presigned has not presigned hash', async () => {
+            const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`);
+            const signatureHex = ethUtil.bufferToHex(signature);
             const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
             const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
                 orderHashHex,
                 signedOrder.makerAddress,
-                signedOrder.signature,
+                signatureHex,
             );
             expect(isValidSignature).to.be.false();
         });