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
0xProject · Oct 9, 2018 · 9e8031d · 9e8031d
1 parent e1236a4
commit 9e8031d
Show file tree

Hide file tree

Showing 23 changed files with 208 additions and 150 deletions.
diff --git a/packages/0x.js/CHANGELOG.json b/packages/0x.js/CHANGELOG.json
@@ -7,7 +7,8 @@
                 "pr": 1102
             },
             {
-                "note": "Added `MetamaskSubprovider` to handle inconsistencies with signing.",
+                "note":
+                    "Added `MetamaskSubprovider` to handle inconsistencies in Metamask's signing JSON RPC endpoints.",
                 "pr": 1102
             },
             {

diff --git a/packages/0x.js/src/index.ts b/packages/0x.js/src/index.ts
@@ -90,6 +90,7 @@ export {
     JSONRPCRequestPayload,
     JSONRPCResponsePayload,
     JSONRPCErrorCallback,
+    JSONRPCResponseError,
     LogEntry,
     DecodedLogArgs,
     LogEntryEvent,

diff --git a/packages/contract-wrappers/src/utils/transaction_encoder.ts b/packages/contract-wrappers/src/utils/transaction_encoder.ts
@@ -33,7 +33,7 @@ export class TransactionEncoder {
             data,
         };
         const typedData = eip712Utils.createZeroExTransactionTypedData(executeTransactionData, exchangeAddress);
-        const eip712MessageBuffer = signTypedDataUtils.signTypedDataHash(typedData);
+        const eip712MessageBuffer = signTypedDataUtils.generateTypedDataHash(typedData);
         const messageHex = `0x${eip712MessageBuffer.toString('hex')}`;
         return messageHex;
     }

diff --git a/packages/contracts/test/utils/transaction_factory.ts b/packages/contracts/test/utils/transaction_factory.ts
@@ -25,7 +25,7 @@ export class TransactionFactory {
         };
 
         const typedData = eip712Utils.createZeroExTransactionTypedData(executeTransactionData, this._exchangeAddress);
-        const eip712MessageBuffer = signTypedDataUtils.signTypedDataHash(typedData);
+        const eip712MessageBuffer = signTypedDataUtils.generateTypedDataHash(typedData);
         const signature = signingUtils.signMessage(eip712MessageBuffer, this._privateKey, signatureType);
         const signedTx = {
             exchangeAddress: this._exchangeAddress,

diff --git a/packages/ethereum-types/CHANGELOG.json b/packages/ethereum-types/CHANGELOG.json
@@ -1,4 +1,13 @@
 [
+    {
+        "version": "1.1.0",
+        "changes": [
+            {
+                "note": "Add `JSONRPCResponseError` and error field on `JSONRPCResponsePayload`.",
+                "pr": 1102
+            }
+        ]
+    },
     {
         "timestamp": 1538693146,
         "version": "1.0.11",

diff --git a/packages/ethereum-types/src/index.ts b/packages/ethereum-types/src/index.ts
@@ -113,10 +113,16 @@ export interface JSONRPCRequestPayload {
     jsonrpc: string;
 }
 
+export interface JSONRPCResponseError {
+    message: string;
+    code: number;
+}
+
 export interface JSONRPCResponsePayload {
     result: any;
     id: number;
     jsonrpc: string;
+    error?: JSONRPCResponseError;
 }
 
 export interface AbstractBlock {

diff --git a/packages/json-schemas/schemas/eip712_typed_data.ts b/packages/json-schemas/schemas/eip712_typed_data.ts
@@ -1,4 +1,4 @@
-export const eip712TypedData = {
+export const eip712TypedDataSchema = {
     id: '/eip712TypedData',
     type: 'object',
     properties: {

diff --git a/packages/json-schemas/schemas/zero_ex_transaction_schema.ts b/packages/json-schemas/schemas/zero_ex_transaction_schema.ts
@@ -0,0 +1,10 @@
+export const zeroExTransactionSchema = {
+    id: '/zeroExTransactionSchema',
+    properties: {
+        data: { $ref: '/hexSchema' },
+        signerAddress: { $ref: '/addressSchema' },
+        salt: { $ref: '/numberSchema' },
+    },
+    required: ['data', 'salt', 'signerAddress'],
+    type: 'object',
+};
diff --git a/packages/json-schemas/src/schemas.ts b/packages/json-schemas/src/schemas.ts
@@ -2,7 +2,7 @@ import { addressSchema, hexSchema, numberSchema } from '../schemas/basic_type_sc
 import { blockParamSchema, blockRangeSchema } from '../schemas/block_range_schema';
 import { callDataSchema } from '../schemas/call_data_schema';
 import { ecSignatureParameterSchema, ecSignatureSchema } from '../schemas/ec_signature_schema';
-import { eip712TypedData } from '../schemas/eip712_typed_data';
+import { eip712TypedDataSchema } from '../schemas/eip712_typed_data';
 import { indexFilterValuesSchema } from '../schemas/index_filter_values_schema';
 import { orderCancellationRequestsSchema } from '../schemas/order_cancel_schema';
 import { orderFillOrKillRequestsSchema } from '../schemas/order_fill_or_kill_requests_schema';
@@ -32,6 +32,7 @@ import { relayerApiOrdersSchema } from '../schemas/relayer_api_orders_schema';
 import { signedOrdersSchema } from '../schemas/signed_orders_schema';
 import { tokenSchema } from '../schemas/token_schema';
 import { jsNumber, txDataSchema } from '../schemas/tx_data_schema';
+import { zeroExTransactionSchema } from '../schemas/zero_ex_transaction_schema';
 
 export const schemas = {
     numberSchema,
@@ -40,7 +41,7 @@ export const schemas = {
     hexSchema,
     ecSignatureParameterSchema,
     ecSignatureSchema,
-    eip712TypedData,
+    eip712TypedDataSchema,
     indexFilterValuesSchema,
     orderCancellationRequestsSchema,
     orderFillOrKillRequestsSchema,
@@ -70,4 +71,5 @@ export const schemas = {
     relayerApiOrdersChannelUpdateSchema,
     relayerApiOrdersResponseSchema,
     relayerApiAssetDataPairsSchema,
+    zeroExTransactionSchema,
 };
diff --git 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
@@ -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
@@ -194,19 +194,26 @@ 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
      *          must be available via the supplied Provider.
      * @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
@@ -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,
             );