Merge pull request #2796 from ethereum/issue/2708

Fixes BatchRequest for locally signed transactions

package.json

@@ -98,7 +98,7 @@
         "eslint-plugin-standard": "^4.0.0",
         "eslint-plugin-unicorn": "^8.0.1",
         "istanbul-combine": "^0.3.0",
-        "jest": "^24.7.1",
+        "jest": "^24.8.0",
         "lerna": "^3.13.4",
         "prettier": "^1.16.4",
         "regenerator-runtime": "^0.13.2",

packages/web3-core-method/src/methods/transaction/EthSendTransactionMethod.js

@@ -41,6 +41,7 @@ export default class EthSendTransactionMethod extends SendTransactionMethod {
     }
 
     /**
+     * TODO: Instead of using the static type property should every method has a static factory method
      * This type will be used in the AbstractMethodFactory.
      *
      * @returns {String}
@@ -49,6 +50,16 @@ export default class EthSendTransactionMethod extends SendTransactionMethod {
         return 'eth-send-transaction-method';
     }
 
+    /**
+     * TODO: Find a better way to have a mangle save method type detection (ES7 decorator?)
+     * The non-static property will be used in the BatchRequest object
+     *
+     * @returns {String}
+     */
+    get Type() {
+        return 'eth-send-transaction-method';
+    }
+
     /**
      * This method will be executed before the RPC request.
      *
@@ -95,12 +106,12 @@ export default class EthSendTransactionMethod extends SendTransactionMethod {
         }
 
         if (this.hasAccounts() && this.isDefaultSigner()) {
-            if (this.moduleInstance.accounts.wallet[this.parameters[0].from]) {
-                this.sendRawTransaction(this.moduleInstance.accounts.wallet[this.parameters[0].from].privateKey).catch(
-                    (error) => {
-                        this.handleError(error, false, 0);
-                    }
-                );
+            const account = this.moduleInstance.accounts.wallet[this.parameters[0].from];
+
+            if (account) {
+                this.sendRawTransaction(account).catch((error) => {
+                    this.handleError(error, false, 0);
+                });
 
                 return this.promiEvent;
             }
@@ -122,36 +133,61 @@ export default class EthSendTransactionMethod extends SendTransactionMethod {
      *
      * @method sendRawTransaction
      *
-     * @param {String} privateKey
+     * @param {Account} account
      *
      * @returns {PromiEvent}
      */
