diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index fa7c46a2b..3889b9fb8 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -1,10 +1,11 @@ import EventEmitter from 'safe-event-emitter'; import { ObservableStore } from '@metamask/obs-store'; import { bufferToHex, keccak, toBuffer } from 'ethereumjs-util'; -import Transaction from 'ethereumjs-tx'; import EthQuery from 'ethjs-query'; import { ethErrors } from 'eth-rpc-errors'; import abi from 'human-standard-token-abi'; +import Common from '@ethereumjs/common'; +import { TransactionFactory } from '@ethereumjs/tx'; import { ethers } from 'ethers'; import NonceTracker from 'nonce-tracker'; import log from 'loglevel'; @@ -24,11 +25,17 @@ import { } from '../../../../shared/constants/transaction'; import { METAMASK_CONTROLLER_EVENTS } from '../../metamask-controller'; import { GAS_LIMITS } from '../../../../shared/constants/gas'; +import { + MAINNET, + NETWORK_TYPE_RPC, +} from '../../../../shared/constants/network'; import TransactionStateManager from './tx-state-manager'; import TxGasUtil from './tx-gas-utils'; import PendingTransactionTracker from './pending-tx-tracker'; import * as txUtils from './lib/util'; +const HARDFORK = 'berlin'; + const hstInterface = new ethers.utils.Interface(abi); const MAX_MEMSTORE_TX_LIST_SIZE = 100; // Number of transactions (by unique nonces) to keep in memory @@ -53,7 +60,7 @@ const MAX_MEMSTORE_TX_LIST_SIZE = 100; // Number of transactions (by unique nonc @param {Object} opts.networkStore - an observable store for network number @param {Object} opts.blockTracker - An instance of eth-blocktracker @param {Object} opts.provider - A network provider. - @param {Function} opts.signTransaction - function the signs an ethereumjs-tx + @param {Function} opts.signTransaction - function the signs an @ethereumjs/tx @param {Object} opts.getPermittedAccounts - get accounts that an origin has permissions for @param {Function} opts.signTransaction - ethTx signer that returns a rawTx @param {number} [opts.txHistoryLimit] - number *optional* for limiting how many transactions are in state @@ -65,6 +72,7 @@ export default class TransactionController extends EventEmitter { super(); this.networkStore = opts.networkStore || new ObservableStore({}); this._getCurrentChainId = opts.getCurrentChainId; + this.getProviderConfig = opts.getProviderConfig; this.preferencesStore = opts.preferencesStore || new ObservableStore({}); this.provider = opts.provider; this.getPermittedAccounts = opts.getPermittedAccounts; @@ -146,6 +154,49 @@ export default class TransactionController extends EventEmitter { return integerChainId; } + /** + * @ethereumjs/tx uses @ethereumjs/common as a configuration tool for + * specifying which chain, network, hardfork and EIPs to support for + * a transaction. By referencing this configuration, and analyzing the fields + * specified in txParams, @ethereumjs/tx is able to determine which EIP-2718 + * transaction type to use. + * @returns {Common} common configuration object + */ + getCommonConfiguration() { + const { type, nickname: name } = this.getProviderConfig(); + + // type will be one of our default network names or 'rpc'. the default + // network names are sufficient configuration, simply pass the name as the + // chain argument in the constructor. + if (type !== NETWORK_TYPE_RPC) { + return new Common({ chain: type, hardfork: HARDFORK }); + } + + // For 'rpc' we need to use the same basic configuration as mainnet, + // since we only support EVM compatible chains, and then override the + // name, chainId and networkId properties. This is done using the + // `forCustomChain` static method on the Common class. + const chainId = parseInt(this._getCurrentChainId(), 16); + const networkId = this.networkStore.getState(); + + const customChainParams = { + name, + chainId, + // It is improbable for a transaction to be signed while the network + // is loading for two reasons. + // 1. Pending, unconfirmed transactions are wiped on network change + // 2. The UI is unusable (loading indicator) when network is loading. + // setting the networkId to 0 is for type safety and to explicity lead + // the transaction to failing if a user is able to get to this branch + // on a custom network that requires valid network id. I have not ran + // into this limitation on any network I have attempted, even when + // hardcoding networkId to 'loading'. + networkId: networkId === 'loading' ? 0 : parseInt(networkId, 10), + }; + + return Common.forCustomChain(MAINNET, customChainParams, HARDFORK); + } + /** Adds a tx to the txlist @emits ${txMeta.id}:unapproved @@ -561,17 +612,22 @@ export default class TransactionController extends EventEmitter { const txMeta = this.txStateManager.getTransaction(txId); // add network/chain id const chainId = this.getChainId(); - const txParams = { ...txMeta.txParams, chainId }; + const txParams = { + ...txMeta.txParams, + chainId, + gasLimit: txMeta.txParams.gas, + }; // sign tx const fromAddress = txParams.from; - const ethTx = new Transaction(txParams); - await this.signEthTx(ethTx, fromAddress); + const common = this.getCommonConfiguration(); + const unsignedEthTx = TransactionFactory.fromTxData(txParams, { common }); + const signedEthTx = await this.signEthTx(unsignedEthTx, fromAddress); // add r,s,v values for provider request purposes see createMetamaskMiddleware // and JSON rpc standard for further explanation - txMeta.r = bufferToHex(ethTx.r); - txMeta.s = bufferToHex(ethTx.s); - txMeta.v = bufferToHex(ethTx.v); + txMeta.r = bufferToHex(signedEthTx.r); + txMeta.s = bufferToHex(signedEthTx.s); + txMeta.v = bufferToHex(signedEthTx.v); this.txStateManager.updateTransaction( txMeta, @@ -580,7 +636,7 @@ export default class TransactionController extends EventEmitter { // set state to signed this.txStateManager.setTxStatusSigned(txMeta.id); - const rawTx = bufferToHex(ethTx.serialize()); + const rawTx = bufferToHex(signedEthTx.serialize()); return rawTx; } diff --git a/app/scripts/controllers/transactions/index.test.js b/app/scripts/controllers/transactions/index.test.js index 3615c33d0..d08aa856a 100644 --- a/app/scripts/controllers/transactions/index.test.js +++ b/app/scripts/controllers/transactions/index.test.js @@ -1,7 +1,7 @@ import { strict as assert } from 'assert'; import EventEmitter from 'events'; import { toBuffer } from 'ethereumjs-util'; -import EthTx from 'ethereumjs-tx'; +import { TransactionFactory } from '@ethereumjs/tx'; import { ObservableStore } from '@metamask/obs-store'; import sinon from 'sinon'; @@ -20,6 +20,9 @@ import TransactionController from '.'; const noop = () => true; const currentNetworkId = '42'; const currentChainId = '0x2a'; +const providerConfig = { + type: 'kovan', +}; const VALID_ADDRESS = '0x0000000000000000000000000000000000000000'; const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001'; @@ -36,6 +39,7 @@ describe('Transaction Controller', function () { }; provider = createTestProviderTools({ scaffold: providerResultStub }) .provider; + fromAccount = getTestAccounts()[0]; const blockTrackerStub = new EventEmitter(); blockTrackerStub.getCurrentBlock = noop; @@ -50,9 +54,9 @@ describe('Transaction Controller', function () { blockTracker: blockTrackerStub, signTransaction: (ethTx) => new Promise((resolve) => { - ethTx.sign(fromAccount.key); - resolve(); + resolve(ethTx.sign(fromAccount.key)); }), + getProviderConfig: () => providerConfig, getPermittedAccounts: () => undefined, getCurrentChainId: () => currentChainId, getParticipateInMetrics: () => false, @@ -519,8 +523,8 @@ describe('Transaction Controller', function () { noop, ); const rawTx = await txController.signTransaction('1'); - const ethTx = new EthTx(toBuffer(rawTx)); - assert.equal(ethTx.getChainId(), 42); + const ethTx = TransactionFactory.fromSerializedData(toBuffer(rawTx)); + assert.equal(ethTx.common.chainIdBN().toNumber(), 42); }); }); diff --git a/app/scripts/controllers/transactions/tx-gas-utils.test.js b/app/scripts/controllers/transactions/tx-gas-utils.test.js index 635802925..15386e319 100644 --- a/app/scripts/controllers/transactions/tx-gas-utils.test.js +++ b/app/scripts/controllers/transactions/tx-gas-utils.test.js @@ -1,5 +1,6 @@ import { strict as assert } from 'assert'; -import Transaction from 'ethereumjs-tx'; +import { TransactionFactory } from '@ethereumjs/tx'; +import Common from '@ethereumjs/common'; import { hexToBn, bnToHex } from '../../lib/util'; import TxUtils from './tx-gas-utils'; @@ -31,8 +32,14 @@ describe('txUtils', function () { nonce: '0x3', chainId: 42, }; - const ethTx = new Transaction(txParams); - assert.equal(ethTx.getChainId(), 42, 'chainId is set from tx params'); + const ethTx = TransactionFactory.fromTxData(txParams, { + common: new Common({ chain: 'kovan' }), + }); + assert.equal( + ethTx.common.chainIdBN().toNumber(), + 42, + 'chainId is set from tx params', + ); }); }); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e2546a91f..304dccf7a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -322,6 +322,9 @@ export default class MetamaskController extends EventEmitter { getPermittedAccounts: this.permissionsController.getAccounts.bind( this.permissionsController, ), + getProviderConfig: this.networkController.getProviderConfig.bind( + this.networkController, + ), networkStore: this.networkController.networkStore, getCurrentChainId: this.networkController.getCurrentChainId.bind( this.networkController, diff --git a/package.json b/package.json index 8e420c948..b6181d71b 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,8 @@ "3box": "^1.10.2", "@babel/runtime": "^7.5.5", "@download/blockies": "^1.0.3", + "@ethereumjs/common": "^2.3.1", + "@ethereumjs/tx": "^3.2.1", "@formatjs/intl-relativetimeformat": "^5.2.6", "@fortawesome/fontawesome-free": "^5.13.0", "@lavamoat/preinstall-always-fail": "^1.0.0", @@ -138,7 +140,6 @@ "eth-trezor-keyring": "^0.7.0", "ethereum-ens-network-map": "^1.0.2", "ethereumjs-abi": "^0.6.4", - "ethereumjs-tx": "1.3.7", "ethereumjs-util": "^7.0.10", "ethereumjs-wallet": "^0.6.4", "ethers": "^5.0.8", diff --git a/yarn.lock b/yarn.lock index 20bc48aae..4861bf74e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1529,7 +1529,7 @@ crc-32 "^1.2.0" ethereumjs-util "^7.0.10" -"@ethereumjs/tx@^3.1.1", "@ethereumjs/tx@^3.1.4": +"@ethereumjs/tx@^3.1.1", "@ethereumjs/tx@^3.1.4", "@ethereumjs/tx@^3.2.1": version "3.2.1" resolved "https://registry.yarnpkg.com/@ethereumjs/tx/-/tx-3.2.1.tgz#65f5f1c11541764f08377a94ba4b0dcbbd67739e" integrity sha512-i9V39OtKvwWos1uVNZxdVhd7zFOyzFLjgt69CoiOY0EmXugS0HjO3uxpLBSglDKFMRriuGqw6ddKEv+RP1UNEw== @@ -10900,14 +10900,6 @@ ethereumjs-common@^1.1.0, ethereumjs-common@^1.3.2, ethereumjs-common@^1.5.0: resolved "https://registry.yarnpkg.com/ethereumjs-common/-/ethereumjs-common-1.5.1.tgz#4e75042473a64daec0ed9fe84323dd9576aa5dba" integrity sha512-aVUPRLgmXORGXXEVkFYgPhr9TGtpBY2tGhZ9Uh0A3lIUzUDr1x6kQx33SbjPUkLkX3eniPQnIL/2psjkjrOfcQ== -ethereumjs-tx@1.3.7, ethereumjs-tx@^1.1.1, ethereumjs-tx@^1.2.0, ethereumjs-tx@^1.2.2, ethereumjs-tx@^1.3.3, ethereumjs-tx@^1.3.4, ethereumjs-tx@^1.3.7: - version "1.3.7" - resolved "https://registry.yarnpkg.com/ethereumjs-tx/-/ethereumjs-tx-1.3.7.tgz#88323a2d875b10549b8347e09f4862b546f3d89a" - integrity sha512-wvLMxzt1RPhAQ9Yi3/HKZTn0FZYpnsmQdbKYfUUpi4j1SEIcbkd9tndVjcPrufY3V7j2IebOpC00Zp2P/Ay2kA== - dependencies: - ethereum-common "^0.0.18" - ethereumjs-util "^5.0.0" - ethereumjs-tx@2.1.2, ethereumjs-tx@^2.1.1, ethereumjs-tx@^2.1.2: version "2.1.2" resolved "https://registry.yarnpkg.com/ethereumjs-tx/-/ethereumjs-tx-2.1.2.tgz#5dfe7688bf177b45c9a23f86cf9104d47ea35fed" @@ -10916,6 +10908,14 @@ ethereumjs-tx@2.1.2, ethereumjs-tx@^2.1.1, ethereumjs-tx@^2.1.2: ethereumjs-common "^1.5.0" ethereumjs-util "^6.0.0" +ethereumjs-tx@^1.1.1, ethereumjs-tx@^1.2.0, ethereumjs-tx@^1.2.2, ethereumjs-tx@^1.3.3, ethereumjs-tx@^1.3.4, ethereumjs-tx@^1.3.7: + version "1.3.7" + resolved "https://registry.yarnpkg.com/ethereumjs-tx/-/ethereumjs-tx-1.3.7.tgz#88323a2d875b10549b8347e09f4862b546f3d89a" + integrity sha512-wvLMxzt1RPhAQ9Yi3/HKZTn0FZYpnsmQdbKYfUUpi4j1SEIcbkd9tndVjcPrufY3V7j2IebOpC00Zp2P/Ay2kA== + dependencies: + ethereum-common "^0.0.18" + ethereumjs-util "^5.0.0" + ethereumjs-util@6.2.1, ethereumjs-util@^6.0.0, ethereumjs-util@^6.1.0, ethereumjs-util@^6.2.0: version "6.2.1" resolved "https://registry.yarnpkg.com/ethereumjs-util/-/ethereumjs-util-6.2.1.tgz#fcb4e4dd5ceacb9d2305426ab1a5cd93e3163b69"