Skip to content

Commit

Permalink
Ensure correct tx category when sending to contracts without tx data (#…
Browse files Browse the repository at this point in the history
…7252)

* Ensure correct transaction category when sending to contracts but there is no txParams data

* Update gas when pasting address in send

* Gracefully fall back is send.util/estimateGas when blockGasLimit from background is falsy

* Remove network request frontend fallback for blockGasLimit

* Add some needed slow downs to e2e tests
  • Loading branch information
danjm authored and danfinlay committed Oct 7, 2019
1 parent d7a4dfe commit 38df647
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 12 deletions.
14 changes: 7 additions & 7 deletions app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/metamask-ui.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
30 changes: 30 additions & 0 deletions test/unit/app/controllers/transactions/tx-controller-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion ui/app/pages/send/send.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
3 changes: 3 additions & 0 deletions ui/app/pages/send/send.constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -58,4 +60,5 @@ module.exports = {
SIMPLE_GAS_COST,
TOKEN_TRANSFER_FUNCTION_SIGNATURE,
BASE_TOKEN_GAS_COST,
ARBITRARY_HIGH_BLOCK_GAS_LIMIT,
}
6 changes: 6 additions & 0 deletions ui/app/pages/send/send.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -243,12 +244,17 @@ async function estimateGas ({
}

// if not, fall back to block gasLimit
if (!blockGasLimit) {
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) => {
Expand Down

0 comments on commit 38df647

Please sign in to comment.