1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-23 09:52:26 +01:00

Allow submission of transactions with dapp suggested gas fees, while estimates are loading (#11858)

* Allow submission of transactions with dapp suggested gas fees, while estimates are loading

* Allow editing of transactions with dapp suggested feeds, while estimates are still loading

* Ensure that advanced gas is always editible inline when gas is loading

* Ensure that insufficient balance error is shown when gas is loading if the user has customized the gas

* Only set gas price insufficient errors if the current network is non-eip-1559, or the txparams actually have a gas price

* Remove unnecessary param

* lint fix

* ensure insufficient balance warning is showing when loading

* Ensure that eip1559 network transactions do not combined eip1559 and non-eip1559 gas fee properties

* Lint fix
This commit is contained in:
Dan J Miller 2021-08-17 17:38:13 -02:30 committed by GitHub
parent 84036aacb6
commit 29487ef277
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 52 additions and 21 deletions

View File

@ -38,3 +38,23 @@ export function isLegacyTransaction(transaction) {
isHexString(transaction.txParams.gasPrice)) isHexString(transaction.txParams.gasPrice))
); );
} }
/**
* Determine if a transactions gas fees in txParams match those in its dappSuggestedGasFees property
* @param {import("../constants/transaction").TransactionMeta} transaction -
* the transaction to check
* @returns {boolean} true if both the txParams and dappSuggestedGasFees are objects with truthy gas fee properties,
* and those properties are strictly equal
*/
export function txParamsAreDappSuggested(transaction) {
const { gasPrice, maxPriorityFeePerGas, maxFeePerGas } =
transaction?.txParams || {};
return (
(gasPrice && gasPrice === transaction?.dappSuggestedGasFees?.gasPrice) ||
(maxPriorityFeePerGas &&
maxFeePerGas &&
transaction?.dappSuggestedGasFees?.maxPriorityFeePerGas ===
maxPriorityFeePerGas &&
transaction?.dappSuggestedGasFees?.maxFeePerGas === maxFeePerGas)
);
}

View File

@ -8,7 +8,6 @@ import { GAS_ESTIMATE_TYPES } from '../../../../shared/constants/gas';
import { getGasFormErrorText } from '../../../helpers/constants/gas'; import { getGasFormErrorText } from '../../../helpers/constants/gas';
import { checkNetworkAndAccountSupports1559 } from '../../../selectors'; import { checkNetworkAndAccountSupports1559 } from '../../../selectors';
import { getIsGasEstimatesLoading } from '../../../ducks/metamask/metamask'; import { getIsGasEstimatesLoading } from '../../../ducks/metamask/metamask';
import { getGasLoadingAnimationIsShowing } from '../../../ducks/app/app';
export default function AdvancedGasControls({ export default function AdvancedGasControls({
gasEstimateType, gasEstimateType,
@ -25,19 +24,12 @@ export default function AdvancedGasControls({
maxFeeFiat, maxFeeFiat,
gasErrors, gasErrors,
minimumGasLimit, minimumGasLimit,
estimateToUse,
}) { }) {
const t = useContext(I18nContext); const t = useContext(I18nContext);
const networkAndAccountSupport1559 = useSelector( const networkAndAccountSupport1559 = useSelector(
checkNetworkAndAccountSupports1559, checkNetworkAndAccountSupports1559,
); );
const isGasEstimatesLoading = useSelector(getIsGasEstimatesLoading); const isGasEstimatesLoading = useSelector(getIsGasEstimatesLoading);
const isGasLoadingAnimationIsShowing = useSelector(
getGasLoadingAnimationIsShowing,
);
const disableFormFields =
estimateToUse !== 'custom' &&
(isGasEstimatesLoading || isGasLoadingAnimationIsShowing);
const showFeeMarketFields = const showFeeMarketFields =
networkAndAccountSupport1559 && networkAndAccountSupport1559 &&
@ -82,7 +74,6 @@ export default function AdvancedGasControls({
? getGasFormErrorText(gasErrors.maxPriorityFee, t) ? getGasFormErrorText(gasErrors.maxPriorityFee, t)
: null : null
} }
disabled={disableFormFields}
/> />
<FormField <FormField
titleText={t('maxFee')} titleText={t('maxFee')}
@ -100,7 +91,6 @@ export default function AdvancedGasControls({
? getGasFormErrorText(gasErrors.maxFee, t) ? getGasFormErrorText(gasErrors.maxFee, t)
: null : null
} }
disabled={disableFormFields}
/> />
</> </>
) : ( ) : (
@ -120,7 +110,6 @@ export default function AdvancedGasControls({
? getGasFormErrorText(gasErrors.gasPrice, t) ? getGasFormErrorText(gasErrors.gasPrice, t)
: null : null
} }
disabled={disableFormFields}
/> />
</> </>
)} )}
@ -143,5 +132,4 @@ AdvancedGasControls.propTypes = {
maxFeeFiat: PropTypes.string, maxFeeFiat: PropTypes.string,
gasErrors: PropTypes.object, gasErrors: PropTypes.object,
minimumGasLimit: PropTypes.string, minimumGasLimit: PropTypes.string,
estimateToUse: PropTypes.bool,
}; };

