Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

[0x.js][order-utils][web3-wrapper] Expose eth_signTypedData functionality for order signing #1102

Merged
merged 10 commits into from
Oct 9, 2018
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"async-child-process": "^1.1.1",
"bundlesize": "^0.17.0",
"coveralls": "^3.0.0",
"ganache-cli": "6.1.3",
"ganache-cli": "6.1.8",
"lcov-result-merger": "^3.0.0",
"npm-cli-login": "^0.0.10",
"npm-run-all": "^4.1.2",
Expand Down
18 changes: 18 additions & 0 deletions packages/0x.js/CHANGELOG.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,22 @@
[
{
"version": "2.0.0",
"changes": [
{
"note": "Add support for `eth_signTypedData`.",
"pr": 1102
},
{
"note": "Added `MetamaskSubprovider` to handle inconsistencies with signing.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inconsistencies in Metamask's signing JSON RPC endpoints

"pr": 1102
},
{
"note":
"Removed `SignerType` (including `SignerType.Metamask`). Please use the `MetamaskSubprovider` to wrap `web3.currentProvider`.",
"pr": 1102
}
]
},
{
"version": "1.0.8",
"changes": [
Expand Down
9 changes: 7 additions & 2 deletions packages/0x.js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,13 @@ export { OrderWatcher, OnOrderStateChangeCallback, OrderWatcherConfig } from '@0

export import Web3ProviderEngine = require('web3-provider-engine');

export { RPCSubprovider, Callback, JSONRPCRequestPayloadWithMethod, ErrorCallback } from '@0xproject/subproviders';
export {
RPCSubprovider,
Callback,
JSONRPCRequestPayloadWithMethod,
ErrorCallback,
MetamaskSubprovider,
} from '@0xproject/subproviders';

export { AbiDecoder } from '@0xproject/utils';

Expand All @@ -68,7 +74,6 @@ export {
OrderStateInvalid,
OrderState,
AssetProxyId,
SignerType,
ERC20AssetData,
ERC721AssetData,
SignatureType,
Expand Down
22 changes: 5 additions & 17 deletions packages/contract-wrappers/src/utils/transaction_encoder.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,13 @@
import { schemas } from '@0xproject/json-schemas';
import { EIP712Schema, EIP712Types, eip712Utils } from '@0xproject/order-utils';
import { eip712Utils } from '@0xproject/order-utils';
import { Order, SignedOrder } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import { BigNumber, signTypedDataUtils } from '@0xproject/utils';
import _ = require('lodash');

import { ExchangeContract } from '../contract_wrappers/generated/exchange';

import { assert } from './assert';

const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = {
name: 'ZeroExTransaction',
parameters: [
{ name: 'salt', type: EIP712Types.Uint256 },
{ name: 'signerAddress', type: EIP712Types.Address },
{ name: 'data', type: EIP712Types.Bytes },
],
};

/**
* Transaction Encoder. Transaction messages exist for the purpose of calling methods on the Exchange contract
* in the context of another address. For example, UserA can encode and sign a fillOrder transaction and UserB
Expand All @@ -41,12 +32,9 @@ export class TransactionEncoder {
signerAddress,
data,
};
const executeTransactionHashBuff = eip712Utils.structHash(
EIP712_ZEROEX_TRANSACTION_SCHEMA,
executeTransactionData,
);
const eip721MessageBuffer = eip712Utils.createEIP712Message(executeTransactionHashBuff, exchangeAddress);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we get rid of createEIP712Message and structHash?

Copy link
Member Author

@dekz dekz Oct 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EIP712 sign typed data was moved to utils to be shared with subproviders and tests and exposed for others to use.

signTypedDataUtils is now a generic implementation (from the EIP712 examples).

Our original implementation was 0x specific and didn't support nested types. The latest version in utils can be used by other developers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, it can be used by external devs but not by us?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order-utils is exporting (and using) 0x specific EIP712 message creation. Which uses the EIP712 generic implementation in the utils package.

const messageHex = `0x${eip721MessageBuffer.toString('hex')}`;
const typedData = eip712Utils.createZeroExTransactionTypedData(executeTransactionData, exchangeAddress);
const eip712MessageBuffer = signTypedDataUtils.signTypedDataHash(typedData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename signTypedDataHash to generateTypedDataHash. There is no signing going on and the current method name suggests as much.

const messageHex = `0x${eip712MessageBuffer.toString('hex')}`;
return messageHex;
}
/**
Expand Down
9 changes: 2 additions & 7 deletions packages/contract-wrappers/test/transaction_encoder_test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { FillScenarios } from '@0xproject/fill-scenarios';
import { assetDataUtils, generatePseudoRandomSalt, orderHashUtils, signatureUtils } from '@0xproject/order-utils';
import { SignedOrder, SignerType } from '@0xproject/types';
import { SignedOrder } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import 'mocha';

Expand Down Expand Up @@ -80,12 +80,7 @@ describe('TransactionEncoder', () => {
): Promise<void> => {
const salt = generatePseudoRandomSalt();
const encodedTransaction = encoder.getTransactionHex(data, salt, signerAddress);
const signature = await signatureUtils.ecSignOrderHashAsync(
provider,
encodedTransaction,
signerAddress,
SignerType.Default,
);
const signature = await signatureUtils.ecSignHashAsync(provider, encodedTransaction, signerAddress);
txHash = await contractWrappers.exchange.executeTransactionAsync(
salt,
signerAddress,
Expand Down
18 changes: 1 addition & 17 deletions packages/contracts/test/exchange/libs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { assetDataUtils, eip712Utils, orderHashUtils } from '@0xproject/order-utils';
import { assetDataUtils, orderHashUtils } from '@0xproject/order-utils';
import { SignedOrder } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
Expand Down Expand Up @@ -126,22 +126,6 @@ describe('Exchange libs', () => {
});

describe('LibOrder', () => {
describe('getOrderSchema', () => {
it('should output the correct order schema hash', async () => {
const orderSchema = await libs.getOrderSchemaHash.callAsync();
const schemaHashBuffer = orderHashUtils._getOrderSchemaBuffer();
const schemaHashHex = `0x${schemaHashBuffer.toString('hex')}`;
expect(schemaHashHex).to.be.equal(orderSchema);
});
});
describe('getDomainSeparatorSchema', () => {
it('should output the correct domain separator schema hash', async () => {
const domainSeparatorSchema = await libs.getDomainSeparatorSchemaHash.callAsync();
const domainSchemaBuffer = eip712Utils._getDomainSeparatorSchemaBuffer();
const schemaHashHex = `0x${domainSchemaBuffer.toString('hex')}`;
expect(schemaHashHex).to.be.equal(domainSeparatorSchema);
});
});
describe('getOrderHash', () => {
it('should output the correct orderHash', async () => {
signedOrder = await orderFactory.newSignedOrderAsync();
Expand Down
12 changes: 3 additions & 9 deletions packages/contracts/test/exchange/signature_validator.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { assetDataUtils, orderHashUtils, signatureUtils } from '@0xproject/order-utils';
import { RevertReason, SignatureType, SignedOrder, SignerType } from '@0xproject/types';
import { RevertReason, SignatureType, SignedOrder } from '@0xproject/types';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
import ethUtil = require('ethereumjs-util');
Expand Down Expand Up @@ -231,10 +231,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 = signatureUtils.addSignedMessagePrefix(
orderHashHex,
SignerType.Default,
);
const orderHashWithEthSignPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex);
const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex);
const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey);
// Create 0x signature from EthSign signature
Expand All @@ -257,10 +254,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 = signatureUtils.addSignedMessagePrefix(
orderHashHex,
SignerType.Default,
);
const orderHashWithEthSignPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex);
const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex);
const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey);
// Create 0x signature from EthSign signature
Expand Down
22 changes: 6 additions & 16 deletions packages/contracts/test/utils/transaction_factory.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,11 @@
import { EIP712Schema, EIP712Types, eip712Utils, generatePseudoRandomSalt } from '@0xproject/order-utils';
import { eip712Utils, generatePseudoRandomSalt } from '@0xproject/order-utils';
import { SignatureType } from '@0xproject/types';
import { signTypedDataUtils } from '@0xproject/utils';
import * as ethUtil from 'ethereumjs-util';

import { signingUtils } from './signing_utils';
import { SignedTransaction } from './types';

const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = {
name: 'ZeroExTransaction',
parameters: [
{ name: 'salt', type: EIP712Types.Uint256 },
{ name: 'signerAddress', type: EIP712Types.Address },
{ name: 'data', type: EIP712Types.Bytes },
],
};

export class TransactionFactory {
private readonly _signerBuff: Buffer;
private readonly _exchangeAddress: string;
Expand All @@ -31,12 +23,10 @@ export class TransactionFactory {
signerAddress,
data,
};
const executeTransactionHashBuff = eip712Utils.structHash(
EIP712_ZEROEX_TRANSACTION_SCHEMA,
executeTransactionData,
);
const txHash = eip712Utils.createEIP712Message(executeTransactionHashBuff, this._exchangeAddress);
const signature = signingUtils.signMessage(txHash, this._privateKey, signatureType);

const typedData = eip712Utils.createZeroExTransactionTypedData(executeTransactionData, this._exchangeAddress);
const eip712MessageBuffer = signTypedDataUtils.signTypedDataHash(typedData);
const signature = signingUtils.signMessage(eip712MessageBuffer, this._privateKey, signatureType);
const signedTx = {
exchangeAddress: this._exchangeAddress,
signature: `0x${signature.toString('hex')}`,
Expand Down
28 changes: 28 additions & 0 deletions packages/json-schemas/schemas/eip712_typed_data.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
export const eip712TypedData = {
id: '/eip712TypedData',
type: 'object',
properties: {
types: {
type: 'object',
properties: {
EIP712Domain: { type: 'array' },
},
additionalProperties: {
type: 'array',
items: {
type: 'object',
properties: {
name: { type: 'string' },
type: { type: 'string' },
},
required: ['name', 'type'],
},
},
required: ['EIP712Domain'],
},
primaryType: { type: 'string' },
domain: { type: 'object' },
message: { type: 'object' },
},
required: ['types', 'primaryType', 'domain', 'message'],
};
2 changes: 2 additions & 0 deletions packages/json-schemas/src/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { addressSchema, hexSchema, numberSchema } from '../schemas/basic_type_sc
import { blockParamSchema, blockRangeSchema } from '../schemas/block_range_schema';
import { callDataSchema } from '../schemas/call_data_schema';
import { ecSignatureParameterSchema, ecSignatureSchema } from '../schemas/ec_signature_schema';
import { eip712TypedData } from '../schemas/eip712_typed_data';
import { indexFilterValuesSchema } from '../schemas/index_filter_values_schema';
import { orderCancellationRequestsSchema } from '../schemas/order_cancel_schema';
import { orderFillOrKillRequestsSchema } from '../schemas/order_fill_or_kill_requests_schema';
Expand Down Expand Up @@ -39,6 +40,7 @@ export const schemas = {
hexSchema,
ecSignatureParameterSchema,
ecSignatureSchema,
eip712TypedData,
indexFilterValuesSchema,
orderCancellationRequestsSchema,
orderFillOrKillRequestsSchema,
Expand Down
17 changes: 17 additions & 0 deletions packages/order-utils/CHANGELOG.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
[
{
"version": "2.0.0",
"changes": [
{
"note": "Added `ecSignOrderAsync` to first sign an order as EIP712 and fallback to EthSign",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's replace as EIP712 and fallback to EthSign with via 'eth_signTypedData' and fallback to 'eth_sign'

"pr": 1102
},
{
"note": "Added `ecSignTypedDataOrderAsync` to sign an order exclusively as EIP712",
"pr": 1102
},
{
"note": "Rename `ecSignOrderHashAsync` to `ecSignHashAsync` removing `SignerType` parameter",
"pr": 1102
}
]
},
{
"version": "1.0.7",
"changes": [
Expand Down
35 changes: 35 additions & 0 deletions packages/order-utils/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,39 @@ export const constants = {
BASE_16: 16,
INFINITE_TIMESTAMP_SEC: new BigNumber(2524604400), // Close to infinite
ZERO_AMOUNT: new BigNumber(0),
EIP712_DOMAIN_NAME: '0x Protocol',
EIP712_DOMAIN_VERSION: '2',
EIP712_DOMAIN_SCHEMA: {
name: 'EIP712Domain',
parameters: [
{ name: 'name', type: 'string' },
{ name: 'version', type: 'string' },
{ name: 'verifyingContract', type: 'address' },
],
},
EIP712_ORDER_SCHEMA: {
name: 'Order',
parameters: [
{ name: 'makerAddress', type: 'address' },
{ name: 'takerAddress', type: 'address' },
{ name: 'feeRecipientAddress', type: 'address' },
{ name: 'senderAddress', type: 'address' },
{ name: 'makerAssetAmount', type: 'uint256' },
{ name: 'takerAssetAmount', type: 'uint256' },
{ name: 'makerFee', type: 'uint256' },
{ name: 'takerFee', type: 'uint256' },
{ name: 'expirationTimeSeconds', type: 'uint256' },
{ name: 'salt', type: 'uint256' },
{ name: 'makerAssetData', type: 'bytes' },
{ name: 'takerAssetData', type: 'bytes' },
],
},
EIP712_ZEROEX_TRANSACTION_SCHEMA: {
name: 'ZeroExTransaction',
parameters: [
{ name: 'salt', type: 'uint256' },
{ name: 'signerAddress', type: 'address' },
{ name: 'data', type: 'bytes' },
],
},
};
dekz marked this conversation as resolved.
Show resolved Hide resolved
Loading