From 748801f4179d353959f40049cf6ca27851eebd0e Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Tue, 18 Jun 2019 09:47:14 -0230 Subject: 4byte fallback (#6551) * Adds 4byte registry fallback to getMethodData() (#6435) * Adds fetchWithCache to guard against unnecessary API calls * Add custom fetch wrapper with abort on timeout * Use opts and cacheRefreshTime in fetch-with-cache util * Use custom fetch wrapper with timeout for fetch-with-cache * Improve contract method data fetching (#6623) * Remove async call from getTransactionActionKey() * Stop blocking confirm screen rendering on method data loading, and base screen route on transactionCategory * Remove use of withMethodData, fix use of knownMethodData, in relation to transaction-list-item.component * Load data contract method data progressively, making it non-blocking; requires simplifying conf-tx-base lifecycle logic. * Allow editing of gas price while loading on the confirm screen. * Fix transactionAction component and its unit tests. * Fix confirm transaction components for cases of route transitions within metamask. * Only call toString on id if truthy in getNavigateTxData() * Fix knownMethodData retrieval and data fetching from fourbyte --- package-lock.json | 6 ++ package.json | 1 + test/helper.js | 1 + .../tests/transaction-action.component.test.js | 63 ++------------ .../transaction-action.component.js | 36 ++------ .../transaction-list-item.component.js | 10 ++- .../transaction-list-item.container.js | 14 ++- ui/app/ducks/app/app.js | 12 +++ .../confirm-transaction.duck.js | 28 +----- .../confirm-transaction.duck.test.js | 11 +-- ui/app/helpers/utils/fetch-with-cache.js | 28 ++++++ ui/app/helpers/utils/fetch.js | 25 ++++++ ui/app/helpers/utils/fetch.test.js | 54 ++++++++++++ ui/app/helpers/utils/transactions.util.js | 99 ++++++++++++---------- .../confirm-transaction-base.component.js | 11 ++- .../confirm-transaction-base.container.js | 19 +++-- .../confirm-transaction-switch.component.js | 23 ++--- .../confirm-transaction-switch.container.js | 31 ++++--- .../confirm-transaction.component.js | 74 +++++++--------- .../confirm-transaction.container.js | 31 +++++-- ui/app/selectors/custom-gas.js | 4 + ui/app/selectors/selectors.js | 15 +++- ui/app/store/actions.js | 42 +++++++++ 23 files changed, 376 insertions(+), 262 deletions(-) create mode 100644 ui/app/helpers/utils/fetch-with-cache.js create mode 100644 ui/app/helpers/utils/fetch.js create mode 100644 ui/app/helpers/utils/fetch.test.js diff --git a/package-lock.json b/package-lock.json index 9fe9a7623..73b748f1b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6233,6 +6233,12 @@ } } }, + "abortcontroller-polyfill": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/abortcontroller-polyfill/-/abortcontroller-polyfill-1.3.0.tgz", + "integrity": "sha512-lbWQgf+eRvku3va8poBlDBO12FigTQr9Zb7NIjXrePrhxWVKdCP2wbDl1tLDaYa18PWTom3UEWwdH13S46I+yA==", + "dev": true + }, "abstract-leveldown": { "version": "2.6.3", "resolved": "https://registry.npmjs.org/abstract-leveldown/-/abstract-leveldown-2.6.3.tgz", diff --git a/package.json b/package.json index 8d700c897..c9c3bd633 100644 --- a/package.json +++ b/package.json @@ -200,6 +200,7 @@ "@storybook/react": "^5.1.1", "addons-linter": "^1.10.0", "babel-core": "^6.26.3", + "abortcontroller-polyfill": "^1.3.0", "babel-eslint": "^8.0.0", "babel-plugin-transform-async-to-generator": "^6.24.1", "babel-plugin-transform-runtime": "^6.23.0", diff --git a/test/helper.js b/test/helper.js index 51f28de17..ddc2aba40 100644 --- a/test/helper.js +++ b/test/helper.js @@ -27,6 +27,7 @@ global.log = log // fetch global.fetch = require('isomorphic-fetch') +require('abortcontroller-polyfill/dist/polyfill-patch-fetch') // dom require('jsdom-global')() diff --git a/ui/app/components/app/transaction-action/tests/transaction-action.component.test.js b/ui/app/components/app/transaction-action/tests/transaction-action.component.test.js index b22a9db39..16f2e256f 100644 --- a/ui/app/components/app/transaction-action/tests/transaction-action.component.test.js +++ b/ui/app/components/app/transaction-action/tests/transaction-action.component.test.js @@ -18,33 +18,6 @@ describe('TransactionAction Component', () => { } }) - it('should render -- when methodData is still fetching', () => { - const methodData = { data: {}, done: false, error: null } - const transaction = { - id: 1, - status: 'confirmed', - submittedTime: 1534045442919, - time: 1534045440641, - txParams: { - from: '0xc5ae6383e126f901dcb06131d97a88745bfa88d6', - gas: '0x5208', - gasPrice: '0x3b9aca00', - nonce: '0x96', - to: '0x50a9d56c2b8ba9a5c7f2c08c3d26e0499f23a706', - value: '0x2386f26fc10000', - }, - } - - const wrapper = shallow(, { context: { t }}) - - assert.equal(wrapper.find('.transaction-action').length, 1) - assert.equal(wrapper.text(), '--') - }) - it('should render Sent Ether', () => { const methodData = { data: {}, done: true, error: null } const transaction = { @@ -75,15 +48,7 @@ describe('TransactionAction Component', () => { it('should render Approved', async () => { const methodData = { - data: { - name: 'Approve', - params: [ - { type: 'address' }, - { type: 'uint256' }, - ], - }, - done: true, - error: null, + name: 'Approve', } const transaction = { id: 1, @@ -99,6 +64,7 @@ describe('TransactionAction Component', () => { value: '0x2386f26fc10000', data: '0x095ea7b300000000000000000000000050a9d56c2b8ba9a5c7f2c08c3d26e0499f23a7060000000000000000000000000000000000000000000000000000000000000003', }, + transactionCategory: 'contractInteraction', } const wrapper = shallow( @@ -111,23 +77,12 @@ describe('TransactionAction Component', () => { ) assert.ok(wrapper) - assert.equal(wrapper.find('.test-class').length, 1) - await wrapper.instance().getTransactionAction() - assert.equal(wrapper.state('transactionAction'), 'approve') + assert.equal(wrapper.find('.transaction-action').length, 1) + assert.equal(wrapper.find('.transaction-action').text().trim(), 'Approve') }) - it('should render Accept Fulfillment', async () => { - const methodData = { - data: { - name: 'AcceptFulfillment', - params: [ - { type: 'address' }, - { type: 'uint256' }, - ], - }, - done: true, - error: null, - } + it('should render contractInteraction', async () => { + const methodData = {} const transaction = { id: 1, status: 'confirmed', @@ -142,6 +97,7 @@ describe('TransactionAction Component', () => { value: '0x2386f26fc10000', data: '0x095ea7b300000000000000000000000050a9d56c2b8ba9a5c7f2c08c3d26e0499f23a7060000000000000000000000000000000000000000000000000000000000000003', }, + transactionCategory: 'contractInteraction', } const wrapper = shallow( @@ -154,9 +110,8 @@ describe('TransactionAction Component', () => { ) assert.ok(wrapper) - assert.equal(wrapper.find('.test-class').length, 1) - await wrapper.instance().getTransactionAction() - assert.equal(wrapper.state('transactionAction'), ' Accept Fulfillment') + assert.equal(wrapper.find('.transaction-action').length, 1) + assert.equal(wrapper.find('.transaction-action').text().trim(), 'contractInteraction') }) }) }) diff --git a/ui/app/components/app/transaction-action/transaction-action.component.js b/ui/app/components/app/transaction-action/transaction-action.component.js index 4a5efdaae..26012ff7f 100644 --- a/ui/app/components/app/transaction-action/transaction-action.component.js +++ b/ui/app/components/app/transaction-action/transaction-action.component.js @@ -15,43 +15,23 @@ export default class TransactionAction extends PureComponent { methodData: PropTypes.object, } - state = { - transactionAction: '', - } - - componentDidMount () { - this.getTransactionAction() - } - - componentDidUpdate () { - this.getTransactionAction() - } - - async getTransactionAction () { - const { transactionAction } = this.state + getTransactionAction () { const { transaction, methodData } = this.props - const { data, done } = methodData - const { name = '' } = data - - if (!done || transactionAction) { - return - } + const { name } = methodData - const actionKey = await getTransactionActionKey(transaction, data) - const action = actionKey - ? this.context.t(actionKey) - : camelCaseToCapitalize(name) + const actionKey = getTransactionActionKey(transaction) + const action = actionKey && this.context.t(actionKey) + const methodName = name && camelCaseToCapitalize(name) - this.setState({ transactionAction: action }) + return methodName || action || '' } render () { - const { className, methodData: { done } } = this.props - const { transactionAction } = this.state + const { className } = this.props return (
- { (done && transactionAction) || '--' } + { this.getTransactionAction() }
) } diff --git a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js index 80b26469b..8bdb6a313 100644 --- a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js +++ b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js @@ -34,6 +34,8 @@ export default class TransactionListItem extends PureComponent { fetchBasicGasAndTimeEstimates: PropTypes.func, fetchGasEstimates: PropTypes.func, rpcPrefs: PropTypes.object, + data: PropTypes.string, + getContractMethodData: PropTypes.func, } static defaultProps = { @@ -150,6 +152,12 @@ export default class TransactionListItem extends PureComponent { ) } + componentDidMount () { + if (this.props.data) { + this.props.getContractMethodData(this.props.data) + } + } + render () { const { assetImages, @@ -214,7 +222,7 @@ export default class TransactionListItem extends PureComponent { { - const { metamask: { knownMethodData, accounts, provider, frequentRpcListDetail } } = state + const { metamask: { accounts, provider, frequentRpcListDetail } } = state const { showFiatInTestnets } = preferencesSelector(state) const isMainnet = getIsMainnet(state) const { transactionGroup: { primaryTransaction } = {} } = ownProps - const { txParams: { gas: gasLimit, gasPrice } = {} } = primaryTransaction + const { txParams: { gas: gasLimit, gasPrice, data } = {} } = primaryTransaction const selectedAccountBalance = accounts[getSelectedAddress(state)].balance const selectRpcInfo = frequentRpcListDetail.find(rpcInfo => rpcInfo.rpcUrl === provider.rpcTarget) const { rpcPrefs } = selectRpcInfo || {} @@ -38,7 +37,7 @@ const mapStateToProps = (state, ownProps) => { }) return { - knownMethodData, + methodData: getKnownMethodData(state, data) || {}, showFiat: (isMainnet || !!showFiatInTestnets), selectedAccountBalance, hasEnoughCancelGas, @@ -51,7 +50,7 @@ const mapDispatchToProps = dispatch => { fetchBasicGasAndTimeEstimates: () => dispatch(fetchBasicGasAndTimeEstimates()), fetchGasEstimates: (blockTime) => dispatch(fetchGasEstimates(blockTime)), setSelectedToken: tokenAddress => dispatch(setSelectedToken(tokenAddress)), - addKnownMethodData: (fourBytePrefix, methodData) => dispatch(addKnownMethodData(fourBytePrefix, methodData)), + getContractMethodData: methodData => dispatch(getContractMethodData(methodData)), retryTransaction: (transaction, gasPrice) => { dispatch(setCustomGasPriceForRetry(gasPrice || transaction.txParams.gasPrice)) dispatch(setCustomGasLimit(transaction.txParams.gas)) @@ -97,5 +96,4 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { export default compose( withRouter, connect(mapStateToProps, mapDispatchToProps, mergeProps), - withMethodData, )(TransactionListItem) diff --git a/ui/app/ducks/app/app.js b/ui/app/ducks/app/app.js index b181092c1..04c8c7422 100644 --- a/ui/app/ducks/app/app.js +++ b/ui/app/ducks/app/app.js @@ -79,6 +79,7 @@ function reduceApp (state, action) { lastSelectedProvider: null, networksTabSelectedRpcUrl: '', networksTabIsInAddMode: false, + loadingMethodData: false, }, state.appState) switch (action.type) { @@ -763,6 +764,17 @@ function reduceApp (state, action) { networksTabIsInAddMode: action.value, }) + case actions.LOADING_METHOD_DATA_STARTED: + return extend(appState, { + loadingMethodData: true, + }) + + case actions.LOADING_METHOD_DATA_FINISHED: + return extend(appState, { + loadingMethodData: false, + }) + + default: return appState } diff --git a/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js b/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js index 58b0ec8e8..b8d1a7e81 100644 --- a/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js +++ b/ui/app/ducks/confirm-transaction/confirm-transaction.duck.js @@ -1,4 +1,3 @@ -import log from 'loglevel' import { conversionRateSelector, currentCurrencySelector, @@ -18,12 +17,9 @@ import { import { getTokenData, - getMethodData, - isSmartContractAddress, sumHexes, } from '../../helpers/utils/transactions.util' -import { getSymbolAndDecimals } from '../../helpers/utils/token-util' import { conversionUtil } from '../../helpers/utils/conversion-util' import { addHexPrefix } from 'ethereumjs-util' @@ -348,7 +344,7 @@ export function updateTxDataAndCalculate (txData) { } export function setTransactionToConfirm (transactionId) { - return async (dispatch, getState) => { + return (dispatch, getState) => { const state = getState() const unconfirmedTransactionsHash = unconfirmedTransactionsHashSelector(state) const transaction = unconfirmedTransactionsHash[transactionId] @@ -364,34 +360,14 @@ export function setTransactionToConfirm (transactionId) { dispatch(updateTxDataAndCalculate(txData)) const { txParams } = transaction - const { to } = txParams if (txParams.data) { - const { tokens: existingTokens } = state - const { data, to: tokenAddress } = txParams - - dispatch(setFetchingData(true)) - const methodData = await getMethodData(data) - dispatch(updateMethodData(methodData)) + const { data } = txParams - try { - const toSmartContract = await isSmartContractAddress(to || '') - dispatch(updateToSmartContract(toSmartContract)) - } catch (error) { - log.error(error) - } - dispatch(setFetchingData(false)) const tokenData = getTokenData(data) dispatch(updateTokenData(tokenData)) - try { - const tokenSymbolData = await getSymbolAndDecimals(tokenAddress, existingTokens) || {} - const { symbol: tokenSymbol = '', decimals: tokenDecimals = '' } = tokenSymbolData - dispatch(updateTokenProps({ tokenSymbol, tokenDecimals })) - } catch (error) { - dispatch(updateTokenProps({ tokenSymbol: '', tokenDecimals: '' })) - } } if (txParams.nonce) { diff --git a/ui/app/ducks/confirm-transaction/confirm-transaction.duck.test.js b/ui/app/ducks/confirm-transaction/confirm-transaction.duck.test.js index d2e344663..9e26314e5 100644 --- a/ui/app/ducks/confirm-transaction/confirm-transaction.duck.test.js +++ b/ui/app/ducks/confirm-transaction/confirm-transaction.duck.test.js @@ -630,7 +630,7 @@ describe('Confirm Transaction Duck', () => { storeActions.forEach((action, index) => assert.equal(action.type, expectedActions[index])) }) - it('updates confirmTransaction transaction', done => { + it('updates confirmTransaction transaction', () => { const mockState = { metamask: { conversionRate: 468.58, @@ -673,13 +673,10 @@ describe('Confirm Transaction Duck', () => { ] store.dispatch(actions.setTransactionToConfirm(2603411941761054)) - .then(() => { - const storeActions = store.getActions() - assert.equal(storeActions.length, expectedActions.length) + const storeActions = store.getActions() + assert.equal(storeActions.length, expectedActions.length) - storeActions.forEach((action, index) => assert.equal(action.type, expectedActions[index])) - done() - }) + storeActions.forEach((action, index) => assert.equal(action.type, expectedActions[index])) }) }) }) diff --git a/ui/app/helpers/utils/fetch-with-cache.js b/ui/app/helpers/utils/fetch-with-cache.js new file mode 100644 index 000000000..ac641c3c4 --- /dev/null +++ b/ui/app/helpers/utils/fetch-with-cache.js @@ -0,0 +1,28 @@ +import { + loadLocalStorageData, + saveLocalStorageData, +} from '../../../lib/local-storage-helpers' +import http from './fetch' + +const fetch = http({ + timeout: 30000, +}) + +export default function fetchWithCache (url, opts, cacheRefreshTime = 360000) { + const currentTime = Date.now() + const cachedFetch = loadLocalStorageData('cachedFetch') || {} + const { cachedUrl, cachedTime } = cachedFetch[url] || {} + if (cachedUrl && currentTime - cachedTime < cacheRefreshTime) { + return cachedFetch[url] + } else { + cachedFetch[url] = { cachedUrl: url, cachedTime: currentTime } + saveLocalStorageData(cachedFetch, 'cachedFetch') + return fetch(url, { + referrerPolicy: 'no-referrer-when-downgrade', + body: null, + method: 'GET', + mode: 'cors', + ...opts, + }) + } +} diff --git a/ui/app/helpers/utils/fetch.js b/ui/app/helpers/utils/fetch.js new file mode 100644 index 000000000..7bb483818 --- /dev/null +++ b/ui/app/helpers/utils/fetch.js @@ -0,0 +1,25 @@ +/* global AbortController */ + +export default function ({ timeout = 120000 } = {}) { + return function _fetch (url, opts) { + return new Promise(async (resolve, reject) => { + const abortController = new AbortController() + const abortSignal = abortController.signal + const f = fetch(url, { + ...opts, + signal: abortSignal, + }) + + const timer = setTimeout(() => abortController.abort(), timeout) + + try { + const res = await f + clearTimeout(timer) + return resolve(res) + } catch (e) { + clearTimeout(timer) + return reject(e) + } + }) + } +} diff --git a/ui/app/helpers/utils/fetch.test.js b/ui/app/helpers/utils/fetch.test.js new file mode 100644 index 000000000..12724525a --- /dev/null +++ b/ui/app/helpers/utils/fetch.test.js @@ -0,0 +1,54 @@ +import assert from 'assert' +import nock from 'nock' + +import http from './fetch' + +describe('custom fetch fn', () => { + it('fetches a url', async () => { + nock('https://api.infura.io') + .get('/money') + .reply(200, '{"hodl": false}') + + const fetch = http() + const response = await (await fetch('https://api.infura.io/money')).json() + assert.deepEqual(response, { + hodl: false, + }) + }) + + it('throws when the request hits a custom timeout', async () => { + nock('https://api.infura.io') + .get('/moon') + .delay(2000) + .reply(200, '{"moon": "2012-12-21T11:11:11Z"}') + + const fetch = http({ + timeout: 123, + }) + + try { + await fetch('https://api.infura.io/moon').then(r => r.json()) + assert.fail('Request should throw') + } catch (e) { + assert.ok(e) + } + }) + + it('should abort the request when the custom timeout is hit', async () => { + nock('https://api.infura.io') + .get('/moon') + .delay(2000) + .reply(200, '{"moon": "2012-12-21T11:11:11Z"}') + + const fetch = http({ + timeout: 123, + }) + + try { + await fetch('https://api.infura.io/moon').then(r => r.json()) + assert.fail('Request should be aborted') + } catch (e) { + assert.deepEqual(e.message, 'Aborted') + } + }) +}) diff --git a/ui/app/helpers/utils/transactions.util.js b/ui/app/helpers/utils/transactions.util.js index 99ccc3478..c84053ec7 100644 --- a/ui/app/helpers/utils/transactions.util.js +++ b/ui/app/helpers/utils/transactions.util.js @@ -7,7 +7,7 @@ import { TRANSACTION_STATUS_CONFIRMED, } from '../../../../app/scripts/controllers/transactions/enums' import prefixForNetwork from '../../../lib/etherscan-prefix-for-network' - +import fetchWithCache from './fetch-with-cache' import { TOKEN_METHOD_TRANSFER, @@ -32,39 +32,57 @@ export function getTokenData (data = '') { return abiDecoder.decodeMethod(data) } +async function getMethodFrom4Byte (fourBytePrefix) { + const fourByteResponse = (await fetchWithCache(`https://www.4byte.directory/api/v1/signatures/?hex_signature=${fourBytePrefix}`, { + referrerPolicy: 'no-referrer-when-downgrade', + body: null, + method: 'GET', + mode: 'cors', + })) + + const fourByteJSON = await fourByteResponse.json() + + if (fourByteJSON.count === 1) { + return fourByteJSON.results[0].text_signature + } else { + return null + } +} + const registry = new MethodRegistry({ provider: global.ethereumProvider }) /** - * Attempts to return the method data from the MethodRegistry library, if the method exists in the - * registry. Otherwise, returns an empty object. - * @param {string} data - The hex data (@code txParams.data) of a transaction + * Attempts to return the method data from the MethodRegistry library, the message registry library and the token abi, in that order of preference + * @param {string} fourBytePrefix - The prefix from the method code associated with the data * @returns {Object} */ - export async function getMethodData (data = '') { - const prefixedData = ethUtil.addHexPrefix(data) - const fourBytePrefix = prefixedData.slice(0, 10) - - try { - const sig = await registry.lookup(fourBytePrefix) - - if (!sig) { - return {} - } - - const parsedResult = registry.parse(sig) - - return { - name: parsedResult.name, - params: parsedResult.args, - } - } catch (error) { - log.error(error) - const contractData = getTokenData(data) - const { name } = contractData || {} - return { name } +export async function getMethodDataAsync (fourBytePrefix) { + try { + const fourByteSig = getMethodFrom4Byte(fourBytePrefix).catch((e) => { + log.error(e) + return null + }) + + let sig = await registry.lookup(fourBytePrefix) + + if (!sig) { + sig = await fourByteSig } + if (!sig) { + return {} + } + + const parsedResult = registry.parse(sig) + return { + name: parsedResult.name, + params: parsedResult.args, + } + } catch (error) { + log.error(error) + return {} + } } export function isConfirmDeployContract (txData = {}) { @@ -87,11 +105,10 @@ export function getFourBytePrefix (data = '') { /** * Returns the action of a transaction as a key to be passed into the translator. * @param {Object} transaction - txData object - * @param {Object} methodData - Data returned from eth-method-registry * @returns {string|undefined} */ -export async function getTransactionActionKey (transaction, methodData) { - const { txParams: { data, to } = {}, msgParams, type } = transaction +export function getTransactionActionKey (transaction) { + const { msgParams, type, transactionCategory } = transaction if (type === 'cancel') { return CANCEL_ATTEMPT_ACTION_KEY @@ -105,27 +122,23 @@ export async function getTransactionActionKey (transaction, methodData) { return DEPLOY_CONTRACT_ACTION_KEY } - if (data) { - const toSmartContract = await isSmartContractAddress(to) - - if (!toSmartContract) { - return SEND_ETHER_ACTION_KEY - } - - const { name } = methodData - const methodName = name && name.toLowerCase() - - if (!methodName) { - return CONTRACT_INTERACTION_KEY - } + const isTokenAction = [ + TOKEN_METHOD_TRANSFER, + TOKEN_METHOD_APPROVE, + TOKEN_METHOD_TRANSFER_FROM, + ].find(actionName => actionName === transactionCategory) + const isNonTokenSmartContract = transactionCategory === CONTRACT_INTERACTION_KEY - switch (methodName) { + if (isTokenAction || isNonTokenSmartContract) { + switch (transactionCategory) { case TOKEN_METHOD_TRANSFER: return SEND_TOKEN_ACTION_KEY case TOKEN_METHOD_APPROVE: return APPROVE_ACTION_KEY case TOKEN_METHOD_TRANSFER_FROM: return TRANSFER_FROM_ACTION_KEY + case CONTRACT_INTERACTION_KEY: + return CONTRACT_INTERACTION_KEY default: return undefined } diff --git a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js index c6a05cf0f..5c46c8449 100644 --- a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -94,6 +94,7 @@ export default class ConfirmTransactionBase extends Component { advancedInlineGasShown: PropTypes.bool, insufficientBalance: PropTypes.bool, hideFiatConversion: PropTypes.bool, + transactionCategory: PropTypes.string, } state = { @@ -268,6 +269,7 @@ export default class ConfirmTransactionBase extends Component { } = {}, hideData, dataComponent, + transactionCategory, } = this.props if (hideData) { @@ -279,7 +281,7 @@ export default class ConfirmTransactionBase extends Component {
{`${t('functionType')}:`} - { name || t('notFound') } + { getMethodName(name) || this.context.tOrKey(transactionCategory) || this.context.t('contractInteraction') }
{ @@ -464,6 +466,7 @@ export default class ConfirmTransactionBase extends Component { handleNextTx (txId) { const { history, clearConfirmTransaction } = this.props + if (txId) { clearConfirmTransaction() history.push(`${CONFIRM_TRANSACTION_ROUTE}/${txId}`) @@ -473,7 +476,7 @@ export default class ConfirmTransactionBase extends Component { getNavigateTxData () { const { currentNetworkUnapprovedTxs, txData: { id } = {} } = this.props const enumUnapprovedTxs = Object.keys(currentNetworkUnapprovedTxs).reverse() - const currentPosition = enumUnapprovedTxs.indexOf(id.toString()) + const currentPosition = enumUnapprovedTxs.indexOf(id ? id.toString() : '') return { totalTx: enumUnapprovedTxs.length, @@ -530,7 +533,6 @@ export default class ConfirmTransactionBase extends Component { valid: propsValid = true, errorMessage, errorKey: propsErrorKey, - actionKey, title, subtitle, hideSubtitle, @@ -542,6 +544,7 @@ export default class ConfirmTransactionBase extends Component { assetImage, warning, unapprovedTxCount, + transactionCategory, } = this.props const { submitting, submitError } = this.state @@ -557,7 +560,7 @@ export default class ConfirmTransactionBase extends Component { toAddress={toAddress} showEdit={onEdit && !isTxReprice} // In the event that the key is falsy (and inherently invalid), use a fallback string - action={this.context.tOrKey(actionKey) || getMethodName(name) || this.context.t('contractInteraction')} + action={getMethodName(name) || this.context.tOrKey(transactionCategory) || this.context.t('contractInteraction')} title={title} titleComponent={this.renderTitleComponent()} subtitle={subtitle} diff --git a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js index 2b087f5cc..6b73b58f0 100644 --- a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -18,7 +18,7 @@ import { isBalanceSufficient, calcGasTotal } from '../send/send.utils' import { conversionGreaterThan } from '../../helpers/utils/conversion-util' import { MIN_GAS_LIMIT_DEC } from '../send/send.constants' import { checksumAddress, addressSlicer, valuesFor } from '../../helpers/utils/util' -import {getMetaMaskAccounts, getAdvancedInlineGasShown, preferencesSelector, getIsMainnet} from '../../selectors/selectors' +import { getMetaMaskAccounts, getAdvancedInlineGasShown, preferencesSelector, getIsMainnet, getKnownMethodData } from '../../selectors/selectors' const casedContractMap = Object.keys(contractMap).reduce((acc, base) => { return { @@ -27,8 +27,9 @@ const casedContractMap = Object.keys(contractMap).reduce((acc, base) => { } }, {}) -const mapStateToProps = (state, props) => { - const { toAddress: propsToAddress } = props +const mapStateToProps = (state, ownProps) => { + const { toAddress: propsToAddress, match: { params = {} } } = ownProps + const { id: paramsTransactionId } = params const { showFiatInTestnets } = preferencesSelector(state) const isMainnet = getIsMainnet(state) const { confirmTransaction, metamask, gas } = state @@ -43,18 +44,18 @@ const mapStateToProps = (state, props) => { hexTransactionFee, hexTransactionTotal, tokenData, - methodData, txData, tokenProps, nonce, } = confirmTransaction - const { txParams = {}, lastGasPrice, id: transactionId } = txData + const { txParams = {}, lastGasPrice, id: transactionId, transactionCategory } = txData const { from: fromAddress, to: txParamsToAddress, gasPrice, gas: gasLimit, value: amount, + data, } = txParams const accounts = getMetaMaskAccounts(state) const { @@ -87,8 +88,7 @@ const mapStateToProps = (state, props) => { ) const isTxReprice = Boolean(lastGasPrice) - - const transaction = R.find(({ id }) => id === transactionId)(selectedAddressTxList) + const transaction = R.find(({ id }) => id === (transactionId || Number(paramsTransactionId)))(selectedAddressTxList) const transactionStatus = transaction ? transaction.status : '' const currentNetworkUnapprovedTxs = R.filter( @@ -104,6 +104,8 @@ const mapStateToProps = (state, props) => { conversionRate, }) + const methodData = getKnownMethodData(state, data) || {} + return { balance, fromAddress, @@ -119,7 +121,7 @@ const mapStateToProps = (state, props) => { hexTransactionAmount, hexTransactionFee, hexTransactionTotal, - txData, + txData: Object.keys(txData).length ? txData : transaction || {}, tokenData, methodData, tokenProps, @@ -141,6 +143,7 @@ const mapStateToProps = (state, props) => { hideSubtitle: (!isMainnet && !showFiatInTestnets), hideFiatConversion: (!isMainnet && !showFiatInTestnets), metaMetricsSendCount, + transactionCategory, } } diff --git a/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.component.js b/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.component.js index 25f2402f1..fc0606365 100644 --- a/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.component.js +++ b/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.component.js @@ -12,18 +12,17 @@ import { CONFIRM_TOKEN_METHOD_PATH, SIGNATURE_REQUEST_PATH, } from '../../helpers/constants/routes' -import { isConfirmDeployContract } from '../../helpers/utils/transactions.util' import { TOKEN_METHOD_TRANSFER, TOKEN_METHOD_APPROVE, TOKEN_METHOD_TRANSFER_FROM, + DEPLOY_CONTRACT_ACTION_KEY, + SEND_ETHER_ACTION_KEY, } from '../../helpers/constants/transactions' export default class ConfirmTransactionSwitch extends Component { static propTypes = { txData: PropTypes.object, - methodData: PropTypes.object, - fetchingData: PropTypes.bool, isEtherTransaction: PropTypes.bool, isTokenMethod: PropTypes.bool, } @@ -31,31 +30,21 @@ export default class ConfirmTransactionSwitch extends Component { redirectToTransaction () { const { txData, - methodData: { name }, - fetchingData, - isEtherTransaction, - isTokenMethod, } = this.props - const { id, txParams: { data } = {} } = txData + const { id, txParams: { data } = {}, transactionCategory } = txData - if (fetchingData) { - return - } - - if (isConfirmDeployContract(txData)) { + if (transactionCategory === DEPLOY_CONTRACT_ACTION_KEY) { const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_DEPLOY_CONTRACT_PATH}` return } - if (isEtherTransaction && !isTokenMethod) { + if (transactionCategory === SEND_ETHER_ACTION_KEY) { const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_SEND_ETHER_PATH}` return } if (data) { - const methodName = name && name.toLowerCase() - - switch (methodName) { + switch (transactionCategory) { case TOKEN_METHOD_TRANSFER: { const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_SEND_TOKEN_PATH}` return diff --git a/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.container.js b/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.container.js index 8213f0964..230a931ad 100644 --- a/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.container.js +++ b/ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.container.js @@ -4,24 +4,27 @@ import { TOKEN_METHOD_TRANSFER, TOKEN_METHOD_APPROVE, TOKEN_METHOD_TRANSFER_FROM, + SEND_ETHER_ACTION_KEY, } from '../../helpers/constants/transactions' +import { unconfirmedTransactionsListSelector } from '../../selectors/confirm-transaction' -const mapStateToProps = state => { - const { - confirmTransaction: { - txData, - methodData, - fetchingData, - toSmartContract, - }, - } = state +const mapStateToProps = (state, ownProps) => { + const { metamask: { unapprovedTxs } } = state + const { match: { params = {}, url } } = ownProps + const urlId = url && url.match(/\d+/) && url.match(/\d+/)[0] + const { id: paramsId } = params + const transactionId = paramsId || urlId + + const unconfirmedTransactions = unconfirmedTransactionsListSelector(state) + const totalUnconfirmed = unconfirmedTransactions.length + const transaction = totalUnconfirmed + ? unapprovedTxs[transactionId] || unconfirmedTransactions[totalUnconfirmed - 1] + : {} return { - txData, - methodData, - fetchingData, - isEtherTransaction: !toSmartContract, - isTokenMethod: [TOKEN_METHOD_APPROVE, TOKEN_METHOD_TRANSFER, TOKEN_METHOD_TRANSFER_FROM].includes(methodData.name && methodData.name.toLowerCase()), + txData: transaction, + isEtherTransaction: transaction && transaction.transactionCategory === SEND_ETHER_ACTION_KEY, + isTokenMethod: [TOKEN_METHOD_APPROVE, TOKEN_METHOD_TRANSFER, TOKEN_METHOD_TRANSFER_FROM].includes(transaction && transaction.transactionCategory && transaction.transactionCategory.toLowerCase()), } } diff --git a/ui/app/pages/confirm-transaction/confirm-transaction.component.js b/ui/app/pages/confirm-transaction/confirm-transaction.component.js index 35b8dc5aa..cca86fa9b 100644 --- a/ui/app/pages/confirm-transaction/confirm-transaction.component.js +++ b/ui/app/pages/confirm-transaction/confirm-transaction.component.js @@ -33,6 +33,10 @@ export default class ConfirmTransaction extends Component { confirmTransaction: PropTypes.object, clearConfirmTransaction: PropTypes.func, fetchBasicGasAndTimeEstimates: PropTypes.func, + transaction: PropTypes.object, + getContractMethodData: PropTypes.func, + transactionId: PropTypes.string, + paramsTransactionId: PropTypes.string, } getParamsTransactionId () { @@ -45,8 +49,11 @@ export default class ConfirmTransaction extends Component { totalUnapprovedCount = 0, send = {}, history, - confirmTransaction: { txData: { id: transactionId } = {} }, + transaction: { txParams: { data } = {} } = {}, fetchBasicGasAndTimeEstimates, + getContractMethodData, + transactionId, + paramsTransactionId, } = this.props if (!totalUnapprovedCount && !send.to) { @@ -54,67 +61,44 @@ export default class ConfirmTransaction extends Component { return } - if (!transactionId) { - fetchBasicGasAndTimeEstimates() - this.setTransactionToConfirm() - } + fetchBasicGasAndTimeEstimates() + getContractMethodData(data) + this.props.setTransactionToConfirm(transactionId || paramsTransactionId) } - componentDidUpdate () { + componentDidUpdate (prevProps) { const { setTransactionToConfirm, - confirmTransaction: { txData: { id: transactionId } = {} }, + transaction: { txData: { txParams: { data } = {} } = {} }, clearConfirmTransaction, + getContractMethodData, + paramsTransactionId, + transactionId, + history, + totalUnapprovedCount, } = this.props - const paramsTransactionId = this.getParamsTransactionId() - if (paramsTransactionId && transactionId && paramsTransactionId !== transactionId + '') { + if (paramsTransactionId && transactionId && prevProps.paramsTransactionId !== paramsTransactionId) { clearConfirmTransaction() + getContractMethodData(data) setTransactionToConfirm(paramsTransactionId) return - } - - if (!transactionId) { - this.setTransactionToConfirm() - } - } - - setTransactionToConfirm () { - const { - history, - unconfirmedTransactions, - setTransactionToConfirm, - } = this.props - const paramsTransactionId = this.getParamsTransactionId() - - if (paramsTransactionId) { - // Check to make sure params ID is valid - const tx = unconfirmedTransactions.find(({ id }) => id + '' === paramsTransactionId) - - if (!tx) { - history.replace(DEFAULT_ROUTE) - } else { - setTransactionToConfirm(paramsTransactionId) - } - } else if (unconfirmedTransactions.length) { - const totalUnconfirmed = unconfirmedTransactions.length - const transaction = unconfirmedTransactions[totalUnconfirmed - 1] - const { id: transactionId, loadingDefaults } = transaction - - if (!loadingDefaults) { - setTransactionToConfirm(transactionId) - } + } else if (prevProps.transactionId && !transactionId && !totalUnapprovedCount) { + history.replace(DEFAULT_ROUTE) + return + } else if (prevProps.transactionId && transactionId && prevProps.transactionId !== transactionId) { + history.replace(DEFAULT_ROUTE) + return } } render () { - const { confirmTransaction: { txData: { id } } = {} } = this.props - const paramsTransactionId = this.getParamsTransactionId() - + const { transactionId, paramsTransactionId } = this.props // Show routes when state.confirmTransaction has been set and when either the ID in the params // isn't specified or is specified and matches the ID in state.confirmTransaction in order to // support URLs of /confirm-transaction or /confirm-transaction/ - return id && (!paramsTransactionId || paramsTransactionId === id + '') + + return transactionId && (!paramsTransactionId || paramsTransactionId === transactionId) ? ( { - const { metamask: { send }, confirmTransaction } = state +const mapStateToProps = (state, ownProps) => { + const { metamask: { send, unapprovedTxs }, confirmTransaction } = state + const { match: { params = {} } } = ownProps + const { id } = params + + const unconfirmedTransactions = unconfirmedTransactionsListSelector(state) + const totalUnconfirmed = unconfirmedTransactions.length + const transaction = totalUnconfirmed + ? unapprovedTxs[id] || unconfirmedTransactions[totalUnconfirmed - 1] + : {} return { - totalUnapprovedCount: getTotalUnapprovedCount(state), + totalUnapprovedCount: totalUnconfirmed, send, confirmTransaction, - unconfirmedTransactions: unconfirmedTransactionsListSelector(state), + unapprovedTxs, + id, + paramsTransactionId: id && String(id), + transactionId: transaction.id && String(transaction.id), + unconfirmedTransactions, + transaction, } } const mapDispatchToProps = dispatch => { return { - setTransactionToConfirm: transactionId => dispatch(setTransactionToConfirm(transactionId)), + setTransactionToConfirm: transactionId => { + dispatch(setTransactionToConfirm(transactionId)) + }, clearConfirmTransaction: () => dispatch(clearConfirmTransaction()), fetchBasicGasAndTimeEstimates: () => dispatch(fetchBasicGasAndTimeEstimates()), + getContractMethodData: (data) => dispatch(getContractMethodData(data)), } } diff --git a/ui/app/selectors/custom-gas.js b/ui/app/selectors/custom-gas.js index 5ba786f0f..1f7ee8f9f 100644 --- a/ui/app/selectors/custom-gas.js +++ b/ui/app/selectors/custom-gas.js @@ -119,6 +119,10 @@ function isCustomPriceSafe (state) { return true } + if (safeLow === null) { + return null + } + const customPriceSafe = conversionGreaterThan( { value: customGasPrice, diff --git a/ui/app/selectors/selectors.js b/ui/app/selectors/selectors.js index c7cb80024..56591b7b0 100644 --- a/ui/app/selectors/selectors.js +++ b/ui/app/selectors/selectors.js @@ -1,5 +1,6 @@ import { NETWORK_TYPES } from '../helpers/constants/common' -import { stripHexPrefix } from 'ethereumjs-util' +import { stripHexPrefix, addHexPrefix } from 'ethereumjs-util' + const abi = require('human-standard-token-abi') import { @@ -50,6 +51,7 @@ const selectors = { isEthereumNetwork, getMetaMetricState, getRpcPrefsForCurrentProvider, + getKnownMethodData, } module.exports = selectors @@ -335,3 +337,14 @@ function getRpcPrefsForCurrentProvider (state) { const { rpcPrefs = {} } = selectRpcInfo || {} return rpcPrefs } + +function getKnownMethodData (state, data) { + if (!data) { + return null + } + const prefixedData = addHexPrefix(data) + const fourBytePrefix = prefixedData.slice(0, 10) + const { knownMethodData } = state.metamask + + return knownMethodData && knownMethodData[fourBytePrefix] +} diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 7f6cbea1f..aff2636ba 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -8,6 +8,7 @@ const { } = require('../pages/send/send.utils') const ethUtil = require('ethereumjs-util') const { fetchLocale } = require('../helpers/utils/i18n-helper') +const { getMethodDataAsync } = require('../helpers/utils/transactions.util') const log = require('loglevel') const { ENVIRONMENT_TYPE_NOTIFICATION } = require('../../../app/scripts/lib/enums') const { hasUnconfirmedTransactions } = require('../helpers/utils/confirm-tx.util') @@ -360,6 +361,12 @@ var actions = { // AppStateController-related actions SET_LAST_ACTIVE_TIME: 'SET_LAST_ACTIVE_TIME', setLastActiveTime, + + getContractMethodData, + loadingMethoDataStarted, + loadingMethoDataFinished, + LOADING_METHOD_DATA_STARTED: 'LOADING_METHOD_DATA_STARTED', + LOADING_METHOD_DATA_FINISHED: 'LOADING_METHOD_DATA_FINISHED', } module.exports = actions @@ -2774,3 +2781,38 @@ function setLastActiveTime () { }) } } + +function loadingMethoDataStarted () { + return { + type: actions.LOADING_METHOD_DATA_STARTED, + } +} + +function loadingMethoDataFinished () { + return { + type: actions.LOADING_METHOD_DATA_FINISHED, + } +} + +function getContractMethodData (data = '') { + return (dispatch, getState) => { + const prefixedData = ethUtil.addHexPrefix(data) + const fourBytePrefix = prefixedData.slice(0, 10) + const { knownMethodData } = getState().metamask + if (knownMethodData && knownMethodData[fourBytePrefix]) { + return Promise.resolve(knownMethodData[fourBytePrefix]) + } + + dispatch(actions.loadingMethoDataStarted()) + log.debug(`loadingMethodData`) + + return getMethodDataAsync(fourBytePrefix) + .then(({ name, params }) => { + dispatch(actions.loadingMethoDataFinished()) + + background.addKnownMethodData(fourBytePrefix, { name, params }) + + return { name, params } + }) + } +} -- cgit v1.2.3 From a37a5acbe1d65571363c10017b24fe0b07ff0398 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 25 Jun 2019 12:42:35 -0700 Subject: Add simulation failure to tx confirmation when transaction simulationFails --- .../confirm-transaction-base/confirm-transaction-base.container.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js index 6b73b58f0..e769d8974 100644 --- a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -91,6 +91,10 @@ const mapStateToProps = (state, ownProps) => { const transaction = R.find(({ id }) => id === (transactionId || Number(paramsTransactionId)))(selectedAddressTxList) const transactionStatus = transaction ? transaction.status : '' + if (transaction && transaction.simulationFails) { + txData.simulationFails = transaction.simulationFails + } + const currentNetworkUnapprovedTxs = R.filter( ({ metamaskNetworkId }) => metamaskNetworkId === network, unapprovedTxs, -- cgit v1.2.3 From 84c28896a677c7a9b6ea006e0ff45caff006fba3 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 25 Jun 2019 12:43:38 -0700 Subject: Version 6.7.0 --- CHANGELOG.md | 9 +++++++++ app/manifest.json | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f4377e14..24af6cb4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ # Changelog ## Current Develop Branch + +## 6.7.0 Tue Jun 18 2019 + +- [#6623](https://github.com/MetaMask/metamask-extension/pull/6623): Improve contract method data fetching (#6623) +- [#6551](https://github.com/MetaMask/metamask-extension/pull/6551): Adds 4byte registry fallback to getMethodData() (#6435) +- [#6718, #6650](https://github.com/MetaMask/metamask-extension/pull/6718, #6650): Add delete to custom RPC form +- [#6700](https://github.com/MetaMask/metamask-extension/pull/6700): Fix styles on 'import account' page, update help link +- [#6714](https://github.com/MetaMask/metamask-extension/pull/6714): Wrap smaller custom block explorer url text +- [#6706](https://github.com/MetaMask/metamask-extension/pull/6706): Pin ethereumjs-tx - [#6700](https://github.com/MetaMask/metamask-extension/pull/6700): Fix styles on 'import account' page, update help link ## 6.6.2 Fri Jun 07 2019 diff --git a/app/manifest.json b/app/manifest.json index 6407c8c1c..27741869e 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "__MSG_appName__", "short_name": "__MSG_appName__", - "version": "6.6.2", + "version": "6.7.0", "manifest_version": 2, "author": "https://metamask.io", "description": "__MSG_appDescription__", -- cgit v1.2.3