View File

@ -67,6 +67,7 @@ export default function EditGasDisplay({
balanceError, balanceError,
estimatesUnavailableWarning, estimatesUnavailableWarning,
hasGasErrors, hasGasErrors,
txParamsHaveBeenCustomized,
}) { }) {
const t = useContext(I18nContext); const t = useContext(I18nContext);
const isMainnet = useSelector(getIsMainnet); const isMainnet = useSelector(getIsMainnet);
@ -96,7 +97,9 @@ export default function EditGasDisplay({
dappSuggestedAndTxParamGasFeesAreTheSame, dappSuggestedAndTxParamGasFeesAreTheSame,
); );
const showTopError = balanceError || estimatesUnavailableWarning; const showTopError =
(balanceError || estimatesUnavailableWarning) &&
(!isGasEstimatesLoading || txParamsHaveBeenCustomized);
const radioButtonsEnabled = const radioButtonsEnabled =
networkAndAccountSupport1559 && networkAndAccountSupport1559 &&
gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKET && gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKET &&
@ -120,7 +123,7 @@ export default function EditGasDisplay({
/> />
</div> </div>
)} )}
{showTopError && !isGasEstimatesLoading && ( {showTopError && (
<div className="edit-gas-display__warning"> <div className="edit-gas-display__warning">
<ErrorMessage errorKey={errorKey} /> <ErrorMessage errorKey={errorKey} />
</div> </div>
@ -270,7 +273,6 @@ export default function EditGasDisplay({
gasErrors={gasErrors} gasErrors={gasErrors}
onManualChange={onManualChange} onManualChange={onManualChange}
minimumGasLimit={minimumGasLimit} minimumGasLimit={minimumGasLimit}
estimateToUse={estimateToUse}
/> />
)} )}
</div> </div>
@ -321,4 +323,5 @@ EditGasDisplay.propTypes = {
balanceError: PropTypes.bool, balanceError: PropTypes.bool,
estimatesUnavailableWarning: PropTypes.bool, estimatesUnavailableWarning: PropTypes.bool,
hasGasErrors: PropTypes.bool, hasGasErrors: PropTypes.bool,
txParamsHaveBeenCustomized: PropTypes.bool,
}; };

View File

@ -64,5 +64,7 @@
&__warning { &__warning {
margin-bottom: 24px; margin-bottom: 24px;
position: relative;
margin-top: 4px;
} }
} }

View File

