From 15721bcff5de6d76e75e092290e1e5ec5cb96008 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?James=20Lefr=C3=A8re?= Date: Sun, 21 Jul 2019 16:25:59 +0200 Subject: [PATCH] Custom chain support * Add (limited) support for custom chains * Add `ethereumjs-common` dependency (also a dependency of `ethereumjs-tx`) * Define `getChainDefinition` helper * Implement custom chains in `ethereumjs-tx` transactions and update tests --- .../@colony/purser-core/helpers.js | 49 +++++++++++++++++ .../@colony/purser-ledger/staticMethods.js | 16 ++---- .../@colony/purser-metamask/staticMethods.js | 10 +--- .../@colony/purser-trezor/staticMethods.js | 10 +--- .../mocks/@colony/purser-core/helpers.js | 4 ++ .../staticMethods/signTransaction.test.js | 48 +++++++++-------- .../staticMethods/signTransaction.test.js | 54 ++++++++++++++++++- .../staticMethods/signTransaction.test.js | 21 ++++++-- package.json | 1 + 9 files changed, 157 insertions(+), 56 deletions(-) diff --git a/modules/node_modules/@colony/purser-core/helpers.js b/modules/node_modules/@colony/purser-core/helpers.js index a68aafea..dc0895ec 100644 --- a/modules/node_modules/@colony/purser-core/helpers.js +++ b/modules/node_modules/@colony/purser-core/helpers.js @@ -1,6 +1,7 @@ /* @flow */ import { hashPersonalMessage, ecrecover } from 'ethereumjs-util'; +import Common from 'ethereumjs-common'; import { safeIntegerValidator, @@ -413,11 +414,59 @@ export const messageOrDataValidator = ( : messageData; }; +/** + * In order to support EIP-155, it's necessary to specify various + * definitions for a given chain (e.g. the chain ID, network ID, hardforks). + * + * Given a chain ID, this function returns a chain definition in the format + * expected by `ethereumjs-tx`. + * + * @param {number} chainId The given chain ID (as defined in EIP-155) + * @return {Object} The common chain definition + */ +export const getChainDefinition = (chainId: number): {| common: Object |} => { + const baseChain = (() => { + switch (chainId) { + /* + * Ganache's default chain ID is 1337, and is also the standard for + * private chains. The assumption is taken here that this inherits + * all of the other properties from mainnet, but that might not be + * the case. + * + * @TODO Provide a means to specify all chain properties for transactions + */ + case 1: + case 1337: + return 'mainnet'; + case 5: + return 'goerli'; + /* + * Other chain IDs _may_ cause validation errors in `ethereumjs-common` + */ + default: + return chainId; + } + })(); + return { + common: Common.forCustomChain( + baseChain, + { chainId }, + /* + * `ethereumjs-common` requires a hardfork to be defined, so we are + * using the current default for this property. This is also an + * assumption, and this should be made configurable. + */ + 'petersburg', + ), + }; +}; + /* * This default export is only here to help us with testing, otherwise * it wound't be needed */ const coreHelpers: Object = { + getChainDefinition, derivationPathSerializer, recoverPublicKey, verifyMessageSignature, diff --git a/modules/node_modules/@colony/purser-ledger/staticMethods.js b/modules/node_modules/@colony/purser-ledger/staticMethods.js index 862de139..3f67b6e3 100644 --- a/modules/node_modules/@colony/purser-ledger/staticMethods.js +++ b/modules/node_modules/@colony/purser-ledger/staticMethods.js @@ -9,8 +9,8 @@ import { import { derivationPathNormalizer, multipleOfTwoHexValueNormalizer, - addressNormalizer, hexSequenceNormalizer, + addressNormalizer, } from '@colony/purser-core/normalizers'; import { warning, objectToErrorString } from '@colony/purser-core/utils'; import { @@ -18,6 +18,7 @@ import { transactionObjectValidator, messageVerificationObjectValidator, messageOrDataValidator, + getChainDefinition, } from '@colony/purser-core/helpers'; import { HEX_HASH_TYPE, SIGNATURE } from '@colony/purser-core/defaults'; import { ledgerConnection, handleLedgerConnectionError } from './helpers'; @@ -73,7 +74,7 @@ export const signTransaction = async ({ * parameter: * https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md */ - const unsignedTransaction = await new EthereumTx( + const unsignedTransaction = new EthereumTx( Object.assign( {}, { @@ -114,7 +115,6 @@ export const signTransaction = async ({ /* $FlowFixMe */ multipleOfTwoHexValueNormalizer(nonce.toString(16)), ), - // to: addressNormalizer(to), value: hexSequenceNormalizer( /* * @TODO Add `bigNumber` `toHexString` wrapper method @@ -150,15 +150,9 @@ export const signTransaction = async ({ multipleOfTwoHexValueNormalizer(chainId.toString(16)), ), }, - { - chain: chainId, - /* - * Only send (and normalize) the destination address if one was - * provided in the initial transaction object. - */ - ...(to ? { to: addressNormalizer(to) } : {}), - }, + to ? { to: addressNormalizer(to) } : {}, ), + getChainDefinition(chainId), ); /* * Sign the transaction object via your Ledger Wallet diff --git a/modules/node_modules/@colony/purser-metamask/staticMethods.js b/modules/node_modules/@colony/purser-metamask/staticMethods.js index bf4924c8..0fe33d97 100644 --- a/modules/node_modules/@colony/purser-metamask/staticMethods.js +++ b/modules/node_modules/@colony/purser-metamask/staticMethods.js @@ -17,6 +17,7 @@ import { transactionObjectValidator, messageVerificationObjectValidator, messageOrDataValidator, + getChainDefinition, } from '@colony/purser-core/helpers'; import { HEX_HASH_TYPE } from '@colony/purser-core/defaults'; @@ -91,14 +92,7 @@ export const signTransactionCallback = ( v, value: new BigNumber(signedValue), }, - { - chain: chainId, - /* - * Only send (and normalize) the destination address if one was - * provided in the initial transaction object. - */ - ...(signedTo ? { to: addressNormalizer(signedTo) } : {}), - }, + getChainDefinition(chainId), ); const serializedSignedTransaction = signedTransaction .serialize() diff --git a/modules/node_modules/@colony/purser-trezor/staticMethods.js b/modules/node_modules/@colony/purser-trezor/staticMethods.js index cd6f97ea..b31047c6 100644 --- a/modules/node_modules/@colony/purser-trezor/staticMethods.js +++ b/modules/node_modules/@colony/purser-trezor/staticMethods.js @@ -22,6 +22,7 @@ import { transactionObjectValidator, messageVerificationObjectValidator, messageOrDataValidator, + getChainDefinition, } from '@colony/purser-core/helpers'; import { HEX_HASH_TYPE, SIGNATURE } from '@colony/purser-core/defaults'; @@ -144,14 +145,7 @@ export const signTransaction = async ({ multipleOfTwoHexValueNormalizer(chainId.toString(16)), ), }, - { - chain: chainId, - /* - * Only send (and normalize) the destination address if one was - * provided in the initial transaction object. - */ - ...(to ? { to: addressNormalizer(to) } : {}), - }, + getChainDefinition(chainId), ); /* * Modify the default payload to set the transaction details diff --git a/modules/tests/mocks/@colony/purser-core/helpers.js b/modules/tests/mocks/@colony/purser-core/helpers.js index abdc30d6..3fb2774e 100644 --- a/modules/tests/mocks/@colony/purser-core/helpers.js +++ b/modules/tests/mocks/@colony/purser-core/helpers.js @@ -28,3 +28,7 @@ export const messageOrDataValidator = jest.fn( return messageData; }, ); + +export const getChainDefinition = jest.fn(() => ({ + common: { chainId: 'mocked-chain-id' }, +})); diff --git a/modules/tests/purser-ledger/staticMethods/signTransaction.test.js b/modules/tests/purser-ledger/staticMethods/signTransaction.test.js index 633e081e..1bdfef64 100644 --- a/modules/tests/purser-ledger/staticMethods/signTransaction.test.js +++ b/modules/tests/purser-ledger/staticMethods/signTransaction.test.js @@ -1,6 +1,9 @@ import { Transaction as EthereumTx } from 'ethereumjs-tx'; -import { transactionObjectValidator } from '@colony/purser-core/helpers'; +import { + getChainDefinition, + transactionObjectValidator, +} from '@colony/purser-core/helpers'; import * as utils from '@colony/purser-core/utils'; import { signTransaction } from '@colony/purser-ledger/staticMethods'; @@ -75,6 +78,7 @@ describe('`Ledger` Hardware Wallet Module Static Methods', () => { ledgerConnection.mockClear(); handleLedgerConnectionError.mockClear(); transactionObjectValidator.mockClear(); + getChainDefinition.mockClear(); derivationPathValidator.mockClear(); utils.warning.mockClear(); }); @@ -108,6 +112,16 @@ describe('`Ledger` Hardware Wallet Module Static Methods', () => { expect(derivationPathValidator).toHaveBeenCalled(); expect(derivationPathValidator).toHaveBeenCalledWith(derivationPath); }); + test('Gets the correct chain definition', async () => { + await signTransaction(mockedArgumentsObject); + /* + * Calls the chain definition helper with the correct value + */ + expect(getChainDefinition).toHaveBeenCalled(); + expect(getChainDefinition).toHaveBeenCalledWith( + mockedTransactionObject.chainId, + ); + }); test('Normalizes the transaction input values', async () => { await signTransaction(mockedArgumentsObject); /* @@ -127,11 +141,6 @@ describe('`Ledger` Hardware Wallet Module Static Methods', () => { */ expect(hexSequenceNormalizer).toHaveBeenCalledWith(nonce); expect(multipleOfTwoHexValueNormalizer).toHaveBeenCalledWith(nonce); - /* - * Normalizes the destination address - */ - expect(addressNormalizer).toHaveBeenCalled(); - expect(addressNormalizer).toHaveBeenCalledWith(to); /* * Normalizes the transaction value */ @@ -170,22 +179,17 @@ describe('`Ledger` Hardware Wallet Module Static Methods', () => { */ expect(EthereumTx).toHaveBeenCalled(); expect(EthereumTx).toHaveBeenCalledWith( - expect.objectContaining( - { - gasPrice, - gasLimit, - nonce, - value, - data: inputData, - r: String(SIGNATURE.R), - s: String(SIGNATURE.S), - v: chainId, - }, - { - chain: chainId, - to, - }, - ), + expect.objectContaining({ + gasPrice, + gasLimit, + nonce, + value, + data: inputData, + r: String(SIGNATURE.R), + s: String(SIGNATURE.S), + v: chainId, + }), + expect.objectContaining({ common: { chainId: 'mocked-chain-id' } }), ); }); test('Normalizes the signed transaction signature components', async () => { diff --git a/modules/tests/purser-metamask/staticMethods/signTransaction.test.js b/modules/tests/purser-metamask/staticMethods/signTransaction.test.js index cf2ba8fd..0b32d562 100644 --- a/modules/tests/purser-metamask/staticMethods/signTransaction.test.js +++ b/modules/tests/purser-metamask/staticMethods/signTransaction.test.js @@ -1,4 +1,8 @@ -import { transactionObjectValidator } from '@colony/purser-core/helpers'; +import { Transaction as EthereumTx } from 'ethereumjs-tx'; +import { + getChainDefinition, + transactionObjectValidator, +} from '@colony/purser-core/helpers'; import { warning } from '@colony/purser-core/utils'; import { signTransaction } from '@colony/purser-metamask/staticMethods'; @@ -15,6 +19,7 @@ import { } from '@colony/purser-core/validators'; import { STD_ERRORS } from '@colony/purser-metamask/defaults'; +import { SIGNATURE } from '@colony/purser-core/defaults'; jest.dontMock('@colony/purser-metamask/staticMethods'); @@ -38,11 +43,23 @@ jest.mock('@colony/purser-metamask/helpers', () => require('@mocks/purser-metamask/helpers'), ); +const chainId = 5; + /* * Mock the injected web3 proxy object */ const mockedTransactionHash = 'mocked-transaction-hash'; -const mockedRawSignedTransaction = {}; +const mockedRawSignedTransaction = { + gas: '1000', + gasPrice: '1', + input: 'mocked-signed-data', + nonce: '10', + r: SIGNATURE.R, + s: SIGNATURE.S, + to: 'mocked-signed-to', + v: SIGNATURE.RECOVERY_EVEN, + value: '0', +}; global.web3 = { eth: { sendTransaction: jest.fn((transactionObject, callback) => @@ -67,6 +84,7 @@ const mockedTransactionObject = { gasPrice, gasLimit, to, + chainId, value, inputData, }; @@ -84,6 +102,7 @@ describe('`Metamask` Wallet Module Static Methods', () => { addressValidator.mockClear(); safeIntegerValidator.mockClear(); hexSequenceValidator.mockClear(); + getChainDefinition.mockClear(); warning.mockClear(); addressNormalizer.mockClear(); hexSequenceNormalizer.mockClear(); @@ -118,6 +137,16 @@ describe('`Metamask` Wallet Module Static Methods', () => { mockedTransactionObject, ); }); + test('Gets the correct chain definition', async () => { + await signTransaction(mockedArgumentsObject); + /* + * Calls the chain definition helper with the correct value + */ + expect(getChainDefinition).toHaveBeenCalled(); + expect(getChainDefinition).toHaveBeenCalledWith( + mockedTransactionObject.chainId, + ); + }); test('Throws if no argument provided', async () => { expect(signTransaction()).rejects.toThrow(); }); @@ -182,6 +211,27 @@ describe('`Metamask` Wallet Module Static Methods', () => { expect(hexSequenceNormalizer).toHaveBeenCalled(); expect(hexSequenceNormalizer).toHaveBeenCalledWith(mockedTransactionHash); }); + test('Creates the unsigned transaction object', async () => { + await signTransaction(mockedArgumentsObject); + /* + * Creates the unsigned transaction, seeding the R,S and V components + */ + expect(EthereumTx).toHaveBeenCalled(); + expect(EthereumTx).toHaveBeenCalledWith( + { + data: mockedRawSignedTransaction.input, + r: mockedRawSignedTransaction.r, + s: mockedRawSignedTransaction.s, + v: mockedRawSignedTransaction.v, + to: mockedRawSignedTransaction.to, + gasLimit: expect.anything(), + gasPrice: expect.anything(), + nonce: expect.anything(), + value: expect.anything(), + }, + { common: { chainId: 'mocked-chain-id' } }, + ); + }); test('Returns the valid hash received from signing', async () => { const signedTransaction = await signTransaction(mockedArgumentsObject); expect(global.web3.eth.getTransaction).toHaveBeenCalled(); diff --git a/modules/tests/purser-trezor/staticMethods/signTransaction.test.js b/modules/tests/purser-trezor/staticMethods/signTransaction.test.js index 871a91fc..80684996 100644 --- a/modules/tests/purser-trezor/staticMethods/signTransaction.test.js +++ b/modules/tests/purser-trezor/staticMethods/signTransaction.test.js @@ -1,6 +1,9 @@ import { Transaction as EthereumTx } from 'ethereumjs-tx'; -import { transactionObjectValidator } from '@colony/purser-core/helpers'; +import { + getChainDefinition, + transactionObjectValidator, +} from '@colony/purser-core/helpers'; import * as utils from '@colony/purser-core/utils'; import { signTransaction } from '@colony/purser-trezor/staticMethods'; @@ -71,6 +74,7 @@ describe('`Trezor` Hardware Wallet Module Static Methods', () => { multipleOfTwoHexValueNormalizer.mockClear(); addressNormalizer.mockClear(); hexSequenceNormalizer.mockClear(); + getChainDefinition.mockClear(); utils.warning.mockClear(); payloadListener.mockClear(); derivationPathValidator.mockClear(); @@ -90,10 +94,7 @@ describe('`Trezor` Hardware Wallet Module Static Methods', () => { s: '0', v: chainId, }, - { - to, - chain: chainId, - }, + { common: { chainId: 'mocked-chain-id' } }, ); }); test('Uses the correct trezor service payload type', async () => { @@ -120,6 +121,16 @@ describe('`Trezor` Hardware Wallet Module Static Methods', () => { mockedTransactionObject, ); }); + test('Gets the correct chain definition', async () => { + await signTransaction(mockedArgumentsObject); + /* + * Calls the chain definition helper with the correct value + */ + expect(getChainDefinition).toHaveBeenCalled(); + expect(getChainDefinition).toHaveBeenCalledWith( + mockedTransactionObject.chainId, + ); + }); test('Validates the derivation path individually', async () => { await signTransaction(mockedArgumentsObject); /* diff --git a/package.json b/package.json index 2d13ce02..ce4e5fdb 100644 --- a/package.json +++ b/package.json @@ -82,6 +82,7 @@ "@ledgerhq/hw-transport-u2f": "^4.50.0", "bip32-path": "^0.4.2", "bn.js": "^5.0.0", + "ethereumjs-common": "^1.3.0", "ethereumjs-tx": "^2.1.0", "ethereumjs-util": "^6.0.0", "ethers": "^4.0.33",