diff --git a/CHANGELOG.md b/CHANGELOG.md index 312260c65..37d9f26b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Current Develop Branch +- [#5283](https://github.com/MetaMask/metamask-extension/pull/5283): Fix bug when eth.getCode() called with no contract - [#5563](https://github.com/MetaMask/metamask-extension/pull/5563#pullrequestreview-166769174) Feature: improve Hatian Creole translations - [#5559](https://github.com/MetaMask/metamask-extension/pull/5559) Localize language names in translation select list diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 5423c76dc..26d0422cb 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1196,6 +1196,9 @@ "transactionError": { "message": "Transaction Error. Exception thrown in contract code." }, + "transactionErrorNoContract": { + "message": "Trying to call a function on a non-contract address." + }, "transactionMemo": { "message": "Transaction memo (optional)" }, diff --git a/app/scripts/controllers/transactions/tx-gas-utils.js b/app/scripts/controllers/transactions/tx-gas-utils.js index 3dd45507f..def67c2c3 100644 --- a/app/scripts/controllers/transactions/tx-gas-utils.js +++ b/app/scripts/controllers/transactions/tx-gas-utils.js @@ -7,6 +7,8 @@ const { const { addHexPrefix } = require('ethereumjs-util') const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send. +import { TRANSACTION_NO_CONTRACT_ERROR_KEY } from '../../../../ui/app/constants/error-keys' + /** tx-gas-utils are gas utility methods for Transaction manager its passed ethquery @@ -32,6 +34,7 @@ class TxGasUtil { } catch (err) { txMeta.simulationFails = { reason: err.message, + errorKey: err.errorKey, } return txMeta } @@ -56,24 +59,38 @@ class TxGasUtil { return txParams.gas } - // if recipient has no code, gas is 21k max: const recipient = txParams.to const hasRecipient = Boolean(recipient) - let code - if (recipient) code = await this.query.getCode(recipient) - if (hasRecipient && (!code || code === '0x')) { - txParams.gas = SIMPLE_GAS_COST - txMeta.simpleSend = true // Prevents buffer addition - return SIMPLE_GAS_COST + // see if we can set the gas based on the recipient + if (hasRecipient) { + const code = await this.query.getCode(recipient) + // 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' + + if (codeIsEmpty) { + // 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') + // set error key so ui can display localized error message + err.errorKey = TRANSACTION_NO_CONTRACT_ERROR_KEY + throw err + } + + // This is a standard ether simple send, gas requirement is exactly 21k + txParams.gas = SIMPLE_GAS_COST + // prevents buffer addition + txMeta.simpleSend = true + return SIMPLE_GAS_COST + } } - // if not, fall back to block gasLimit + // fallback to block gasLimit const blockGasLimitBN = hexToBn(blockGasLimitHex) const saferGasLimitBN = BnMultiplyByFraction(blockGasLimitBN, 19, 20) txParams.gas = bnToHex(saferGasLimitBN) - // run tx + // estimate tx gas requirements return await this.query.estimateGas(txParams) } diff --git a/old-ui/app/components/pending-tx.js b/old-ui/app/components/pending-tx.js index c8132539c..35e81210e 100644 --- a/old-ui/app/components/pending-tx.js +++ b/old-ui/app/components/pending-tx.js @@ -1,4 +1,5 @@ const Component = require('react').Component +const connect = require('react-redux').connect const h = require('react-hyperscript') const inherits = require('util').inherits const actions = require('../../../ui/app/actions') @@ -19,7 +20,9 @@ const BNInput = require('./bn-as-decimal-input') const MIN_GAS_PRICE_BN = new BN('0') const MIN_GAS_LIMIT_BN = new BN('21000') -module.exports = PendingTx +module.exports = connect()(PendingTx) + + inherits(PendingTx, Component) function PendingTx () { Component.call(this) @@ -445,7 +448,8 @@ PendingTx.prototype.onSubmit = function (event) { const txMeta = this.gatherTxMeta() const valid = this.checkValidity() this.setState({ valid, submitting: true }) - if (valid && this.verifyGasParams()) { + const validGasParams = this.verifyGasParams() + if (valid && validGasParams) { this.props.sendTransaction(txMeta, event) } else { this.props.dispatch(actions.displayWarning('Invalid Gas Parameters')) @@ -488,8 +492,12 @@ PendingTx.prototype.verifyGasParams = function () { ) } -PendingTx.prototype._notZeroOrEmptyString = function (obj) { - return obj !== '' && obj !== '0x0' +PendingTx.prototype._notZeroOrEmptyString = function (value) { + // allow undefined values + if (value === undefined) return true + // Geth will return '0x', and ganache-core v2.2.1 will return '0x0' + const valueIsEmpty = !value || value === '0x' || value === '0x0' + return !valueIsEmpty } PendingTx.prototype.bnMultiplyByFraction = function (targetBN, numerator, denominator) { diff --git a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js index c92867afe..7d01aaffb 100644 --- a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -136,7 +136,7 @@ export default class ConfirmTransactionBase extends Component { if (simulationFails) { return { valid: true, - errorKey: TRANSACTION_ERROR_KEY, + errorKey: simulationFails.errorKey ? simulationFails.errorKey : TRANSACTION_ERROR_KEY, } } diff --git a/ui/app/components/send/send.utils.js b/ui/app/components/send/send.utils.js index a18a9e4b3..eb1667c63 100644 --- a/ui/app/components/send/send.utils.js +++ b/ui/app/components/send/send.utils.js @@ -215,7 +215,9 @@ async function estimateGas ({ // if recipient has no code, gas is 21k max: if (!selectedToken && !data) { const code = Boolean(to) && await global.eth.getCode(to) - if (!code || code === '0x') { + // Geth will return '0x', and ganache-core v2.2.1 will return '0x0' + const codeIsEmpty = !code || code === '0x' || code === '0x0' + if (codeIsEmpty) { return SIMPLE_GAS_COST } } else if (selectedToken && !to) { diff --git a/ui/app/constants/error-keys.js b/ui/app/constants/error-keys.js index f70ed3b19..704064c96 100644 --- a/ui/app/constants/error-keys.js +++ b/ui/app/constants/error-keys.js @@ -1,3 +1,4 @@ export const INSUFFICIENT_FUNDS_ERROR_KEY = 'insufficientFunds' export const GAS_LIMIT_TOO_LOW_ERROR_KEY = 'gasLimitTooLow' export const TRANSACTION_ERROR_KEY = 'transactionError' +export const TRANSACTION_NO_CONTRACT_ERROR_KEY = 'transactionErrorNoContract' diff --git a/ui/app/helpers/transactions.util.js b/ui/app/helpers/transactions.util.js index 64ec82225..2f4b1d095 100644 --- a/ui/app/helpers/transactions.util.js +++ b/ui/app/helpers/transactions.util.js @@ -125,7 +125,9 @@ export function getLatestSubmittedTxWithNonce (transactions = [], nonce = '0x0') export async function isSmartContractAddress (address) { const code = await global.eth.getCode(address) - return code && code !== '0x' + // Geth will return '0x', and ganache-core v2.2.1 will return '0x0' + const codeIsEmpty = !code || code === '0x' || code === '0x0' + return !codeIsEmpty } export function sumHexes (...args) {