Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve contract method data fetching #6623

Merged
merged 8 commits into from
May 28, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -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(<TransactionAction
methodData={methodData}
transaction={transaction}
className="transaction-action"
/>, { 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 = {
Expand Down Expand Up @@ -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,
Expand All @@ -99,6 +64,7 @@ describe('TransactionAction Component', () => {
value: '0x2386f26fc10000',
data: '0x095ea7b300000000000000000000000050a9d56c2b8ba9a5c7f2c08c3d26e0499f23a7060000000000000000000000000000000000000000000000000000000000000003',
},
transactionCategory: 'contractInteraction',
}

const wrapper = shallow(
Expand All @@ -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',
Expand All @@ -142,6 +97,7 @@ describe('TransactionAction Component', () => {
value: '0x2386f26fc10000',
data: '0x095ea7b300000000000000000000000050a9d56c2b8ba9a5c7f2c08c3d26e0499f23a7060000000000000000000000000000000000000000000000000000000000000003',
},
transactionCategory: 'contractInteraction',
}

const wrapper = shallow(
Expand All @@ -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')
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<div className={classnames('transaction-action', className)}>
{ (done && transactionAction) || '--' }
{ this.getTransactionAction() }
</div>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -150,6 +152,12 @@ export default class TransactionListItem extends PureComponent {
)
}

componentDidMount () {
if (this.props.data) {
this.props.getContractMethodData(this.props.data)
}
}

render () {
const {
assetImages,
Expand Down Expand Up @@ -214,7 +222,7 @@ export default class TransactionListItem extends PureComponent {
<TransactionListItemDetails
transactionGroup={transactionGroup}
onRetry={this.handleRetry}
showRetry={showRetry && methodData.done}
showRetry={showRetry}
onCancel={this.handleCancel}
showCancel={showCancel}
cancelDisabled={!hasEnoughCancelGas}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { connect } from 'react-redux'
import { withRouter } from 'react-router-dom'
import { compose } from 'recompose'
import withMethodData from '../../../helpers/higher-order-components/with-method-data'
import TransactionListItem from './transaction-list-item.component'
import { setSelectedToken, showModal, showSidebar, addKnownMethodData } from '../../../store/actions'
import { setSelectedToken, showModal, showSidebar, getContractMethodData } from '../../../store/actions'
import { hexToDecimal } from '../../../helpers/utils/conversions.util'
import { getTokenData } from '../../../helpers/utils/transactions.util'
import { getHexGasTotal, increaseLastGasPrice } from '../../../helpers/utils/confirm-tx.util'
Expand All @@ -14,15 +13,15 @@ import {
setCustomGasPriceForRetry,
setCustomGasLimit,
} from '../../../ducks/gas/gas.duck'
import { getIsMainnet, preferencesSelector, getSelectedAddress, conversionRateSelector } from '../../../selectors/selectors'
import { getIsMainnet, preferencesSelector, getSelectedAddress, conversionRateSelector, getKnownMethodData } from '../../../selectors/selectors'
import { isBalanceSufficient } from '../../../pages/send/send.utils'

const mapStateToProps = (state, ownProps) => {
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 || {}
Expand All @@ -38,7 +37,7 @@ const mapStateToProps = (state, ownProps) => {
})

return {
knownMethodData,
methodData: getKnownMethodData(state, data) || {},
showFiat: (isMainnet || !!showFiatInTestnets),
selectedAccountBalance,
hasEnoughCancelGas,
Expand All @@ -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))
Expand Down Expand Up @@ -97,5 +96,4 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
export default compose(
withRouter,
connect(mapStateToProps, mapDispatchToProps, mergeProps),
withMethodData,
)(TransactionListItem)
12 changes: 12 additions & 0 deletions ui/app/ducks/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ function reduceApp (state, action) {
lastSelectedProvider: null,
networksTabSelectedRpcUrl: '',
networksTabIsInAddMode: false,
loadingMethodData: false,
}, state.appState)

switch (action.type) {
Expand Down Expand Up @@ -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
}
Expand Down
28 changes: 2 additions & 26 deletions ui/app/ducks/confirm-transaction/confirm-transaction.duck.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import log from 'loglevel'
import {
conversionRateSelector,
currentCurrencySelector,
Expand All @@ -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'

Expand Down Expand Up @@ -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]
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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]))
})
})
})
Loading