Rename from SignerProviderType.EthSign to SignerType.Default

3 files changed, 98 insertions, 92 deletions

diff --git a/packages/order-utils/src/order_factory.ts b/packages/order-utils/src/order_factory.ts
index bd6bb84b8..4a6f3924b 100644
--- a/packages/order-utils/src/order_factory.ts
+++ b/packages/order-utils/src/order_factory.ts
@@ -1,4 +1,4 @@
-import { Order, SignedOrder, SignerProviderType } from '@0xproject/types';
+import { Order, SignedOrder, SignerType } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import { Provider } from 'ethereum-types';
 import * as _ from 'lodash';
@@ -58,12 +58,11 @@ export const orderFactory = {
             createOrderOpts,
         );
         const orderHash = orderHashUtils.getOrderHashHex(order);
-        const signature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, SignerProviderType.EthSign);
+        const signature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, SignerType.Default);
         const signedOrder: SignedOrder = _.assign(order, { signature });
         return signedOrder;
     },
 };
-<<<<<<< HEAD
 
 function generateDefaultCreateOrderOpts(): {
     takerAddress: string;
@@ -84,18 +83,3 @@ function generateDefaultCreateOrderOpts(): {
         expirationTimeSeconds: constants.INFINITE_TIMESTAMP_SEC,
     };
 }
-
-function getVRSHexString(ecSignature: ECSignature): string {
-    const ETH_SIGN_SIGNATURE_TYPE = '03';
-    const vrs = `${intToHex(ecSignature.v)}${ethUtil.stripHexPrefix(ecSignature.r)}${ethUtil.stripHexPrefix(
-        ecSignature.s,
-    )}${ETH_SIGN_SIGNATURE_TYPE}`;
-    return vrs;
-}
-
-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 db5a35f80..5a58edf38 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, SignerProviderType, ValidatorSignature } from '@0xproject/types';
+import { ECSignature, SignatureType, SignerType, ValidatorSignature } from '@0xproject/types';
 import { Web3Wrapper } from '@0xproject/web3-wrapper';
 import { Provider } from 'ethereum-types';
 import * as ethUtil from 'ethereumjs-util';
