fix(@embark/rpc-manager): fix eth_signTypedData method + tests

The signTypedData rpc method was broken, because it didn't check for
the node accounts, meaning that if you wanted to sign with a node
account that was unlocked, like in the tests, it would throw

Author: jrainville

dapps/tests/app/test/another_storage_spec.js

@@ -1,4 +1,4 @@
-/*global artifacts, contract, config, it, web3*/
+/*global artifacts, contract, config, it, web3, evmMethod*/
 const assert = require('assert');
 const AnotherStorage = artifacts.require('AnotherStorage');
 const SimpleStorage = artifacts.require('SimpleStorage');
@@ -63,4 +63,51 @@ contract("AnotherStorage", function() {
     let result = await AnotherStorage.simpleStorageAddress();
     assert.equal(result.toString(), SimpleStorage.options.address);
   });
+
+  it("can sign using eth_signTypedData with a custom account", async function() {
+    const chainId = await web3.eth.net.getId();
+
+    const domain = [
+      {name: "name", type: "string"},
+      {name: "version", type: "string"},
+      {name: "chainId", type: "uint256"},
+      {name: "verifyingContract", type: "address"}
+    ];
+
+    const redeem = [
+      {name: "keycard", type: "address"},
+      {name: "receiver", type: "address"},
+      {name: "code", type: "bytes32"}
+    ];
+
+    const domainData = {
+      name: "KeycardGift",
+      version: "1",
+      chainId,
+      verifyingContract: SimpleStorage.options.address
+    };
+
+    const message = {
+      keycard: accounts[1],
+      receiver: accounts[2],
+      code: web3.utils.sha3("hello world")
+    };
+
+    const data = {
+      types: {
+        EIP712Domain: domain,
+        Redeem: redeem
+      },
+      primaryType: "Redeem",
+      domain: domainData,
+      message
+    };
+
+    const signature = await evmMethod("eth_signTypedData", [
+      accounts[0],
+      data
+    ]);
+    assert.strictEqual(signature, '0x5dcbab53809985222a62807dd2f23551902fa4471377e319d5d682e1458646714cc71' +
+      'faa76cf6de3e0d871edbfa85628db552619d681594d5af2f34be2c33cdd1b');
+  });
 });

dapps/tests/app/test/array_references_spec.js

@@ -1,8 +1,9 @@
-/*global artifacts, contract, config, it, web3*/
+/*global artifacts, contract, config, it, web3, evmMethod*/
 const assert = require('assert');
 const SomeContract = artifacts.require('SomeContract');
 const MyToken2 = artifacts.require('MyToken2');
 
