Use PrivateKeySubprovider inside eth lightwallet

There's a loss of information when hex encoding and passing to eth light wallet (chain id is lost). This results in a different signature. While it may work on testnets it is not sufficient for our test cases. We can export the private key and use it in our PrivateKeyWalletSubprovider

3 files changed, 57 insertions, 71 deletions

diff --git a/packages/subproviders/src/subproviders/eth_lightwallet_subprovider.ts b/packages/subproviders/src/subproviders/eth_lightwallet_subprovider.ts
index 64d984996..b594ffb24 100644
--- a/packages/subproviders/src/subproviders/eth_lightwallet_subprovider.ts
+++ b/packages/subproviders/src/subproviders/eth_lightwallet_subprovider.ts
@@ -2,11 +2,13 @@ import { assert } from '@0xproject/assert';
 import { addressUtils } from '@0xproject/utils';
 import * as lightwallet from 'eth-lightwallet';
 import EthereumTx = require('ethereumjs-tx');
+import * as ethUtil from 'ethereumjs-util';
 import * as _ from 'lodash';
 
 import { PartialTxParams, WalletSubproviderErrors } from '../types';
 
 import { BaseWalletSubprovider } from './base_wallet_subprovider';
+import { PrivateKeyWalletSubprovider } from './private_key_wallet';
 
 /*
  * This class implements the web3-provider-engine subprovider interface and forwards
@@ -42,17 +44,14 @@ export class EthLightwalletSubprovider extends BaseWalletSubprovider {
      * @return Signed transaction hex string
      */
     public async signTransactionAsync(txParams: PartialTxParams): Promise<string> {
-        if (_.isUndefined(txParams.from) || !addressUtils.isAddress(txParams.from)) {
-            throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid);
-        }
-
-        const tx = new EthereumTx(txParams);
-        const txHex = tx.serialize().toString('hex');
-        let signedTxHex: string = lightwallet.signing.signTx(this._keystore, this._pwDerivedKey, txHex, txParams.from);
-
-        signedTxHex = `0x${signedTxHex}`;
-
-        return signedTxHex;
+        // Lightwallet loses the chain id information when hex encoding the transaction
+        // this results in a different signature on certain networks. PrivateKeyWallet
+        // respects this as it uses the parameters passed in
+        let privKey = this._keystore.exportPrivateKey(txParams.from, this._pwDerivedKey);
+        const privKeyWallet = new PrivateKeyWalletSubprovider(privKey);
+        const privKeySignature = await privKeyWallet.signTransactionAsync(txParams);
+        privKey = '';
+        return privKeySignature;
     }
     /**
      * Sign a personal Ethereum signed message. The signing account will be the account
@@ -65,19 +64,10 @@ export class EthLightwalletSubprovider extends BaseWalletSubprovider {
      * @return Signature hex string (order: rsv)
      */
     public async signPersonalMessageAsync(data: string, address: string): Promise<string> {
-        if (_.isUndefined(data)) {
-            throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage);
-        }
-        assert.isHexString('data', data);
-        assert.isETHAddressHex('address', address);
-        const result: ECSignatureBuffer = lightwallet.signing.signMsgHash(
-            this._keystore,
-            this._pwDerivedKey,
-            data,
-            address,
-        );
-
-        const signature = lightwallet.signing.concatSig(result);
-        return signature;
+        let privKey = this._keystore.exportPrivateKey(address, this._pwDerivedKey);
+        const privKeyWallet = new PrivateKeyWalletSubprovider(privKey);
+        privKey = '';
+        const result = privKeyWallet.signPersonalMessageAsync(data, address);
+        return result;
     }
 }
diff --git a/packages/subproviders/test/unit/eth_lightwallet_subprovider_test.ts b/packages/subproviders/test/unit/eth_lightwallet_subprovider_test.ts
index c0adb9225..f17c21f02 100644
--- a/packages/subproviders/test/unit/eth_lightwallet_subprovider_test.ts
+++ b/packages/subproviders/test/unit/eth_lightwallet_subprovider_test.ts
@@ -1,34 +1,32 @@
 import * as chai from 'chai';
 import * as lightwallet from 'eth-lightwallet';
 import { JSONRPCResponsePayload } from 'ethereum-types';
+import * as ethUtils from 'ethereumjs-util';
 import Web3ProviderEngine = require('web3-provider-engine');
 
 import { EthLightwalletSubprovider } from '../../src';
 import { DoneCallback } from '../../src/types';
 import { chaiSetup } from '../chai_setup';