-    async sendRawTransaction(privateKey = null) {
+    async sendRawTransaction(account = {}) {
+        const response = await this.signTransaction(account);
+
+        this.parameters = [response.rawTransaction];
+        this.rpcMethod = 'eth_sendRawTransaction';
+
+        return super.execute();
+    }
+
+    /**
+     * Signs the transaction locally
+     *
+     * @method signTransaction
+     *
+     * @param {Account} account
+     *
+     * @returns {Promise<void>}
+     */
+    async signTransaction(account = {}) {
         this.beforeExecution(this.moduleInstance);
 
         if (!this.parameters[0].chainId) {
             this.parameters[0].chainId = await this.chainIdMethod.execute();
         }
 
         if (!this.parameters[0].nonce && this.parameters[0].nonce !== 0) {
-            this.getTransactionCountMethod.parameters = [this.parameters[0].from, 'latest'];
+            let nonce;
+
+            if (account.nonce) {
+                account.nonce = account.nonce + 1;
+                nonce = account.nonce;
+            }
 
-            this.parameters[0].nonce = await this.getTransactionCountMethod.execute();
+            if (!nonce) {
+                this.getTransactionCountMethod.parameters = [this.parameters[0].from, 'latest'];
+                nonce = await this.getTransactionCountMethod.execute();
+                account.nonce = nonce;
+            }
+
+            this.parameters[0].nonce = nonce;
         }
 
         let transaction = this.parameters[0];
         transaction.to = transaction.to || '0x';
         transaction.data = transaction.data || '0x';
         transaction.value = transaction.value || '0x';
         transaction.chainId = this.utils.numberToHex(transaction.chainId);
-        delete transaction.from;
 
-        const response = await this.moduleInstance.transactionSigner.sign(transaction, privateKey);
-
-        this.parameters = [response.rawTransaction];
-        this.rpcMethod = 'eth_sendRawTransaction';
+        delete transaction.from;
 
-        return super.execute();
+        return this.moduleInstance.transactionSigner.sign(transaction, account.privateKey);
     }
 
     /**

packages/web3-core-method/tests/src/methods/transaction/EthSendTransactionMethodTest.js

@@ -204,7 +204,7 @@ describe('EthSendTransactionMethodTest', () => {
 
             expect(hash).toEqual('0x0');
 
-            expect(customSigner.sign).toHaveBeenCalledWith(mappedTransaction, null);
+            expect(customSigner.sign).toHaveBeenCalledWith(mappedTransaction, undefined);
 
             expect(formatters.inputTransactionFormatter).toHaveBeenCalledWith(transaction, moduleInstanceMock);
 
@@ -246,7 +246,7 @@ describe('EthSendTransactionMethodTest', () => {
 
         await expect(method.execute()).rejects.toThrow('ERROR');
 
-        expect(customSigner.sign).toHaveBeenCalledWith(mappedTransaction, null);
+        expect(customSigner.sign).toHaveBeenCalledWith(mappedTransaction, undefined);
 
         expect(formatters.inputTransactionFormatter).toHaveBeenCalledWith(transaction, moduleInstanceMock);
     });

packages/web3-core-subscriptions/tests/src/subscriptions/eth/SyncingSubscriptionTest.js

@@ -50,8 +50,6 @@ describe('SyncingSubscriptionTest', () => {
     });
 
     it('calls onNewSubscriptionItem and returns the syncing status', () => {
-        expect(
-            syncingSubscription.onNewSubscriptionItem({status: true, syncing: true})
-        ).toEqual(true);
+        expect(syncingSubscription.onNewSubscriptionItem({status: true, syncing: true})).toEqual(true);
     });
 });

packages/web3-eth-accounts/src/models/Account.js

@@ -38,6 +38,7 @@ export default class Account {
     constructor(options, accounts = null) {
         this.address = options.address;
         this.privateKey = options.privateKey;
+        this.nonce = options.nonce;
         this.accounts = accounts;
     }
 

packages/web3-providers/src/batch-request/BatchRequest.js

@@ -23,6 +23,7 @@
 import isArray from 'lodash/isArray';
 import isObject from 'lodash/isObject';
 import JsonRpcResponseValidator from '../validators/JsonRpcResponseValidator';
+import JsonRpcMapper from '../mappers/JsonRpcMapper';
 
 export default class BatchRequest {
     /**
@@ -33,6 +34,7 @@ export default class BatchRequest {
     constructor(moduleInstance) {
         this.moduleInstance = moduleInstance;
         this.methods = [];
+        this.accounts = [];
     }
 
     /**
@@ -57,51 +59,110 @@ export default class BatchRequest {
      *
      * @returns Promise<{methods: AbstractMethod[], response: Object[]}|Error[]>
      */
-    execute() {
-        return this.moduleInstance.currentProvider.sendBatch(this.methods, this.moduleInstance).then((response) => {
-            let errors = [];
-            this.methods.forEach((method, index) => {
-                if (!isArray(response)) {
+    async execute() {
+        const payload = await this.toPayload();
+        const response = await this.moduleInstance.currentProvider.sendPayload(payload);
+
+        let errors = [];
+        this.methods.forEach((method, index) => {
+            if (!isArray(response)) {
+                if (method.callback) {
                     method.callback(
                         new Error(`BatchRequest error: Response should be of type Array but is: ${typeof response}`),
                         null
                     );
 
-                    errors.push(`Response should be of type Array but is: ${typeof response}`);
-
                     return;
                 }
 
-                const responseItem = response[index] || null;
+                throw new Error(`BatchRequest error: Response should be of type Array but is: ${typeof response}`);
+            }
+
+            const responseItem = response[index] || null;
+            const validationResult = JsonRpcResponseValidator.validate(responseItem);
 
-                const validationResult = JsonRpcResponseValidator.validate(responseItem);
+            if (validationResult === true) {
+                try {
+                    let mappedResult;
+
+                    // TODO: Find a better handling for custom behaviours in a batch request (afterBatchRequest?)
+                    if (method.Type === 'eth-send-transaction-method') {
+                        mappedResult = responseItem.result;
+                    } else {
+                        mappedResult = method.afterExecution(responseItem.result);
+                    }
 
-                if (validationResult) {
-                    try {
-                        const mappedResult = method.afterExecution(responseItem.result);
+                    response[index] = mappedResult;
 
-                        response[index] = mappedResult;
+                    if (method.callback) {
                         method.callback(false, mappedResult);
-                    } catch (error) {
-                        errors.push(error);
-                        method.callback(error, null);
                     }
+                } catch (error) {
+                    errors[index] = {method, error};
 
-                    return;
+                    if (method.callback) {
+                        method.callback(error, null);
+                    }
                 }
 
-                errors.push(validationResult);
-                method.callback(validationResult, null);
-            });
+                return;
+            }
+
+            errors[index] = {method, error: validationResult};
+
+            if (this.accounts[index] && this.accounts[index].nonce) {
+                this.accounts[index].nonce--;
+            }
 
-            if (errors.length > 0) {
-                throw new Error(`BatchRequest error: ${JSON.stringify(errors)}`);
+            if (method.callback) {
+                method.callback(validationResult, null);
             }
+        });
 
-            return {
-                methods: this.methods,
+        if (errors.length > 0) {
+            // eslint-disable-next-line no-throw-literal
+            throw {
+                errors,
                 response
             };
-        });
+        }
+
+        return {
+            methods: this.methods,
+            response
+        };
+    }
+
+    /**
+     *  Creates a payload and signs the transactions locally if required.
+     *
+     * @method toPayload
+     *
+     * @returns {Promise<Array>}
+     */
+    async toPayload() {
+        let payload = [];
+
+        for (let i = 0; i < this.methods.length; i++) {
+            const method = this.methods[i];
+            method.beforeExecution(this.moduleInstance);
+
+            // TODO: The method type specific handling shouldn't be done here.
+            if (this.moduleInstance.accounts && method.Type === 'eth-send-transaction-method' && method.hasAccounts()) {
+                const account = this.moduleInstance.accounts.wallet[method.parameters[0].from];
+
+                if (account) {
+                    const response = await method.signTransaction(account);
+                    method.parameters = [response.rawTransaction];
+                    method.rpcMethod = 'eth_sendRawTransaction';
+
+                    this.accounts[i] = account;
+                }
+            }
+
+            payload.push(JsonRpcMapper.toPayload(method.rpcMethod, method.parameters));
+        }
+
+        return payload;
     }
 }