Skip to content

Commit

Permalink
Allow submission of transactions with dapp suggested gas fees, while …
Browse files Browse the repository at this point in the history
…estimates are loading (MetaMask#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
  • Loading branch information
danjm authored and ryanml committed Aug 17, 2021
1 parent da42d60 commit 17ae98a
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 21 deletions.
20 changes: 20 additions & 0 deletions shared/modules/transaction.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,23 @@ export function isLegacyTransaction(transaction) {
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)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { GAS_ESTIMATE_TYPES } from '../../../../shared/constants/gas';
import { getGasFormErrorText } from '../../../helpers/constants/gas';
import { checkNetworkAndAccountSupports1559 } from '../../../selectors';
import { getIsGasEstimatesLoading } from '../../../ducks/metamask/metamask';
import { getGasLoadingAnimationIsShowing } from '../../../ducks/app/app';

export default function AdvancedGasControls({
gasEstimateType,
Expand All @@ -25,19 +24,12 @@ export default function AdvancedGasControls({
maxFeeFiat,
gasErrors,
minimumGasLimit,
estimateToUse,
}) {
const t = useContext(I18nContext);
const networkAndAccountSupport1559 = useSelector(
checkNetworkAndAccountSupports1559,
);
const isGasEstimatesLoading = useSelector(getIsGasEstimatesLoading);
const isGasLoadingAnimationIsShowing = useSelector(
getGasLoadingAnimationIsShowing,
);
const disableFormFields =
estimateToUse !== 'custom' &&
(isGasEstimatesLoading || isGasLoadingAnimationIsShowing);

const showFeeMarketFields =
networkAndAccountSupport1559 &&
Expand Down Expand Up @@ -82,7 +74,6 @@ export default function AdvancedGasControls({
? getGasFormErrorText(gasErrors.maxPriorityFee, t)
: null
}
disabled={disableFormFields}
/>
<FormField
titleText={t('maxFee')}
Expand All @@ -100,7 +91,6 @@ export default function AdvancedGasControls({
? getGasFormErrorText(gasErrors.maxFee, t)
: null
}
disabled={disableFormFields}
/>
</>
) : (
Expand All @@ -120,7 +110,6 @@ export default function AdvancedGasControls({
? getGasFormErrorText(gasErrors.gasPrice, t)
: null
}
disabled={disableFormFields}
/>
</>
)}
Expand All @@ -143,5 +132,4 @@ AdvancedGasControls.propTypes = {
maxFeeFiat: PropTypes.string,
gasErrors: PropTypes.object,
minimumGasLimit: PropTypes.number,
estimateToUse: PropTypes.bool,
};
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export default function EditGasDisplay({
balanceError,
estimatesUnavailableWarning,
hasGasErrors,
txParamsHaveBeenCustomized,
}) {
const t = useContext(I18nContext);
const isMainnet = useSelector(getIsMainnet);
Expand Down Expand Up @@ -96,7 +97,9 @@ export default function EditGasDisplay({
dappSuggestedAndTxParamGasFeesAreTheSame,
);

const showTopError = balanceError || estimatesUnavailableWarning;
const showTopError =
(balanceError || estimatesUnavailableWarning) &&
(!isGasEstimatesLoading || txParamsHaveBeenCustomized);
const radioButtonsEnabled =
networkAndAccountSupport1559 &&
gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKET &&
Expand All @@ -120,7 +123,7 @@ export default function EditGasDisplay({
/>
</div>
)}
{showTopError && !isGasEstimatesLoading && (
{showTopError && (
<div className="edit-gas-display__warning">
<ErrorMessage errorKey={errorKey} />
</div>
Expand Down Expand Up @@ -266,7 +269,6 @@ export default function EditGasDisplay({
gasErrors={gasErrors}
onManualChange={onManualChange}
minimumGasLimit={minimumGasLimit}
estimateToUse={estimateToUse}
/>
)}
</div>
Expand Down Expand Up @@ -317,4 +319,5 @@ EditGasDisplay.propTypes = {
balanceError: PropTypes.bool,
estimatesUnavailableWarning: PropTypes.bool,
hasGasErrors: PropTypes.bool,
txParamsHaveBeenCustomized: PropTypes.bool,
};
2 changes: 2 additions & 0 deletions ui/components/app/edit-gas-display/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,7 @@

&__warning {
margin-bottom: 24px;
position: relative;
margin-top: 4px;
}
}
16 changes: 13 additions & 3 deletions ui/components/app/edit-gas-popover/edit-gas-popover.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
import { useDispatch, useSelector } from 'react-redux';
import { useGasFeeInputs } from '../../../hooks/useGasFeeInputs';
import { getGasLoadingAnimationIsShowing } from '../../../ducks/app/app';

import { txParamsAreDappSuggested } from '../../../../shared/modules/transaction.utils';
import { EDIT_GAS_MODES, GAS_LIMITS } from '../../../../shared/constants/gas';

import {
Expand Down Expand Up @@ -93,6 +93,9 @@ export default function EditGasPopover({
estimatedBaseFee,
} = useGasFeeInputs(defaultEstimateToUse, transaction, minimumGasLimit, mode);

const txParamsHaveBeenCustomized =
estimateToUse === 'custom' || txParamsAreDappSuggested(transaction);

/**
* Temporary placeholder, this should be managed by the parent component but
* we will be extracting this component from the hard to maintain modal/
Expand Down Expand Up @@ -129,11 +132,17 @@ export default function EditGasPopover({
gasPrice: decGWEIToHexWEI(gasPrice),
};

const cleanTransactionParams = { ...transaction.txParams };

if (networkAndAccountSupport1559) {
delete cleanTransactionParams.gasPrice;
}

const updatedTxMeta = {
...transaction,
userFeeLevel: estimateToUse || 'custom',
txParams: {
...transaction.txParams,
...cleanTransactionParams,
...newGasSettings,
},
};
Expand Down Expand Up @@ -212,7 +221,7 @@ export default function EditGasPopover({
hasGasErrors ||
balanceError ||
((isGasEstimatesLoading || gasLoadingAnimationIsShowing) &&
!estimateToUse === 'custom')
!txParamsHaveBeenCustomized)
}
>
{footerButtonText}
Expand Down Expand Up @@ -262,6 +271,7 @@ export default function EditGasPopover({
balanceError={balanceError}
estimatesUnavailableWarning={estimatesUnavailableWarning}
hasGasErrors={hasGasErrors}
txParamsHaveBeenCustomized={txParamsHaveBeenCustomized}
{...editGasDisplayProps}
/>
</>
Expand Down
5 changes: 4 additions & 1 deletion ui/hooks/useGasFeeInputs.js
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,10 @@ export function useGasFeeInputs(
if (networkAndAccountSupports1559) {
estimatesUnavailableWarning = true;
}
if (bnLessThanEqualTo(gasPriceToUse, 0)) {
if (
(!networkAndAccountSupports1559 || transaction?.txParams?.gasPrice) &&
bnLessThanEqualTo(gasPriceToUse, 0)
) {
gasErrors.gasPrice = GAS_FORM_ERRORS.GAS_PRICE_TOO_LOW;
}
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ import {
getPreferences,
} from '../../selectors';
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 {
updateTransactionGasFees,
Expand Down Expand Up @@ -155,7 +158,9 @@ const mapStateToProps = (state, ownProps) => {
const isEthGasPrice = getIsEthGasPriceFetched(state);
const noGasPrice = !supportsEIP1599 && getNoGasPriceFetched(state);
const { useNativeCurrencyAsPrimaryCurrency } = getPreferences(state);
const gasFeeIsCustom = fullTxData.userFeeLevel === 'custom';
const gasFeeIsCustom =
fullTxData.userFeeLevel === 'custom' ||
txParamsAreDappSuggested(fullTxData);

return {
balance,
Expand Down

0 comments on commit 17ae98a

Please sign in to comment.