+import { fixtureData } from '../utils/fixture_data';
 import { ganacheSubprovider } from '../utils/ganache_subprovider';
 import { reportCallbackErrors } from '../utils/report_callback_errors';
 
 chaiSetup.configure();
 const expect = chai.expect;
 
-const FAKE_ADDRESS = '0x44be42fd88e22387c43ba9b75941aa3e680dae25';
-const NUM_GENERATED_ADDRESSES = 10;
+const DEFAULT_NUM_ACCOUNTS = 10;
 const PASSWORD = 'supersecretpassword99';
-const SEED_PHRASE = 'dilemma hollow outer pony cube season start stereo surprise when edit blast';
 const SALT = 'kvODghzs7Ff1uqHyI0P3wI4Hso4w4iWT2e9qmrWz0y4';
-const HD_PATH_STRING = `m/44'/60'/0'`;
 
 describe('EthLightwalletSubprovider', () => {
     let ethLightwalletSubprovider: EthLightwalletSubprovider;
     before(async () => {
         const options = {
             password: PASSWORD,
-            seedPhrase: SEED_PHRASE,
+            seedPhrase: fixtureData.TEST_RPC_MNEMONIC,
             salt: SALT,
-            hdPathString: HD_PATH_STRING,
+            hdPathString: fixtureData.TESTRPC_BASE_DERIVATION_PATH,
         };
-
         const createVaultAsync = async (vaultOptions: lightwallet.VaultOptions) => {
             return new Promise<lightwallet.keystore>(resolve => {
                 lightwallet.keystore.createVault(vaultOptions, (err: Error, vaultKeystore) => {
@@ -53,7 +51,7 @@ describe('EthLightwalletSubprovider', () => {
         const pwDerivedKey: Uint8Array = await deriveKeyFromPasswordAsync(keystore);
 
         // Generate 10 addresses
-        keystore.generateNewAddress(pwDerivedKey, NUM_GENERATED_ADDRESSES);
+        keystore.generateNewAddress(pwDerivedKey, DEFAULT_NUM_ACCOUNTS);
 
         ethLightwalletSubprovider = new EthLightwalletSubprovider(keystore, pwDerivedKey);
     });
@@ -61,22 +59,20 @@ describe('EthLightwalletSubprovider', () => {
         describe('success cases', () => {
             it('returns a list of accounts', async () => {
                 const accounts = await ethLightwalletSubprovider.getAccountsAsync();
-                expect(accounts[0]).to.be.equal(FAKE_ADDRESS);
-                expect(accounts.length).to.be.equal(NUM_GENERATED_ADDRESSES);
+                expect(accounts[0]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_0);
+                expect(accounts[1]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_1);
+                expect(accounts.length).to.be.equal(DEFAULT_NUM_ACCOUNTS);
             });
             it('signs a personal message hash', async () => {
-                const signingAccount = (await ethLightwalletSubprovider.getAccountsAsync())[0];
-
-                // Keccak-256 hash of 'hello world'
-                const messageHash = '0x47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad';
-                const ecSignatureHex = await ethLightwalletSubprovider.signPersonalMessageAsync(
-                    messageHash,
-                    signingAccount,
-                );
-                expect(ecSignatureHex).to.be.equal(
-                    // tslint:disable-next-line:max-line-length
-                    '0xa46b696c1aa8f91dbb33d1a66f6440bf3cf334c9dc45dc389668c1e60e2db31e259400b41f31632fa994837054c5345c88dc455c13931332489029adee6fd24d1b',
-                );
+                const accounts = await ethLightwalletSubprovider.getAccountsAsync();
+                const signingAccount = accounts[0];
+                const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING));
+                const ecSignatureHex = await ethLightwalletSubprovider.signPersonalMessageAsync(data, signingAccount);
+                expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT);
+            });
+            it('signs a transaction', async () => {
+                const txHex = await ethLightwalletSubprovider.signTransactionAsync(fixtureData.TX_DATA);
+                expect(txHex).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT);
             });
         });
     });
@@ -98,52 +94,38 @@ describe('EthLightwalletSubprovider', () => {
                 };
                 const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => {
                     expect(err).to.be.a('null');
-                    expect(response.result.length).to.be.equal(NUM_GENERATED_ADDRESSES);
-                    expect(response.result[0]).to.be.equal(FAKE_ADDRESS);
+                    expect(response.result[0]).to.be.equal(fixtureData.TEST_RPC_ACCOUNT_0);
+                    expect(response.result.length).to.be.equal(DEFAULT_NUM_ACCOUNTS);
                     done();
                 });
                 provider.sendAsync(payload, callback);
             });
             it('signs a personal message hash with eth_sign', (done: DoneCallback) => {
-                // Keccak-256 hash of 'hello world'
-                const messageHash = '0x47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad';
+                const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING));
+                const account = fixtureData.TEST_RPC_ACCOUNT_0;
                 const payload = {
                     jsonrpc: '2.0',
                     method: 'eth_sign',
-                    params: ['0x44be42fd88e22387c43ba9b75941aa3e680dae25', messageHash],
+                    params: [account, data],
                     id: 1,
                 };
                 const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => {
                     expect(err).to.be.a('null');
-                    expect(response.result).to.be.equal(
-                        // tslint:disable-next-line:max-line-length
-                        '0xa46b696c1aa8f91dbb33d1a66f6440bf3cf334c9dc45dc389668c1e60e2db31e259400b41f31632fa994837054c5345c88dc455c13931332489029adee6fd24d1b',
-                    );
+                    expect(response.result).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT);
                     done();
                 });
                 provider.sendAsync(payload, callback);
             });
             it('signs a transaction', (done: DoneCallback) => {
-                const tx = {
-                    to: '0xafa3f8684e54059998bc3a7b0d2b0da075154d66',
-                    value: '0x00',
-                    gasPrice: '0x00',
-                    nonce: '0x00',
-                    gas: '0x00',
-                    from: '0x44be42fd88e22387c43ba9b75941aa3e680dae25',
-                };
                 const payload = {
                     jsonrpc: '2.0',
                     method: 'eth_signTransaction',
-                    params: [tx],
+                    params: [fixtureData.TX_DATA],
                     id: 1,
                 };
                 const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => {
-                    const expectedResponseLength = 192;
-
                     expect(err).to.be.a('null');
-                    expect(response.result.raw.length).to.be.equal(expectedResponseLength);
-                    expect(response.result.raw.substr(0, 2)).to.be.equal('0x');
+                    expect(response.result.raw).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT);
                     done();
                 });
                 provider.sendAsync(payload, callback);
