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 --- .../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 +++++++-- 6 files changed, 95 insertions(+), 94 deletions(-) (limited to 'ui/app/pages') 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)), } } -- cgit