From 6157275cfc326c16c0acfb83e15ab55e226d0902 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Fri, 4 Oct 2019 16:22:53 -0230 Subject: [PATCH 1/5] Ensure correct transaction category when sending to contracts but there is no txParams data --- app/scripts/controllers/transactions/index.js | 14 ++++----- .../transactions/tx-controller-test.js | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index c69b82673d03..22ea58142b09 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -603,21 +603,21 @@ class TransactionController extends EventEmitter { ].find(tokenMethodName => tokenMethodName === name && name.toLowerCase()) let result - let code - if (!txParams.data) { - result = SEND_ETHER_ACTION_KEY - } else if (tokenMethodName) { + if (txParams.data && tokenMethodName) { result = tokenMethodName - } else if (!to) { + } else if (txParams.data && !to) { result = DEPLOY_CONTRACT_ACTION_KEY - } else { + } + + let code + if (!result) { 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' + const codeIsEmpty = !code || code === '0x' || code === '0x0' result = codeIsEmpty ? SEND_ETHER_ACTION_KEY : CONTRACT_INTERACTION_KEY diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js index 9072dc684caf..642e1b6afd8b 100644 --- a/test/unit/app/controllers/transactions/tx-controller-test.js +++ b/test/unit/app/controllers/transactions/tx-controller-test.js @@ -622,6 +622,36 @@ describe('Transaction Controller', function () { }) assert.deepEqual(result, { transactionCategory: CONTRACT_INTERACTION_KEY, getCodeResponse: '0x0a' }) }) + + it('should return a contract interaction transactionCategory with the correct getCodeResponse when to is a contract address and data is falsey', 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: '', + }) + assert.deepEqual(result, { transactionCategory: CONTRACT_INTERACTION_KEY, getCodeResponse: '0x0a' }) + }) }) describe('#getPendingTransactions', function () { From 1a8344828192a1fff5d6d987fb5ecbc2c381f79b Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Fri, 4 Oct 2019 17:28:36 -0230 Subject: [PATCH 2/5] Gracefully fall back is send.util/estimateGas when blockGasLimit from background is falsy --- ui/app/pages/send/send.constants.js | 3 +++ ui/app/pages/send/send.utils.js | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/ui/app/pages/send/send.constants.js b/ui/app/pages/send/send.constants.js index d3fa38d10f6c..52ff823cc6c6 100644 --- a/ui/app/pages/send/send.constants.js +++ b/ui/app/pages/send/send.constants.js @@ -6,6 +6,8 @@ const MIN_GAS_PRICE_HEX = (parseInt(MIN_GAS_PRICE_DEC)).toString(16) const MIN_GAS_LIMIT_DEC = '21000' const MIN_GAS_LIMIT_HEX = (parseInt(MIN_GAS_LIMIT_DEC)).toString(16) +const ARBITRARY_HIGH_BLOCK_GAS_LIMIT = (parseInt('8000000')).toString(16) + const MIN_GAS_PRICE_GWEI = ethUtil.addHexPrefix(conversionUtil(MIN_GAS_PRICE_HEX, { fromDenomination: 'WEI', toDenomination: 'GWEI', @@ -58,4 +60,5 @@ module.exports = { SIMPLE_GAS_COST, TOKEN_TRANSFER_FUNCTION_SIGNATURE, BASE_TOKEN_GAS_COST, + ARBITRARY_HIGH_BLOCK_GAS_LIMIT, } diff --git a/ui/app/pages/send/send.utils.js b/ui/app/pages/send/send.utils.js index 32c293701295..ba287db40947 100644 --- a/ui/app/pages/send/send.utils.js +++ b/ui/app/pages/send/send.utils.js @@ -18,6 +18,7 @@ const { ONE_GWEI_IN_WEI_HEX, SIMPLE_GAS_COST, TOKEN_TRANSFER_FUNCTION_SIGNATURE, + ARBITRARY_HIGH_BLOCK_GAS_LIMIT, } = require('./send.constants') const abi = require('ethereumjs-abi') const ethUtil = require('ethereumjs-util') @@ -243,12 +244,21 @@ async function estimateGas ({ } // if not, fall back to block gasLimit + if (!blockGasLimit) { + try { + blockGasLimit = await this.query.getBlockByNumber('latest', false) + } catch (e) { + blockGasLimit = ARBITRARY_HIGH_BLOCK_GAS_LIMIT + } + } + paramsForGasEstimate.gas = ethUtil.addHexPrefix(multiplyCurrencies(blockGasLimit, 0.95, { multiplicandBase: 16, multiplierBase: 10, roundDown: '0', toNumericBase: 'hex', })) + // run tx return new Promise((resolve, reject) => { return estimateGasMethod(paramsForGasEstimate, (err, estimatedGas) => { From d8cbddbe0e61871bfcf2c20a9e73c05e1c86c323 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Fri, 4 Oct 2019 17:27:15 -0230 Subject: [PATCH 3/5] Update gas when pasting address in send --- ui/app/pages/send/send.component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/pages/send/send.component.js b/ui/app/pages/send/send.component.js index cb07dcb599db..d699e27a7104 100644 --- a/ui/app/pages/send/send.component.js +++ b/ui/app/pages/send/send.component.js @@ -304,7 +304,7 @@ export default class SendTransactionScreen extends PersistentForm { }} onChange={this.onRecipientInputChange} onValidAddressTyped={(address) => this.props.updateSendTo(address, '')} - onPaste={text => this.props.updateSendTo(text)} + onPaste={text => { this.props.updateSendTo(text) && this.updateGas() }} onReset={() => this.props.updateSendTo('', '')} updateEnsResolution={this.props.updateSendEnsResolution} updateEnsResolutionError={this.props.updateSendEnsResolutionError} From 202af788132361fafbd098af66ad7d766583f8e8 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Fri, 4 Oct 2019 19:57:50 -0230 Subject: [PATCH 4/5] Remove network request frontend fallback for blockGasLimit --- ui/app/pages/send/send.utils.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ui/app/pages/send/send.utils.js b/ui/app/pages/send/send.utils.js index ba287db40947..f4985e4a0653 100644 --- a/ui/app/pages/send/send.utils.js +++ b/ui/app/pages/send/send.utils.js @@ -245,11 +245,7 @@ async function estimateGas ({ // if not, fall back to block gasLimit if (!blockGasLimit) { - try { - blockGasLimit = await this.query.getBlockByNumber('latest', false) - } catch (e) { - blockGasLimit = ARBITRARY_HIGH_BLOCK_GAS_LIMIT - } + blockGasLimit = ARBITRARY_HIGH_BLOCK_GAS_LIMIT } paramsForGasEstimate.gas = ethUtil.addHexPrefix(multiplyCurrencies(blockGasLimit, 0.95, { From 11c5b15c7d8cb960daa38596a235aaab84b6a71b Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Fri, 4 Oct 2019 19:58:11 -0230 Subject: [PATCH 5/5] Add some needed slow downs to e2e tests --- test/e2e/metamask-ui.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/metamask-ui.spec.js b/test/e2e/metamask-ui.spec.js index 664cc4b6bed3..138967738afb 100644 --- a/test/e2e/metamask-ui.spec.js +++ b/test/e2e/metamask-ui.spec.js @@ -567,18 +567,18 @@ describe('MetaMask', function () { const send3eth = await findElement(driver, By.xpath(`//button[contains(text(), 'Send')]`), 10000) await send3eth.click() - await delay(regularDelayMs) + await delay(largeDelayMs * 2) const contractDeployment = await findElement(driver, By.xpath(`//button[contains(text(), 'Deploy Contract')]`), 10000) await contractDeployment.click() - await delay(regularDelayMs) + await delay(largeDelayMs * 2) await send3eth.click() await contractDeployment.click() - await delay(regularDelayMs) + await delay(largeDelayMs * 2) await driver.switchTo().window(extension) - await delay(regularDelayMs) + await delay(largeDelayMs * 2) let transactions = await findElements(driver, By.css('.transaction-list-item')) await transactions[3].click()