From b81c4e5c98cdf3f5e6ebb05e57c2be993cdf5da0 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 14 May 2019 15:44:07 -0230 Subject: Set a default value for code in _determineTransactionCategory (#6604) * Set a default value for code in _determineTransactionCategory * Adds e2e tests that fail when token txs without gas param are not properly handled. * Adds unit tests for _determineTransactionCategory * Base error throwing and simple gas setting in estimateTxGas on transactionCategory --- app/scripts/controllers/transactions/index.js | 11 ++++++----- app/scripts/controllers/transactions/tx-gas-utils.js | 15 +++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) (limited to 'app') diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index dc6a043e4..79dba7833 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -191,7 +191,7 @@ class TransactionController extends EventEmitter { } txUtils.validateTxParams(normalizedTxParams) // construct txMeta - const { transactionCategory, code } = await this._determineTransactionCategory(txParams) + const { transactionCategory, getCodeResponse } = await this._determineTransactionCategory(txParams) let txMeta = this.txStateManager.generateTxMeta({ txParams: normalizedTxParams, type: TRANSACTION_TYPE_STANDARD, @@ -204,7 +204,7 @@ class TransactionController extends EventEmitter { // check whether recipient account is blacklisted recipientBlacklistChecker.checkAccount(txMeta.metamaskNetworkId, normalizedTxParams.to) // add default tx params - txMeta = await this.addTxGasDefaults(txMeta, code) + txMeta = await this.addTxGasDefaults(txMeta, getCodeResponse) } catch (error) { log.warn(error) txMeta.loadingDefaults = false @@ -224,7 +224,7 @@ class TransactionController extends EventEmitter { @param txMeta {Object} - the txMeta object @returns {Promise} resolves with txMeta */ - async addTxGasDefaults (txMeta, code) { + async addTxGasDefaults (txMeta, getCodeResponse) { const txParams = txMeta.txParams // ensure value txParams.value = txParams.value ? ethUtil.addHexPrefix(txParams.value) : '0x0' @@ -235,7 +235,7 @@ class TransactionController extends EventEmitter { } txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16)) // set gasLimit - return await this.txGasUtil.analyzeGasUsage(txMeta, code) + return await this.txGasUtil.analyzeGasUsage(txMeta, getCodeResponse) } /** @@ -593,6 +593,7 @@ class TransactionController extends EventEmitter { try { code = await this.query.getCode(to) } catch (e) { + code = null log.warn(e) } // For an address with no code, geth will return '0x', and ganache-core v2.2.1 will return '0x0' @@ -601,7 +602,7 @@ class TransactionController extends EventEmitter { result = codeIsEmpty ? SEND_ETHER_ACTION_KEY : CONTRACT_INTERACTION_KEY } - return { transactionCategory: result, code } + return { transactionCategory: result, getCodeResponse: code } } /** diff --git a/app/scripts/controllers/transactions/tx-gas-utils.js b/app/scripts/controllers/transactions/tx-gas-utils.js index 2dc461f48..287fb6f44 100644 --- a/app/scripts/controllers/transactions/tx-gas-utils.js +++ b/app/scripts/controllers/transactions/tx-gas-utils.js @@ -6,6 +6,7 @@ const { } = require('../../lib/util') const log = require('loglevel') const { addHexPrefix } = require('ethereumjs-util') +const { SEND_ETHER_ACTION_KEY } = require('../../../../ui/app/helpers/constants/transactions.js') const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send. import { TRANSACTION_NO_CONTRACT_ERROR_KEY } from '../../../../ui/app/helpers/constants/error-keys' @@ -25,14 +26,13 @@ class TxGasUtil { /** @param txMeta {Object} - the txMeta object - @param code {string} - the code at the txs address, as returned by this.query.getCode(to) @returns {object} the txMeta object with the gas written to the txParams */ - async analyzeGasUsage (txMeta, code) { + async analyzeGasUsage (txMeta, getCodeResponse) { const block = await this.query.getBlockByNumber('latest', false) let estimatedGasHex try { - estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit, code) + estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit, getCodeResponse) } catch (err) { log.warn(err) txMeta.simulationFails = { @@ -55,10 +55,9 @@ class TxGasUtil { Estimates the tx's gas usage @param txMeta {Object} - the txMeta object @param blockGasLimitHex {string} - hex string of the block's gas limit - @param code {string} - the code at the txs address, as returned by this.query.getCode(to) @returns {string} the estimated gas limit as a hex string */ - async estimateTxGas (txMeta, blockGasLimitHex, code) { + async estimateTxGas (txMeta, blockGasLimitHex, getCodeResponse) { const txParams = txMeta.txParams // check if gasLimit is already specified @@ -75,9 +74,9 @@ class TxGasUtil { // see if we can set the gas based on the recipient if (hasRecipient) { // For an address with no code, geth will return '0x', and ganache-core v2.2.1 will return '0x0' - const codeIsEmpty = !code || code === '0x' || code === '0x0' + const categorizedAsSimple = txMeta.transactionCategory === SEND_ETHER_ACTION_KEY - if (codeIsEmpty) { + if (categorizedAsSimple) { // if there's data in the params, but there's no contract code, it's not a valid transaction if (txParams.data) { const err = new Error('TxGasUtil - Trying to call a function on a non-contract address') @@ -85,7 +84,7 @@ class TxGasUtil { err.errorKey = TRANSACTION_NO_CONTRACT_ERROR_KEY // set the response on the error so that we can see in logs what the actual response was - err.getCodeResponse = code + err.getCodeResponse = getCodeResponse throw err } -- cgit