@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
import { useDispatch, useSelector } from 'react-redux'; import { useDispatch, useSelector } from 'react-redux';
import { useGasFeeInputs } from '../../../hooks/useGasFeeInputs'; import { useGasFeeInputs } from '../../../hooks/useGasFeeInputs';
import { getGasLoadingAnimationIsShowing } from '../../../ducks/app/app'; import { getGasLoadingAnimationIsShowing } from '../../../ducks/app/app';
import { txParamsAreDappSuggested } from '../../../../shared/modules/transaction.utils';
import { EDIT_GAS_MODES, GAS_LIMITS } from '../../../../shared/constants/gas'; import { EDIT_GAS_MODES, GAS_LIMITS } from '../../../../shared/constants/gas';
import { import {
@ -93,6 +93,9 @@ export default function EditGasPopover({
estimatedBaseFee, estimatedBaseFee,
} = useGasFeeInputs(defaultEstimateToUse, transaction, minimumGasLimit, mode); } = useGasFeeInputs(defaultEstimateToUse, transaction, minimumGasLimit, mode);
const txParamsHaveBeenCustomized =
estimateToUse === 'custom' || txParamsAreDappSuggested(transaction);
/** /**
* Temporary placeholder, this should be managed by the parent component but * Temporary placeholder, this should be managed by the parent component but
* we will be extracting this component from the hard to maintain modal/ * we will be extracting this component from the hard to maintain modal/
@ -129,11 +132,17 @@ export default function EditGasPopover({
gasPrice: decGWEIToHexWEI(gasPrice), gasPrice: decGWEIToHexWEI(gasPrice),
}; };
const cleanTransactionParams = { ...transaction.txParams };
if (networkAndAccountSupport1559) {
delete cleanTransactionParams.gasPrice;
}
const updatedTxMeta = { const updatedTxMeta = {
...transaction, ...transaction,
userFeeLevel: estimateToUse || 'custom', userFeeLevel: estimateToUse || 'custom',
txParams: { txParams: {
...transaction.txParams, ...cleanTransactionParams,
...newGasSettings, ...newGasSettings,
}, },
}; };
@ -212,7 +221,7 @@ export default function EditGasPopover({
hasGasErrors || hasGasErrors ||
balanceError || balanceError ||
((isGasEstimatesLoading || gasLoadingAnimationIsShowing) && ((isGasEstimatesLoading || gasLoadingAnimationIsShowing) &&
!estimateToUse === 'custom') !txParamsHaveBeenCustomized)
} }
> >
{footerButtonText} {footerButtonText}
@ -262,6 +271,7 @@ export default function EditGasPopover({
balanceError={balanceError} balanceError={balanceError}
estimatesUnavailableWarning={estimatesUnavailableWarning} estimatesUnavailableWarning={estimatesUnavailableWarning}
hasGasErrors={hasGasErrors} hasGasErrors={hasGasErrors}
txParamsHaveBeenCustomized={txParamsHaveBeenCustomized}
{...editGasDisplayProps} {...editGasDisplayProps}
/> />
</> </>

View File

@ -456,7 +456,10 @@ export function useGasFeeInputs(
if (networkAndAccountSupports1559) { if (networkAndAccountSupports1559) {
estimatesUnavailableWarning = true; estimatesUnavailableWarning = true;
} }
if (bnLessThanEqualTo(gasPriceToUse, 0)) { if (
(!networkAndAccountSupports1559 || transaction?.txParams?.gasPrice) &&
bnLessThanEqualTo(gasPriceToUse, 0)
) {
gasErrors.gasPrice = GAS_FORM_ERRORS.GAS_PRICE_TOO_LOW; gasErrors.gasPrice = GAS_FORM_ERRORS.GAS_PRICE_TOO_LOW;
} }
break; break;

View File

@ -31,7 +31,10 @@ import {
getPreferences, getPreferences,
} from '../../selectors'; } from '../../selectors';
import { getMostRecentOverviewPage } from '../../ducks/history/history'; import { getMostRecentOverviewPage } from '../../ducks/history/history';
import { transactionMatchesNetwork } from '../../../shared/modules/transaction.utils'; import {
transactionMatchesNetwork,
txParamsAreDappSuggested,
} from '../../../shared/modules/transaction.utils';
import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils'; import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils';
import { import {
updateTransactionGasFees, updateTransactionGasFees,
@ -155,7 +158,9 @@ const mapStateToProps = (state, ownProps) => {
const isEthGasPrice = getIsEthGasPriceFetched(state); const isEthGasPrice = getIsEthGasPriceFetched(state);
const noGasPrice = !supportsEIP1599 && getNoGasPriceFetched(state); const noGasPrice = !supportsEIP1599 && getNoGasPriceFetched(state);
const { useNativeCurrencyAsPrimaryCurrency } = getPreferences(state); const { useNativeCurrencyAsPrimaryCurrency } = getPreferences(state);
const gasFeeIsCustom = fullTxData.userFeeLevel === 'custom'; const gasFeeIsCustom =
fullTxData.userFeeLevel === 'custom' ||
txParamsAreDappSuggested(fullTxData);
return { return {
balance, balance,