Skip to content

Commit

Permalink
fix(@embark/rpc-manager): fix eth_signTypedData method + tests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jrainville committed Mar 4, 2020
1 parent 06ce73a commit 63ccbe4
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 30 deletions.
49 changes: 48 additions & 1 deletion 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');
Expand Down Expand Up @@ -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');
});
});
51 changes: 50 additions & 1 deletion 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: {
Expand All @@ -22,6 +23,8 @@ config({
}
}
}
}, (err, theAccounts) => {
accounts = theAccounts;
});

contract("SomeContract", function() {
Expand All @@ -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);
});
});
17 changes: 2 additions & 15 deletions packages/plugins/rpc-manager/src/lib/eth_signData.ts
Expand Up @@ -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) {
Expand All @@ -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>) {
Expand Down
37 changes: 24 additions & 13 deletions 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) {
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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);
}
Expand Down
23 changes: 23 additions & 0 deletions 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);
}

0 comments on commit 63ccbe4

Please sign in to comment.