@@ -48,7 +48,7 @@ export async function isValidSignatureAsync(
 
         case SignatureType.EthSign: {
             const ecSignature = parseECSignature(signature);
-            const prefixedMessageHex = addSignedMessagePrefix(data, SignerProviderType.EthSign);
+            const prefixedMessageHex = addSignedMessagePrefix(data, SignerType.Default);
             return isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
         }
 
@@ -72,7 +72,7 @@ export async function isValidSignatureAsync(
         }
 
         case SignatureType.Trezor: {
-            const prefixedMessageHex = addSignedMessagePrefix(data, SignerProviderType.Trezor);
+            const prefixedMessageHex = addSignedMessagePrefix(data, SignerType.Trezor);
             const ecSignature = parseECSignature(signature);
             return isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
         }
@@ -196,15 +196,15 @@ export function isValidECSignature(data: string, signature: ECSignature, signerA
  * @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   signerProviderType Different signers add/require different prefixes to be prepended to the message being signed.
- *          Since we cannot know ahead of time which signer you are using, you must supply a SignerProviderType.
+ * @param   signerType Different signers add/require different prefixes to be prepended to the message being signed.
+ *          Since we cannot know ahead of time which signer you are using, you must supply a SignerType.
  * @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,
-    signerProviderType: SignerProviderType,
+    signerType: SignerType,
 ): Promise<string> {
     assert.isWeb3Provider('provider', provider);
     assert.isHexString('orderHash', orderHash);
@@ -214,9 +214,9 @@ export async function ecSignOrderHashAsync(
     const normalizedSignerAddress = signerAddress.toLowerCase();
 
     let msgHashHex = orderHash;
-    const prefixedMsgHashHex = addSignedMessagePrefix(orderHash, signerProviderType);
+    const prefixedMsgHashHex = addSignedMessagePrefix(orderHash, signerType);
     // Metamask incorrectly implements eth_sign and does not prefix the message as per the spec
-    if (signerProviderType === SignerProviderType.Metamask) {
+    if (signerType === SignerType.Metamask) {
         msgHashHex = prefixedMsgHashHex;
     }
     const signature = await web3Wrapper.signMessageAsync(normalizedSignerAddress, msgHashHex);
@@ -225,22 +225,22 @@ export async function ecSignOrderHashAsync(
     // v + r + s OR r + s + v, and different clients (even different versions of the same client)
     // return the signature params in different orders. In order to support all client implementations,
     // we parse the signature in both ways, and evaluate if either one is a valid signature.
+    // r + s + v is the most prevalent format from eth_sign, so we attempt this first.
     // tslint:disable-next-line:custom-no-magic-numbers
     const validVParamValues = [27, 28];
-    const ecSignatureVRS = parseSignatureHexAsVRS(signature);
-    if (_.includes(validVParamValues, ecSignatureVRS.v)) {
-        const isValidVRSSignature = isValidECSignature(prefixedMsgHashHex, ecSignatureVRS, normalizedSignerAddress);
-        if (isValidVRSSignature) {
-            const convertedSignatureHex = convertECSignatureToSignatureHex(ecSignatureVRS, signerProviderType);
-            return convertedSignatureHex;
-        }
-    }
-
     const ecSignatureRSV = parseSignatureHexAsRSV(signature);
     if (_.includes(validVParamValues, ecSignatureRSV.v)) {
         const isValidRSVSignature = isValidECSignature(prefixedMsgHashHex, ecSignatureRSV, normalizedSignerAddress);
         if (isValidRSVSignature) {
-            const convertedSignatureHex = convertECSignatureToSignatureHex(ecSignatureRSV, signerProviderType);
+            const convertedSignatureHex = convertECSignatureToSignatureHex(ecSignatureRSV, signerType);
+            return convertedSignatureHex;
+        }
+    }
+    const ecSignatureVRS = parseSignatureHexAsVRS(signature);
+    if (_.includes(validVParamValues, ecSignatureVRS.v)) {
+        const isValidVRSSignature = isValidECSignature(prefixedMsgHashHex, ecSignatureVRS, normalizedSignerAddress);
+        if (isValidVRSSignature) {
+            const convertedSignatureHex = convertECSignatureToSignatureHex(ecSignatureVRS, signerType);
             return convertedSignatureHex;
         }
     }
@@ -250,23 +250,32 @@ export async function ecSignOrderHashAsync(
 /**
  * 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 signerProviderType The SignerProviderType of the signed data
- * @return Hex encoded string of signature with Signature Type
+ * @param signerType The SignerType of the signed data
+ * @return Hex encoded string of signature (v,r,s) with Signature Type
  */
-export function convertECSignatureToSignatureHex(
-    ecSignature: ECSignature,
-    signerProviderType: SignerProviderType,
-): string {
+export function convertECSignatureToSignatureHex(ecSignature: ECSignature, signerType: SignerType): 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;
+    switch (signerType) {
+        case SignerType.Metamask:
+        case SignerType.Ledger:
+        case SignerType.Default: {
+            const signatureType = SignatureType.EthSign;
+            const signatureWithType = convertToSignatureWithType(signatureHex, signatureType);
+            return signatureWithType;
+        }
+        case SignerType.Trezor: {
+            const signatureType = SignatureType.Trezor;
+            const signatureWithType = convertToSignatureWithType(signatureHex, signatureType);
+            return signatureWithType;
+        }
+        default:
+            throw new Error(`Unrecognized SignerType: ${signerType}`);
+    }
 }
 /**
  * Combines the signature proof and the Signature Type.
@@ -282,30 +291,30 @@ export function convertToSignatureWithType(signature: string, signatureType: Sig
 /**
  * Adds the relevant prefix to the message being signed.
  * @param message Message to sign
- * @param signerProviderType The type of message prefix to add for a given SignerProviderType. Different signers expect
+ * @param signerType The type of message prefix to add for a given SignerType. Different signers expect
  *                          specific message prefixes.
  * @return Prefixed message
  */
-export function addSignedMessagePrefix(message: string, signerProviderType: SignerProviderType): string {
+export function addSignedMessagePrefix(message: string, signerType: SignerType = SignerType.Default): string {
     assert.isString('message', message);
-    assert.doesBelongToStringEnum('signerProviderType', signerProviderType, SignerProviderType);
-    switch (signerProviderType) {
-        case SignerProviderType.Metamask:
-        case SignerProviderType.Ledger:
-        case SignerProviderType.EthSign: {
+    assert.doesBelongToStringEnum('signerType', signerType, SignerType);
+    switch (signerType) {
+        case SignerType.Metamask:
+        case SignerType.Ledger:
+        case SignerType.Default: {
             const msgBuff = ethUtil.toBuffer(message);
             const prefixedMsgBuff = ethUtil.hashPersonalMessage(msgBuff);
             const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
             return prefixedMsgHex;
         }
-        case SignerProviderType.Trezor: {
+        case SignerType.Trezor: {
             const msgBuff = ethUtil.toBuffer(message);
             const prefixedMsgBuff = hashTrezorPersonalMessage(msgBuff);
             const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
             return prefixedMsgHex;
         }
         default:
-            throw new Error(`Unrecognized SignerProviderType: ${signerProviderType}`);
+            throw new Error(`Unrecognized SignerType: ${signerType}`);
     }
 }
 
diff --git a/packages/order-utils/test/signature_utils_test.ts b/packages/order-utils/test/signature_utils_test.ts
index de76e82ac..a25d2afd6 100644
--- a/packages/order-utils/test/signature_utils_test.ts
+++ b/packages/order-utils/test/signature_utils_test.ts
@@ -1,4 +1,4 @@
-import { SignerProviderType } from '@0xproject/types';
+import { SignerType } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import * as chai from 'chai';
 import { JSONRPCErrorCallback, JSONRPCRequestPayload } from 'ethereum-types';
@@ -7,7 +7,7 @@ import 'mocha';
 import * as Sinon from 'sinon';
 
 import { ecSignOrderHashAsync, generatePseudoRandomSalt } from '../src';
-import { isValidECSignature, isValidSignatureAsync } from '../src/signature_utils';
+import { convertECSignatureToSignatureHex, isValidECSignature, isValidSignatureAsync } from '../src/signature_utils';
 
 import { chaiSetup } from './utils/chai_setup';
 import { provider, web3Wrapper } from './utils/web3_wrapper';
@@ -124,12 +124,7 @@ describe('Signature utils', () => {
             const orderHash = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0';
             const expectedSignature =
                 '0x1b61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc3340349190569279751135161d22529dc25add4f6069af05be04cacbda2ace225403';
-            const ecSignature = await ecSignOrderHashAsync(
-                provider,
-                orderHash,
-                makerAddress,
-                SignerProviderType.EthSign,
-            );
+            const ecSignature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, SignerType.Default);
             expect(ecSignature).to.equal(expectedSignature);
         });
         it('should return the correct Signature for signatureHex concatenated as R + S + V', async () => {
@@ -155,12 +150,7 @@ describe('Signature utils', () => {
                     }
                 },
             };
-            const ecSignature = await ecSignOrderHashAsync(
-                fakeProvider,
-                orderHash,
-                makerAddress,
-                SignerProviderType.EthSign,
-            );
+            const ecSignature = await ecSignOrderHashAsync(fakeProvider, orderHash, makerAddress, SignerType.Default);
             expect(ecSignature).to.equal(expectedSignature);
         });
         it('should return the correct Signature for signatureHex concatenated as V + R + S', async () => {
@@ -183,32 +173,28 @@ describe('Signature utils', () => {
                 },
             };
 
-            const ecSignature = await ecSignOrderHashAsync(
-                fakeProvider,
-                orderHash,
-                makerAddress,
-                SignerProviderType.EthSign,
-            );
+            const ecSignature = await ecSignOrderHashAsync(fakeProvider, orderHash, makerAddress, SignerType.Default);
             expect(ecSignature).to.equal(expectedSignature);
         });
         // Note this is due to a bug in Metamask where it does not prefix before signing, this is a known issue and is to be fixed in the future
         // Source: https://github.com/MetaMask/metamask-extension/commit/a9d36860bec424dcee8db043d3e7da6a5ff5672e
-        it('should receive a payload modified with a prefix when Metamask is SignerProviderType', async () => {
+        it('should receive a payload modified with a prefix when Metamask is SignerType', async () => {
             const orderHash = '0x34decbedc118904df65f379a175bb39ca18209d6ce41d5ed549d54e6e0a95004';
             const orderHashPrefixed = '0xae70f31d26096291aa681b26cb7574563956221d0b4213631e1ef9df675d4cba';
             const expectedSignature =
                 '0x1b117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d872871137feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b03';
+            // Generated from a MM eth_sign request from 0x5409ed021d9299bf6814279a6a1411a7e866a631 signing 0xae70f31d26096291aa681b26cb7574563956221d0b4213631e1ef9df675d4cba
+            const metamaskSignature =
+                '0x117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d872871137feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b1b';
             const fakeProvider = {
                 async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise<void> {
                     if (payload.method === 'eth_sign') {
-                        const [address, message] = payload.params;
+                        const [, message] = payload.params;
                         expect(message).to.equal(orderHashPrefixed);
-                        const signature =
-                            '0x1b117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d872871137feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b';
                         callback(null, {
                             id: 42,
                             jsonrpc: '2.0',
-                            result: signature,
+                            result: metamaskSignature,
                         });
                     } else {
                         callback(null, { id: 42, jsonrpc: '2.0', result: [makerAddress] });
@@ -216,25 +202,52 @@ describe('Signature utils', () => {
                 },
             };
 
-            const ecSignature = await ecSignOrderHashAsync(
-                fakeProvider,
-                orderHash,
-                makerAddress,
-                SignerProviderType.Metamask,
-            );
+            const ecSignature = await ecSignOrderHashAsync(fakeProvider, orderHash, makerAddress, SignerType.Metamask);
             expect(ecSignature).to.equal(expectedSignature);
         });
         it('should return a valid signature', async () => {
             const orderHash = '0x34decbedc118904df65f379a175bb39ca18209d6ce41d5ed549d54e6e0a95004';
-            const ecSignature = await ecSignOrderHashAsync(
-                provider,
-                orderHash,
-                makerAddress,
-                SignerProviderType.EthSign,
-            );
+            const ecSignature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, SignerType.Default);
 
             const isValidSignature = await isValidSignatureAsync(provider, orderHash, ecSignature, makerAddress);
             expect(isValidSignature).to.be.true();
         });
     });
+    describe('#convertECSignatureToSignatureHex', () => {
+        const ecSignature: ECSignature = {
+            v: 27,
+            r: '0xaca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d64393',
+            s: '0x46b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf2',
+        };
+        it('should concatenate v,r,s and append the Trezor signature type', async () => {
+            const expectedSignatureWithSignatureType =
+                '0x1baca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d6439346b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf208';
+            const signatureWithSignatureType = convertECSignatureToSignatureHex(ecSignature, SignerType.Trezor);
+            expect(signatureWithSignatureType).to.equal(expectedSignatureWithSignatureType);
+        });
+        it('should concatenate v,r,s and append the EthSign signature type when SignerType is Default', async () => {
+            const expectedSignatureWithSignatureType =
+                '0x1baca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d6439346b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf203';
+            const signatureWithSignatureType = convertECSignatureToSignatureHex(ecSignature, SignerType.Default);
+            expect(signatureWithSignatureType).to.equal(expectedSignatureWithSignatureType);
+        });
+        it('should concatenate v,r,s and append the EthSign signature type when SignerType is Ledger', async () => {
+            const expectedSignatureWithSignatureType =
+                '0x1baca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d6439346b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf203';
+            const signatureWithSignatureType = convertECSignatureToSignatureHex(ecSignature, SignerType.Ledger);
+            expect(signatureWithSignatureType).to.equal(expectedSignatureWithSignatureType);
+        });
+        it('should concatenate v,r,s and append the EthSign signature type when SignerType is Metamask', async () => {
+            const expectedSignatureWithSignatureType =
+                '0x1baca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d6439346b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf203';
+            const signatureWithSignatureType = convertECSignatureToSignatureHex(ecSignature, SignerType.Metamask);
+            expect(signatureWithSignatureType).to.equal(expectedSignatureWithSignatureType);
+        });
+        it('should throw if the SignerType is invalid', async () => {
+            const expectedMessage = 'Unrecognized SignerType: INVALID_SIGNER';
+            expect(() => convertECSignatureToSignatureHex(ecSignature, 'INVALID_SIGNER' as SignerType)).to.throw(
+                expectedMessage,
+            );
+        });
+    });
 });