From ad90bddf85477d8ad25dbf4a804097e13383cf63 Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Wed, 27 Mar 2019 21:46:45 +0100 Subject: [PATCH 1/8] recover method in Accounts module optimized and tWallet types updated --- packages/web3-eth-accounts/src/Accounts.js | 11 +++++------ packages/web3-eth-accounts/types/index.d.ts | 19 +++++++++++++++---- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/web3-eth-accounts/src/Accounts.js b/packages/web3-eth-accounts/src/Accounts.js index 57c4d87d8c1..93599ac579c 100644 --- a/packages/web3-eth-accounts/src/Accounts.js +++ b/packages/web3-eth-accounts/src/Accounts.js @@ -27,7 +27,6 @@ import Hash from 'eth-lib/lib/hash'; import RLP from 'eth-lib/lib/rlp'; import Bytes from 'eth-lib/lib/bytes'; import {encodeSignature, recover} from 'eth-lib/lib/account'; // TODO: Remove this dependency -import {hexToBytes, isHexStrict} from 'web3-utils'; // TODO: Use the VO's of a web3-types module. import {AbstractWeb3Module} from 'web3-core'; import Account from './models/Account'; import Wallet from './models/Wallet'; @@ -91,8 +90,8 @@ export default class Accounts extends AbstractWeb3Module { * @returns {String} */ hashMessage(data) { - if (isHexStrict(data)) { - data = hexToBytes(data); + if (this.utils.isHexStrict(data)) { + data = this.utils.hexToBytes(data); } const messageBuffer = Buffer.from(data); @@ -201,8 +200,6 @@ export default class Accounts extends AbstractWeb3Module { * @returns {String} */ recover(message, signature, preFixed) { - const args = [].slice.apply(arguments); - if (isObject(message)) { return this.recover(message.messageHash, encodeSignature([message.v, message.r, message.s]), true); } @@ -211,7 +208,9 @@ export default class Accounts extends AbstractWeb3Module { message = this.hashMessage(message); } - if (args.length >= 4) { + if (arguments.length >= 4) { + const args = [...arguments]; + preFixed = args.slice(-1)[0]; preFixed = isBoolean(preFixed) ? preFixed : false; diff --git a/packages/web3-eth-accounts/types/index.d.ts b/packages/web3-eth-accounts/types/index.d.ts index 9c440da403c..46951aaee0d 100644 --- a/packages/web3-eth-accounts/types/index.d.ts +++ b/packages/web3-eth-accounts/types/index.d.ts @@ -13,16 +13,16 @@ */ /** * @file index.d.ts - * @author Josh Stevens + * @author Josh Stevens , Samuel Furter * @date 2018 */ -import {AbstractWeb3Module, TransactionConfig, Web3ModuleOptions, SignedTransaction} from 'web3-core'; +import {AbstractWeb3Module, SignedTransaction, TransactionConfig, Web3ModuleOptions} from 'web3-core'; import {provider} from 'web3-providers'; import * as net from 'net'; export class Accounts extends AbstractWeb3Module { - constructor(provider: provider, net?: net.Socket|null, options?: Web3ModuleOptions); + constructor(provider: provider, net?: net.Socket | null, options?: Web3ModuleOptions); create(entropy?: string): Account; @@ -36,7 +36,7 @@ export class Accounts extends AbstractWeb3Module { sign(data: string, privateKey: string): Sign; - recover(signedTransaction: SignedTransaction): string; + recover(signatureObject: SignatureObject): string; recover(message: string, signature: string, preFixed?: boolean): string; recover(message: string, v: string, r: string, s: string, preFixed?: boolean): string; @@ -50,6 +50,10 @@ export class Accounts extends AbstractWeb3Module { export class Wallet { constructor(accounts: Accounts); + accountsIndex: number; + length: number; + defaultKeyName: string; + create(numberOfAccounts: number, entropy?: string): Wallet; add(account: string | AddAccount): AddedAccount; @@ -111,3 +115,10 @@ export interface Sign extends SignedTransaction { message: string; signature: string; } + +export interface SignatureObject { + messageHash: string; + r: string; + s: string; + v: string; +} From a8a4002754de4f8e4a8fd8e54b0762955bea79aa Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Wed, 27 Mar 2019 21:51:19 +0100 Subject: [PATCH 2/8] [].slice removed in recover method of the Accounts module --- packages/web3-eth-accounts/src/Accounts.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/web3-eth-accounts/src/Accounts.js b/packages/web3-eth-accounts/src/Accounts.js index 93599ac579c..0e051d32897 100644 --- a/packages/web3-eth-accounts/src/Accounts.js +++ b/packages/web3-eth-accounts/src/Accounts.js @@ -211,10 +211,10 @@ export default class Accounts extends AbstractWeb3Module { if (arguments.length >= 4) { const args = [...arguments]; - preFixed = args.slice(-1)[0]; + preFixed = args[args.length - 1]; preFixed = isBoolean(preFixed) ? preFixed : false; - return this.recover(message, encodeSignature(args.slice(1, 4)), preFixed); // v, r, s + return this.recover(message, encodeSignature([args[1], args[2], args[3]]), preFixed); // v, r, s } return recover(message, signature); From a67cd7964acd2319e969b5e8dff4389f980c0993 Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Wed, 27 Mar 2019 21:54:02 +0100 Subject: [PATCH 3/8] recover method improved of the accounts module --- packages/web3-eth-accounts/src/Accounts.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/web3-eth-accounts/src/Accounts.js b/packages/web3-eth-accounts/src/Accounts.js index 0e051d32897..b3241c14931 100644 --- a/packages/web3-eth-accounts/src/Accounts.js +++ b/packages/web3-eth-accounts/src/Accounts.js @@ -209,12 +209,8 @@ export default class Accounts extends AbstractWeb3Module { } if (arguments.length >= 4) { - const args = [...arguments]; - - preFixed = args[args.length - 1]; - preFixed = isBoolean(preFixed) ? preFixed : false; - - return this.recover(message, encodeSignature([args[1], args[2], args[3]]), preFixed); // v, r, s + // v, r, s + return this.recover(message, encodeSignature([arguments[1], arguments[2], arguments[3]]), !!arguments[4]); } return recover(message, signature); From 00b6c59de162b1f6303f45958f2371400e0c7383 Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Thu, 28 Mar 2019 11:05:36 +0100 Subject: [PATCH 4/8] accounts module improved --- packages/web3-eth-accounts/src/Accounts.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/web3-eth-accounts/src/Accounts.js b/packages/web3-eth-accounts/src/Accounts.js index b3241c14931..b1a0635639a 100644 --- a/packages/web3-eth-accounts/src/Accounts.js +++ b/packages/web3-eth-accounts/src/Accounts.js @@ -22,7 +22,6 @@ import isFunction from 'lodash/isFunction'; import isObject from 'lodash/isObject'; -import isBoolean from 'lodash/isBoolean'; import Hash from 'eth-lib/lib/hash'; import RLP from 'eth-lib/lib/rlp'; import Bytes from 'eth-lib/lib/bytes'; @@ -95,8 +94,7 @@ export default class Accounts extends AbstractWeb3Module { } const messageBuffer = Buffer.from(data); - const preamble = `\u0019Ethereum Signed Message:\n${data.length}`; - const preambleBuffer = Buffer.from(preamble); + const preambleBuffer = Buffer.from(`\u0019Ethereum Signed Message:\n${data.length}`); const ethMessage = Buffer.concat([preambleBuffer, messageBuffer]); return Hash.keccak256s(ethMessage); @@ -181,8 +179,8 @@ export default class Accounts extends AbstractWeb3Module { * @returns {Object} */ sign(data, privateKey) { - if (isHexStrict(data)) { - data = hexToBytes(data); + if (this.utils.isHexStrict(data)) { + data = this.utils.hexToBytes(data); } return Account.fromPrivateKey(privateKey, this).sign(data); @@ -210,7 +208,11 @@ export default class Accounts extends AbstractWeb3Module { if (arguments.length >= 4) { // v, r, s - return this.recover(message, encodeSignature([arguments[1], arguments[2], arguments[3]]), !!arguments[4]); + return this.recover( + arguments[0], + encodeSignature([arguments[1], arguments[2], arguments[3]]), + !!arguments[4] + ); } return recover(message, signature); From bc0ec7ef0e566fa380b1bfd479468dea4e44831d Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Thu, 28 Mar 2019 11:29:34 +0100 Subject: [PATCH 5/8] tests updated --- packages/web3-eth-accounts/jest.config.js | 4 ++-- packages/web3-eth-accounts/src/Accounts.js | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/web3-eth-accounts/jest.config.js b/packages/web3-eth-accounts/jest.config.js index b1d032186ed..30dd5b6f742 100644 --- a/packages/web3-eth-accounts/jest.config.js +++ b/packages/web3-eth-accounts/jest.config.js @@ -1,9 +1,9 @@ const jestConfig = require('../../jest.config'); module.exports = jestConfig({ - 'hexToBytes': 'web3-utils', - 'isHexStrict': 'web3-utils', 'Utils': 'web3-utils', + 'isHexStrict': 'web3-utils', + 'hexToBytes': 'web3-utils', 'formatters': 'web3-core-helpers', 'EthAccount': 'eth-lib/lib/account', 'scryptsy': 'scrypt.js' diff --git a/packages/web3-eth-accounts/src/Accounts.js b/packages/web3-eth-accounts/src/Accounts.js index b1a0635639a..aa7319c45fe 100644 --- a/packages/web3-eth-accounts/src/Accounts.js +++ b/packages/web3-eth-accounts/src/Accounts.js @@ -45,6 +45,8 @@ export default class Accounts extends AbstractWeb3Module { constructor(provider, utils, formatters, methodFactory, options, net) { super(provider, options, methodFactory, net); + this.utils = utils; + this.formatters = formatters; this.transactionSigner = options.transactionSigner; this.formatters = formatters; this.defaultKeyName = 'web3js_wallet'; From 067c03057993762af6fb53ce65403615ab860e5a Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Thu, 28 Mar 2019 11:34:18 +0100 Subject: [PATCH 6/8] inputCallFormatter added to signTransaction --- packages/web3-eth-accounts/src/Accounts.js | 3 ++- packages/web3-eth-accounts/src/index.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/web3-eth-accounts/src/Accounts.js b/packages/web3-eth-accounts/src/Accounts.js index aa7319c45fe..e1d6317a2b3 100644 --- a/packages/web3-eth-accounts/src/Accounts.js +++ b/packages/web3-eth-accounts/src/Accounts.js @@ -48,7 +48,6 @@ export default class Accounts extends AbstractWeb3Module { this.utils = utils; this.formatters = formatters; this.transactionSigner = options.transactionSigner; - this.formatters = formatters; this.defaultKeyName = 'web3js_wallet'; this.accounts = {}; this.accountsIndex = 0; @@ -132,6 +131,8 @@ export default class Accounts extends AbstractWeb3Module { tx.nonce = await this.getTransactionCount(account.address); } + tx = this.formatters.inputCallFormatter(tx, this.moduleInstance); + const signedTransaction = await this.transactionSigner.sign(tx, account.privateKey); if (isFunction(callback)) { diff --git a/packages/web3-eth-accounts/src/index.js b/packages/web3-eth-accounts/src/index.js index e10c257d865..d6f0f00e5b0 100644 --- a/packages/web3-eth-accounts/src/index.js +++ b/packages/web3-eth-accounts/src/index.js @@ -36,5 +36,5 @@ import AccountsModule from './Accounts'; * @constructor */ export function Accounts(provider, net = null, options = {}) { - return new AccountsModule(provider, Utils, formatters, new MethodFactory(Utils, formatters), options, net); + return new AccountsModule(provider, Utils, new MethodFactory(Utils, formatters), options, net); } From 128cf7ef4c9c928bd504eeb9ce707ad0c9584d38 Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Thu, 28 Mar 2019 11:38:19 +0100 Subject: [PATCH 7/8] AccountsTest updated --- packages/web3-eth-accounts/src/Accounts.js | 7 +++--- .../tests/src/AccountsTest.js | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/web3-eth-accounts/src/Accounts.js b/packages/web3-eth-accounts/src/Accounts.js index e1d6317a2b3..5b9a6155784 100644 --- a/packages/web3-eth-accounts/src/Accounts.js +++ b/packages/web3-eth-accounts/src/Accounts.js @@ -131,9 +131,10 @@ export default class Accounts extends AbstractWeb3Module { tx.nonce = await this.getTransactionCount(account.address); } - tx = this.formatters.inputCallFormatter(tx, this.moduleInstance); - - const signedTransaction = await this.transactionSigner.sign(tx, account.privateKey); + const signedTransaction = await this.transactionSigner.sign( + this.formatters.inputCallFormatter(tx, this), + account.privateKey + ); if (isFunction(callback)) { callback(false, signedTransaction); diff --git a/packages/web3-eth-accounts/tests/src/AccountsTest.js b/packages/web3-eth-accounts/tests/src/AccountsTest.js index 3083968bf6a..e86f49fdabb 100644 --- a/packages/web3-eth-accounts/tests/src/AccountsTest.js +++ b/packages/web3-eth-accounts/tests/src/AccountsTest.js @@ -91,6 +91,8 @@ describe('AccountsTest', () => { return Promise.resolve('signed-transaction'); }); + formatters.inputCallFormatter.mockReturnValueOnce(transaction); + const response = await accounts.signTransaction(transaction, 'pk', callback); expect(response).toEqual('signed-transaction'); @@ -99,6 +101,8 @@ describe('AccountsTest', () => { expect(Account.fromPrivateKey).toHaveBeenCalledWith('pk', accounts); + expect(formatters.inputCallFormatter).toHaveBeenCalledWith(transaction, accounts); + expect(transactionSignerMock.sign).toHaveBeenCalledWith(transaction, account.privateKey); }); @@ -132,6 +136,8 @@ describe('AccountsTest', () => { return Promise.resolve(1); }); + formatters.inputCallFormatter.mockReturnValueOnce(transaction); + await expect(accounts.signTransaction(transaction, 'pk', callback)).resolves.toEqual('signed-transaction'); expect(callback).toHaveBeenCalledWith(false, 'signed-transaction'); @@ -140,6 +146,8 @@ describe('AccountsTest', () => { expect(transactionSignerMock.sign).toHaveBeenCalledWith(mappedTransaction, account.privateKey); + expect(formatters.inputCallFormatter).toHaveBeenCalledWith(transaction, accounts); + expect(accounts.getChainId).toHaveBeenCalled(); }); @@ -173,6 +181,8 @@ describe('AccountsTest', () => { return Promise.resolve(1); }); + formatters.inputCallFormatter.mockReturnValueOnce(transaction); + await expect(accounts.signTransaction(transaction, 'pk', callback)).resolves.toEqual('signed-transaction'); expect(callback).toHaveBeenCalledWith(false, 'signed-transaction'); @@ -181,6 +191,8 @@ describe('AccountsTest', () => { expect(transactionSignerMock.sign).toHaveBeenCalledWith(mappedTransaction, account.privateKey); + expect(formatters.inputCallFormatter).toHaveBeenCalledWith(transaction, accounts); + expect(accounts.getGasPrice).toHaveBeenCalled(); }); @@ -214,6 +226,8 @@ describe('AccountsTest', () => { return Promise.resolve(1); }); + formatters.inputCallFormatter.mockReturnValueOnce(transaction); + await expect(accounts.signTransaction(transaction, 'pk', callback)).resolves.toEqual('signed-transaction'); expect(callback).toHaveBeenCalledWith(false, 'signed-transaction'); @@ -222,6 +236,8 @@ describe('AccountsTest', () => { expect(transactionSignerMock.sign).toHaveBeenCalledWith(mappedTransaction, account.privateKey); + expect(formatters.inputCallFormatter).toHaveBeenCalledWith(transaction, accounts); + expect(accounts.getTransactionCount).toHaveBeenCalledWith('0x0'); }); @@ -241,10 +257,14 @@ describe('AccountsTest', () => { return Promise.reject(new Error('ERROR')); }); + formatters.inputCallFormatter.mockReturnValueOnce(transaction); + await expect(accounts.signTransaction(transaction, 'pk')).rejects.toThrow('ERROR'); expect(Account.fromPrivateKey).toHaveBeenCalledWith('pk', accounts); + expect(formatters.inputCallFormatter).toHaveBeenCalledWith(transaction, accounts); + expect(transactionSignerMock.sign).toHaveBeenCalledWith(transaction, 'pk'); }); @@ -264,6 +284,8 @@ describe('AccountsTest', () => { return Promise.reject(new Error('ERROR')); }); + formatters.inputCallFormatter.mockReturnValueOnce(transaction); + accounts.signTransaction(transaction, 'pk', (error, response) => { expect(error).toEqual(new Error('ERROR')); @@ -271,6 +293,8 @@ describe('AccountsTest', () => { expect(transactionSignerMock.sign).toHaveBeenCalledWith(transaction, 'pk'); + expect(formatters.inputCallFormatter).toHaveBeenCalledWith(transaction, accounts); + done(); }); }); From 881b7df13b9b2fa131c0711690ad08266e4a67a9 Mon Sep 17 00:00:00 2001 From: Samuel Furter Date: Thu, 28 Mar 2019 11:45:39 +0100 Subject: [PATCH 8/8] eslint executed --- packages/web3-eth-accounts/tests/src/models/WalletTest.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/web3-eth-accounts/tests/src/models/WalletTest.js b/packages/web3-eth-accounts/tests/src/models/WalletTest.js index 79d810c7398..1b99cd22e6d 100644 --- a/packages/web3-eth-accounts/tests/src/models/WalletTest.js +++ b/packages/web3-eth-accounts/tests/src/models/WalletTest.js @@ -35,8 +35,8 @@ describe('WalletTest', () => { it('calls the length property and returns the accountsIndex', () => { wallet.accountsIndex = 99; - - expect(wallet.length).toEqual(99); + + expect(wallet).toHaveLength(99); }); it('calls create and returns the expected value', () => { @@ -170,7 +170,7 @@ describe('WalletTest', () => { expect(() => { wallet.decrypt([true], 'pw'); - }).toThrow('Couldn\'t decrypt accounts. Password wrong?'); + }).toThrow("Couldn't decrypt accounts. Password wrong?"); expect(Account.fromV3Keystore).toHaveBeenCalledWith(true, 'pw', false, accountsMock); });