refactor(@embark/rpc-manager): Simplify RPC modifications

Managing account details inside of the RPC Manager became a bit convulted and difficult to follow due to any web3 requests inside of an `RpcModifier` communicating over the proxy and therefore to other `RpcModifier`’s or itself. It also created cases where node accounts were duplicated by way of running the `eth_accounts` modifier multiple times (the first time getting accounts from the node and subsequent times getting accounts from the modified `eth_accounts` response.

This has been simplified by having the entry point of the `rpc-manager` (`index.js`) talk directly to the node via `web3`. This allowed account/nodeAccount management to also be handled by the entry point, removing the need for each individual `RpcModifier` from having to handle these account details. The result is a much more simplified and and much easier to maintain code for RPC Manager.

The cases for which accounts can be modified (via `personal_newAccount` RPC call, and via test configuration change) are now handled in one place (the entry point) and propagated to the each `RpcModifier`.

Add `blockchain:started` command to request when the blockchain has been started. In this case, this is needed so that we know when we can create a direct connection to the node, instead of the proxy (as is the case in almost all other modules).

Extend action timeout when in debug mode.

1. These changs have made the `RpcModifier` base class essentially useless, however, it has been kept in place because it will be used for future DRY improvements to the `rpc-manager`.
2. These changes have been tested with the following DApps:
- Demo
- Test DApp
- Contracts test DApp
- Teller

packages/core/core/src/index.ts

@@ -84,6 +84,10 @@ export interface Configuration {
     wsRPC: boolean;
     isDev: boolean;
     client: string;
+    enabled: boolean;
+    clientConfig: {
+      miningMode: string
+    }
   };
   webServerConfig: {
     certOptions: {

packages/core/core/src/processes/processLauncher.js

@@ -1,6 +1,7 @@
 const child_process = require('child_process');
 import { readJsonSync } from 'fs-extra';
 const path = require('path');
+import { isDebug } from 'embark-utils';
 
 const constants = readJsonSync(path.join(__dirname, '../../constants.json'));
 
@@ -18,7 +19,7 @@ export class ProcessLauncher {
   constructor(options) {
     this.name = options.name || path.basename(options.modulePath);
 
-    if (this._isDebug()) {
+    if (isDebug()) {
       const childOptions = {stdio: 'pipe', execArgv: ['--inspect-brk=' + (60000 + processCount)]};
       processCount++;
       this.process = child_process.fork(options.modulePath, [], childOptions);
@@ -45,11 +46,6 @@ export class ProcessLauncher {
     this.events.request('process:logs:register', {processName: this.name, eventName: `process:${this.name}:log`, silent: this.silent});
   }
 
-  _isDebug() {
-    const argvString= process.execArgv.join();
-    return argvString.includes('--debug') || argvString.includes('--inspect');
-  }
-
   // Subscribes to messages from the child process and delegates to the right methods
   _subscribeToMessages() {
     const self = this;

packages/core/utils/src/accountParser.js

@@ -13,7 +13,9 @@ export default class AccountParser {
   static parseAccountsConfig(accountsConfig, web3, dappPath, logger, nodeAccounts) {
     let accounts = [];
     if (!(accountsConfig && accountsConfig.length)) {
-      return nodeAccounts;
+      return nodeAccounts.map(account => {
+        return (typeof account === 'string') ? { address: account } : account;
+      });
     }
     if (accountsConfig && accountsConfig.length) {
       accountsConfig.forEach(accountConfig => {

packages/core/utils/src/env.js

@@ -49,3 +49,8 @@ export function setUpEnv(defaultEmbarkPath) {
     (process.env[NODE_PATH] ? delimiter : '') +
     (process.env[NODE_PATH] || '');
 }
+
+export function isDebug() {
+  const argvString= process.execArgv.join();
+  return argvString.includes('--debug') || argvString.includes('--inspect');
+}

packages/core/utils/src/index.ts

@@ -57,7 +57,7 @@ export {
   normalizePath,
   toForwardSlashes
 } from './pathUtils';
-export { setUpEnv } from './env';
+export { setUpEnv, isDebug } from './env';
 
 import {
   dappPath

packages/embark/src/cmd/cmd.js

@@ -332,7 +332,8 @@ class Cmd {
           txDetails: options.txDetails,
           node: options.node,
           coverage: options.coverage,
-          env: options.env || 'test'
+          env: options.env || 'test',
+          sol: options.solc
         });
       });
   }

packages/plugins/geth/src/devtxs.ts

@@ -1,12 +1,12 @@
 import { __ } from 'embark-i18n';
-import { Embark, EmbarkEvents } from "embark-core";
+import { Embark, EmbarkEvents, Configuration } from "embark-core";
 import { Logger } from "embark-logger";
 import Web3 from "web3";
 import { TransactionReceipt } from "web3-eth";
 import constants from "embark-core/constants.json";
 export default class DevTxs {
   private embark: Embark;
-  private blockchainConfig: any;
+  private blockchainConfig: Configuration["blockchainConfig"];
   private events: EmbarkEvents;
   private logger: Logger;
   private web3?: Web3;

packages/plugins/rpc-manager/src/lib/eth_accounts.ts

@@ -1,7 +1,6 @@
 import { Callback, Embark, EmbarkEvents } from "embark-core";
 const { blockchain: blockchainConstants } = require("embark-core/constants");
 import { __ } from "embark-i18n";
-import { Logger } from "embark-logger";
 import Web3 from "web3";
 import RpcModifier from "./rpcModifier";
 
@@ -10,24 +9,9 @@ const METHODS_TO_MODIFY = [
   blockchainConstants.transactionMethods.personal_listAccounts,
 ];
 
-function arrayEqual(arrayA: string[], arrayB: string[]) {
-  if (!(arrayA && arrayB) || arrayA.length !== arrayB.length) {
-    return false;
-  } else {
-    return arrayA.every((address, index) => Web3.utils.toChecksumAddress(address) === Web3.utils.toChecksumAddress(arrayB[index]));
-  }
-}
-
 export default class EthAccounts extends RpcModifier {
-  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents) {
-    super(embark, rpcModifierEvents);
-
-    this.init();
-  }
-
-  private async init() {
-    const nodeAccounts = await this.nodeAccounts;
-    this.rpcModifierEvents.request2("nodeAccounts:updated", nodeAccounts);
+  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
+    super(embark, rpcModifierEvents, nodeAccounts, accounts, web3);
 
     this.embark.registerActionForEvent("blockchain:proxy:response", this.ethAccountsResponse.bind(this));
   }
@@ -42,16 +26,11 @@ export default class EthAccounts extends RpcModifier {
     this.logger.trace(__(`Original request/response data: ${JSON.stringify({ request: params.request, response: params.response })}`));
 
     try {
-      if (!arrayEqual(params.response.result, this._nodeAccounts || [])) {
-        // reset backing variables so accounts is recalculated
-        await this.rpcModifierEvents.request2("nodeAccounts:updated", params.response.result);
-      }
-      const accounts = await this.accounts;
-      if (!(accounts && accounts.length)) {
+      if (!(this.accounts && this.accounts.length)) {
         return callback(null, params);
       }
 
-      params.response.result = accounts.map((acc) => acc.address || acc);
+      params.response.result = this.accounts.map((acc) => acc.address);
       this.logger.trace(__(`Modified request/response data: ${JSON.stringify({ request: params.request, response: params.response })}`));
     } catch (err) {
       return callback(err);

packages/plugins/rpc-manager/src/lib/eth_sendTransaction.ts

@@ -1,7 +1,6 @@
 import async from "async";
 import { Callback, Embark, EmbarkEvents } from "embark-core";
 import { __ } from "embark-i18n";
-import { Logger } from "embark-logger";
 import Web3 from "web3";
 const { blockchain: blockchainConstants } = require("embark-core/constants");
 import RpcModifier from "./rpcModifier";
@@ -9,8 +8,8 @@ import RpcModifier from "./rpcModifier";
 export default class EthSendTransaction extends RpcModifier {
   private signTransactionQueue: any;
   private nonceCache: any = {};
-  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents) {
-    super(embark, rpcModifierEvents);
+  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
+    super(embark, rpcModifierEvents, nodeAccounts, accounts, web3);
 
     embark.registerActionForEvent("blockchain:proxy:request", this.ethSendTransactionRequest.bind(this));
 
@@ -23,9 +22,8 @@ export default class EthSendTransaction extends RpcModifier {
           return callback(err, null);
         }
         payload.nonce = newNonce;
-        const web3 = await this.web3;
         try {
-          const result = await web3.eth.accounts.signTransaction(payload, account.privateKey);
+          const result = await this.web3.eth.accounts.signTransaction(payload, account.privateKey);
           callback(null, result.rawTransaction);
         } catch (err) {
           callback(err);
@@ -35,8 +33,7 @@ export default class EthSendTransaction extends RpcModifier {
   }
 
   private async getNonce(address: string, callback: Callback<any>) {
-    const web3 = await this.web3;
-    web3.eth.getTransactionCount(address, (error: any, transactionCount: number) => {
+    this.web3.eth.getTransactionCount(address, (error: any, transactionCount: number) => {
       if (error) {
         return callback(error, null);
       }
@@ -57,8 +54,7 @@ export default class EthSendTransaction extends RpcModifier {
     if (!(params.request.method === blockchainConstants.transactionMethods.eth_sendTransaction)) {
       return callback(null, params);
     }
-    const accounts = await this.accounts;
-    if (!(accounts && accounts.length)) {
+    if (!(this.accounts && this.accounts.length)) {
       return callback(null, params);
     }
 
@@ -67,7 +63,7 @@ export default class EthSendTransaction extends RpcModifier {
 
     try {
       // Check if we have that account in our wallet
-      const account = accounts.find((acc) => Web3.utils.toChecksumAddress(acc.address) === Web3.utils.toChecksumAddress(params.request.params[0].from));
+      const account = this.accounts.find((acc) => Web3.utils.toChecksumAddress(acc.address) === Web3.utils.toChecksumAddress(params.request.params[0].from));
       if (account && account.privateKey) {
         return this.signTransactionQueue.push({ payload: params.request.params[0], account }, (err: any, newPayload: any) => {
           if (err) {

packages/plugins/rpc-manager/src/lib/eth_signData.ts

@@ -4,8 +4,8 @@ import Web3 from "web3";
 import RpcModifier from "./rpcModifier";
 
 export default class EthSignData extends RpcModifier {
-  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents) {
-    super(embark, rpcModifierEvents);
+  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
+    super(embark, rpcModifierEvents, nodeAccounts, accounts, web3);
 
     this.embark.registerActionForEvent("blockchain:proxy:request", this.ethSignDataRequest.bind(this));
     this.embark.registerActionForEvent("blockchain:proxy:response", this.ethSignDataResponse.bind(this));
@@ -17,10 +17,9 @@ export default class EthSignData extends RpcModifier {
     }
 
     try {
-      const nodeAccounts = await this.nodeAccounts;
       const [fromAddr] = params.request.params;
 
-      const account = nodeAccounts.find(acc => (
+      const account = this.nodeAccounts.find(acc => (
         Web3.utils.toChecksumAddress(acc) ===
         Web3.utils.toChecksumAddress(fromAddr)
       ));
@@ -40,11 +39,9 @@ export default class EthSignData extends RpcModifier {
     }
 
     try {
-      const accounts = await this.accounts;
-      const nodeAccounts = await this.nodeAccounts;
       const [fromAddr, data] = params.request.params;
 
-      const nodeAccount = nodeAccounts.find(acc => (
+      const nodeAccount = this.nodeAccounts.find(acc => (
         Web3.utils.toChecksumAddress(acc) ===
         Web3.utils.toChecksumAddress(fromAddr)
       ));
@@ -55,7 +52,7 @@ export default class EthSignData extends RpcModifier {
       this.logger.trace(__(`Modifying blockchain '${params.request.method}' response:`));
       this.logger.trace(__(`Original request/response data: ${JSON.stringify(params)}`));
 
-      const account = accounts.find(acc => (
+      const account = this.accounts.find(acc => (
         Web3.utils.toChecksumAddress(acc.address) ===
           Web3.utils.toChecksumAddress(fromAddr)
       ));

packages/plugins/rpc-manager/src/lib/eth_signTypedData.ts

@@ -1,13 +1,12 @@
 import { sign, transaction } from "@omisego/omg-js-util";
 import { Callback, Embark, EmbarkEvents } from "embark-core";
 import { __ } from "embark-i18n";
-import { Logger } from "embark-logger";
 import Web3 from "web3";
 import RpcModifier from "./rpcModifier";
 
 export default class EthSignTypedData extends RpcModifier {
-  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents) {
-    super(embark, rpcModifierEvents);
+  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
+    super(embark, rpcModifierEvents, nodeAccounts, accounts, web3);
 
     this.embark.registerActionForEvent("blockchain:proxy:request", this.ethSignTypedDataRequest.bind(this));
     this.embark.registerActionForEvent("blockchain:proxy:response", this.ethSignTypedDataResponse.bind(this));
@@ -41,9 +40,8 @@ export default class EthSignTypedData extends RpcModifier {
     this.logger.trace(__(`Original request/response data: ${JSON.stringify({ request: params.request, response: params.response })}`));
 
     try {
-      const accounts = await this.accounts;
       const [fromAddr, typedData] = params.request.params;
-      const account = accounts.find((acc) => Web3.utils.toChecksumAddress(acc.address) === Web3.utils.toChecksumAddress(fromAddr));
+      const account = this.accounts.find((acc) => Web3.utils.toChecksumAddress(acc.address) === Web3.utils.toChecksumAddress(fromAddr));
       if (!(account && account.privateKey)) {
         return callback(
           new Error(__("Could not sign transaction because Embark does not have a private key associated with '%s'. " +

packages/plugins/rpc-manager/src/lib/eth_subscribe.ts

@@ -1,10 +1,11 @@
 import { Callback, Embark, EmbarkEvents } from "embark-core";
 import { __ } from "embark-i18n";
 import RpcModifier from "./rpcModifier";
+import Web3 from "web3";
 
 export default class EthSubscribe extends RpcModifier {
-  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents) {
-    super(embark, rpcModifierEvents);
+  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
+    super(embark, rpcModifierEvents, nodeAccounts, accounts, web3);
 
     embark.registerActionForEvent("blockchain:proxy:request", this.ethSubscribeRequest.bind(this));
     embark.registerActionForEvent("blockchain:proxy:response", this.ethSubscribeResponse.bind(this));

packages/plugins/rpc-manager/src/lib/eth_unsubscribe.ts

@@ -1,11 +1,12 @@
 import { Callback, Embark, EmbarkEvents } from "embark-core";
 import { __ } from "embark-i18n";
 import RpcModifier from "./rpcModifier";
+import Web3 from "web3";
 
 export default class EthUnsubscribe extends RpcModifier {
 
-  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents) {
-    super(embark, rpcModifierEvents);
+  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
+    super(embark, rpcModifierEvents, nodeAccounts, accounts, web3);
 
     embark.registerActionForEvent("blockchain:proxy:request", this.ethUnsubscribeRequest.bind(this));
     embark.registerActionForEvent("blockchain:proxy:response", this.ethUnsubscribeResponse.bind(this));

packages/plugins/rpc-manager/src/lib/index.ts

@@ -1,6 +1,7 @@
-import { Callback, Embark, EmbarkEvents } from "embark-core";
+import { Callback, Embark, Configuration, EmbarkEvents } from "embark-core";
 import { Events } from "embark-core";
 import { Logger } from 'embark-logger';
+import { AccountParser, dappPath } from "embark-utils";
 import Web3 from "web3";
 import EthAccounts from "./eth_accounts";
 import EthSendTransaction from "./eth_sendTransaction";
@@ -27,18 +28,56 @@ export default class RpcManager {
     this.init();
   }
 
+  protected get web3() {
+    return (async () => {
+      if (!this._web3) {
+        await this.events.request2("blockchain:started");
+        // get connection directly to the node
+        const provider = await this.events.request2("blockchain:node:provider", "ethereum");
+        this._web3 = new Web3(provider);
+      }
+      return this._web3;
+    })();
+  }
+
+  private get nodeAccounts() {
+    return (async () => {
+      if (!this._nodeAccounts) {
+        const web3 = await this.web3;
+        this._nodeAccounts = await web3.eth.getAccounts();
+      }
+      return this._nodeAccounts || [];
+    })();
+  }
+
+  private get accounts() {
+    return (async () => {
+      if (!this._accounts) {
+        const web3 = await this.web3;
+        const nodeAccounts = await this.nodeAccounts;
+        this._accounts = AccountParser.parseAccountsConfig(this.embark.config.blockchainConfig.accounts, web3, dappPath(), this.logger, nodeAccounts);
+      }
+      return this._accounts || [];
+    })();
+  }
+
   private async init() {
 
-    this.rpcModifierEvents.setCommandHandler("nodeAccounts:updated", this.updateAccounts.bind(this));
-    this.rpcModifierEvents.setCommandHandler("nodeAccounts:added", async (addedNodeAccount: any, cb: Callback<null>) => {
-      if (!this._nodeAccounts) {
-        this._nodeAccounts = [addedNodeAccount];
-      } else {
-        this._nodeAccounts.push(addedNodeAccount);
-      }
-      return this.updateAccounts(this._nodeAccounts, cb);
+    this.embark.registerActionForEvent("tests:config:updated", { priority: 40 }, (_params, cb) => {
+      // blockchain configs may have changed (ie endpoint)
+      this._web3 = null;
+
+      // web.eth.getAccounts may return a different value now
+      // update accounts across all modifiers
+      this.updateAccounts(cb);
     });
 
+    this.rpcModifierEvents.setCommandHandler("nodeAccounts:updated", this.updateAccounts.bind(this));
+
+    const web3 = await this.web3;
+    const nodeAccounts = await this.nodeAccounts;
+    const accounts = await this.accounts;
+
     this.modifiers = [
       PersonalNewAccount,
       EthAccounts,
@@ -47,13 +86,15 @@ export default class RpcManager {
       EthSignData,
       EthSubscribe,
       EthUnsubscribe
-    ].map((rpcModifier) => new rpcModifier(this.embark, this.rpcModifierEvents));
+    ].map((RpcMod) => new RpcMod(this.embark, this.rpcModifierEvents, nodeAccounts, accounts, web3));
   }
 
-  private async updateAccounts(updatedNodeAccounts: any[], cb: Callback<null>) {
-    this._nodeAccounts = updatedNodeAccounts;
+  private async updateAccounts(cb: Callback<null>) {
+    this._nodeAccounts = null;
+    this._accounts = null;
     for (const modifier of this.modifiers) {
-      await (modifier.nodeAccounts = Promise.resolve(updatedNodeAccounts));
+      modifier.nodeAccounts = await this.nodeAccounts;
+      modifier.accounts = await this.accounts;
     }
     cb();
   }

packages/plugins/rpc-manager/src/lib/personal_newAccount.ts

@@ -4,8 +4,8 @@ const { blockchain: blockchainConstants } = require("embark-core/constants");
 import { __ } from "embark-i18n";
 import RpcModifier from "./rpcModifier";
 export default class PersonalNewAccount extends RpcModifier {
-  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents) {
-    super(embark, rpcModifierEvents);
+  constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
+    super(embark, rpcModifierEvents, nodeAccounts, accounts, web3);
 
     embark.registerActionForEvent("blockchain:proxy:response", this.personalNewAccountResponse.bind(this));
   }
@@ -16,7 +16,7 @@ export default class PersonalNewAccount extends RpcModifier {
     }
 
     // emit event so tx modifiers can refresh accounts
-    await this.rpcModifierEvents.request2("nodeAccounts:added", params.response.result);
+    await this.rpcModifierEvents.request2("nodeAccounts:updated");
 
     callback(null, params);
   }

packages/plugins/rpc-manager/src/lib/rpcModifier.ts

@@ -1,69 +1,13 @@
-import { Embark, EmbarkEvents } /* supplied by @types/embark in packages/core/typings */ from "embark-core";
+import { Embark, EmbarkConfig, EmbarkEvents } /* supplied by @types/embark in packages/core/typings */ from "embark-core";
 import { Logger } from "embark-logger";
-import { AccountParser, dappPath } from "embark-utils";
 import Web3 from "web3";
 
 export default class RpcModifier {
   public events: EmbarkEvents;
   public logger: Logger;
-  private _web3: Web3 | null = null;
-  private _accounts: any[] | null = null;
   protected _nodeAccounts: any[] | null = null;
-  constructor(readonly embark: Embark, readonly rpcModifierEvents: EmbarkEvents) {
+  constructor(readonly embark: Embark, readonly rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web: Web3) {
     this.events = embark.events;
     this.logger = embark.logger;
-
-    this.embark.registerActionForEvent("tests:config:updated", { priority: 40 }, async (_params, cb) => {
-      // reset accounts backing variable as the accounts in config may have changed and
-      // web.eth.getAccounts may return a different value now
-      this._accounts = null;
-      this._nodeAccounts = null;
-      cb(null, null);
-    });
-  }
-
-  protected get web3() {
-    return (async () => {
-      if (!this._web3) {
-        const provider = await this.events.request2("blockchain:client:provider", "ethereum");
-        this._web3 = new Web3(provider);
-      }
-      return this._web3;
-    })();
-  }
-
-  public get nodeAccounts() {
-    return (async () => {
-      if (!this._nodeAccounts) {
-        const web3 = await this.web3;
-        this._nodeAccounts = await web3.eth.getAccounts();
-      }
-      return this._nodeAccounts || [];
-    })();
-  }
-
-  public set nodeAccounts(nodeAccounts: Promise<any[]>) {
-    (async () => {
-      this._nodeAccounts = await nodeAccounts;
-      // reset accounts backing variable as it needs to be recalculated
-      this._accounts = null;
-    })();
-  }
-
-  protected get accounts() {
-    return (async () => {
-      if (!this._accounts) {
-        const web3 = await this.web3;
-        const nodeAccounts = await this.nodeAccounts;
-        this._accounts = AccountParser.parseAccountsConfig(this.embark.config.blockchainConfig.accounts, web3, dappPath(), this.logger, nodeAccounts);
-      }
-      return this._accounts || [];
-    })();
-  }
-
-  protected set accounts(accounts) {
-    (async () => {
-      this._accounts = await accounts;
-    })();
   }
 }

packages/stack/blockchain/src/index.js

@@ -151,6 +151,12 @@ export default class Blockchain {
         return cb(`Error getting provider: ${err.message || err}`);
       }
     });
+    this.events.setCommandHandler("blockchain:started", (cb) => {
+      if (this.startedClient) {
+        return cb(null, this.startedClient);
+      }
+      this.events.on("blockchain:started", (clientName) => { cb(null, clientName); });
+    });
     this.blockchainApi.registerAPIs("ethereum");
     this.blockchainApi.registerRequests("ethereum");
 

packages/stack/proxy/src/proxy.js

@@ -2,9 +2,10 @@ import { __ } from 'embark-i18n';
 import express from 'express';
 import expressWs from 'express-ws';
 import cors from 'cors';
+import { isDebug } from 'embark-utils';
 const Web3RequestManager = require('web3-core-requestmanager');
 
-const ACTION_TIMEOUT = 5000;
+const ACTION_TIMEOUT = isDebug() ? 20000 : 5000;
 
 export class Proxy {
   constructor(options) {