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 } diff --git a/test/e2e/beta/contract-test/contract.js b/test/e2e/beta/contract-test/contract.js index e247e26ea..e1f886c58 100644 --- a/test/e2e/beta/contract-test/contract.js +++ b/test/e2e/beta/contract-test/contract.js @@ -37,6 +37,8 @@ web3.currentProvider.enable().then(() => { const createToken = document.getElementById('createToken') const transferTokens = document.getElementById('transferTokens') const approveTokens = document.getElementById('approveTokens') + const transferTokensWithoutGas = document.getElementById('transferTokensWithoutGas') + const approveTokensWithoutGas = document.getElementById('approveTokensWithoutGas') deployButton.addEventListener('click', async function () { document.getElementById('contractStatus').innerHTML = 'Deploying' @@ -135,6 +137,29 @@ web3.currentProvider.enable().then(() => { console.log(result) }) }) + + transferTokensWithoutGas.addEventListener('click', function (event) { + console.log(`event`, event) + contract.transfer('0x2f318C334780961FB129D2a6c30D0763d9a5C970', '7', { + from: web3.eth.accounts[0], + to: contract.address, + data: '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a', + gasPrice: '20000000000', + }, function (result) { + console.log('result', result) + }) + }) + + approveTokensWithoutGas.addEventListener('click', function () { + contract.approve('0x2f318C334780961FB129D2a6c30D0763d9a5C970', '7', { + from: web3.eth.accounts[0], + to: contract.address, + data: '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005', + gasPrice: '20000000000', + }, function (result) { + console.log(result) + }) + }) } }) diff --git a/test/e2e/beta/contract-test/index.html b/test/e2e/beta/contract-test/index.html index 0d422ef20..6e134dc36 100644 --- a/test/e2e/beta/contract-test/index.html +++ b/test/e2e/beta/contract-test/index.html @@ -27,6 +27,8 @@ + + diff --git a/test/e2e/beta/metamask-beta-ui.spec.js b/test/e2e/beta/metamask-beta-ui.spec.js index 7fb3de6e4..06778ab99 100644 --- a/test/e2e/beta/metamask-beta-ui.spec.js +++ b/test/e2e/beta/metamask-beta-ui.spec.js @@ -812,10 +812,31 @@ describe('MetaMask', function () { await delay(regularDelayMs) const [gasPriceInput, gasLimitInput] = await findElements(driver, By.css('.advanced-tab__gas-edit-row__input')) - await gasPriceInput.clear() + await gasPriceInput.sendKeys(Key.chord(Key.CONTROL, 'a')) + await delay(50) + + await gasPriceInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasPriceInput.sendKeys(Key.BACK_SPACE) + await delay(50) await gasPriceInput.sendKeys('10') - await gasLimitInput.clear() + await delay(50) + await gasLimitInput.sendKeys(Key.chord(Key.CONTROL, 'a')) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) await gasLimitInput.sendKeys('60001') + await delay(50) + await gasLimitInput.sendKeys(Key.chord(Key.CONTROL, 'e')) + await delay(50) const save = await findElement(driver, By.xpath(`//button[contains(text(), 'Save')]`)) await save.click() @@ -1175,7 +1196,7 @@ describe('MetaMask', function () { const transferTokens = await findElement(driver, By.xpath(`//button[contains(text(), 'Approve Tokens')]`)) await transferTokens.click() - await closeAllWindowHandlesExcept(driver, extension) + await closeAllWindowHandlesExcept(driver, [extension, dapp]) await driver.switchTo().window(extension) await delay(regularDelayMs) @@ -1228,21 +1249,31 @@ describe('MetaMask', function () { await delay(regularDelayMs) const [gasPriceInput, gasLimitInput] = await findElements(driver, By.css('.advanced-tab__gas-edit-row__input')) - await gasPriceInput.clear() - await delay(tinyDelayMs) + await gasPriceInput.sendKeys(Key.chord(Key.CONTROL, 'a')) + await delay(50) + + await gasPriceInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasPriceInput.sendKeys(Key.BACK_SPACE) + await delay(50) await gasPriceInput.sendKeys('10') - await delay(tinyDelayMs) - await gasLimitInput.clear() - await delay(tinyDelayMs) + await delay(50) await gasLimitInput.sendKeys(Key.chord(Key.CONTROL, 'a')) - await gasLimitInput.sendKeys('60000') + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys(Key.BACK_SPACE) + await delay(50) + await gasLimitInput.sendKeys('60001') + await delay(50) await gasLimitInput.sendKeys(Key.chord(Key.CONTROL, 'e')) - - // Needed for different behaviour of input in different versions of firefox - const gasLimitInputValue = await gasLimitInput.getAttribute('value') - if (gasLimitInputValue === '600001') { - await gasLimitInput.sendKeys(Key.BACK_SPACE) - } + await delay(50) const save = await findElement(driver, By.css('.page-container__footer-button')) await save.click() @@ -1271,6 +1302,105 @@ describe('MetaMask', function () { }) }) + describe('Tranfers a custom token from dapp when no gas value is specified', () => { + it('transfers an already created token, without specifying gas', async () => { + const windowHandles = await driver.getAllWindowHandles() + const extension = windowHandles[0] + const dapp = await switchToWindowWithTitle(driver, 'E2E Test Dapp', windowHandles) + await closeAllWindowHandlesExcept(driver, [extension, dapp]) + await delay(regularDelayMs) + + await driver.switchTo().window(dapp) + + const transferTokens = await findElement(driver, By.xpath(`//button[contains(text(), 'Transfer Tokens Without Gas')]`)) + await transferTokens.click() + + await closeAllWindowHandlesExcept(driver, [extension, dapp]) + await driver.switchTo().window(extension) + await delay(regularDelayMs) + + await driver.wait(async () => { + const pendingTxes = await findElements(driver, By.css('.transaction-list__pending-transactions .transaction-list-item')) + return pendingTxes.length === 1 + }, 10000) + + const [txListItem] = await findElements(driver, By.css('.transaction-list-item')) + const [txListValue] = await findElements(driver, By.css('.transaction-list-item__amount--primary')) + await driver.wait(until.elementTextMatches(txListValue, /-7\s*TST/)) + await txListItem.click() + await delay(regularDelayMs) + }) + + it('submits the transaction', async function () { + await delay(regularDelayMs) + const confirmButton = await findElement(driver, By.xpath(`//button[contains(text(), 'Confirm')]`)) + await confirmButton.click() + await delay(regularDelayMs) + }) + + it('finds the transaction in the transactions list', async function () { + await driver.wait(async () => { + const confirmedTxes = await findElements(driver, By.css('.transaction-list__completed-transactions .transaction-list-item')) + return confirmedTxes.length === 4 + }, 10000) + + const txValues = await findElements(driver, By.css('.transaction-list-item__amount--primary')) + await driver.wait(until.elementTextMatches(txValues[0], /-7\s*TST/)) + const txStatuses = await findElements(driver, By.css('.transaction-list-item__action')) + await driver.wait(until.elementTextMatches(txStatuses[0], /Sent Tokens/)) + }) + }) + + describe('Approves a custom token from dapp when no gas value is specified', () => { + it('approves an already created token', async () => { + const windowHandles = await driver.getAllWindowHandles() + const extension = windowHandles[0] + const dapp = await switchToWindowWithTitle(driver, 'E2E Test Dapp', windowHandles) + await closeAllWindowHandlesExcept(driver, [extension, dapp]) + await delay(regularDelayMs) + + await driver.switchTo().window(dapp) + await delay(tinyDelayMs) + + const transferTokens = await findElement(driver, By.xpath(`//button[contains(text(), 'Approve Tokens Without Gas')]`)) + await transferTokens.click() + + await closeAllWindowHandlesExcept(driver, extension) + await driver.switchTo().window(extension) + await delay(regularDelayMs) + + await driver.wait(async () => { + const pendingTxes = await findElements(driver, By.css('.transaction-list__pending-transactions .transaction-list-item')) + return pendingTxes.length === 1 + }, 10000) + + const [txListItem] = await findElements(driver, By.css('.transaction-list-item')) + const [txListValue] = await findElements(driver, By.css('.transaction-list-item__amount--primary')) + await driver.wait(until.elementTextMatches(txListValue, /-7\s*TST/)) + await txListItem.click() + await delay(regularDelayMs) + }) + + it('submits the transaction', async function () { + await delay(regularDelayMs) + const confirmButton = await findElement(driver, By.xpath(`//button[contains(text(), 'Confirm')]`)) + await confirmButton.click() + await delay(regularDelayMs) + }) + + it('finds the transaction in the transactions list', async function () { + await driver.wait(async () => { + const confirmedTxes = await findElements(driver, By.css('.transaction-list__completed-transactions .transaction-list-item')) + return confirmedTxes.length === 5 + }, 10000) + + const txValues = await findElements(driver, By.css('.transaction-list-item__amount--primary')) + await driver.wait(until.elementTextMatches(txValues[0], /-7\s*TST/)) + const txStatuses = await findElements(driver, By.css('.transaction-list-item__action')) + await driver.wait(until.elementTextMatches(txStatuses[0], /Approve/)) + }) + }) + describe('Hide token', () => { it('hides the token when clicked', async () => { const [hideTokenEllipsis] = await findElements(driver, By.css('.token-list-item__ellipsis')) diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js index 9000cd364..8ff409207 100644 --- a/test/unit/app/controllers/transactions/tx-controller-test.js +++ b/test/unit/app/controllers/transactions/tx-controller-test.js @@ -8,6 +8,13 @@ const TransactionController = require('../../../../../app/scripts/controllers/tr const { TRANSACTION_TYPE_RETRY, } = require('../../../../../app/scripts/controllers/transactions/enums') +const { + TOKEN_METHOD_APPROVE, + TOKEN_METHOD_TRANSFER, + SEND_ETHER_ACTION_KEY, + DEPLOY_CONTRACT_ACTION_KEY, + CONTRACT_INTERACTION_KEY, +} = require('../../../../../ui/app/helpers/constants/transactions.js') const { createTestProviderTools, getTestAccounts } = require('../../../../stub/provider') const noop = () => true @@ -537,6 +544,86 @@ describe('Transaction Controller', function () { }) }) + describe('#_determineTransactionCategory', function () { + it('should return a simple send transactionCategory when to is truthy but data is falsey', async function () { + const result = await txController._determineTransactionCategory({ + to: '0xabc', + data: '', + }) + assert.deepEqual(result, { transactionCategory: SEND_ETHER_ACTION_KEY, getCodeResponse: undefined }) + }) + + it('should return a token transfer transactionCategory when data is for the respective method call', async function () { + const result = await txController._determineTransactionCategory({ + to: '0xabc', + data: '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a', + }) + assert.deepEqual(result, { transactionCategory: TOKEN_METHOD_TRANSFER, getCodeResponse: undefined }) + }) + + it('should return a token approve transactionCategory when data is for the respective method call', async function () { + const result = await txController._determineTransactionCategory({ + to: '0xabc', + data: '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005', + }) + assert.deepEqual(result, { transactionCategory: TOKEN_METHOD_APPROVE, getCodeResponse: undefined }) + }) + + it('should return a contract deployment transactionCategory when to is falsey and there is data', async function () { + const result = await txController._determineTransactionCategory({ + to: '', + data: '0xabd', + }) + assert.deepEqual(result, { transactionCategory: DEPLOY_CONTRACT_ACTION_KEY, getCodeResponse: undefined }) + }) + + it('should return a simple send transactionCategory with a 0x getCodeResponse when there is data and but the to address is not a contract address', async function () { + const result = await txController._determineTransactionCategory({ + to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9', + data: '0xabd', + }) + assert.deepEqual(result, { transactionCategory: SEND_ETHER_ACTION_KEY, getCodeResponse: '0x' }) + }) + + it('should return a simple send transactionCategory with a null getCodeResponse when to is truthy and there is data and but getCode returns an error', async function () { + const result = await txController._determineTransactionCategory({ + to: '0xabc', + data: '0xabd', + }) + assert.deepEqual(result, { transactionCategory: SEND_ETHER_ACTION_KEY, getCodeResponse: null }) + }) + + it('should return a contract interaction transactionCategory with the correct getCodeResponse when to is truthy and there is data and it is not a token transaction', async function () { + const _providerResultStub = { + // 1 gwei + eth_gasPrice: '0x0de0b6b3a7640000', + // by default, all accounts are external accounts (not contracts) + eth_getCode: '0xa', + } + const _provider = createTestProviderTools({ scaffold: _providerResultStub }).provider + const _fromAccount = getTestAccounts()[0] + const _blockTrackerStub = new EventEmitter() + _blockTrackerStub.getCurrentBlock = noop + _blockTrackerStub.getLatestBlock = noop + const _txController = new TransactionController({ + provider: _provider, + getGasPrice: function () { return '0xee6b2800' }, + networkStore: new ObservableStore(currentNetworkId), + txHistoryLimit: 10, + blockTracker: _blockTrackerStub, + signTransaction: (ethTx) => new Promise((resolve) => { + ethTx.sign(_fromAccount.key) + resolve() + }), + }) + const result = await _txController._determineTransactionCategory({ + to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9', + data: 'abd', + }) + assert.deepEqual(result, { transactionCategory: CONTRACT_INTERACTION_KEY, getCodeResponse: '0x0a' }) + }) + }) + describe('#getPendingTransactions', function () { beforeEach(function () { txController.txStateManager._saveTxList([