Introduce SignerProviderType

This allows the developer to indicate the nuanced signer provider. Some have different implementations (trezor, ledger) and others have different implementations (metamask). Breaking the abstraction of eth_sign. EthSign assumes a spec compliant implementation and can be used as a default

4 files changed, 62 insertions, 65 deletions

diff --git a/packages/order-utils/src/index.ts b/packages/order-utils/src/index.ts
index 858f500c6..681fbc904 100644
--- a/packages/order-utils/src/index.ts
+++ b/packages/order-utils/src/index.ts
@@ -13,15 +13,7 @@ export { orderFactory } from './order_factory';
 export { constants } from './constants';
 export { crypto } from './crypto';
 export { generatePseudoRandomSalt } from './salt';
-export {
-    CreateOrderOpts,
-    OrderError,
-    MessagePrefixType,
-    MessagePrefixOpts,
-    EIP712Parameter,
-    EIP712Schema,
-    EIP712Types,
-} from './types';
+export { CreateOrderOpts, OrderError, EIP712Parameter, EIP712Schema, EIP712Types } from './types';
 export { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher';
 export { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher';
 export { BalanceAndProxyAllowanceLazyStore } from './store/balance_and_proxy_allowance_lazy_store';
diff --git a/packages/order-utils/src/order_factory.ts b/packages/order-utils/src/order_factory.ts
index 14727fd97..bd6bb84b8 100644
--- a/packages/order-utils/src/order_factory.ts
+++ b/packages/order-utils/src/order_factory.ts
@@ -1,14 +1,13 @@
-import { ECSignature, Order, SignedOrder } from '@0xproject/types';
+import { Order, SignedOrder, SignerProviderType } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import { Provider } from 'ethereum-types';
-import * as ethUtil from 'ethereumjs-util';
 import * as _ from 'lodash';
 
 import { constants } from './constants';
 import { orderHashUtils } from './order_hash';
 import { generatePseudoRandomSalt } from './salt';
 import { ecSignOrderHashAsync } from './signature_utils';
-import { CreateOrderOpts, MessagePrefixType } from './types';
+import { CreateOrderOpts } from './types';
 
 export const orderFactory = {
     createOrder(
@@ -59,16 +58,12 @@ export const orderFactory = {
             createOrderOpts,
         );
         const orderHash = orderHashUtils.getOrderHashHex(order);
-        const messagePrefixOpts = {
-            prefixType: MessagePrefixType.EthSign,
-            shouldAddPrefixBeforeCallingEthSign: false,
-        };
-        const ecSignature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, messagePrefixOpts);
-        const signature = getVRSHexString(ecSignature);
+        const signature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, SignerProviderType.EthSign);
         const signedOrder: SignedOrder = _.assign(order, { signature });
         return signedOrder;
     },
 };
