Fix order-utils tests

This commit is contained in:
Fabio Berger 2018-05-30 14:08:43 -07:00
parent 3302c89284
commit 32833b7301
7 changed files with 123 additions and 81 deletions

View File

@ -1,10 +1,10 @@
export { orderHashUtils } from './order_hash';
export { isValidSignatureAsync, ecSignOrderHashAsync } from './signature_utils';
export { isValidSignatureAsync, ecSignOrderHashAsync, addSignedMessagePrefix } from './signature_utils';
export { orderFactory } from './order_factory';
export { constants } from './constants';
export { crypto } from './crypto';
export { generatePseudoRandomSalt } from './salt';
export { OrderError } from './types';
export { OrderError, MessagePrefixType, MessagePrefixOpts } from './types';
export { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher';
export { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher';
export { RemainingFillableCalculator } from './remaining_fillable_calculator';

View File

@ -1,12 +1,12 @@
import { Provider, SignedOrder } from '@0xproject/types';
import { ECSignature, Provider, SignedOrder } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as ethUtil from 'ethereumjs-util';
import * as _ from 'lodash';
import { orderHashUtils } from './order_hash';
import { generatePseudoRandomSalt } from './salt';
import { ecSignOrderHashAsync, getVRSHexString } from './signature_utils';
const SHOULD_ADD_PERSONAL_MESSAGE_PREFIX = false;
import { ecSignOrderHashAsync } from './signature_utils';
import { MessagePrefixType } from './types';
export const orderFactory = {
async createSignedOrderAsync(
@ -44,14 +44,25 @@ export const orderFactory = {
expirationTimeSeconds,
};
const orderHash = orderHashUtils.getOrderHashHex(order);
const ecSignature = await ecSignOrderHashAsync(
provider,
orderHash,
makerAddress,
SHOULD_ADD_PERSONAL_MESSAGE_PREFIX,
);
const messagePrefixOpts = {
prefixType: MessagePrefixType.EthSign,
shouldAddPrefixBeforeCallingEthSign: false,
};
const ecSignature = await ecSignOrderHashAsync(provider, orderHash, makerAddress, messagePrefixOpts);
const signature = getVRSHexString(ecSignature);
const signedOrder: SignedOrder = _.assign(order, { signature });
return signedOrder;
},
};
function getVRSHexString(ecSignature: ECSignature): string {
const vrs = `0x${intToHex(ecSignature.v)}${ethUtil.stripHexPrefix(ecSignature.r)}${ethUtil.stripHexPrefix(
ecSignature.s,
)}`;
return vrs;
}
function intToHex(i: number): string {
const hex = ethUtil.bufferToHex(ethUtil.toBuffer(i));
return hex;
}

View File

@ -9,7 +9,7 @@ import * as _ from 'lodash';
import { assert } from './assert';
import { crypto } from './crypto';
const INVALID_TAKER_FORMAT = 'instance.taker is not of a type(s) string';
const INVALID_TAKER_FORMAT = 'instance.takerAddress is not of a type(s) string';
export const orderHashUtils = {
/**
@ -34,7 +34,7 @@ export const orderHashUtils = {
*/
getOrderHashHex(order: SignedOrder | Order): string {
try {
assert.doesConformToSchema('order', order, schemas.orderSchema);
assert.doesConformToSchema('order', order, schemas.orderSchema, [schemas.hexSchema]);
} catch (error) {
if (_.includes(error.message, INVALID_TAKER_FORMAT)) {
const errMsg =

View File

@ -8,12 +8,13 @@ import { artifacts } from './artifacts';
import { assert } from './assert';
import { ExchangeContract } from './generated_contract_wrappers/exchange';
import { ISignerContract } from './generated_contract_wrappers/i_signer';
import { OrderError } from './types';
import { MessagePrefixOpts, MessagePrefixType, OrderError } from './types';
/**
* Verifies that the provided signature is valid according to the 0x Protocol smart contracts
* @param data The hex encoded data signed by the supplied signature.
* @param signature An object containing the elliptic curve signature parameters.
* @param signature A hex encoded 0x Protocol signature made up of: [SignatureType][TypeSpecificData].
* E.g [SignatureType.EIP712][vrs]
* @param signerAddress The hex encoded address that signed the data, producing the supplied signature.
* @return Whether the signature is valid for the supplied signerAddress and data.
*/
@ -41,10 +42,8 @@ export async function isValidSignatureAsync(
// types use ECRecover...
case SignatureType.Ecrecover: {
const ecSignature = parseECSignature(signature);
const dataBuff = ethUtil.toBuffer(data);
const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff);
const msgHash = ethUtil.bufferToHex(msgHashBuff);
return isValidECSignature(msgHash, ecSignature, signerAddress);
const prefixedMessageHex = addSignedMessagePrefix(data, MessagePrefixType.EthSign);
return isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
}
case SignatureType.EIP712: {
@ -53,11 +52,9 @@ export async function isValidSignatureAsync(
}
case SignatureType.Trezor: {
const dataBuff = ethUtil.toBuffer(data);
const msgHashBuff = hashTrezorPersonalMessage(dataBuff);
const msgHash = ethUtil.bufferToHex(msgHashBuff);
const prefixedMessageHex = addSignedMessagePrefix(data, MessagePrefixType.Trezor);
const ecSignature = parseECSignature(signature);
return isValidECSignature(msgHash, ecSignature, signerAddress);
return isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
}
// TODO: Rename Contract -> Wallet
@ -78,13 +75,14 @@ export async function isValidSignatureAsync(
}
}
export function getVRSHexString(ecSignature: ECSignature): string {
const vrs = `0x${intToHex(ecSignature.v)}${ethUtil.stripHexPrefix(ecSignature.r)}${ethUtil.stripHexPrefix(
ecSignature.s,
)}`;
return vrs;
}
/**
* Checks if the supplied elliptic curve signature corresponds to signing `data` with
* the private key corresponding to `signerAddress`
* @param data The hex encoded data signed by the supplied signature.
* @param signature An object containing the elliptic curve signature parameters.
* @param signerAddress The hex encoded address that signed the data, producing the supplied signature.
* @return Whether the ECSignature is valid.
*/
export function isValidECSignature(data: string, signature: ECSignature, signerAddress: string): boolean {
assert.isHexString('data', data);
assert.doesConformToSchema('signature', signature, schemas.ecSignatureSchema);
@ -112,17 +110,16 @@ 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 shouldAddPersonalMessagePrefix Some signers add the personal message prefix `\x19Ethereum Signed Message`
* themselves (e.g Parity Signer, Ledger, TestRPC) and others expect it to already be done by the client
* (e.g Metamask). Depending on which signer this request is going to, decide on whether to add the prefix
* before sending the request.
* @param hashPrefixOpts Different signers add/require different prefixes be appended 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.
*/
export async function ecSignOrderHashAsync(
provider: Provider,
orderHash: string,
signerAddress: string,
shouldAddPersonalMessagePrefix: boolean,
messagePrefixOpts: MessagePrefixOpts,
): Promise<ECSignature> {
assert.isHexString('orderHash', orderHash);
const web3Wrapper = new Web3Wrapper(provider);
@ -130,12 +127,10 @@ export async function ecSignOrderHashAsync(
const normalizedSignerAddress = signerAddress.toLowerCase();
let msgHashHex = orderHash;
if (shouldAddPersonalMessagePrefix) {
const orderHashBuff = ethUtil.toBuffer(orderHash);
const msgHashBuff = ethUtil.hashPersonalMessage(orderHashBuff);
msgHashHex = ethUtil.bufferToHex(msgHashBuff);
const prefixedMsgHashHex = addSignedMessagePrefix(orderHash, messagePrefixOpts.prefixType);
if (messagePrefixOpts.shouldAddPrefixBeforeCallingEthSign) {
msgHashHex = prefixedMsgHashHex;
}
const signature = await web3Wrapper.signMessageAsync(normalizedSignerAddress, msgHashHex);
// HACK: There is no consensus on whether the signatureHex string should be formatted as
@ -146,7 +141,7 @@ export async function ecSignOrderHashAsync(
const validVParamValues = [27, 28];
const ecSignatureVRS = parseSignatureHexAsVRS(signature);
if (_.includes(validVParamValues, ecSignatureVRS.v)) {
const isValidVRSSignature = isValidECSignature(orderHash, ecSignatureVRS, normalizedSignerAddress);
const isValidVRSSignature = isValidECSignature(prefixedMsgHashHex, ecSignatureVRS, normalizedSignerAddress);
if (isValidVRSSignature) {
return ecSignatureVRS;
}
@ -154,7 +149,7 @@ export async function ecSignOrderHashAsync(
const ecSignatureRSV = parseSignatureHexAsRSV(signature);
if (_.includes(validVParamValues, ecSignatureRSV.v)) {
const isValidRSVSignature = isValidECSignature(orderHash, ecSignatureRSV, normalizedSignerAddress);
const isValidRSVSignature = isValidECSignature(prefixedMsgHashHex, ecSignatureRSV, normalizedSignerAddress);
if (isValidRSVSignature) {
return ecSignatureRSV;
}
@ -163,6 +158,30 @@ export async function ecSignOrderHashAsync(
throw new Error(OrderError.InvalidSignature);
}
export function addSignedMessagePrefix(message: string, messagePrefixType: MessagePrefixType): string {
switch (messagePrefixType) {
case MessagePrefixType.None:
return message;
case MessagePrefixType.EthSign: {
const msgBuff = ethUtil.toBuffer(message);
const prefixedMsgBuff = ethUtil.hashPersonalMessage(msgBuff);
const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
return prefixedMsgHex;
}
case MessagePrefixType.Trezor: {
const msgBuff = ethUtil.toBuffer(message);
const prefixedMsgBuff = hashTrezorPersonalMessage(msgBuff);
const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
return prefixedMsgHex;
}
default:
throw new Error(`Unrecognized MessagePrefixType: ${messagePrefixType}`);
}
}
function hashTrezorPersonalMessage(message: Buffer): Buffer {
const prefix = ethUtil.toBuffer('\x19Ethereum Signed Message:\n' + String.fromCharCode(message.length));
return ethUtil.sha3(Buffer.concat([prefix, message]));
@ -183,11 +202,6 @@ function parseECSignature(signature: string): ECSignature {
return ecSignature;
}
function intToHex(i: number): string {
const hex = ethUtil.bufferToHex(ethUtil.toBuffer(i));
return hex;
}
function getSignatureTypeIndexIfExists(signature: string): number {
const unprefixedSignature = ethUtil.stripHexPrefix(signature);
const signatureTypeHex = unprefixedSignature.substr(0, 2);

View File

@ -1,3 +1,25 @@
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;
}

View File

@ -39,7 +39,7 @@ describe('Order hashing', () => {
it('throws a readable error message if taker format is invalid', async () => {
const orderWithInvalidtakerFormat = {
...order,
taker: (null as any) as string,
takerAddress: (null as any) as string,
};
const expectedErrorMessage =
'Order taker must be of type string. If you want anyone to be able to fill an order - pass ZeroEx.NULL_ADDRESS';

View File

@ -7,7 +7,7 @@ import 'make-promises-safe';
import 'mocha';
import * as Sinon from 'sinon';
import { ecSignOrderHashAsync, generatePseudoRandomSalt, orderHashUtils } from '../src';
import { ecSignOrderHashAsync, generatePseudoRandomSalt, MessagePrefixType, orderHashUtils } from '../src';
import { isValidECSignature, isValidSignatureAsync } from '../src/signature_utils';
import { chaiSetup } from './utils/chai_setup';
@ -16,8 +16,6 @@ import { provider, web3Wrapper } from './utils/web3_wrapper';
chaiSetup.configure();
const expect = chai.expect;
const SHOULD_ADD_PERSONAL_MESSAGE_PREFIX = false;
describe('Signature utils', () => {
describe('#isValidSignature', () => {
let dataHex = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0';
@ -75,28 +73,28 @@ describe('Signature utils', () => {
// TODO: remaining sigs
});
describe('#isValidECSignature', () => {
// The Exchange smart contract `isValidECSignature` method only validates orderHashes and assumes
// the length of the data is exactly 32 bytes. Thus for these tests, we use data of this size.
const dataHex = '0x6927e990021d23b1eb7b8789f6a6feaf98fe104bb0cf8259421b79f9a34222b0';
// TODO: Replace this with a vanilla signature without ANY prefix or modification
const signature = {
v: 27,
r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33',
s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254',
r: '0xaca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d64393',
s: '0x46b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf2',
};
const address = '0x5409ed021d9299bf6814279a6a1411a7e866a631';
const data = '0x47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad';
const address = '0x0e5cb767cce09a7f3ca594df118aa519be5e2b5a';
it("should return false if the data doesn't pertain to the signature & address", async () => {
expect(isValidECSignature('0x0', signature, address)).to.be.false();
});
it("should return false if the address doesn't pertain to the signature & data", async () => {
const validUnrelatedAddress = '0x8b0292b11a196601ed2ce54b665cafeca0347d42';
expect(isValidECSignature(dataHex, signature, validUnrelatedAddress)).to.be.false();
expect(isValidECSignature(data, signature, validUnrelatedAddress)).to.be.false();
});
it("should return false if the signature doesn't pertain to the dataHex & address", async () => {
it("should return false if the signature doesn't pertain to the data & address", async () => {
const wrongSignature = _.assign({}, signature, { v: 28 });
expect(isValidECSignature(dataHex, wrongSignature, address)).to.be.false();
expect(isValidECSignature(data, wrongSignature, address)).to.be.false();
});
it('should return true if the signature does pertain to the dataHex & address', async () => {
const isValidSignatureLocal = isValidECSignature(dataHex, signature, address);
it('should return true if the signature does pertain to the data & address', async () => {
const isValidSignatureLocal = isValidECSignature(data, signature, address);
expect(isValidSignatureLocal).to.be.true();
});
});
@ -147,12 +145,11 @@ describe('Signature utils', () => {
r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33',
s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254',
};
const ecSignature = await ecSignOrderHashAsync(
provider,
orderHash,
makerAddress,
SHOULD_ADD_PERSONAL_MESSAGE_PREFIX,
);
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 () => {
@ -181,12 +178,11 @@ describe('Signature utils', () => {
},
};
const ecSignature = await ecSignOrderHashAsync(
fakeProvider,
orderHash,
makerAddress,
SHOULD_ADD_PERSONAL_MESSAGE_PREFIX,
);
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 () => {
@ -212,12 +208,11 @@ describe('Signature utils', () => {
},
};
const ecSignature = await ecSignOrderHashAsync(
fakeProvider,
orderHash,
makerAddress,
SHOULD_ADD_PERSONAL_MESSAGE_PREFIX,
);
const messagePrefixOpts = {
prefixType: MessagePrefixType.EthSign,
shouldAddPrefixBeforeCallingEthSign: false,
};
const ecSignature = await ecSignOrderHashAsync(fakeProvider, orderHash, makerAddress, messagePrefixOpts);
expect(ecSignature).to.deep.equal(expectedECSignature);
});
});