From 45e9fbe8f93f68f3786629fff1861b1a66b90635 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 31 Jul 2018 17:24:19 +0800 Subject: 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 --- packages/0x.js/CHANGELOG.json | 3 + packages/0x.js/src/0x.ts | 20 ++-- packages/0x.js/src/index.ts | 2 +- .../contracts/test/exchange/signature_validator.ts | 12 +-- packages/order-utils/CHANGELOG.json | 3 + packages/order-utils/src/index.ts | 10 +- packages/order-utils/src/order_factory.ts | 15 ++- packages/order-utils/src/signature_utils.ts | 80 +++++++++----- packages/order-utils/src/types.ts | 22 ---- packages/order-utils/test/signature_utils_test.ts | 117 ++++++++++++++------- packages/types/src/index.ts | 10 ++ 11 files changed, 176 insertions(+), 118 deletions(-) (limited to 'packages') diff --git a/packages/0x.js/CHANGELOG.json b/packages/0x.js/CHANGELOG.json index 913b7a76e..0a10e1ccf 100644 --- a/packages/0x.js/CHANGELOG.json +++ b/packages/0x.js/CHANGELOG.json @@ -21,6 +21,9 @@ "changes": [ { "note": "Dependencies updated" + }, + { + "note": "Update ecSignOrderHashAsync to return the signature as a string for immediate use in contracts" } ], "timestamp": 1532605697 diff --git a/packages/0x.js/src/0x.ts b/packages/0x.js/src/0x.ts index 2a2b82f63..86859c368 100644 --- a/packages/0x.js/src/0x.ts +++ b/packages/0x.js/src/0x.ts @@ -14,13 +14,19 @@ import { ecSignOrderHashAsync, generatePseudoRandomSalt, isValidSignatureAsync, - MessagePrefixOpts, orderHashUtils, } from '@0xproject/order-utils'; // HACK: Since we export assetDataUtils from ZeroEx and it has AssetProxyId, ERC20AssetData and ERC721AssetData // in it's public interface, we need to import these types here. // tslint:disable-next-line:no-unused-variable -import { AssetProxyId, ECSignature, ERC20AssetData, ERC721AssetData, Order, SignedOrder } from '@0xproject/types'; +import { + AssetProxyId, + ERC20AssetData, + ERC721AssetData, + Order, + SignedOrder, + SignerProviderType, +} from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { Provider, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; @@ -238,19 +244,19 @@ export class ZeroEx { * @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 MessagePrefixOpts Options regarding the desired prefix and whether to add it before calling `eth_sign` - * @return An object containing the Elliptic curve signature parameters generated by signing the orderHash. + * @param SignerProviderType The type of Signer Provider which implements `eth_sign`. E.g Metamask, Ledger, Trezor or EthSign. + * @return A hex encoded string of the Elliptic curve signature parameters generated by signing the orderHash and signature type. */ public async ecSignOrderHashAsync( orderHash: string, signerAddress: string, - messagePrefixOpts: MessagePrefixOpts, - ): Promise { + signerProviderType: SignerProviderType, + ): Promise { const signature = await ecSignOrderHashAsync( this._contractWrappers.getProvider(), orderHash, signerAddress, - messagePrefixOpts, + signerProviderType, ); return signature; } diff --git a/packages/0x.js/src/index.ts b/packages/0x.js/src/index.ts index 95ca07eea..1e5c0c270 100644 --- a/packages/0x.js/src/index.ts +++ b/packages/0x.js/src/index.ts @@ -1,12 +1,12 @@ export { ZeroEx } from './0x'; -export { MessagePrefixType, MessagePrefixOpts } from '@0xproject/order-utils'; export { Web3ProviderEngine, RPCSubprovider } from '@0xproject/subproviders'; export { ExchangeContractErrs, Order, SignedOrder, + SignerProviderType, ECSignature, OrderStateValid, OrderStateInvalid, diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index bef0547bd..7e9f4fa59 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { addSignedMessagePrefix, assetDataUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils'; -import { RevertReason, SignatureType, SignedOrder } from '@0xproject/types'; +import { addSignedMessagePrefix, assetDataUtils, orderHashUtils } from '@0xproject/order-utils'; +import { RevertReason, SignatureType, SignedOrder, SignerProviderType } from '@0xproject/types'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); @@ -213,7 +213,7 @@ describe('MixinSignatureValidator', () => { 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 orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.EthSign); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -236,7 +236,7 @@ describe('MixinSignatureValidator', () => { it('should return false when SignatureType=EthSign and signature is invalid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.EthSign); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -385,7 +385,7 @@ describe('MixinSignatureValidator', () => { 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 orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.Trezor); const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); // Create 0x signature from Trezor signature @@ -408,7 +408,7 @@ describe('MixinSignatureValidator', () => { it('should return false when SignatureType=Trezor and signature is invalid', async () => { // Create Trezor signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.Trezor); const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); // Create 0x signature from Trezor signature diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index fa82976ad..60f0855c2 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -30,6 +30,9 @@ "changes": [ { "note": "Dependencies updated" + }, + { + "note": "Update ecSignOrderHashAsync to return signature string with signature type byte" } ], "timestamp": 1532605697 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 { + signerProviderType: SignerProviderType, +): Promise { 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', diff --git a/packages/order-utils/test/signature_utils_test.ts b/packages/order-utils/test/signature_utils_test.ts index baae2b414..179905b18 100644 --- a/packages/order-utils/test/signature_utils_test.ts +++ b/packages/order-utils/test/signature_utils_test.ts @@ -1,3 +1,4 @@ +import { SignerProviderType } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { JSONRPCErrorCallback, JSONRPCRequestPayload } from 'ethereum-types'; @@ -5,7 +6,7 @@ import * as _ from 'lodash'; import 'mocha'; import * as Sinon from 'sinon'; -import { ecSignOrderHashAsync, generatePseudoRandomSalt, MessagePrefixType } from '../src'; +import { ecSignOrderHashAsync, generatePseudoRandomSalt } from '../src'; import { isValidECSignature, isValidSignatureAsync } from '../src/signature_utils'; import { chaiSetup } from './utils/chai_setup'; @@ -119,32 +120,28 @@ describe('Signature utils', () => { _.each(stubs, s => s.restore()); stubs = []; }); - it('Should return the correct ECSignature', async () => { + it('Should return the correct Signature', async () => { const orderHash = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0'; - const expectedECSignature = { - v: 27, - r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33', - s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254', - }; - const messagePrefixOpts = { - prefixType: MessagePrefixType.EthSign, - shouldAddPrefixBeforeCallingEthSign: false, - }; - const ecSignature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, messagePrefixOpts); - expect(ecSignature).to.deep.equal(expectedECSignature); - }); - it('should return the correct ECSignature for signatureHex concatenated as R + S + V', async () => { + const expectedSignature = + '0x1b61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc3340349190569279751135161d22529dc25add4f6069af05be04cacbda2ace225403'; + const ecSignature = await ecSignOrderHashAsync( + provider, + orderHash, + makerAddress, + SignerProviderType.EthSign, + ); + expect(ecSignature).to.equal(expectedSignature); + }); + it('should return the correct Signature for signatureHex concatenated as R + S + V', async () => { const orderHash = '0x34decbedc118904df65f379a175bb39ca18209d6ce41d5ed549d54e6e0a95004'; - const expectedECSignature = { - v: 27, - r: '0x117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d87287113', - s: '0x7feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b', - }; + const expectedSignature = + '0x1b117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d872871137feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b03'; const fakeProvider = { async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise { if (payload.method === 'eth_sign') { const [address, message] = payload.params; + expect(message).to.equal(orderHash); const signature = await web3Wrapper.signMessageAsync(address, message); // tslint:disable-next-line:custom-no-magic-numbers const rsvHex = `0x${signature.substr(130)}${signature.substr(2, 128)}`; @@ -158,21 +155,18 @@ describe('Signature utils', () => { } }, }; - - const messagePrefixOpts = { - prefixType: MessagePrefixType.EthSign, - shouldAddPrefixBeforeCallingEthSign: false, - }; - const ecSignature = await ecSignOrderHashAsync(fakeProvider, orderHash, makerAddress, messagePrefixOpts); - expect(ecSignature).to.deep.equal(expectedECSignature); - }); - it('should return the correct ECSignature for signatureHex concatenated as V + R + S', async () => { + const ecSignature = await ecSignOrderHashAsync( + fakeProvider, + orderHash, + makerAddress, + SignerProviderType.EthSign, + ); + expect(ecSignature).to.equal(expectedSignature); + }); + it('should return the correct Signature for signatureHex concatenated as V + R + S', async () => { const orderHash = '0x34decbedc118904df65f379a175bb39ca18209d6ce41d5ed549d54e6e0a95004'; - const expectedECSignature = { - v: 27, - r: '0x117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d87287113', - s: '0x7feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b', - }; + const expectedSignature = + '0x1b117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d872871137feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b03'; const fakeProvider = { async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise { if (payload.method === 'eth_sign') { @@ -189,12 +183,57 @@ describe('Signature utils', () => { }, }; - const messagePrefixOpts = { - prefixType: MessagePrefixType.EthSign, - shouldAddPrefixBeforeCallingEthSign: false, + const ecSignature = await ecSignOrderHashAsync( + fakeProvider, + orderHash, + makerAddress, + SignerProviderType.EthSign, + ); + 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 + it('should receive a payload modified with a prefix when Metamask is SignerProviderType', async () => { + const orderHash = '0x34decbedc118904df65f379a175bb39ca18209d6ce41d5ed549d54e6e0a95004'; + const orderHashPrefixed = '0xae70f31d26096291aa681b26cb7574563956221d0b4213631e1ef9df675d4cba'; + const expectedSignature = + '0x1b117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d872871137feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b03'; + const fakeProvider = { + async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise { + if (payload.method === 'eth_sign') { + const [address, message] = payload.params; + expect(message).to.equal(orderHashPrefixed); + const signature = + '0x1b117902c86dfb95fe0d1badd983ee166ad259b27acb220174cbb4460d872871137feabdfe76e05924b484789f79af4ee7fa29ec006cedce1bbf369320d034e10b'; + callback(null, { + id: 42, + jsonrpc: '2.0', + result: signature, + }); + } else { + callback(null, { id: 42, jsonrpc: '2.0', result: [makerAddress] }); + } + }, }; - const ecSignature = await ecSignOrderHashAsync(fakeProvider, orderHash, makerAddress, messagePrefixOpts); - expect(ecSignature).to.deep.equal(expectedECSignature); + + const ecSignature = await ecSignOrderHashAsync( + fakeProvider, + orderHash, + makerAddress, + SignerProviderType.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 isValidSignature = await isValidSignatureAsync(provider, orderHash, ecSignature, makerAddress); + expect(isValidSignature).to.be.true(); }); }); }); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 7774b61b0..ba384dbb8 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -142,6 +142,16 @@ export enum SignatureType { NSignatureTypes, } +/** + * The Signer Provider Type. Some Signer implementations use different message prefixes or implement different + * eth_sign behaviour. Note EthSign is compatible with the Ledger device. + */ +export enum SignerProviderType { + EthSign = 'ETH_SIGN', + Metamask = 'METAMASK', + Trezor = 'TREZOR', +} + /** * Elliptic Curve signature */ -- cgit v1.2.3 From 9dd6ba78250d8bbde1d5023ce4ac4254884f4115 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Fri, 3 Aug 2018 11:35:03 +0800 Subject: Update jsdoc --- packages/order-utils/src/signature_utils.ts | 9 ++++----- packages/order-utils/test/signature_utils_test.ts | 1 + 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'packages') diff --git a/packages/order-utils/src/signature_utils.ts b/packages/order-utils/src/signature_utils.ts index 3237259c9..07644ebe2 100644 --- a/packages/order-utils/src/signature_utils.ts +++ b/packages/order-utils/src/signature_utils.ts @@ -196,9 +196,8 @@ 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 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) + * @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. * @return A hex encoded string containing the Elliptic curve signature generated by signing the orderHash and the Signature Type. */ export async function ecSignOrderHashAsync( @@ -251,7 +250,7 @@ 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 messagePrefixType The MessagePrefixType of the signed data + * @param signerProviderType The SignerProviderType of the signed data * @return Hex encoded string of signature with Signature Type */ export function convertECSignatureToSignatureHex( @@ -283,7 +282,7 @@ export function convertToSignatureWithType(signature: string, type: SignatureTyp /** * Adds the relevant prefix to the message being signed. * @param message Message to sign - * @param messagePrefixType The type of message prefix to add. Different signers expect + * @param signerProviderType The type of message prefix to add for a given SignerProviderType. Different signers expect * specific message prefixes. * @return Prefixed message */ diff --git a/packages/order-utils/test/signature_utils_test.ts b/packages/order-utils/test/signature_utils_test.ts index 179905b18..de76e82ac 100644 --- a/packages/order-utils/test/signature_utils_test.ts +++ b/packages/order-utils/test/signature_utils_test.ts @@ -192,6 +192,7 @@ describe('Signature utils', () => { 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 () => { const orderHash = '0x34decbedc118904df65f379a175bb39ca18209d6ce41d5ed549d54e6e0a95004'; const orderHashPrefixed = '0xae70f31d26096291aa681b26cb7574563956221d0b4213631e1ef9df675d4cba'; -- cgit v1.2.3 From 5d4dd406f2946b43377049d7422c72b433bc64ab Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Fri, 3 Aug 2018 11:53:26 +0800 Subject: Update Changelogs. Rebased from development --- packages/0x.js/CHANGELOG.json | 12 +++++++++--- packages/order-utils/CHANGELOG.json | 13 ++++++++++--- packages/order-utils/src/signature_utils.ts | 7 ++++--- packages/types/CHANGELOG.json | 9 +++++++++ packages/types/src/index.ts | 5 +++-- 5 files changed, 35 insertions(+), 11 deletions(-) (limited to 'packages') diff --git a/packages/0x.js/CHANGELOG.json b/packages/0x.js/CHANGELOG.json index 0a10e1ccf..88c1bd3a3 100644 --- a/packages/0x.js/CHANGELOG.json +++ b/packages/0x.js/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "pending", + "changes": [ + { + "pr": 914, + "note": "Update ecSignOrderHashAsync to return the signature as a string for immediate use in contracts" + } + ] + }, { "version": "1.0.1-rc.3", "changes": [ @@ -21,9 +30,6 @@ "changes": [ { "note": "Dependencies updated" - }, - { - "note": "Update ecSignOrderHashAsync to return the signature as a string for immediate use in contracts" } ], "timestamp": 1532605697 diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index 60f0855c2..c70448d69 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -1,4 +1,14 @@ [ + { + "version": "pending", + "changes": [ + { + "pr": 914, + "note": + "Update ecSignOrderHashAsync to return signature string with signature type byte. Removes messagePrefixOpts." + } + ] + }, { "version": "1.0.1-rc.3", "changes": [ @@ -30,9 +40,6 @@ "changes": [ { "note": "Dependencies updated" - }, - { - "note": "Update ecSignOrderHashAsync to return signature string with signature type byte" } ], "timestamp": 1532605697 diff --git a/packages/order-utils/src/signature_utils.ts b/packages/order-utils/src/signature_utils.ts index 07644ebe2..db5a35f80 100644 --- a/packages/order-utils/src/signature_utils.ts +++ b/packages/order-utils/src/signature_utils.ts @@ -271,11 +271,11 @@ export function convertECSignatureToSignatureHex( /** * 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. + * @param signatureType 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)]); +export function convertToSignatureWithType(signature: string, signatureType: SignatureType): string { + const signatureBuffer = Buffer.concat([ethUtil.toBuffer(signature), ethUtil.toBuffer(signatureType)]); const signatureHex = `0x${signatureBuffer.toString('hex')}`; return signatureHex; } @@ -291,6 +291,7 @@ export function addSignedMessagePrefix(message: string, signerProviderType: Sign assert.doesBelongToStringEnum('signerProviderType', signerProviderType, SignerProviderType); switch (signerProviderType) { case SignerProviderType.Metamask: + case SignerProviderType.Ledger: case SignerProviderType.EthSign: { const msgBuff = ethUtil.toBuffer(message); const prefixedMsgBuff = ethUtil.hashPersonalMessage(msgBuff); diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json index 8520c5146..057d3451c 100644 --- a/packages/types/CHANGELOG.json +++ b/packages/types/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "pending", + "changes": [ + { + "pr": 914, + "note": "Added SignerProviderType" + } + ] + }, { "version": "1.0.1-rc.3", "changes": [ diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index ba384dbb8..58399de4c 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -143,11 +143,12 @@ export enum SignatureType { } /** - * The Signer Provider Type. Some Signer implementations use different message prefixes or implement different - * eth_sign behaviour. Note EthSign is compatible with the Ledger device. + * The Signer Provider Type. Some Signer provider implementations use different message prefixes or implement different + * eth_sign behaviour. */ export enum SignerProviderType { EthSign = 'ETH_SIGN', + Ledger = 'LEDGER', Metamask = 'METAMASK', Trezor = 'TREZOR', } -- cgit v1.2.3 From ca4905c3436931684d113ec66882836a4d0b265b Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 9 Aug 2018 12:24:52 +1000 Subject: Rename from SignerProviderType.EthSign to SignerType.Default --- packages/0x.js/src/0x.ts | 17 ++--- packages/0x.js/src/index.ts | 2 +- .../contracts/test/exchange/signature_validator.ts | 10 +-- packages/order-utils/src/order_factory.ts | 20 +---- packages/order-utils/src/signature_utils.ts | 83 ++++++++++++--------- packages/order-utils/test/signature_utils_test.ts | 87 +++++++++++++--------- packages/subproviders/test/utils/fixture_data.ts | 2 +- packages/types/CHANGELOG.json | 2 +- packages/types/src/index.ts | 8 +- 9 files changed, 116 insertions(+), 115 deletions(-) (limited to 'packages') diff --git a/packages/0x.js/src/0x.ts b/packages/0x.js/src/0x.ts index 86859c368..80d784552 100644 --- a/packages/0x.js/src/0x.ts +++ b/packages/0x.js/src/0x.ts @@ -19,14 +19,7 @@ import { // HACK: Since we export assetDataUtils from ZeroEx and it has AssetProxyId, ERC20AssetData and ERC721AssetData // in it's public interface, we need to import these types here. // tslint:disable-next-line:no-unused-variable -import { - AssetProxyId, - ERC20AssetData, - ERC721AssetData, - Order, - SignedOrder, - SignerProviderType, -} from '@0xproject/types'; +import { AssetProxyId, ERC20AssetData, ERC721AssetData, Order, SignedOrder, SignerType } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { Provider, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; @@ -244,19 +237,21 @@ export class ZeroEx { * @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 The type of Signer Provider which implements `eth_sign`. E.g Metamask, Ledger, Trezor or EthSign. + * @param signerType the type signer that will perform the `eth_sign` operation. E.g Default, Metamask, Ledger or Trezor. + * Some implementations exhibit different behaviour. Default will assume a spec compliant eth_sign implementation. + * This parameter is defaulted to `SignerType.Default`. * @return A hex encoded string of the Elliptic curve signature parameters generated by signing the orderHash and signature type. */ public async ecSignOrderHashAsync( orderHash: string, signerAddress: string, - signerProviderType: SignerProviderType, + signerType: SignerType = SignerType.Default, ): Promise { const signature = await ecSignOrderHashAsync( this._contractWrappers.getProvider(), orderHash, signerAddress, - signerProviderType, + signerType, ); return signature; } diff --git a/packages/0x.js/src/index.ts b/packages/0x.js/src/index.ts index 1e5c0c270..2ba60e730 100644 --- a/packages/0x.js/src/index.ts +++ b/packages/0x.js/src/index.ts @@ -6,7 +6,7 @@ export { ExchangeContractErrs, Order, SignedOrder, - SignerProviderType, + SignerType, ECSignature, OrderStateValid, OrderStateInvalid, diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 7e9f4fa59..56a06c247 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { addSignedMessagePrefix, assetDataUtils, orderHashUtils } from '@0xproject/order-utils'; -import { RevertReason, SignatureType, SignedOrder, SignerProviderType } from '@0xproject/types'; +import { RevertReason, SignatureType, SignedOrder, SignerType } from '@0xproject/types'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); @@ -213,7 +213,7 @@ describe('MixinSignatureValidator', () => { it('should return true when SignatureType=EthSign and signature is valid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.EthSign); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Default); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -236,7 +236,7 @@ describe('MixinSignatureValidator', () => { it('should return false when SignatureType=EthSign and signature is invalid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.EthSign); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Default); const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); // Create 0x signature from EthSign signature @@ -385,7 +385,7 @@ describe('MixinSignatureValidator', () => { it('should return true when SignatureType=Trezor and signature is valid', async () => { // Create Trezor signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.Trezor); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Trezor); const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); // Create 0x signature from Trezor signature @@ -408,7 +408,7 @@ describe('MixinSignatureValidator', () => { it('should return false when SignatureType=Trezor and signature is invalid', async () => { // Create Trezor signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerProviderType.Trezor); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, SignerType.Trezor); const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); // Create 0x signature from Trezor signature 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 { 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 { 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, + ); + }); + }); }); diff --git a/packages/subproviders/test/utils/fixture_data.ts b/packages/subproviders/test/utils/fixture_data.ts index 3137e08b0..7cf502c97 100644 --- a/packages/subproviders/test/utils/fixture_data.ts +++ b/packages/subproviders/test/utils/fixture_data.ts @@ -25,7 +25,7 @@ export const fixtureData = { chainId: networkId, from: TEST_RPC_ACCOUNT_0, }, - // This is the signed result of the abouve Transaction Data + // This is the signed result of the above Transaction Data TX_DATA_SIGNED_RESULT: '0xf85f8080822710940000000000000000000000000000000000000000808078a0712854c73c69445cc1b22a7c3d7312ff9a97fe4ffba35fd636e8236b211b6e7ca0647cee031615e52d916c7c707025bc64ad525d8f1b9876c3435a863b42743178', TX_DATA_ACCOUNT_1_SIGNED_RESULT: diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json index 057d3451c..4f60310e6 100644 --- a/packages/types/CHANGELOG.json +++ b/packages/types/CHANGELOG.json @@ -4,7 +4,7 @@ "changes": [ { "pr": 914, - "note": "Added SignerProviderType" + "note": "Added SignerType to handle different signing prefix scenarios" } ] }, diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 58399de4c..fa634420d 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -143,11 +143,11 @@ export enum SignatureType { } /** - * The Signer Provider Type. Some Signer provider implementations use different message prefixes or implement different - * eth_sign behaviour. + * The type of the Signer implementation. Some signer implementations use different message prefixes (e.g Trezor) or implement different + * eth_sign behaviour (e.g Metamask). Default assumes a spec compliant `eth_sign`. */ -export enum SignerProviderType { - EthSign = 'ETH_SIGN', +export enum SignerType { + Default = 'DEFAULT', Ledger = 'LEDGER', Metamask = 'METAMASK', Trezor = 'TREZOR', -- cgit v1.2.3 From a3517574936aa6a4911003dbff06302926b04cb4 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Tue, 14 Aug 2018 08:32:16 +1000 Subject: Update version numbers. Add source for Metamask future fix. Consolidate switch statement to one return --- packages/0x.js/CHANGELOG.json | 2 +- packages/0x.js/src/0x.ts | 2 +- packages/order-utils/CHANGELOG.json | 2 +- packages/order-utils/src/signature_utils.ts | 14 ++++++++------ packages/types/CHANGELOG.json | 2 +- 5 files changed, 12 insertions(+), 10 deletions(-) (limited to 'packages') diff --git a/packages/0x.js/CHANGELOG.json b/packages/0x.js/CHANGELOG.json index 88c1bd3a3..a1bdcc506 100644 --- a/packages/0x.js/CHANGELOG.json +++ b/packages/0x.js/CHANGELOG.json @@ -1,6 +1,6 @@ [ { - "version": "pending", + "version": "1.0.1-rc.4", "changes": [ { "pr": 914, diff --git a/packages/0x.js/src/0x.ts b/packages/0x.js/src/0x.ts index 80d784552..48d00c1ac 100644 --- a/packages/0x.js/src/0x.ts +++ b/packages/0x.js/src/0x.ts @@ -237,7 +237,7 @@ export class ZeroEx { * @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 signerType the type signer that will perform the `eth_sign` operation. E.g Default, Metamask, Ledger or Trezor. + * @param signerType the signer type that will perform the `eth_sign` operation. E.g Default, Metamask, Ledger or Trezor. * Some implementations exhibit different behaviour. Default will assume a spec compliant eth_sign implementation. * This parameter is defaulted to `SignerType.Default`. * @return A hex encoded string of the Elliptic curve signature parameters generated by signing the orderHash and signature type. diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index c70448d69..6f59580b3 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -1,6 +1,6 @@ [ { - "version": "pending", + "version": "1.0.1-rc.4", "changes": [ { "pr": 914, diff --git a/packages/order-utils/src/signature_utils.ts b/packages/order-utils/src/signature_utils.ts index 5a58edf38..870aef2ed 100644 --- a/packages/order-utils/src/signature_utils.ts +++ b/packages/order-utils/src/signature_utils.ts @@ -216,6 +216,7 @@ export async function ecSignOrderHashAsync( let msgHashHex = orderHash; const prefixedMsgHashHex = addSignedMessagePrefix(orderHash, signerType); // Metamask incorrectly implements eth_sign and does not prefix the message as per the spec + // Source: https://github.com/MetaMask/metamask-extension/commit/a9d36860bec424dcee8db043d3e7da6a5ff5672e if (signerType === SignerType.Metamask) { msgHashHex = prefixedMsgHashHex; } @@ -260,22 +261,23 @@ export function convertECSignatureToSignatureHex(ecSignature: ECSignature, signe ethUtil.toBuffer(ecSignature.s), ]); const signatureHex = `0x${signatureBuffer.toString('hex')}`; + let signatureType; switch (signerType) { case SignerType.Metamask: case SignerType.Ledger: case SignerType.Default: { - const signatureType = SignatureType.EthSign; - const signatureWithType = convertToSignatureWithType(signatureHex, signatureType); - return signatureWithType; + signatureType = SignatureType.EthSign; + break; } case SignerType.Trezor: { - const signatureType = SignatureType.Trezor; - const signatureWithType = convertToSignatureWithType(signatureHex, signatureType); - return signatureWithType; + signatureType = SignatureType.Trezor; + break; } default: throw new Error(`Unrecognized SignerType: ${signerType}`); } + const signatureWithType = convertToSignatureWithType(signatureHex, signatureType); + return signatureWithType; } /** * Combines the signature proof and the Signature Type. diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json index 4f60310e6..9d78d3dc0 100644 --- a/packages/types/CHANGELOG.json +++ b/packages/types/CHANGELOG.json @@ -1,6 +1,6 @@ [ { - "version": "pending", + "version": "1.0.1-rc.4", "changes": [ { "pr": 914, -- cgit v1.2.3