From b9165c03af40983d885af2b18e729f11746de91d Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Mon, 2 Jul 2018 11:07:51 +1000 Subject: 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 --- .../subproviders/eth_lightwallet_subprovider.ts | 40 +++++------- .../test/unit/eth_lightwallet_subprovider_test.ts | 74 ++++++++-------------- .../unit/private_key_wallet_subprovider_test.ts | 14 ++++ 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 { - 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 { - 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(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 = { -- cgit v1.2.3