+<<<<<<< HEAD
 
 function generateDefaultCreateOrderOpts(): {
     takerAddress: string;
@@ -102,3 +97,5 @@ function intToHex(i: number): string {
     const hex = ethUtil.bufferToHex(ethUtil.toBuffer(i));
     return hex;
 }
+=======
+>>>>>>> Introduce SignerProviderType
diff --git a/packages/order-utils/src/signature_utils.ts b/packages/order-utils/src/signature_utils.ts
index 26fb24705..3237259c9 100644
--- a/packages/order-utils/src/signature_utils.ts
+++ b/packages/order-utils/src/signature_utils.ts
@@ -1,5 +1,5 @@
 import { schemas } from '@0xproject/json-schemas';
-import { ECSignature, SignatureType, ValidatorSignature } from '@0xproject/types';
+import { ECSignature, SignatureType, SignerProviderType, ValidatorSignature } from '@0xproject/types';
 import { Web3Wrapper } from '@0xproject/web3-wrapper';
 import { Provider } from 'ethereum-types';
 import * as ethUtil from 'ethereumjs-util';
@@ -10,7 +10,7 @@ import { assert } from './assert';
 import { ExchangeContract } from './generated_contract_wrappers/exchange';
 import { IValidatorContract } from './generated_contract_wrappers/i_validator';
 import { IWalletContract } from './generated_contract_wrappers/i_wallet';
-import { MessagePrefixOpts, MessagePrefixType, OrderError } from './types';
+import { OrderError } from './types';
 import { utils } from './utils';
 
 /**
@@ -48,7 +48,7 @@ export async function isValidSignatureAsync(
 
         case SignatureType.EthSign: {
             const ecSignature = parseECSignature(signature);
-            const prefixedMessageHex = addSignedMessagePrefix(data, MessagePrefixType.EthSign);
+            const prefixedMessageHex = addSignedMessagePrefix(data, SignerProviderType.EthSign);
             return isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
         }
 
@@ -72,7 +72,7 @@ export async function isValidSignatureAsync(
         }
 
         case SignatureType.Trezor: {
-            const prefixedMessageHex = addSignedMessagePrefix(data, MessagePrefixType.Trezor);
+            const prefixedMessageHex = addSignedMessagePrefix(data, SignerProviderType.Trezor);
             const ecSignature = parseECSignature(signature);
             return isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
         }
@@ -191,22 +191,22 @@ export function isValidECSignature(data: string, signature: ECSignature, signerA
 }
 
 /**
- * Signs an orderHash and returns it's elliptic curve signature.
+ * Signs an orderHash and returns it's elliptic curve signature and signature type.
  * This method currently supports TestRPC, Geth and Parity above and below V1.6.6
  * @param   orderHash       Hex encoded orderHash to sign.
  * @param   signerAddress   The hex encoded Ethereum address you wish to sign it with. This address
  *          must be available via the Provider supplied to 0x.js.
- * @param   hashPrefixOpts Different signers add/require different prefixes be appended to the message being signed.
+ * @param   messagePrefixOpts Different signers add/require different prefixes be prepended to the message being signed.
  *          Since we cannot know ahead of time which signer you are using, you must supply both a prefixType and
  *          whether it must be added before calling `eth_sign` (some signers add it themselves)
- * @return  An object containing the Elliptic curve signature parameters generated by signing the orderHash.
+ * @return  A hex encoded string containing the Elliptic curve signature generated by signing the orderHash and the Signature Type.
  */
 export async function ecSignOrderHashAsync(
     provider: Provider,
     orderHash: string,
     signerAddress: string,
-    messagePrefixOpts: MessagePrefixOpts,
-): Promise<ECSignature> {
+    signerProviderType: SignerProviderType,
+): Promise<string> {
     assert.isWeb3Provider('provider', provider);
     assert.isHexString('orderHash', orderHash);
     assert.isETHAddressHex('signerAddress', signerAddress);
@@ -215,8 +215,9 @@ export async function ecSignOrderHashAsync(
     const normalizedSignerAddress = signerAddress.toLowerCase();
 
     let msgHashHex = orderHash;
-    const prefixedMsgHashHex = addSignedMessagePrefix(orderHash, messagePrefixOpts.prefixType);
-    if (messagePrefixOpts.shouldAddPrefixBeforeCallingEthSign) {
+    const prefixedMsgHashHex = addSignedMessagePrefix(orderHash, signerProviderType);
+    // Metamask incorrectly implements eth_sign and does not prefix the message as per the spec
+    if (signerProviderType === SignerProviderType.Metamask) {
         msgHashHex = prefixedMsgHashHex;
     }
     const signature = await web3Wrapper.signMessageAsync(normalizedSignerAddress, msgHashHex);
@@ -231,7 +232,8 @@ export async function ecSignOrderHashAsync(
     if (_.includes(validVParamValues, ecSignatureVRS.v)) {
         const isValidVRSSignature = isValidECSignature(prefixedMsgHashHex, ecSignatureVRS, normalizedSignerAddress);
         if (isValidVRSSignature) {
-            return ecSignatureVRS;
+            const convertedSignatureHex = convertECSignatureToSignatureHex(ecSignatureVRS, signerProviderType);
+            return convertedSignatureHex;
         }
     }
 
@@ -239,13 +241,45 @@ export async function ecSignOrderHashAsync(
     if (_.includes(validVParamValues, ecSignatureRSV.v)) {
         const isValidRSVSignature = isValidECSignature(prefixedMsgHashHex, ecSignatureRSV, normalizedSignerAddress);
         if (isValidRSVSignature) {
-            return ecSignatureRSV;
+            const convertedSignatureHex = convertECSignatureToSignatureHex(ecSignatureRSV, signerProviderType);
+            return convertedSignatureHex;
         }
     }
 
     throw new Error(OrderError.InvalidSignature);
 }
-
+/**
+ * Combines ECSignature with V,R,S and the relevant signature type for use in 0x protocol
+ * @param ecSignature The ECSignature of the signed data
+ * @param messagePrefixType The MessagePrefixType of the signed data
+ * @return Hex encoded string of signature with Signature Type
+ */
+export function convertECSignatureToSignatureHex(
+    ecSignature: ECSignature,
+    signerProviderType: SignerProviderType,
+): string {
+    const signatureBuffer = Buffer.concat([
+        ethUtil.toBuffer(ecSignature.v),
+        ethUtil.toBuffer(ecSignature.r),
+        ethUtil.toBuffer(ecSignature.s),
+    ]);
+    const signatureHex = `0x${signatureBuffer.toString('hex')}`;
+    const signatureType =
+        signerProviderType === SignerProviderType.Trezor ? SignatureType.Trezor : SignatureType.EthSign;
+    const signatureWithType = convertToSignatureWithType(signatureHex, signatureType);
+    return signatureWithType;
+}
+/**
+ * Combines the signature proof and the Signature Type.
+ * @param signature The hex encoded signature proof
+ * @param type The signature type, i.e EthSign, Trezor, Wallet etc.
+ * @return Hex encoded string of signature proof with Signature Type
+ */
+export function convertToSignatureWithType(signature: string, type: SignatureType): string {
+    const signatureBuffer = Buffer.concat([ethUtil.toBuffer(signature), ethUtil.toBuffer(type)]);
+    const signatureHex = `0x${signatureBuffer.toString('hex')}`;
+    return signatureHex;
+}
 /**
  * Adds the relevant prefix to the message being signed.
  * @param message Message to sign
@@ -253,29 +287,25 @@ export async function ecSignOrderHashAsync(
  *                          specific message prefixes.
  * @return Prefixed message
  */
-export function addSignedMessagePrefix(message: string, messagePrefixType: MessagePrefixType): string {
+export function addSignedMessagePrefix(message: string, signerProviderType: SignerProviderType): string {
     assert.isString('message', message);
-    assert.doesBelongToStringEnum('messagePrefixType', messagePrefixType, MessagePrefixType);
-    switch (messagePrefixType) {
-        case MessagePrefixType.None:
-            return message;
-
-        case MessagePrefixType.EthSign: {
+    assert.doesBelongToStringEnum('signerProviderType', signerProviderType, SignerProviderType);
+    switch (signerProviderType) {
+        case SignerProviderType.Metamask:
+        case SignerProviderType.EthSign: {
             const msgBuff = ethUtil.toBuffer(message);
             const prefixedMsgBuff = ethUtil.hashPersonalMessage(msgBuff);
             const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
             return prefixedMsgHex;
         }
-
-        case MessagePrefixType.Trezor: {
+        case SignerProviderType.Trezor: {
             const msgBuff = ethUtil.toBuffer(message);
             const prefixedMsgBuff = hashTrezorPersonalMessage(msgBuff);
             const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
             return prefixedMsgHex;
         }
-
         default:
-            throw new Error(`Unrecognized MessagePrefixType: ${messagePrefixType}`);
+            throw new Error(`Unrecognized SignerProviderType: ${signerProviderType}`);
     }
 }
 
diff --git a/packages/order-utils/src/types.ts b/packages/order-utils/src/types.ts
index f44e94349..1fbd8cf7b 100644
--- a/packages/order-utils/src/types.ts
+++ b/packages/order-utils/src/types.ts
@@ -4,28 +4,6 @@ export enum OrderError {
     InvalidSignature = 'INVALID_SIGNATURE',
 }
 
-/**
- * The requisite message prefix (is any) to add to an `eth_sign` request.
- */
-export enum MessagePrefixType {
-    None = 'NONE',
-    EthSign = 'ETH_SIGN',
-    Trezor = 'TREZOR',
-}
-
-/**
- * Options related to message prefixing of messages sent to `eth_sign`
- * Some signers prepend a message prefix (e.g Parity Signer, Ledger, TestRPC), while
- * others require it already be prepended (e.g Metamask). In addition, different signers
- * expect slightly different prefixes (See: https://github.com/ethereum/go-ethereum/issues/14794).
- * Depending on the signer that will receive your signing request, you must specify the
- * desired prefix and whether it should be added before making the `eth_sign` request.
- */
-export interface MessagePrefixOpts {
-    prefixType: MessagePrefixType;
-    shouldAddPrefixBeforeCallingEthSign: boolean;
-}
-
 export enum TradeSide {
     Maker = 'maker',
     Taker = 'taker',