Merge pull request #349 from 0xProject/fix/0x.js/signOrderHash

Add shouldAddPersonalMessagePrefix param to signOrderHashAsync

ISSUE_TEMPLATE.md

@@ -0,0 +1,62 @@
+<!--- Thank you for taking the time to file an Issue -->
+
+<!--- Before submitting please check to see if this issue was already reported -->
+
+<!--- Provide a general summary of the issue in the Title above -->
+
+## Expected Behavior
+
+<!--- If you're describing a bug, tell us what should happen -->
+
+<!--- If you're suggesting a package change/improvement, tell us how it should work -->
+
+<!--- If you're suggesting a contract or protocol change/improvement, visit our ZEIPs repo -->
+
+## Current Behavior
+
+<!--- If describing a bug, tell us what happens instead of the expected behavior -->
+
+<!--- If suggesting a change/improvement, explain the difference from current behavior -->
+
+## Possible Solution
+
+<!--- Not obligatory, but suggest a fix/reason for the bug, -->
+
+<!--- or ideas how to implement the addition or change -->
+
+## Steps to Reproduce (for bugs)
+
+<!--- Provide a link to a live example, or an unambiguous set of steps to -->
+
+<!--- reproduce this bug. Include code to reproduce, if relevant -->
+
+```
+1.
+2.
+3.
+```
+
+## Context
+
+<!--- How has this issue affected you? What are you trying to accomplish? -->
+
+<!--- Providing context helps us come up with a solution that is most useful in the real world -->
+
+## Your Environment
+
+<!--- Include as many relevant details about the environment you experienced the bug in -->
+
+|             Package | Version |
+| ------------------: | :------ |
+|             `0x.js` | 0.25.0  |
+| `Exchange Contract` | v1      |
+
+| Network |
+| ------- |
+| NAME    |
+
+<!-- For example:
+| mainnet |
+| kovan |
+| testrpc |
+-->

PULL_REQUEST_TEMPLATE.md

@@ -1,3 +1,36 @@
-This PR:
+<!--- Thank you for taking the time to submit a Pull Request -->
 
-\*
+<!--- Provide a general summary of the issue in the Title above -->
+
+## Description
+
+<!--- Describe your changes in detail -->
+
+## Motivation and Context
+
+<!--- Why is this change required? What problem does it solve? -->
+
+<!--- If it fixes an open issue, please link to the issue here. -->
+
+## How Has This Been Tested?
+
+<!--- Please describe in detail how you tested your changes. -->
+
+<!--- Include details of your testing environment, and the tests you ran to -->
+
+<!--- see how your change affects other areas of the code, etc. -->
+
+## Types of changes
+
+<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply:
+- [ ] Bug fix (non-breaking change which fixes an issue)
+- [ ] New feature (non-breaking change which adds functionality)
+- [ ] Breaking change (fix or feature that would cause existing functionality to change)
+
+## Checklist:
+<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
+
+<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
+
+* [ ] Change requires a change to the documentation.
+* [ ] Added tests to cover my changes.

package.json