@@ -155,7 +137,7 @@ describe('EthLightwalletSubprovider', () => {
                 const payload = {
                     jsonrpc: '2.0',
                     method: 'eth_sign',
-                    params: ['0x0000000000000000000000000000000000000000', nonHexMessage],
+                    params: [fixtureData.TEST_RPC_ACCOUNT_0, nonHexMessage],
                     id: 1,
                 };
                 const callback = reportCallbackErrors(done)((err: Error, _response: JSONRPCResponsePayload) => {
@@ -170,7 +152,7 @@ describe('EthLightwalletSubprovider', () => {
                 const payload = {
                     jsonrpc: '2.0',
                     method: 'personal_sign',
-                    params: [nonHexMessage, '0x0000000000000000000000000000000000000000'],
+                    params: [nonHexMessage, fixtureData.TEST_RPC_ACCOUNT_0],
                     id: 1,
                 };
                 const callback = reportCallbackErrors(done)((err: Error, _response: JSONRPCResponsePayload) => {
diff --git a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts
index a41ad7790..ab321bcff 100644
--- a/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts
+++ b/packages/subproviders/test/unit/private_key_wallet_subprovider_test.ts
@@ -60,6 +60,20 @@ describe('PrivateKeyWalletSubprovider', () => {
                 });
                 provider.sendAsync(payload, callback);
             });
+            it('signs a transaction', (done: DoneCallback) => {
+                const payload = {
+                    jsonrpc: '2.0',
+                    method: 'eth_signTransaction',
+                    params: [fixtureData.TX_DATA],
+                    id: 1,
+                };
+                const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => {
+                    expect(err).to.be.a('null');
+                    expect(response.result.raw).to.be.equal(fixtureData.TX_DATA_SIGNED_RESULT);
+                    done();
+                });
+                provider.sendAsync(payload, callback);
+            });
             it('signs a personal message with eth_sign', (done: DoneCallback) => {
                 const messageHex = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING));
                 const payload = {