From 3e79f4961d567e77c35112e17cbdc7ed96756f51 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 12:48:36 +0100 Subject: [PATCH 01/12] Fix send max eth --- app/actions/transaction/index.js | 14 ++++ .../confirmations/SendFlow/Amount/index.js | 20 ++++- .../confirmations/SendFlow/Confirm/index.js | 82 +++++++++++++++---- .../TransactionReviewEIP1559Update/index.tsx | 7 +- app/reducers/transaction/index.js | 13 +++ 5 files changed, 117 insertions(+), 19 deletions(-) diff --git a/app/actions/transaction/index.js b/app/actions/transaction/index.js index 7cc0d95a3e0..0d327b73458 100644 --- a/app/actions/transaction/index.js +++ b/app/actions/transaction/index.js @@ -157,3 +157,17 @@ export function setProposedNonce(proposedNonce) { proposedNonce, }; } + +export function setMaxValueMode(maxValueMode) { + return { + type: 'SET_MAX_VALUE_MODE', + maxValueMode, + }; +} + +export function setTransactionValue(value) { + return { + type: 'SET_TRANSACTION_VALUE', + value, + }; +} diff --git a/app/components/Views/confirmations/SendFlow/Amount/index.js b/app/components/Views/confirmations/SendFlow/Amount/index.js index 10a38a98a6a..0e9116591be 100644 --- a/app/components/Views/confirmations/SendFlow/Amount/index.js +++ b/app/components/Views/confirmations/SendFlow/Amount/index.js @@ -17,6 +17,7 @@ import { prepareTransaction, setTransactionObject, resetTransaction, + setMaxValueMode, } from '../../../../../actions/transaction'; import { getSendFlowTitle } from '../../../../UI/Navbar'; import StyledButton from '../../../../UI/StyledButton'; @@ -926,9 +927,22 @@ class Amount extends PureComponent { }; onInputChange = (inputValue, selectedAsset, useMax) => { - const { contractExchangeRates, conversionRate, currentCurrency, ticker } = - this.props; + const { + contractExchangeRates, + conversionRate, + currentCurrency, + ticker, + maxValueMode: currentMaxValueMode, + setMaxValueMode, + } = this.props; const { internalPrimaryCurrencyIsCrypto } = this.state; + + if (useMax) { + setMaxValueMode(true); + } else if (currentMaxValueMode) { + setMaxValueMode(false); + } + let inputValueConversion, renderableInputValueConversion, hasExchangeRate, @@ -1557,6 +1571,7 @@ const mapStateToProps = (state, ownProps) => ({ ), swapsIsLive: swapsLivenessSelector(state), chainId: selectChainId(state), + maxValueMode: state.transaction.maxValueMode, }); const mapDispatchToProps = (dispatch) => ({ @@ -1567,6 +1582,7 @@ const mapDispatchToProps = (dispatch) => ({ setSelectedAsset: (selectedAsset) => dispatch(setSelectedAsset(selectedAsset)), resetTransaction: () => dispatch(resetTransaction()), + setMaxValueMode: (maxValueMode) => dispatch(setMaxValueMode(maxValueMode)), }); export default connect( diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js index 8c06868102f..bcb983c59a7 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/index.js @@ -1,4 +1,6 @@ import React, { PureComponent } from 'react'; +import BN from 'bn.js'; +import convert from '@metamask/ethjs-unit'; import { baseStyles } from '../../../../../styles/common'; import { InteractionManager, @@ -30,12 +32,14 @@ import StyledButton from '../../../../UI/StyledButton'; import { WalletDevice } from '@metamask/transaction-controller'; import { ChainId } from '@metamask/controller-utils'; import { GAS_ESTIMATE_TYPES } from '@metamask/gas-fee-controller'; +import { remove0x } from '@metamask/utils'; import { prepareTransaction, resetTransaction, setNonce, setProposedNonce, setTransactionId, + setTransactionValue, } from '../../../../../actions/transaction'; import { getGasLimit } from '../../../../../util/custom-gas'; import Engine from '../../../../../core/Engine'; @@ -320,7 +324,8 @@ class Confirm extends PureComponent { ); setNetworkNonce = async () => { - const { networkClientId, setNonce, setProposedNonce, transaction } = this.props; + const { networkClientId, setNonce, setProposedNonce, transaction } = + this.props; const proposedNonce = await getNetworkNonce(transaction, networkClientId); setNonce(proposedNonce); setProposedNonce(proposedNonce); @@ -546,13 +551,20 @@ class Confirm extends PureComponent { componentDidUpdate = (prevProps, prevState) => { const { + accounts, transactionState: { transactionTo, - transaction: { value, gas }, + transaction: { value, gas, from }, }, contractBalances, selectedAsset, + maxValueMode, + gasFeeEstimates, } = this.props; + + const { transactionMeta } = this.state; + const { id: transactionId } = transactionMeta; + this.updateNavBar(); const transaction = this.prepareTransactionToSend(); @@ -575,6 +587,13 @@ class Confirm extends PureComponent { const haveEIP1559TotalMaxHexChanged = EIP1559GasTransaction.totalMaxHex !== prevState.EIP1559GasTransaction.totalMaxHex; + const isEIP1559Transaction = + this.props.gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKET; + const haveGasFeeMaxNativeChanged = isEIP1559Transaction + ? EIP1559GasTransaction.gasFeeMaxNative !== + prevState.EIP1559GasTransaction.gasFeeMaxNative + : legacyGasTransaction.gasFeeMaxNative !== + prevState.legacyGasTransaction.gasFeeMaxNative; const haveGasPropertiesChanged = (this.props.gasFeeEstimates && @@ -604,6 +623,39 @@ class Confirm extends PureComponent { ? AppConstants.GAS_OPTIONS.MEDIUM : this.state.gasSelected; + if ( + maxValueMode && + selectedAsset.isETH && + !isEmpty(gasFeeEstimates) && + haveGasFeeMaxNativeChanged + ) { + const { TransactionController } = Engine.context; + const { gasFeeMaxNative } = isEIP1559Transaction + ? EIP1559GasTransaction + : legacyGasTransaction; + + const accountBalanceBN = new BN(remove0x(accounts[from].balance), 16); + const transactionFeeMax = new BN( + convert.toWei(gasFeeMaxNative, 'ether'), + 16, + ); + + const maxTransactionValueBN = accountBalanceBN.sub(transactionFeeMax); + + const maxTransactionValueHex = + '0x' + maxTransactionValueBN.toString(16); + + TransactionController.updateEditableParams(transactionId, { + value: maxTransactionValueHex, + }).then((newMeta) => { + this.props.setTransactionValue(maxTransactionValueHex); + }); + + // In order to prevent race condition do not remove this early return. + // Another update will be triggered by `updateEditableParams` and validateAmount will be called next update. + return; + } + if ( (!this.state.stopUpdateGas && !this.state.advancedGasInserted) || gasEstimateTypeChanged @@ -611,7 +663,6 @@ class Confirm extends PureComponent { if (this.props.gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKET) { error = this.validateAmount({ transaction, - total: EIP1559GasTransaction.totalMaxHex, }); this.setError(error); // eslint-disable-next-line react/no-did-update-set-state @@ -641,7 +692,6 @@ class Confirm extends PureComponent { } else { error = this.validateAmount({ transaction, - total: legacyGasTransaction.totalHex, }); this.setError(error); } @@ -805,7 +855,7 @@ class Confirm extends PureComponent { * Validates transaction balances * @returns - Whether there is an error with the amount */ - validateAmount = ({ transaction, total }) => { + validateAmount = ({ transaction }) => { const { accounts, contractBalances, @@ -821,7 +871,7 @@ class Confirm extends PureComponent { const selectedAddress = transaction?.from; const weiBalance = hexToBN(accounts[selectedAddress].balance); - const totalTransactionValue = hexToBN(total); + const totalTransactionValue = hexToBN(value); if (!isDecimal(value)) { return strings('transaction.invalid_amount'); @@ -951,12 +1001,10 @@ class Confirm extends PureComponent { if (gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKET) { error = this.validateAmount({ transaction, - total: EIP1559GasTransaction.totalMaxHex, }); } else { error = this.validateAmount({ transaction, - total: legacyGasTransaction.totalHex, }); } this.setError(error); @@ -1244,15 +1292,15 @@ class Confirm extends PureComponent { closeModal: true, ...(txnType ? { - legacyGasTransaction: gasTxn, - legacyGasObject: gasObj, - advancedGasInserted: !gasSelect, - stopUpdateGas: false, - } + legacyGasTransaction: gasTxn, + legacyGasObject: gasObj, + advancedGasInserted: !gasSelect, + stopUpdateGas: false, + } : { - EIP1559GasTransaction: gasTxn, - EIP1559GasObject: gasObj, - }), + EIP1559GasTransaction: gasTxn, + EIP1559GasObject: gasObj, + }), }); }; @@ -1579,6 +1627,7 @@ const mapStateToProps = (state) => ({ selectCurrentTransactionMetadata(state)?.simulationData, useTransactionSimulations: selectUseTransactionSimulations(state), securityAlertResponse: selectCurrentTransactionSecurityAlertResponse(state), + maxValueMode: state.transaction.maxValueMode, }); const mapDispatchToProps = (dispatch) => ({ @@ -1594,6 +1643,7 @@ const mapDispatchToProps = (dispatch) => ({ showAlert: (config) => dispatch(showAlert(config)), updateTransactionMetrics: ({ transactionId, params }) => dispatch(updateTransactionMetrics({ transactionId, params })), + setTransactionValue: (value) => dispatch(setTransactionValue(value)), }); export default connect( diff --git a/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx b/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx index e0986b3c59b..261f5e45ea0 100644 --- a/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx +++ b/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx @@ -93,7 +93,12 @@ const TransactionReviewEIP1559Update = ({ updateTransactionState(gasTransaction); } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [gasEstimationReady, updateTransactionState, suggestedGasLimit]); + }, [ + gasEstimationReady, + updateTransactionState, + suggestedGasLimit, + renderableGasFeeMaxNative, + ]); const openLinkAboutGas = useCallback( () => Linking.openURL(AppConstants.URLS.WHY_TRANSACTION_TAKE_TIME), diff --git a/app/reducers/transaction/index.js b/app/reducers/transaction/index.js index d3c4ed135fe..5d1ab2efb94 100644 --- a/app/reducers/transaction/index.js +++ b/app/reducers/transaction/index.js @@ -29,6 +29,7 @@ const initialState = { proposedNonce: undefined, nonce: undefined, securityAlertResponses: {}, + useMax: false, }; const getAssetType = (selectedAsset) => { @@ -146,6 +147,18 @@ const transactionReducer = (state = initialState, action) => { id: transactionId, }; } + case 'SET_MAX_VALUE_MODE': { + return { + ...state, + maxValueMode: action.maxValueMode, + }; + } + case 'SET_TRANSACTION_VALUE': { + return { + ...state, + transaction: { ...state.transaction, value: action.value }, + }; + } default: return state; } From 45c706e714fadc0d7955be0616bccafd05ab22ac Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 12:57:50 +0100 Subject: [PATCH 02/12] Fix dep --- app/components/Views/confirmations/SendFlow/Confirm/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js index bcb983c59a7..4faf82bcc5d 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/index.js @@ -1,5 +1,5 @@ import React, { PureComponent } from 'react'; -import BN from 'bn.js'; +import { BN } from 'ethereumjs-util'; import convert from '@metamask/ethjs-unit'; import { baseStyles } from '../../../../../styles/common'; import { From defb9ba6057e6e845c556c146fc730b52c9a3fd6 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 13:03:03 +0100 Subject: [PATCH 03/12] Change dependent property --- .../TransactionReview/TransactionReviewEIP1559Update/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx b/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx index 261f5e45ea0..c927fdee7a6 100644 --- a/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx +++ b/app/components/Views/confirmations/components/TransactionReview/TransactionReviewEIP1559Update/index.tsx @@ -71,6 +71,7 @@ const TransactionReviewEIP1559Update = ({ }); const { + gasFeeMaxNative, renderableGasFeeMinNative, renderableGasFeeMinConversion, renderableGasFeeMaxNative, @@ -97,7 +98,7 @@ const TransactionReviewEIP1559Update = ({ gasEstimationReady, updateTransactionState, suggestedGasLimit, - renderableGasFeeMaxNative, + gasFeeMaxNative, ]); const openLinkAboutGas = useCallback( From 1d384770a8ce00df32fc601367e20685d3de272b Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 13:07:59 +0100 Subject: [PATCH 04/12] Fix lint --- .../Views/confirmations/SendFlow/Confirm/index.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js index 4faf82bcc5d..4828a07eab0 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/index.js @@ -356,8 +356,8 @@ class Confirm extends PureComponent { request_source: this.originIsMMSDKRemoteConn ? AppConstants.REQUEST_SOURCES.SDK_REMOTE_CONN : this.originIsWalletConnect - ? AppConstants.REQUEST_SOURCES.WC - : AppConstants.REQUEST_SOURCES.IN_APP_BROWSER, + ? AppConstants.REQUEST_SOURCES.WC + : AppConstants.REQUEST_SOURCES.IN_APP_BROWSER, is_smart_transaction: shouldUseSmartTransaction || false, }; @@ -550,6 +550,7 @@ class Confirm extends PureComponent { }; componentDidUpdate = (prevProps, prevState) => { + console.log('componentDidUpdate'); const { accounts, transactionState: { @@ -968,12 +969,7 @@ class Confirm extends PureComponent { transactionSimulationData: { isUpdatedAfterSecurityCheck } = {}, } = this.props; - const { - legacyGasTransaction, - transactionConfirmed, - EIP1559GasTransaction, - isChangeInSimulationModalShown, - } = this.state; + const { transactionConfirmed, isChangeInSimulationModalShown } = this.state; if (transactionConfirmed) return; if (isUpdatedAfterSecurityCheck && !isChangeInSimulationModalShown) { From 8c8771e6920239ce79e24950a5f9528edbd1826c Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 13:09:34 +0100 Subject: [PATCH 05/12] Remove log --- app/components/Views/confirmations/SendFlow/Confirm/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js index 4828a07eab0..facbe3f78f3 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/index.js @@ -550,7 +550,6 @@ class Confirm extends PureComponent { }; componentDidUpdate = (prevProps, prevState) => { - console.log('componentDidUpdate'); const { accounts, transactionState: { From 51423396b867b8c843b6b846eb0fdc3190797966 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 14:06:13 +0100 Subject: [PATCH 06/12] Fix lint --- .../confirmations/SendFlow/Amount/index.js | 8 ++++ .../confirmations/SendFlow/Confirm/index.js | 39 +++++++-------- .../confirmations/SendFlow/Confirm/utils.js | 34 ++++++++++++++ .../SendFlow/Confirm/utils.test.js | 47 +++++++++++++++++++ 4 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 app/components/Views/confirmations/SendFlow/Confirm/utils.js create mode 100644 app/components/Views/confirmations/SendFlow/Confirm/utils.test.js diff --git a/app/components/Views/confirmations/SendFlow/Amount/index.js b/app/components/Views/confirmations/SendFlow/Amount/index.js index 0e9116591be..f6597376424 100644 --- a/app/components/Views/confirmations/SendFlow/Amount/index.js +++ b/app/components/Views/confirmations/SendFlow/Amount/index.js @@ -486,6 +486,14 @@ class Amount extends PureComponent { * Type of gas fee estimate provided by the gas fee controller. */ gasEstimateType: PropTypes.string, + /** + * Boolean that indicates if the max value mode is enabled + */ + maxValueMode: PropTypes.bool, + /** + * Function that sets the transaction value + */ + setTransactionValue: PropTypes.func, }; state = { diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js index facbe3f78f3..c57c8b181df 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/index.js @@ -1,6 +1,4 @@ import React, { PureComponent } from 'react'; -import { BN } from 'ethereumjs-util'; -import convert from '@metamask/ethjs-unit'; import { baseStyles } from '../../../../../styles/common'; import { InteractionManager, @@ -32,7 +30,6 @@ import StyledButton from '../../../../UI/StyledButton'; import { WalletDevice } from '@metamask/transaction-controller'; import { ChainId } from '@metamask/controller-utils'; import { GAS_ESTIMATE_TYPES } from '@metamask/gas-fee-controller'; -import { remove0x } from '@metamask/utils'; import { prepareTransaction, resetTransaction, @@ -139,6 +136,7 @@ import { validateSufficientBalance, } from './validation'; import { buildTransactionParams } from '../../../../../util/confirmation/transactions'; +import { updateTransactionToMaxValue } from './utils'; const EDIT = 'edit'; const EDIT_NONCE = 'edit_nonce'; @@ -290,6 +288,14 @@ class Confirm extends PureComponent { * Object containing blockaid validation response for confirmation */ securityAlertResponse: PropTypes.object, + /** + * Boolean that indicates if the max value mode is enabled + */ + maxValueMode: PropTypes.bool, + /** + * Function that sets the transaction value + */ + setTransactionValue: PropTypes.func, }; state = { @@ -629,26 +635,13 @@ class Confirm extends PureComponent { !isEmpty(gasFeeEstimates) && haveGasFeeMaxNativeChanged ) { - const { TransactionController } = Engine.context; - const { gasFeeMaxNative } = isEIP1559Transaction - ? EIP1559GasTransaction - : legacyGasTransaction; - - const accountBalanceBN = new BN(remove0x(accounts[from].balance), 16); - const transactionFeeMax = new BN( - convert.toWei(gasFeeMaxNative, 'ether'), - 16, - ); - - const maxTransactionValueBN = accountBalanceBN.sub(transactionFeeMax); - - const maxTransactionValueHex = - '0x' + maxTransactionValueBN.toString(16); - - TransactionController.updateEditableParams(transactionId, { - value: maxTransactionValueHex, - }).then((newMeta) => { - this.props.setTransactionValue(maxTransactionValueHex); + updateTransactionToMaxValue({ + transactionId, + isEIP1559Transaction, + EIP1559GasTransaction, + legacyGasTransaction, + accountBalance: accounts[from].balance, + setTransactionValue: this.props.setTransactionValue, }); // In order to prevent race condition do not remove this early return. diff --git a/app/components/Views/confirmations/SendFlow/Confirm/utils.js b/app/components/Views/confirmations/SendFlow/Confirm/utils.js new file mode 100644 index 00000000000..102b3891db8 --- /dev/null +++ b/app/components/Views/confirmations/SendFlow/Confirm/utils.js @@ -0,0 +1,34 @@ +import { BN } from 'ethereumjs-util'; +import convert from '@metamask/ethjs-unit'; +import { remove0x, add0x } from '@metamask/utils'; +import Engine from '../../../../../core/Engine'; + +export const updateTransactionToMaxValue = async ({ + transactionId, + isEIP1559Transaction, + EIP1559GasTransaction, + legacyGasTransaction, + accountBalance, + setTransactionValue, +}) => { + const { TransactionController } = Engine.context; + const { gasFeeMaxNative } = isEIP1559Transaction + ? EIP1559GasTransaction + : legacyGasTransaction; + + const accountBalanceBN = new BN(remove0x(accountBalance), 16); + const transactionFeeMax = new BN(convert.toWei(gasFeeMaxNative, 'ether'), 16); + + const maxTransactionValueBN = accountBalanceBN.sub(transactionFeeMax); + + const maxTransactionValueHex = add0x(maxTransactionValueBN.toString(16)); + + const transactionMeta = await TransactionController.updateEditableParams( + transactionId, + { + value: maxTransactionValueHex, + }, + ); + + setTransactionValue(transactionMeta.txParams.value); +}; diff --git a/app/components/Views/confirmations/SendFlow/Confirm/utils.test.js b/app/components/Views/confirmations/SendFlow/Confirm/utils.test.js new file mode 100644 index 00000000000..24240e8a2a6 --- /dev/null +++ b/app/components/Views/confirmations/SendFlow/Confirm/utils.test.js @@ -0,0 +1,47 @@ +import { updateTransactionToMaxValue } from './utils'; +import { BN } from 'ethereumjs-util'; +import convert from '@metamask/ethjs-unit'; + +// Mock the Engine and its context +jest.mock('../../../../../core/Engine', () => ({ + context: { + TransactionController: { + updateEditableParams: jest.fn().mockResolvedValue({ + txParams: { value: '0x0' }, + }), + }, + }, +})); + +describe('updateTransactionToMaxValue', () => { + it('should update the transaction value correctly', async () => { + const transactionId = 'testTransactionId'; + const isEIP1559Transaction = true; + const EIP1559GasTransaction = { gasFeeMaxNative: '0.01' }; + const legacyGasTransaction = { gasFeeMaxNative: '0.02' }; + const accountBalance = '0x2386f26fc10000'; // 0.1 ether in wei + const setTransactionValue = jest.fn(); + + await updateTransactionToMaxValue({ + transactionId, + isEIP1559Transaction, + EIP1559GasTransaction, + legacyGasTransaction, + accountBalance, + setTransactionValue, + }); + + // Calculate expected max transaction value + const accountBalanceBN = new BN('2386f26fc10000', 16); // 0.1 ether in wei + const transactionFeeMax = new BN(convert.toWei('0.01', 'ether'), 10); + const expectedMaxTransactionValueBN = + accountBalanceBN.sub(transactionFeeMax); + const expectedMaxTransactionValueHex = + '0x' + expectedMaxTransactionValueBN.toString(16); + + // Check if setTransactionValue was called with the correct value + expect(setTransactionValue).toHaveBeenCalledWith( + expectedMaxTransactionValueHex, + ); + }); +}); From 1d98cb6d64434091c5e939c1dd9a89569d53136e Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 14:27:34 +0100 Subject: [PATCH 07/12] Add extra test --- .../Views/confirmations/SendFlow/Amount/index.js | 7 +++++-- .../confirmations/SendFlow/Amount/index.test.tsx | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/components/Views/confirmations/SendFlow/Amount/index.js b/app/components/Views/confirmations/SendFlow/Amount/index.js index f6597376424..6c36198c17e 100644 --- a/app/components/Views/confirmations/SendFlow/Amount/index.js +++ b/app/components/Views/confirmations/SendFlow/Amount/index.js @@ -491,9 +491,9 @@ class Amount extends PureComponent { */ maxValueMode: PropTypes.bool, /** - * Function that sets the transaction value + * Function that sets the max value mode */ - setTransactionValue: PropTypes.func, + setMaxValueMode: PropTypes.func, }; state = { @@ -945,6 +945,9 @@ class Amount extends PureComponent { } = this.props; const { internalPrimaryCurrencyIsCrypto } = this.state; + console.log('useMax', useMax); + console.log({ setMaxValueMode }); + if (useMax) { setMaxValueMode(true); } else if (currentMaxValueMode) { diff --git a/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx b/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx index 4f57005c85d..6f3edec25d4 100644 --- a/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx +++ b/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx @@ -9,6 +9,7 @@ import TransactionTypes from '../../../../../core/TransactionTypes'; import { AmountViewSelectorsIDs } from '../../../../../../e2e/selectors/SendFlow/AmountView.selectors'; import { backgroundState } from '../../../../../util/test/initial-root-state'; +import { setMaxValueMode } from '../../../../../actions/transaction'; const mockTransactionTypes = TransactionTypes; @@ -67,6 +68,12 @@ jest.mock('../../../../../util/transaction-controller', () => ({ ), })); +jest.mock('../../../../../actions/transaction', () => ({ + setMaxValueMode: jest.fn().mockReturnValue({ + type: 'SET_MAX_VALUE_MODE', + }), +})); + const mockNavigate = jest.fn(); const CURRENT_ACCOUNT = '0x76cf1CdD1fcC252442b50D6e97207228aA4aefC3'; @@ -236,6 +243,15 @@ describe('Amount', () => { expect(toJSON()).toMatchSnapshot(); }); + it('should set max value mode when toggled on', () => { + const { getByText } = renderComponent(initialState); + + const useMaxButton = getByText(/Use max/); + fireEvent.press(useMaxButton); + + expect(setMaxValueMode).toHaveBeenCalled(); + }); + it('should proceed if balance is sufficient while on Native primary currency', async () => { const { getByText, getByTestId, toJSON } = renderComponent({ engine: { From cc288c2c31058a72d8b2ba8e280a360d9eeb5a09 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 14:36:06 +0100 Subject: [PATCH 08/12] Fix tests --- .../confirmations/SendFlow/Amount/index.js | 3 --- .../SendFlow/Amount/index.test.tsx | 1 + .../Confirm/{utils.test.js => utils.test.ts} | 4 ++-- .../SendFlow/Confirm/{utils.js => utils.ts} | 22 ++++++++++++++----- 4 files changed, 20 insertions(+), 10 deletions(-) rename app/components/Views/confirmations/SendFlow/Confirm/{utils.test.js => utils.test.ts} (92%) rename app/components/Views/confirmations/SendFlow/Confirm/{utils.js => utils.ts} (56%) diff --git a/app/components/Views/confirmations/SendFlow/Amount/index.js b/app/components/Views/confirmations/SendFlow/Amount/index.js index 6c36198c17e..0e0f8171eac 100644 --- a/app/components/Views/confirmations/SendFlow/Amount/index.js +++ b/app/components/Views/confirmations/SendFlow/Amount/index.js @@ -945,9 +945,6 @@ class Amount extends PureComponent { } = this.props; const { internalPrimaryCurrencyIsCrypto } = this.state; - console.log('useMax', useMax); - console.log({ setMaxValueMode }); - if (useMax) { setMaxValueMode(true); } else if (currentMaxValueMode) { diff --git a/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx b/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx index 6f3edec25d4..05d8097eea6 100644 --- a/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx +++ b/app/components/Views/confirmations/SendFlow/Amount/index.test.tsx @@ -69,6 +69,7 @@ jest.mock('../../../../../util/transaction-controller', () => ({ })); jest.mock('../../../../../actions/transaction', () => ({ + ...jest.requireActual('../../../../../actions/transaction'), setMaxValueMode: jest.fn().mockReturnValue({ type: 'SET_MAX_VALUE_MODE', }), diff --git a/app/components/Views/confirmations/SendFlow/Confirm/utils.test.js b/app/components/Views/confirmations/SendFlow/Confirm/utils.test.ts similarity index 92% rename from app/components/Views/confirmations/SendFlow/Confirm/utils.test.js rename to app/components/Views/confirmations/SendFlow/Confirm/utils.test.ts index 24240e8a2a6..f7e79395415 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/utils.test.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/utils.test.ts @@ -1,6 +1,6 @@ import { updateTransactionToMaxValue } from './utils'; import { BN } from 'ethereumjs-util'; -import convert from '@metamask/ethjs-unit'; +import { toWei } from '../../../../../util/number'; // Mock the Engine and its context jest.mock('../../../../../core/Engine', () => ({ @@ -33,7 +33,7 @@ describe('updateTransactionToMaxValue', () => { // Calculate expected max transaction value const accountBalanceBN = new BN('2386f26fc10000', 16); // 0.1 ether in wei - const transactionFeeMax = new BN(convert.toWei('0.01', 'ether'), 10); + const transactionFeeMax = new BN(toWei('0.01', 'ether'), 10); const expectedMaxTransactionValueBN = accountBalanceBN.sub(transactionFeeMax); const expectedMaxTransactionValueHex = diff --git a/app/components/Views/confirmations/SendFlow/Confirm/utils.js b/app/components/Views/confirmations/SendFlow/Confirm/utils.ts similarity index 56% rename from app/components/Views/confirmations/SendFlow/Confirm/utils.js rename to app/components/Views/confirmations/SendFlow/Confirm/utils.ts index 102b3891db8..730e205c7d8 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/utils.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/utils.ts @@ -1,7 +1,8 @@ import { BN } from 'ethereumjs-util'; -import convert from '@metamask/ethjs-unit'; +import type { TransactionMeta } from '@metamask/transaction-controller'; import { remove0x, add0x } from '@metamask/utils'; import Engine from '../../../../../core/Engine'; +import { toWei } from '../../../../../util/number'; export const updateTransactionToMaxValue = async ({ transactionId, @@ -10,6 +11,17 @@ export const updateTransactionToMaxValue = async ({ legacyGasTransaction, accountBalance, setTransactionValue, +}: { + transactionId: string; + isEIP1559Transaction: boolean; + EIP1559GasTransaction: { + gasFeeMaxNative: string; + }; + legacyGasTransaction: { + gasFeeMaxNative: string; + }; + accountBalance: string; + setTransactionValue: (value: string) => void; }) => { const { TransactionController } = Engine.context; const { gasFeeMaxNative } = isEIP1559Transaction @@ -17,18 +29,18 @@ export const updateTransactionToMaxValue = async ({ : legacyGasTransaction; const accountBalanceBN = new BN(remove0x(accountBalance), 16); - const transactionFeeMax = new BN(convert.toWei(gasFeeMaxNative, 'ether'), 16); + const transactionFeeMax = new BN(toWei(gasFeeMaxNative, 'ether'), 16); const maxTransactionValueBN = accountBalanceBN.sub(transactionFeeMax); const maxTransactionValueHex = add0x(maxTransactionValueBN.toString(16)); - const transactionMeta = await TransactionController.updateEditableParams( + const txMeta = (await TransactionController.updateEditableParams( transactionId, { value: maxTransactionValueHex, }, - ); + )) as TransactionMeta; - setTransactionValue(transactionMeta.txParams.value); + setTransactionValue(txMeta.txParams.value as string); }; From b00befdd1cb39c0bdbfa61eef8491aa379facfcc Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 14:48:48 +0100 Subject: [PATCH 09/12] Fix lint --- .../confirmations/SendFlow/Confirm/utils.test.ts | 12 ++++-------- .../Views/confirmations/SendFlow/Confirm/utils.ts | 12 ++++-------- app/util/transaction-controller/index.test.ts | 1 + app/util/transaction-controller/index.ts | 7 +++++++ 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/components/Views/confirmations/SendFlow/Confirm/utils.test.ts b/app/components/Views/confirmations/SendFlow/Confirm/utils.test.ts index f7e79395415..2ffef529897 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/utils.test.ts +++ b/app/components/Views/confirmations/SendFlow/Confirm/utils.test.ts @@ -3,14 +3,10 @@ import { BN } from 'ethereumjs-util'; import { toWei } from '../../../../../util/number'; // Mock the Engine and its context -jest.mock('../../../../../core/Engine', () => ({ - context: { - TransactionController: { - updateEditableParams: jest.fn().mockResolvedValue({ - txParams: { value: '0x0' }, - }), - }, - }, +jest.mock('../../../../../util/transaction-controller', () => ({ + updateEditableParams: jest.fn().mockResolvedValue({ + txParams: { value: '0x0' }, + }), })); describe('updateTransactionToMaxValue', () => { diff --git a/app/components/Views/confirmations/SendFlow/Confirm/utils.ts b/app/components/Views/confirmations/SendFlow/Confirm/utils.ts index 730e205c7d8..965b4525bd5 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/utils.ts +++ b/app/components/Views/confirmations/SendFlow/Confirm/utils.ts @@ -1,8 +1,8 @@ import { BN } from 'ethereumjs-util'; import type { TransactionMeta } from '@metamask/transaction-controller'; import { remove0x, add0x } from '@metamask/utils'; -import Engine from '../../../../../core/Engine'; import { toWei } from '../../../../../util/number'; +import { updateEditableParams } from '../../../../../util/transaction-controller'; export const updateTransactionToMaxValue = async ({ transactionId, @@ -23,7 +23,6 @@ export const updateTransactionToMaxValue = async ({ accountBalance: string; setTransactionValue: (value: string) => void; }) => { - const { TransactionController } = Engine.context; const { gasFeeMaxNative } = isEIP1559Transaction ? EIP1559GasTransaction : legacyGasTransaction; @@ -35,12 +34,9 @@ export const updateTransactionToMaxValue = async ({ const maxTransactionValueHex = add0x(maxTransactionValueBN.toString(16)); - const txMeta = (await TransactionController.updateEditableParams( - transactionId, - { - value: maxTransactionValueHex, - }, - )) as TransactionMeta; + const txMeta = (await updateEditableParams(transactionId, { + value: maxTransactionValueHex, + })) as TransactionMeta; setTransactionValue(txMeta.txParams.value as string); }; diff --git a/app/util/transaction-controller/index.test.ts b/app/util/transaction-controller/index.test.ts index 739678d5253..f3990dc5095 100644 --- a/app/util/transaction-controller/index.test.ts +++ b/app/util/transaction-controller/index.test.ts @@ -37,6 +37,7 @@ jest.mock('../../core/Engine', () => ({ updateSecurityAlertResponse: jest.fn(), updateTransaction: jest.fn(), wipeTransactions: jest.fn(), + updateEditableParams: jest.fn(), }, }, })); diff --git a/app/util/transaction-controller/index.ts b/app/util/transaction-controller/index.ts index fecf09e428b..9700fbb497c 100644 --- a/app/util/transaction-controller/index.ts +++ b/app/util/transaction-controller/index.ts @@ -108,6 +108,13 @@ export function wipeTransactions( return TransactionController.wipeTransactions(...args); } +export function updateEditableParams( + ...args: Parameters +) { + const { TransactionController } = Engine.context; + return TransactionController.updateEditableParams(...args); +} + export const getNetworkNonce = async ( { from }: { from: string }, networkClientId: NetworkClientId, From 39d4b19548545f4ee813d36e1735b242500636dd Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 14:51:08 +0100 Subject: [PATCH 10/12] Remove unnecessary if clause --- .../Views/confirmations/SendFlow/Confirm/index.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js index c57c8b181df..0a0c1e30f81 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/index.js @@ -985,16 +985,9 @@ class Confirm extends PureComponent { try { const transaction = this.prepareTransactionToSend(); - let error; - if (gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKET) { - error = this.validateAmount({ - transaction, - }); - } else { - error = this.validateAmount({ - transaction, - }); - } + const error = this.validateAmount({ + transaction, + }); this.setError(error); if (error) { this.setState({ transactionConfirmed: false, stopUpdateGas: true }); From f33da7b5eca21fb5a72c0c77b72fe56c7b8692f5 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 18 Dec 2024 15:03:41 +0100 Subject: [PATCH 11/12] Remove unused prop --- app/components/Views/confirmations/SendFlow/Confirm/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/components/Views/confirmations/SendFlow/Confirm/index.js b/app/components/Views/confirmations/SendFlow/Confirm/index.js index 0a0c1e30f81..1e1c11ef7ca 100644 --- a/app/components/Views/confirmations/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/SendFlow/Confirm/index.js @@ -956,7 +956,6 @@ class Confirm extends PureComponent { transactionState: { assetType }, navigation, resetTransaction, - gasEstimateType, shouldUseSmartTransaction, transactionSimulationData: { isUpdatedAfterSecurityCheck } = {}, } = this.props; From 63bf323d4d86c72baca3e0b9f90a52165068af36 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 8 Jan 2025 16:23:01 +0100 Subject: [PATCH 12/12] Simplify usemax --- .../Views/confirmations/SendFlow/Amount/index.js | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/app/components/Views/confirmations/SendFlow/Amount/index.js b/app/components/Views/confirmations/SendFlow/Amount/index.js index 0e0f8171eac..cf6319415fe 100644 --- a/app/components/Views/confirmations/SendFlow/Amount/index.js +++ b/app/components/Views/confirmations/SendFlow/Amount/index.js @@ -486,10 +486,6 @@ class Amount extends PureComponent { * Type of gas fee estimate provided by the gas fee controller. */ gasEstimateType: PropTypes.string, - /** - * Boolean that indicates if the max value mode is enabled - */ - maxValueMode: PropTypes.bool, /** * Function that sets the max value mode */ @@ -940,16 +936,11 @@ class Amount extends PureComponent { conversionRate, currentCurrency, ticker, - maxValueMode: currentMaxValueMode, setMaxValueMode, } = this.props; const { internalPrimaryCurrencyIsCrypto } = this.state; - if (useMax) { - setMaxValueMode(true); - } else if (currentMaxValueMode) { - setMaxValueMode(false); - } + setMaxValueMode(useMax ?? false) let inputValueConversion, renderableInputValueConversion, @@ -1579,7 +1570,6 @@ const mapStateToProps = (state, ownProps) => ({ ), swapsIsLive: swapsLivenessSelector(state), chainId: selectChainId(state), - maxValueMode: state.transaction.maxValueMode, }); const mapDispatchToProps = (dispatch) => ({