Merge pull request #2596 from ethereum/testing/accounts-module

Testing Accounts Module
web3 · Mar 28, 2019 · 894c55c · 894c55c
2 parents 4a96e25 + 296dad3
commit 894c55c
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 27 deletions.
diff --git 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
@@ -22,12 +22,10 @@
 
 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';
 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';
@@ -47,8 +45,9 @@ export default class Accounts extends AbstractWeb3Module {
     constructor(provider, utils, formatters, methodFactory, options, net) {
         super(provider, options, methodFactory, net);
 
-        this.transactionSigner = options.transactionSigner;
+        this.utils = utils;
         this.formatters = formatters;
+        this.transactionSigner = options.transactionSigner;
         this.defaultKeyName = 'web3js_wallet';
         this.accounts = {};
         this.accountsIndex = 0;
@@ -91,13 +90,12 @@ 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);
-        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);
@@ -133,7 +131,10 @@ export default class Accounts extends AbstractWeb3Module {
                 tx.nonce = await this.getTransactionCount(account.address);
             }
 
-            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);
@@ -182,8 +183,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);
@@ -201,8 +202,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,11 +210,13 @@ export default class Accounts extends AbstractWeb3Module {
             message = this.hashMessage(message);
         }
 
-        if (args.length >= 4) {
-            preFixed = args.slice(-1)[0];
-            preFixed = isBoolean(preFixed) ? preFixed : false;
-
-            return this.recover(message, encodeSignature(args.slice(1, 4)), preFixed); // v, r, s
+        if (arguments.length >= 4) {
+            // v, r, s
+            return this.recover(
+                arguments[0],
+                encodeSignature([arguments[1], arguments[2], arguments[3]]),
+                !!arguments[4]
+            );
         }
 
         return recover(message, signature);

diff --git 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);
 }
diff --git 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,13 +284,17 @@ describe('AccountsTest', () => {
             return Promise.reject(new Error('ERROR'));
         });
 
+        formatters.inputCallFormatter.mockReturnValueOnce(transaction);
+
         accounts.signTransaction(transaction, 'pk', (error, response) => {
             expect(error).toEqual(new Error('ERROR'));
 
             expect(Account.fromPrivateKey).toHaveBeenCalledWith('pk', accounts);
 
             expect(transactionSignerMock.sign).toHaveBeenCalledWith(transaction, 'pk');
 
+            expect(formatters.inputCallFormatter).toHaveBeenCalledWith(transaction, accounts);
+
             done();
         });
     });

diff --git 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);
     });

diff --git 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 <[email protected]>
+ * @author Josh Stevens <[email protected]>, Samuel Furter <[email protected]>
  * @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;
+}