Throw and handle errors from Providers.

In web3 wrapper when a response contains an error field we throw this rather than return response.result which is often undefined.
In Signature Utils we handle the error thrown when a user rejects the signing dialogue to prevent double signing.
Exposed the ZeroExTransaction JSON schema.
In Website only use the MetamaskSubprovider if we can detect the provider is Metamask

5 files changed, 116 insertions, 71 deletions

diff --git a/packages/order-utils/src/eip712_utils.ts b/packages/order-utils/src/eip712_utils.ts
index 43b421de7..56f736500 100644
--- a/packages/order-utils/src/eip712_utils.ts
+++ b/packages/order-utils/src/eip712_utils.ts
@@ -1,3 +1,5 @@
+import { assert } from '@0xproject/assert';
+import { schemas } from '@0xproject/json-schemas';
 import { EIP712Object, EIP712TypedData, EIP712Types, Order, ZeroExTransaction } from '@0xproject/types';
 import * as _ from 'lodash';
 
@@ -18,6 +20,8 @@ export const eip712Utils = {
         message: EIP712Object,
         exchangeAddress: string,
     ): EIP712TypedData => {
+        assert.isETHAddressHex('exchangeAddress', exchangeAddress);
+        assert.isString('primaryType', primaryType);
         const typedData = {
             types: {
                 EIP712Domain: constants.EIP712_DOMAIN_SCHEMA.parameters,
@@ -31,6 +35,7 @@ export const eip712Utils = {
             message,
             primaryType,
         };
+        assert.doesConformToSchema('typedData', typedData, schemas.eip712TypedDataSchema);
         return typedData;
     },
     /**
@@ -39,6 +44,7 @@ export const eip712Utils = {
      * @return  A typed data object
      */
     createOrderTypedData: (order: Order): EIP712TypedData => {
+        assert.doesConformToSchema('order', order, schemas.orderSchema, [schemas.hexSchema]);
         const normalizedOrder = _.mapValues(order, value => {
             return !_.isString(value) ? value.toString() : value;
         });
@@ -61,6 +67,8 @@ export const eip712Utils = {
         zeroExTransaction: ZeroExTransaction,
         exchangeAddress: string,
     ): EIP712TypedData => {
+        assert.isETHAddressHex('exchangeAddress', exchangeAddress);
+        assert.doesConformToSchema('zeroExTransaction', zeroExTransaction, schemas.zeroExTransactionSchema);
         const normalizedTransaction = _.mapValues(zeroExTransaction, value => {
             return !_.isString(value) ? value.toString() : value;
         });
diff --git a/packages/order-utils/src/order_hash.ts b/packages/order-utils/src/order_hash.ts
index a6dd6688c..b523a3523 100644
--- a/packages/order-utils/src/order_hash.ts
+++ b/packages/order-utils/src/order_hash.ts
@@ -52,7 +52,7 @@ export const orderHashUtils = {
      */
     getOrderHashBuffer(order: SignedOrder | Order): Buffer {
         const typedData = eip712Utils.createOrderTypedData(order);
-        const orderHashBuff = signTypedDataUtils.signTypedDataHash(typedData);
+        const orderHashBuff = signTypedDataUtils.generateTypedDataHash(typedData);
         return orderHashBuff;
     },
 };
diff --git a/packages/order-utils/src/signature_utils.ts b/packages/order-utils/src/signature_utils.ts
index e518b29a0..2605ccd32 100644
--- a/packages/order-utils/src/signature_utils.ts
+++ b/packages/order-utils/src/signature_utils.ts
@@ -194,7 +194,7 @@ export const signatureUtils = {
         }
     },
     /**
-     * Signs an order and returns its elliptic curve signature and signature type. First `eth_signTypedData` is requested
+     * Signs an order and returns a SignedOrder. First `eth_signTypedData` is requested
      * then a fallback to `eth_sign` if not available on the supplied provider.
      * @param   order The Order to sign.
      * @param   signerAddress   The hex encoded Ethereum address you wish to sign it with. This address
@@ -202,11 +202,18 @@ export const signatureUtils = {
      * @return  A SignedOrder containing the order and Elliptic curve signature with Signature Type.
      */
     async ecSignOrderAsync(provider: Provider, order: Order, signerAddress: string): Promise<SignedOrder> {
+        assert.doesConformToSchema('order', order, schemas.orderSchema, [schemas.hexSchema]);
         try {
-            const signedOrder = signatureUtils.ecSignTypedDataOrderAsync(provider, order, signerAddress);
+            const signedOrder = await signatureUtils.ecSignTypedDataOrderAsync(provider, order, signerAddress);
             return signedOrder;
         } catch (err) {
-            // Fallback to using EthSign when ethSignTypedData is not supported
+            // HACK: We are unable to handle specific errors thrown since provider is not an object
+            //       under our control. It could be Metamask Web3, Ethers, or any general RPC provider.
+            //       We check for a user denying the signature request in a way that supports Metamask and
+            //       Coinbase Wallet
+            if (err.message.includes('User denied message signature')) {
+                throw err;
+            }
             const orderHash = orderHashUtils.getOrderHashHex(order);
             const signatureHex = await signatureUtils.ecSignHashAsync(provider, orderHash, signerAddress);
             const signedOrder = {
@@ -217,7 +224,7 @@ export const signatureUtils = {
         }
     },
     /**
-     * Signs an order using `eth_signTypedData` and returns its elliptic curve signature and signature type.
+     * Signs an order using `eth_signTypedData` and returns a SignedOrder.
      * @param   order The Order to sign.
      * @param   signerAddress   The hex encoded Ethereum address you wish to sign it with. This address
      *          must be available via the supplied Provider.
@@ -226,6 +233,7 @@ export const signatureUtils = {
     async ecSignTypedDataOrderAsync(provider: Provider, order: Order, signerAddress: string): Promise<SignedOrder> {
         assert.isWeb3Provider('provider', provider);
         assert.isETHAddressHex('signerAddress', signerAddress);
+        assert.doesConformToSchema('order', order, schemas.orderSchema, [schemas.hexSchema]);
         const web3Wrapper = new Web3Wrapper(provider);
         await assert.isSenderAddressAsync('signerAddress', signerAddress, web3Wrapper);
         const normalizedSignerAddress = signerAddress.toLowerCase();
@@ -247,14 +255,16 @@ export const signatureUtils = {
         } catch (err) {
             // Detect if Metamask to transition users to the MetamaskSubprovider
             if ((provider as any).isMetaMask) {
-                throw new Error(`Unsupported Provider, please use MetamaskSubprovider: ${err.message}`);
+                throw new Error(
+                    `MetaMask provider must be wrapped in a MetamaskSubprovider (from the '@0xproject/subproviders' package) in order to work with this method.`,
+                );
             } else {
                 throw err;
             }
         }
     },
     /**
-     * Signs a hash and returns its elliptic curve signature and signature type.
+     * Signs a hash using `eth_sign` and returns its elliptic curve signature and signature type.
      * @param   msgHash       Hex encoded message to sign.
      * @param   signerAddress   The hex encoded Ethereum address you wish to sign it with. This address
      *          must be available via the supplied Provider.
@@ -303,7 +313,9 @@ export const signatureUtils = {
         }
         // Detect if Metamask to transition users to the MetamaskSubprovider
         if ((provider as any).isMetaMask) {
-            throw new Error('Unsupported Provider, please use MetamaskSubprovider.');
+            throw new Error(
+                `MetaMask provider must be wrapped in a MetamaskSubprovider (from the '@0xproject/subproviders' package) in order to work with this method.`,
+            );
         } else {
             throw new Error(OrderError.InvalidSignature);
         }
diff --git a/packages/order-utils/test/eip712_utils_test.ts b/packages/order-utils/test/eip712_utils_test.ts
index dc76595db..d65cabe9c 100644
--- a/packages/order-utils/test/eip712_utils_test.ts
+++ b/packages/order-utils/test/eip712_utils_test.ts
@@ -33,7 +33,7 @@ describe('EIP712 Utils', () => {
                 {
                     salt: new BigNumber('0'),
                     data: constants.NULL_BYTES,
-                    signerAddress: constants.NULL_BYTES,
+                    signerAddress: constants.NULL_ADDRESS,
                 },
                 constants.NULL_ADDRESS,
             );
diff --git a/packages/order-utils/test/signature_utils_test.ts b/packages/order-utils/test/signature_utils_test.ts
index 7f6987f6a..f2d6790fb 100644
--- a/packages/order-utils/test/signature_utils_test.ts
+++ b/packages/order-utils/test/signature_utils_test.ts
@@ -17,6 +17,28 @@ chaiSetup.configure();
 const expect = chai.expect;
 
 describe('Signature utils', () => {
+    let makerAddress: string;
+    const fakeExchangeContractAddress = '0x1dc4c1cefef38a777b15aa20260a54e584b16c48';
+    let order: Order;
+    before(async () => {
+        const availableAddreses = await web3Wrapper.getAvailableAddressesAsync();
+        makerAddress = availableAddreses[0];
+        order = {
+            makerAddress,
+            takerAddress: constants.NULL_ADDRESS,
+            senderAddress: constants.NULL_ADDRESS,
+            feeRecipientAddress: constants.NULL_ADDRESS,
+            makerAssetData: constants.NULL_ADDRESS,
+            takerAssetData: constants.NULL_ADDRESS,
+            exchangeAddress: fakeExchangeContractAddress,
+            salt: new BigNumber(0),
+            makerFee: new BigNumber(0),
+            takerFee: new BigNumber(0),
+            makerAssetAmount: new BigNumber(0),
+            takerAssetAmount: new BigNumber(0),
+            expirationTimeSeconds: new BigNumber(0),
+        };
+    });
     describe('#isValidSignatureAsync', () => {
         let dataHex = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0';
         const ethSignSignature =
@@ -117,54 +139,54 @@ describe('Signature utils', () => {
         });
     });
     describe('#ecSignOrderAsync', () => {
-        let makerAddress: string;
-        const fakeExchangeContractAddress = '0x1dc4c1cefef38a777b15aa20260a54e584b16c48';
-        let order: Order;
-        before(async () => {
-            const availableAddreses = await web3Wrapper.getAvailableAddressesAsync();
-            makerAddress = availableAddreses[0];
-            order = {
-                makerAddress,
-                takerAddress: constants.NULL_ADDRESS,
-                senderAddress: constants.NULL_ADDRESS,
-                feeRecipientAddress: constants.NULL_ADDRESS,
-                makerAssetData: constants.NULL_ADDRESS,
-                takerAssetData: constants.NULL_ADDRESS,
-                exchangeAddress: fakeExchangeContractAddress,
-                salt: new BigNumber(0),
-                makerFee: new BigNumber(0),
-                takerFee: new BigNumber(0),
-                makerAssetAmount: new BigNumber(0),
-                takerAssetAmount: new BigNumber(0),
-                expirationTimeSeconds: new BigNumber(0),
+        it('should default to eth_sign if eth_signTypedData is unavailable', async () => {
+            const expectedSignature =
+                '0x1c3582f06356a1314dbf1c0e534c4d8e92e59b056ee607a7ff5a825f5f2cc5e6151c5cc7fdd420f5608e4d5bef108e42ad90c7a4b408caef32e24374cf387b0d7603';
+
+            const fakeProvider = {
+                async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise<void> {
+                    if (payload.method === 'eth_signTypedData') {
+                        callback(new Error('Internal RPC Error'));
+                    } else if (payload.method === 'eth_sign') {
+                        const [address, message] = payload.params;
+                        const signature = await web3Wrapper.signMessageAsync(address, message);
+                        callback(null, {
+                            id: 42,
+                            jsonrpc: '2.0',
+                            result: signature,
+                        });
+                    } else {
+                        callback(null, { id: 42, jsonrpc: '2.0', result: [makerAddress] });
+                    }
+                },
             };
+            const signedOrder = await signatureUtils.ecSignOrderAsync(fakeProvider, order, makerAddress);
+            expect(signedOrder.signature).to.equal(expectedSignature);
         });
-        it('should result in the same signature as signing order hash without prefix', async () => {
-            const orderHashHex = orderHashUtils.getOrderHashHex(order);
-            const sig = ethUtil.ecsign(
-                ethUtil.toBuffer(orderHashHex),
-                Buffer.from('F2F48EE19680706196E2E339E5DA3491186E0C4C5030670656B0E0164837257D', 'hex'),
-            );
-            const signatureBuffer = Buffer.concat([
-                ethUtil.toBuffer(sig.v),
-                ethUtil.toBuffer(sig.r),
-                ethUtil.toBuffer(sig.s),
-                ethUtil.toBuffer(SignatureType.EIP712),
-            ]);
-            const signatureHex = `0x${signatureBuffer.toString('hex')}`;
-            const signedOrder = await signatureUtils.ecSignOrderAsync(provider, order, makerAddress);
-            const isValidSignature = await signatureUtils.isValidSignatureAsync(
-                provider,
-                orderHashHex,
-                signedOrder.signature,
-                makerAddress,
+        it('should throw if the user denies the signing request', async () => {
+            const fakeProvider = {
+                async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise<void> {
+                    if (payload.method === 'eth_signTypedData') {
+                        callback(new Error('User denied message signature'));
+                    } else if (payload.method === 'eth_sign') {
+                        const [address, message] = payload.params;
+                        const signature = await web3Wrapper.signMessageAsync(address, message);
+                        callback(null, {
+                            id: 42,
+                            jsonrpc: '2.0',
+                            result: signature,
+                        });
+                    } else {
+                        callback(null, { id: 42, jsonrpc: '2.0', result: [makerAddress] });
+                    }
+                },
+            };
+            expect(signatureUtils.ecSignOrderAsync(fakeProvider, order, makerAddress)).to.to.be.rejectedWith(
+                'User denied message signature',
             );
-            expect(signatureHex).to.eq(signedOrder.signature);
-            expect(isValidSignature).to.eq(true);
         });
     });
     describe('#ecSignHashAsync', () => {
-        let makerAddress: string;
         before(async () => {
             const availableAddreses = await web3Wrapper.getAvailableAddressesAsync();
             makerAddress = availableAddreses[0];
@@ -239,27 +261,30 @@ describe('Signature utils', () => {
         });
     });
     describe('#ecSignTypedDataOrderAsync', () => {
-        let makerAddress: string;
-        const fakeExchangeContractAddress = '0x1dc4c1cefef38a777b15aa20260a54e584b16c48';
-        let order: Order;
-        before(async () => {
-            const availableAddreses = await web3Wrapper.getAvailableAddressesAsync();
-            makerAddress = availableAddreses[0];
-            order = {
+        it('should result in the same signature as signing the order hash without an ethereum message prefix', async () => {
+            // Note: Since order hash is an EIP712 hash the result of a valid EIP712 signature
+            //       of order hash is the same as signing the order without the Ethereum Message prefix.
+            const orderHashHex = orderHashUtils.getOrderHashHex(order);
+            const sig = ethUtil.ecsign(
+                ethUtil.toBuffer(orderHashHex),
+                Buffer.from('F2F48EE19680706196E2E339E5DA3491186E0C4C5030670656B0E0164837257D', 'hex'),
+            );
+            const signatureBuffer = Buffer.concat([
+                ethUtil.toBuffer(sig.v),
+                ethUtil.toBuffer(sig.r),
+                ethUtil.toBuffer(sig.s),
+                ethUtil.toBuffer(SignatureType.EIP712),
+            ]);
+            const signatureHex = `0x${signatureBuffer.toString('hex')}`;
+            const signedOrder = await signatureUtils.ecSignTypedDataOrderAsync(provider, order, makerAddress);
+            const isValidSignature = await signatureUtils.isValidSignatureAsync(
+                provider,
+                orderHashHex,
+                signedOrder.signature,
                 makerAddress,
-                takerAddress: constants.NULL_ADDRESS,
-                senderAddress: constants.NULL_ADDRESS,
-                feeRecipientAddress: constants.NULL_ADDRESS,
-                makerAssetData: constants.NULL_ADDRESS,
-                takerAssetData: constants.NULL_ADDRESS,
-                exchangeAddress: fakeExchangeContractAddress,
-                salt: new BigNumber(0),
-                makerFee: new BigNumber(0),
-                takerFee: new BigNumber(0),
-                makerAssetAmount: new BigNumber(0),
-                takerAssetAmount: new BigNumber(0),
-                expirationTimeSeconds: new BigNumber(0),
-            };
+            );
+            expect(signatureHex).to.eq(signedOrder.signature);
+            expect(isValidSignature).to.eq(true);
         });
         it('should return the correct Signature for signatureHex concatenated as R + S + V', async () => {
             const expectedSignature =