+let accounts;
 config({
   contracts: {
     deploy: {
@@ -22,6 +23,8 @@ config({
       }
     }
   }
+}, (err, theAccounts) => {
+  accounts = theAccounts;
 });
 
 contract("SomeContract", function() {
@@ -37,4 +40,50 @@ contract("SomeContract", function() {
     assert.strictEqual(address, web3.eth.defaultAccount);
   });
 
+  it("can sign using eth_signTypedData with a node account", async function() {
+    const chainId = await web3.eth.net.getId();
+
+    const domain = [
+      {name: "name", type: "string"},
+      {name: "version", type: "string"},
+      {name: "chainId", type: "uint256"},
+      {name: "verifyingContract", type: "address"}
+    ];
+
+    const redeem = [
+      {name: "keycard", type: "address"},
+      {name: "receiver", type: "address"},
+      {name: "code", type: "bytes32"}
+    ];
+
+    const domainData = {
+      name: "KeycardGift",
+      version: "1",
+      chainId,
+      verifyingContract: SomeContract.options.address
+    };
+
+    const message = {
+      keycard: accounts[1],
+      receiver: accounts[2],
+      code: web3.utils.sha3("hello world")
+    };
+
+    const data = {
+      types: {
+        EIP712Domain: domain,
+        Redeem: redeem
+      },
+      primaryType: "Redeem",
+      domain: domainData,
+      message
+    };
+
+    const signature = await evmMethod("eth_signTypedData", [
+      accounts[0],
+      data
+    ]);
+    // Impossible to tell what the signature will be because the account is not deterministic
+    assert.ok(signature);
+  });
 });

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

@@ -2,6 +2,7 @@ import { Callback, Embark, EmbarkEvents } from "embark-core";
 import { __ } from "embark-i18n";
 import Web3 from "web3";
 import RpcModifier from "./rpcModifier";
+import {handleSignRequest} from './utils/signUtils';
 
 export default class EthSignData extends RpcModifier {
   constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
@@ -16,21 +17,7 @@ export default class EthSignData extends RpcModifier {
       return callback(null, params);
     }
 
-    try {
-      const [fromAddr] = params.request.params;
-
-      const account = this.nodeAccounts.find(acc => (
-        Web3.utils.toChecksumAddress(acc) ===
-        Web3.utils.toChecksumAddress(fromAddr)
-      ));
-
-      if (!account) {
-        params.sendToNode = false;
-      }
-    } catch (err) {
-      return callback(err);
-    }
-    callback(null, params);
+    handleSignRequest(this.nodeAccounts, params, callback);
   }
 
   private async ethSignDataResponse(params: any, callback: Callback<any>) {

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

@@ -1,8 +1,9 @@
-import { sign, transaction } from "@omisego/omg-js-util";
-import { Callback, Embark, EmbarkEvents } from "embark-core";
-import { __ } from "embark-i18n";
+import {sign, transaction} from "@omisego/omg-js-util";
+import {Callback, Embark, EmbarkEvents} from "embark-core";
+import {__} from "embark-i18n";
 import Web3 from "web3";
 import RpcModifier from "./rpcModifier";
+import {handleSignRequest, isNodeAccount} from './utils/signUtils';
 
 export default class EthSignTypedData extends RpcModifier {
   constructor(embark: Embark, rpcModifierEvents: EmbarkEvents, public nodeAccounts: string[], public accounts: any[], protected web3: Web3) {
@@ -18,15 +19,14 @@ export default class EthSignTypedData extends RpcModifier {
     // - eth_signTypedData_v3
     // - eth_signTypedData_v4
     // - personal_signTypedData (parity)
-    if (params.request.method.includes("signTypedData")) {
-      // indicate that we do not want this call to go to the node
-      params.sendToNode = false;
+    if (!params.request.method.includes("signTypedData")) {
       return callback(null, params);
     }
-    callback(null, params);
+
+    handleSignRequest(this.nodeAccounts, params, callback);
   }
-  private async ethSignTypedDataResponse(params: any, callback: Callback<any>) {
 
+  private async ethSignTypedDataResponse(params: any, callback: Callback<any>) {
     // check for:
     // - eth_signTypedData
     // - eth_signTypedData_v3
@@ -35,12 +35,20 @@ export default class EthSignTypedData extends RpcModifier {
     if (!params.request.method.includes("signTypedData")) {
       return callback(null, params);
     }
-
-    this.logger.trace(__(`Modifying blockchain '${params.request.method}' response:`));
-    this.logger.trace(__(`Original request/response data: ${JSON.stringify({ request: params.request, response: params.response })}`));
-
     try {
       const [fromAddr, typedData] = params.request.params;
+
+      if (isNodeAccount(this.nodeAccounts, fromAddr)) {
+        // If it's a node account, we send the result because it should already be signed
+        return callback(null, params);
+      }
+
+      this.logger.trace(__(`Modifying blockchain '${params.request.method}' response:`));
+      this.logger.trace(__(`Original request/response data: ${JSON.stringify({
+        request: params.request,
+        response: params.response
+      })}`));
+
       const account = this.accounts.find((acc) => Web3.utils.toChecksumAddress(acc.address) === Web3.utils.toChecksumAddress(fromAddr));
       if (!(account && account.privateKey)) {
         return callback(
@@ -51,7 +59,10 @@ export default class EthSignTypedData extends RpcModifier {
       const signature = sign(toSign, [account.privateKey]);
 
       params.response.result = signature[0];
-      this.logger.trace(__(`Modified request/response data: ${JSON.stringify({ request: params.request, response: params.response })}`));
+      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/utils/signUtils.ts

@@ -0,0 +1,23 @@
+import Web3 from "web3";
+
+export function isNodeAccount(nodeAccounts, fromAddr) {
+  const account = nodeAccounts.find(acc => (
+    Web3.utils.toChecksumAddress(acc) ===
+    Web3.utils.toChecksumAddress(fromAddr)
+  ));
+  return !!account;
+}
+
+export function handleSignRequest(nodeAccounts, params, callback) {
+  try {
+    const [fromAddr] = params.request.params;
+
+    // If it's not a node account, we don't send it to the Node as it won't understand it
+    if (!isNodeAccount(nodeAccounts, fromAddr)) {
+      params.sendToNode = false;
+    }
+  } catch (err) {
+    return callback(err);
+  }
+  callback(null, params);
+}