From 865a0d8173e27c073f00d0d156b95e8d92208a94 Mon Sep 17 00:00:00 2001 From: Alexander Tseung Date: Fri, 17 Aug 2018 14:34:14 -0700 Subject: Use eth_getCode to sort transaction action type --- .../confirm-transaction-switch.component.js | 13 +++- .../confirm-transaction-switch.container.js | 6 +- ui/app/ducks/confirm-transaction.duck.js | 42 +++++++++---- .../ducks/tests/confirm-transaction.duck.test.js | 70 +++++++++++++++------- ui/app/helpers/confirm-transaction/util.js | 5 ++ 5 files changed, 99 insertions(+), 37 deletions(-) (limited to 'ui') diff --git a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js index 0280f73c6..d494977cd 100644 --- a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js +++ b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js @@ -23,14 +23,16 @@ export default class ConfirmTransactionSwitch extends Component { static propTypes = { txData: PropTypes.object, methodData: PropTypes.object, - fetchingMethodData: PropTypes.bool, + fetchingData: PropTypes.bool, + isEtherTransaction: PropTypes.bool, } redirectToTransaction () { const { txData, methodData: { name }, - fetchingMethodData, + fetchingData, + isEtherTransaction, } = this.props const { id, txParams: { data } = {} } = txData @@ -39,10 +41,15 @@ export default class ConfirmTransactionSwitch extends Component { return } - if (fetchingMethodData) { + if (fetchingData) { return } + if (isEtherTransaction) { + const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_SEND_ETHER_PATH}` + return + } + if (data) { const methodName = name && name.toLowerCase() diff --git a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js index 3d7fc78cc..7f2c36af2 100644 --- a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js +++ b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js @@ -6,14 +6,16 @@ const mapStateToProps = state => { confirmTransaction: { txData, methodData, - fetchingMethodData, + fetchingData, + toSmartContract, }, } = state return { txData, methodData, - fetchingMethodData, + fetchingData, + isEtherTransaction: !toSmartContract, } } diff --git a/ui/app/ducks/confirm-transaction.duck.js b/ui/app/ducks/confirm-transaction.duck.js index 1885e12d1..f17933ddd 100644 --- a/ui/app/ducks/confirm-transaction.duck.js +++ b/ui/app/ducks/confirm-transaction.duck.js @@ -14,6 +14,7 @@ import { addEth, increaseLastGasPrice, hexGreaterThan, + isSmartContractAddress, } from '../helpers/confirm-transaction/util' import { getSymbolAndDecimals } from '../token-util' @@ -35,8 +36,9 @@ const UPDATE_TRANSACTION_TOTALS = createActionType('UPDATE_TRANSACTION_TOTALS') const UPDATE_HEX_GAS_TOTAL = createActionType('UPDATE_HEX_GAS_TOTAL') const UPDATE_TOKEN_PROPS = createActionType('UPDATE_TOKEN_PROPS') const UPDATE_NONCE = createActionType('UPDATE_NONCE') -const FETCH_METHOD_DATA_START = createActionType('FETCH_METHOD_DATA_START') -const FETCH_METHOD_DATA_END = createActionType('FETCH_METHOD_DATA_END') +const UPDATE_TO_SMART_CONTRACT = createActionType('UPDATE_TO_SMART_CONTRACT') +const FETCH_DATA_START = createActionType('FETCH_DATA_START') +const FETCH_DATA_END = createActionType('FETCH_DATA_END') // Initial state const initState = { @@ -55,7 +57,8 @@ const initState = { ethTransactionTotal: '', hexGasTotal: '', nonce: '', - fetchingMethodData: false, + toSmartContract: false, + fetchingData: false, } // Reducer @@ -138,15 +141,20 @@ export default function reducer ({ confirmTransaction: confirmState = initState ...confirmState, nonce: action.payload, } - case FETCH_METHOD_DATA_START: + case UPDATE_TO_SMART_CONTRACT: return { ...confirmState, - fetchingMethodData: true, + toSmartContract: action.payload, } - case FETCH_METHOD_DATA_END: + case FETCH_DATA_START: return { ...confirmState, - fetchingMethodData: false, + fetchingData: true, + } + case FETCH_DATA_END: + return { + ...confirmState, + fetchingData: false, } case CLEAR_CONFIRM_TRANSACTION: return initState @@ -237,9 +245,16 @@ export function updateNonce (nonce) { } } -export function setFetchingMethodData (isFetching) { +export function updateToSmartContract (toSmartContract) { + return { + type: UPDATE_TO_SMART_CONTRACT, + payload: toSmartContract, + } +} + +export function setFetchingData (isFetching) { return { - type: isFetching ? FETCH_METHOD_DATA_START : FETCH_METHOD_DATA_END, + type: isFetching ? FETCH_DATA_START : FETCH_DATA_END, } } @@ -338,19 +353,22 @@ 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 try { - dispatch(setFetchingMethodData(true)) + dispatch(setFetchingData(true)) const methodData = await getMethodData(data) dispatch(updateMethodData(methodData)) - dispatch(setFetchingMethodData(false)) + const toSmartContract = await isSmartContractAddress(to) + dispatch(updateToSmartContract(toSmartContract)) + dispatch(setFetchingData(false)) } catch (error) { dispatch(updateMethodData({})) - dispatch(setFetchingMethodData(false)) + dispatch(setFetchingData(false)) } const tokenData = getTokenData(data) diff --git a/ui/app/ducks/tests/confirm-transaction.duck.test.js b/ui/app/ducks/tests/confirm-transaction.duck.test.js index 111674e33..1bab0add0 100644 --- a/ui/app/ducks/tests/confirm-transaction.duck.test.js +++ b/ui/app/ducks/tests/confirm-transaction.duck.test.js @@ -1,6 +1,7 @@ import assert from 'assert' import configureMockStore from 'redux-mock-store' import thunk from 'redux-thunk' +import sinon from 'sinon' import ConfirmTransactionReducer, * as actions from '../confirm-transaction.duck.js' @@ -20,7 +21,8 @@ const initialState = { ethTransactionTotal: '', hexGasTotal: '', nonce: '', - fetchingMethodData: false, + toSmartContract: false, + fetchingData: false, } const UPDATE_TX_DATA = 'metamask/confirm-transaction/UPDATE_TX_DATA' @@ -35,8 +37,9 @@ const UPDATE_TRANSACTION_TOTALS = 'metamask/confirm-transaction/UPDATE_TRANSACTI const UPDATE_HEX_GAS_TOTAL = 'metamask/confirm-transaction/UPDATE_HEX_GAS_TOTAL' const UPDATE_TOKEN_PROPS = 'metamask/confirm-transaction/UPDATE_TOKEN_PROPS' const UPDATE_NONCE = 'metamask/confirm-transaction/UPDATE_NONCE' -const FETCH_METHOD_DATA_START = 'metamask/confirm-transaction/FETCH_METHOD_DATA_START' -const FETCH_METHOD_DATA_END = 'metamask/confirm-transaction/FETCH_METHOD_DATA_END' +const UPDATE_TO_SMART_CONTRACT = 'metamask/confirm-transaction/UPDATE_TO_SMART_CONTRACT' +const FETCH_DATA_START = 'metamask/confirm-transaction/FETCH_DATA_START' +const FETCH_DATA_END = 'metamask/confirm-transaction/FETCH_DATA_END' const CLEAR_CONFIRM_TRANSACTION = 'metamask/confirm-transaction/CLEAR_CONFIRM_TRANSACTION' describe('Confirm Transaction Duck', () => { @@ -64,7 +67,8 @@ describe('Confirm Transaction Duck', () => { ethTransactionTotal: '469.27', hexGasTotal: '0x1319718a5000', nonce: '0x0', - fetchingMethodData: false, + toSmartContract: false, + fetchingData: false, }, } @@ -271,30 +275,43 @@ describe('Confirm Transaction Duck', () => { ) }) - it('should set fetchingMethodData to true when receiving a FETCH_METHOD_DATA_START action', () => { + it('should update nonce when receiving an UPDATE_TO_SMART_CONTRACT action', () => { assert.deepEqual( ConfirmTransactionReducer(mockState, { - type: FETCH_METHOD_DATA_START, + type: UPDATE_TO_SMART_CONTRACT, + payload: true, }), { ...mockState.confirmTransaction, - fetchingMethodData: true, + toSmartContract: true, } ) }) - it('should set fetchingMethodData to false when receiving a FETCH_METHOD_DATA_END action', () => { + it('should set fetchingData to true when receiving a FETCH_DATA_START action', () => { assert.deepEqual( - ConfirmTransactionReducer({ confirmTransaction: { fetchingMethodData: true } }, { - type: FETCH_METHOD_DATA_END, + ConfirmTransactionReducer(mockState, { + type: FETCH_DATA_START, }), { - fetchingMethodData: false, + ...mockState.confirmTransaction, + fetchingData: true, } ) }) - it('should clear confirmTransaction when receiving a FETCH_METHOD_DATA_END action', () => { + it('should set fetchingData to false when receiving a FETCH_DATA_END action', () => { + assert.deepEqual( + ConfirmTransactionReducer({ confirmTransaction: { fetchingData: true } }, { + type: FETCH_DATA_END, + }), + { + fetchingData: false, + } + ) + }) + + it('should clear confirmTransaction when receiving a FETCH_DATA_END action', () => { assert.deepEqual( ConfirmTransactionReducer(mockState, { type: CLEAR_CONFIRM_TRANSACTION, @@ -460,24 +477,24 @@ describe('Confirm Transaction Duck', () => { ) }) - it('should create an action to set fetchingMethodData to true', () => { + it('should create an action to set fetchingData to true', () => { const expectedAction = { - type: FETCH_METHOD_DATA_START, + type: FETCH_DATA_START, } assert.deepEqual( - actions.setFetchingMethodData(true), + actions.setFetchingData(true), expectedAction ) }) - it('should create an action to set fetchingMethodData to false', () => { + it('should create an action to set fetchingData to false', () => { const expectedAction = { - type: FETCH_METHOD_DATA_END, + type: FETCH_DATA_END, } assert.deepEqual( - actions.setFetchingMethodData(false), + actions.setFetchingData(false), expectedAction ) }) @@ -495,6 +512,18 @@ describe('Confirm Transaction Duck', () => { }) describe('Thunk actions', done => { + beforeEach(() => { + global.eth = { + getCode: sinon.stub().callsFake( + address => Promise.resolve(address && address.match(/isContract/) ? 'not-0x' : '0x') + ), + } + }) + + afterEach(() => { + global.eth.getCode.resetHistory() + }) + it('updates txData and gas on an existing transaction in confirmTransaction', () => { const mockState = { metamask: { @@ -505,7 +534,7 @@ describe('Confirm Transaction Duck', () => { ethTransactionAmount: '1', ethTransactionFee: '0.000021', ethTransactionTotal: '1.000021', - fetchingMethodData: false, + fetchingData: false, fiatTransactionAmount: '469.26', fiatTransactionFee: '0.01', fiatTransactionTotal: '469.27', @@ -581,7 +610,7 @@ describe('Confirm Transaction Duck', () => { ethTransactionAmount: '1', ethTransactionFee: '0.000021', ethTransactionTotal: '1.000021', - fetchingMethodData: false, + fetchingData: false, fiatTransactionAmount: '469.26', fiatTransactionFee: '0.01', fiatTransactionTotal: '469.27', @@ -667,6 +696,7 @@ describe('Confirm Transaction Duck', () => { .then(() => { const storeActions = store.getActions() assert.equal(storeActions.length, expectedActions.length) + storeActions.forEach((action, index) => assert.equal(action.type, expectedActions[index])) done() }) diff --git a/ui/app/helpers/confirm-transaction/util.js b/ui/app/helpers/confirm-transaction/util.js index 76e80a8ac..3d0cb57e7 100644 --- a/ui/app/helpers/confirm-transaction/util.js +++ b/ui/app/helpers/confirm-transaction/util.js @@ -146,3 +146,8 @@ export function roundExponential (value) { // In JS, numbers with exponentials greater than 20 get displayed as an exponential. return bigNumberValue.e > 20 ? Number(bigNumberValue.toPrecision(PRECISION)) : value } + +export async function isSmartContractAddress (address) { + const code = await global.eth.getCode(address) + return code && code !== '0x' +} -- cgit