@@ -4,8 +4,8 @@
     "workspaces": ["packages/*"],
     "scripts": {
         "testrpc": "testrpc -p 8545 --networkId 50 -m \"${npm_package_config_mnemonic}\"",
-        "prettier": "prettier --write '**/*.{ts,tsx,json,md}'",
-        "prettier:ci": "prettier --list-different '**/*.{ts,tsx,json,md}'",
+        "prettier": "prettier --write '**/*.{ts,tsx,json,md}' --config .prettierrc",
+        "prettier:ci": "prettier --list-different '**/*.{ts,tsx,json,md}' --config .prettierrc",
         "lerna:run": "lerna run",
         "lerna:rebuild": "lerna run clean; lerna run build;",
         "lerna:publish":

packages/0x.js/CHANGELOG.md

@@ -1,10 +1,21 @@
 # CHANGELOG
 
-## v0.30.1 - _TBD, 2018_
+## v0.31.0 - _January 30, 2018_
+
+    * Add the `shouldAddPersonalMessagePrefix` parameter to `signOrderHashAsync` so that the
+    caller can decide on whether to add the personalMessage prefix before relaying the request
+    to the signer. Parity Signer, Ledger and TestRPC add the prefix themselves, Metamask expects
+    it to have already been added. (#349)
+
+## v0.30.2 - _January 29, 2018_
+
+    * Add Rinkeby testnet addresses to artifacts  (#337)
+    * Move @0xproject/types to dependencies from devDependencies fixing missing type errors
+
+## v0.30.1 - _January 24, 2018_
 
     * Fix a bug allowing negative fill values  (#212)
     * Fix a bug that made it impossible to pass a custom ZRX address  (#341)
-    * Add Rinkeby testnet addresses to artifacts  (#337)
 
 ## v0.30.0 - _January 17, 2018_
 

packages/0x.js/package.json

@@ -2,27 +2,24 @@
     "name": "0x.js",
     "version": "0.30.2",
     "description": "A javascript library for interacting with the 0x protocol",
-    "keywords": [
-        "0x.js",
-        "0xproject",
-        "ethereum",
-        "tokens",
-        "exchange"
-    ],
+    "keywords": ["0x.js", "0xproject", "ethereum", "tokens", "exchange"],
     "main": "lib/src/index.js",
     "types": "lib/src/index.d.ts",
     "scripts": {
         "prebuild": "run-s clean generate_contract_wrappers",
         "build": "run-p build:umd:prod build:commonjs; exit 0;",
         "docs:json": "typedoc --excludePrivate --excludeExternals --target ES5 --json $JSON_FILE_PATH $PROJECT_DIR",
-        "upload_docs_json": "aws s3 cp generated_docs/index.json $S3_URL --profile 0xproject --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers --content-type application/json",
-        "generate_contract_wrappers": "node ../abi-gen/lib/index.js --abiGlob 'src/artifacts/@(Exchange|Token|TokenTransferProxy|EtherToken|TokenRegistry|DummyToken).json' --templates contract_templates --output src/contract_wrappers/generated",
+        "upload_docs_json":
+            "aws s3 cp generated_docs/index.json $S3_URL --profile 0xproject --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers --content-type application/json",
+        "generate_contract_wrappers":
+            "node ../abi-gen/lib/index.js --abiGlob 'src/artifacts/@(Exchange|Token|TokenTransferProxy|EtherToken|TokenRegistry|DummyToken).json' --templates contract_templates --output src/contract_wrappers/generated",
         "lint": "tslint --project . 'src/**/*.ts' 'test/**/*.ts'",
         "test:circleci": "run-s test:coverage report_test_coverage",
         "test": "run-s clean test:commonjs",
         "test:coverage": "nyc npm run test --all",
         "report_test_coverage": "nyc report --reporter=text-lcov | coveralls",
-        "update_contracts": "for i in ${npm_package_config_artifacts}; do copyfiles -u 4 ../contracts/build/contracts/$i.json ../0x.js/src/artifacts; done;",
+        "update_contracts":
+            "for i in ${npm_package_config_artifacts}; do copyfiles -u 4 ../contracts/build/contracts/$i.json ../0x.js/src/artifacts; done;",
         "clean": "shx rm -rf _bundles lib test_temp",
         "build:umd:prod": "NODE_ENV=production webpack",
         "build:commonjs": "tsc && copyfiles -u 2 './src/artifacts/**/*.json' ./lib/src/artifacts;",

packages/0x.js/src/0x.ts

@@ -240,20 +240,22 @@ 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 Web3.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.
      * @return  An object containing the Elliptic curve signature parameters generated by signing the orderHash.
      */
-    public async signOrderHashAsync(orderHash: string, signerAddress: string): Promise<ECSignature> {
+    public async signOrderHashAsync(
+        orderHash: string,
+        signerAddress: string,
+        shouldAddPersonalMessagePrefix: boolean,
+    ): Promise<ECSignature> {
         assert.isHexString('orderHash', orderHash);
         await assert.isSenderAddressAsync('signerAddress', signerAddress, this._web3Wrapper);
 
-        let msgHashHex;
-        const nodeVersion = await this._web3Wrapper.getNodeVersionAsync();
-        const isParityNode = utils.isParityNode(nodeVersion);
-        const isTestRpc = utils.isTestRpc(nodeVersion);
-        if (isParityNode || isTestRpc) {
-            // Parity and TestRpc nodes add the personalMessage prefix itself
-            msgHashHex = orderHash;
-        } else {
+        let msgHashHex = orderHash;
+        if (shouldAddPersonalMessagePrefix) {
             const orderHashBuff = ethUtil.toBuffer(orderHash);
             const msgHashBuff = ethUtil.hashPersonalMessage(orderHashBuff);
             msgHashHex = ethUtil.bufferToHex(msgHashBuff);

packages/0x.js/src/utils/utils.ts

@@ -20,12 +20,6 @@ export const utils = {
         // tslint:disable-next-line: no-console
         console.log(message);
     },
-    isParityNode(nodeVersion: string): boolean {
-        return _.includes(nodeVersion, 'Parity');
-    },
-    isTestRpc(nodeVersion: string): boolean {
-        return _.includes(nodeVersion, 'TestRPC');
-    },
     spawnSwitchErr(name: string, value: any): Error {
         return new Error(`Unexpected switch value: ${value} encountered for ${name}`);
     },

packages/0x.js/test/0x.js_test.ts

@@ -16,6 +16,8 @@ const blockchainLifecycle = new BlockchainLifecycle(constants.RPC_URL);
 chaiSetup.configure();
 const expect = chai.expect;
 
+const SHOULD_ADD_PERSONAL_MESSAGE_PREFIX = false;
+
 describe('ZeroEx library', () => {
     const web3 = web3Factory.create();
     const config = {
@@ -198,7 +200,11 @@ describe('ZeroEx library', () => {
                 r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33',
                 s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254',
             };
-            const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress);
+            const ecSignature = await zeroEx.signOrderHashAsync(
+                orderHash,
+                makerAddress,
+                SHOULD_ADD_PERSONAL_MESSAGE_PREFIX,
+            );
             expect(ecSignature).to.deep.equal(expectedECSignature);
         });
         it('should return the correct ECSignature for signatureHex concatenated as R + S + V', async () => {
@@ -215,7 +221,11 @@ describe('ZeroEx library', () => {
                 Sinon.stub(ZeroEx, 'isValidSignature').returns(true),
             ];
 
-            const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress);
+            const ecSignature = await zeroEx.signOrderHashAsync(
+                orderHash,
+                makerAddress,
+                SHOULD_ADD_PERSONAL_MESSAGE_PREFIX,
+            );
             expect(ecSignature).to.deep.equal(expectedECSignature);
         });
         it('should return the correct ECSignature for signatureHex concatenated as V + R + S', async () => {
@@ -232,7 +242,11 @@ describe('ZeroEx library', () => {
                 Sinon.stub(ZeroEx, 'isValidSignature').returns(true),
             ];
 
-            const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress);
+            const ecSignature = await zeroEx.signOrderHashAsync(
+                orderHash,
+                makerAddress,
+                SHOULD_ADD_PERSONAL_MESSAGE_PREFIX,
+            );
             expect(ecSignature).to.deep.equal(expectedECSignature);
         });
     });

packages/0x.js/test/utils/order_factory.ts

@@ -3,6 +3,8 @@ import * as _ from 'lodash';
 
 import { SignedOrder, ZeroEx } from '../../src';
 
+const SHOULD_ADD_PERSONAL_MESSAGE_PREFIX = false;
+
 export const orderFactory = {
     async createSignedOrderAsync(
         zeroEx: ZeroEx,
@@ -37,7 +39,7 @@ export const orderFactory = {
             expirationUnixTimestampSec,
         };
         const orderHash = ZeroEx.getOrderHashHex(order);
-        const ecSignature = await zeroEx.signOrderHashAsync(orderHash, maker);
+        const ecSignature = await zeroEx.signOrderHashAsync(orderHash, maker, SHOULD_ADD_PERSONAL_MESSAGE_PREFIX);
         const signedOrder: SignedOrder = _.assign(order, { ecSignature });
         return signedOrder;
     },

packages/chai-as-promised-typescript-typings/package.json

@@ -9,9 +9,7 @@
         "url": "git+https://github.com/0xProject/0x.js.git"
     },
     "author": "Fabio Berger",
-    "contributors": [
-        "Leonid Logvinov <logvinov.leon@gmail.com>"
-    ],
+    "contributors": ["Leonid Logvinov <logvinov.leon@gmail.com>"],
     "license": "Apache-2.0",
     "bugs": {
         "url": "https://github.com/0xProject/0x.js/issues"

packages/connect/package.json

@@ -2,20 +2,15 @@
     "name": "@0xproject/connect",
     "version": "0.5.2",
     "description": "A javascript library for interacting with the standard relayer api",
-    "keywords": [
-        "connect",
-        "0xproject",
-        "ethereum",
-        "tokens",
-        "exchange"
-    ],
+    "keywords": ["connect", "0xproject", "ethereum", "tokens", "exchange"],
     "main": "lib/src/index.js",
     "types": "lib/src/index.d.ts",
     "scripts": {
         "build": "tsc",
         "clean": "shx rm -rf _bundles lib test_temp",
         "docs:json": "typedoc --excludePrivate --excludeExternals --target ES5 --json $JSON_FILE_PATH $PROJECT_DIR",
-        "upload_docs_json": "aws s3 cp generated_docs/index.json $S3_URL --profile 0xproject --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers --content-type application/json",
+        "upload_docs_json":
+            "aws s3 cp generated_docs/index.json $S3_URL --profile 0xproject --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers --content-type application/json",
         "copy_test_fixtures": "copyfiles -u 2 './test/fixtures/**/*.json' ./lib/test/fixtures",
         "lint": "tslint --project . 'src/**/*.ts' 'test/**/*.ts'",
         "run_mocha": "mocha lib/test/**/*_test.js",

packages/contracts/package.json

@@ -8,9 +8,11 @@
         "test": "test"
     },
     "scripts": {
-        "build": "rm -rf ./lib; copyfiles ./build/**/* ./deploy/solc/solc_bin/* ./deploy/test/fixtures/contracts/**/* ./deploy/test/fixtures/contracts/* ./lib; tsc;",
+        "build":
+            "rm -rf ./lib; copyfiles ./build/**/* ./deploy/solc/solc_bin/* ./deploy/test/fixtures/contracts/**/* ./deploy/test/fixtures/contracts/* ./lib; tsc;",
         "test": "npm run build; truffle test",
-        "compile:comment": "Yarn workspaces do not link binaries correctly so we need to reference them directly https://github.com/yarnpkg/yarn/issues/3846",
+        "compile:comment":
+            "Yarn workspaces do not link binaries correctly so we need to reference them directly https://github.com/yarnpkg/yarn/issues/3846",
         "compile": "node ../deployer/lib/src/cli.js compile",
         "clean": "rm -rf ./lib",
         "migrate": "node ../deployer/lib/src/cli.js migrate",

packages/tslint-config/package.json

@@ -8,22 +8,12 @@
         "clean": "shx rm -rf lib",
         "lint": "tslint --project . 'rules/**/*.ts'"
     },
-    "files": [
-        "tslint.js",
-        "README.md",
-        "LICENSE"
-    ],
+    "files": ["tslint.js", "README.md", "LICENSE"],
     "repository": {
         "type": "git",
         "url": "git://github.com/0xProject/0x.js.git"
     },
-    "keywords": [
-        "tslint",
-        "config",
-        "0xProject",
-        "typescript",
-        "ts"
-    ],
+    "keywords": ["tslint", "config", "0xProject", "typescript", "ts"],
     "author": {
         "name": "Fabio Berger",
         "email": "fabio@0xproject.com"

packages/web3-typescript-typings/package.json

@@ -12,9 +12,7 @@
         "url": "git+https://github.com/0xProject/0x.js.git"
     },
     "author": "Fabio Berger",
-    "contributors": [
-        "Leonid Logvinov <logvinov.leon@gmail.com>"
-    ],
+    "contributors": ["Leonid Logvinov <logvinov.leon@gmail.com>"],
     "license": "Apache-2.0",
     "bugs": {
         "url": "https://github.com/0xProject/0x.js/issues"

packages/website/package.json

@@ -8,9 +8,12 @@
         "clean": "shx rm -f public/bundle*",
         "lint": "tslint --project . 'ts/**/*.ts' 'ts/**/*.tsx'",
         "dev": "webpack-dev-server --content-base public --https",
-        "update_contracts": "for i in ${npm_package_config_artifacts}; do copyfiles -u 4 ../contracts/build/contracts/$i.json ../website/contracts; done;",
-        "deploy_staging": "npm run build; aws s3 sync ./public/. s3://staging-0xproject --profile 0xproject --region us-east-1 --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers",
-        "deploy_live": "npm run build; aws s3 sync ./public/. s3://0xproject.com --profile 0xproject --region us-east-1 --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers"
+        "update_contracts":
+            "for i in ${npm_package_config_artifacts}; do copyfiles -u 4 ../contracts/build/contracts/$i.json ../website/contracts; done;",
+        "deploy_staging":
+            "npm run build; aws s3 sync ./public/. s3://staging-0xproject --profile 0xproject --region us-east-1 --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers",
+        "deploy_live":
+            "npm run build; aws s3 sync ./public/. s3://0xproject.com --profile 0xproject --region us-east-1 --grants read=uri=http://acs.amazonaws.com/groups/global/AllUsers"
     },
     "config": {
         "artifacts": "Mintable"