1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-10-22 19:26:13 +02:00

fix: speedup cancellation (#10579)

fixes #7305
This commit is contained in:
Shane 2021-03-12 11:26:07 -08:00 committed by GitHub
parent 84b1379b40
commit b21cc5660f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 193 additions and 272 deletions

View File

@ -385,7 +385,7 @@ export default class TransactionController extends EventEmitter {
* @param {string} [customGasPrice] - the hex value to use for the cancel transaction
* @returns {txMeta}
*/
async createCancelTransaction(originalTxId, customGasPrice) {
async createCancelTransaction(originalTxId, customGasPrice, customGasLimit) {
const originalTxMeta = this.txStateManager.getTx(originalTxId);
const { txParams } = originalTxMeta;
const { gasPrice: lastGasPrice, from, nonce } = txParams;
@ -398,7 +398,7 @@ export default class TransactionController extends EventEmitter {
from,
to: from,
nonce,
gas: '0x5208',
gas: customGasLimit || '0x5208',
value: '0x0',
gasPrice: newGasPrice,
},

View File

@ -1858,10 +1858,11 @@ export default class MetamaskController extends EventEmitter {
* @param {string} [customGasPrice] - the hex value to use for the cancel transaction
* @returns {Object} MetaMask state
*/
async createCancelTransaction(originalTxId, customGasPrice) {
async createCancelTransaction(originalTxId, customGasPrice, customGasLimit) {
await this.txController.createCancelTransaction(
originalTxId,
customGasPrice,
customGasLimit,
);
const state = await this.getState();
return state;

View File

@ -61,7 +61,7 @@ const mapStateToProps = (state, ownProps) => {
const { currentNetworkTxList, send } = state.metamask;
const { modalState: { props: modalProps } = {} } = state.appState.modal || {};
const { txData = {} } = modalProps || {};
const { transaction = {} } = ownProps;
const { transaction = {}, onSubmit } = ownProps;
const selectedTransaction = currentNetworkTxList.find(
({ id }) => id === (transaction.id || txData.id),
);
@ -77,7 +77,8 @@ const mapStateToProps = (state, ownProps) => {
value: sendToken ? '0x0' : send.amount,
};
const { gasPrice: currentGasPrice, gas: currentGasLimit, value } = txParams;
const { gasPrice: currentGasPrice, gas: currentGasLimit } = txParams;
const value = ownProps.transaction?.txParams?.value || txParams.value;
const customModalGasPriceInHex = getCustomGasPrice(state) || currentGasPrice;
const customModalGasLimitInHex =
getCustomGasLimit(state) || currentGasLimit || '0x5208';
@ -175,6 +176,7 @@ const mapStateToProps = (state, ownProps) => {
tokenBalance: getTokenBalance(state),
conversionRate,
value,
onSubmit,
};
};
@ -253,6 +255,12 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
...otherDispatchProps,
...ownProps,
onSubmit: (gasLimit, gasPrice) => {
if (ownProps.onSubmit) {
dispatchHideSidebar();
dispatchCancelAndClose();
ownProps.onSubmit(gasLimit, gasPrice);
return;
}
if (isConfirm) {
const updatedTx = {
...transaction,

View File

@ -1,9 +1,7 @@
import assert from 'assert';
import proxyquire from 'proxyquire';
import sinon from 'sinon';
import { TRANSACTION_STATUSES } from '../../../../../../../shared/constants/transaction';
let mapStateToProps;
let mapDispatchToProps;
let mergeProps;
@ -25,8 +23,7 @@ const sendActionSpies = {
proxyquire('../gas-modal-page-container.container.js', {
'react-redux': {
connect: (ms, md, mp) => {
mapStateToProps = ms;
connect: (_, md, mp) => {
mapDispatchToProps = md;
mergeProps = mp;
return () => ({});
@ -48,225 +45,6 @@ proxyquire('../gas-modal-page-container.container.js', {
});
describe('gas-modal-page-container container', function () {
describe('mapStateToProps()', function () {
it('should map the correct properties to props', function () {
const baseMockState = {
appState: {
modal: {
modalState: {
props: {
hideBasic: true,
txData: {
id: 34,
},
},
},
},
},
metamask: {
send: {
gasLimit: '16',
gasPrice: '32',
amount: '64',
maxModeOn: false,
},
currentCurrency: 'abc',
conversionRate: 50,
usdConversionRate: 123,
preferences: {
showFiatInTestnets: false,
},
provider: {
type: 'mainnet',
chainId: '0x1',
},
currentNetworkTxList: [
{
id: 34,
txParams: {
gas: '0x1600000',
gasPrice: '0x3200000',
value: '0x640000000000000',
},
},
],
},
gas: {
basicEstimates: {
blockTime: 12,
safeLow: 2,
},
customData: {
limit: 'aaaaaaaa',
price: 'ffffffff',
},
priceAndTimeEstimates: [
{ gasprice: 3, expectedTime: 31 },
{ gasprice: 4, expectedTime: 62 },
{ gasprice: 5, expectedTime: 93 },
{ gasprice: 6, expectedTime: 124 },
],
},
confirmTransaction: {
txData: {
txParams: {
gas: '0x1600000',
gasPrice: '0x3200000',
value: '0x640000000000000',
},
},
},
};
const baseExpectedResult = {
balance: '0x0',
isConfirm: true,
customGasPrice: 4.294967295,
customGasLimit: 2863311530,
newTotalFiat: '637.41',
conversionRate: 50,
customModalGasLimitInHex: 'aaaaaaaa',
customModalGasPriceInHex: 'ffffffff',
customPriceIsExcessive: false,
customGasTotal: 'aaaaaaa955555556',
customPriceIsSafe: true,
gasPriceButtonGroupProps: {
buttonDataLoading: 'mockBasicGasEstimateLoadingStatus:4',
defaultActiveButtonIndex: 'mockRenderableBasicEstimateData:4ffffffff',
gasButtonInfo: 'mockRenderableBasicEstimateData:4',
},
hideBasic: true,
infoRowProps: {
originalTotalFiat: '637.41',
originalTotalEth: '12.748189 ETH',
newTotalFiat: '637.41',
newTotalEth: '12.748189 ETH',
sendAmount: '0.45036 ETH',
transactionFee: '12.297829 ETH',
},
insufficientBalance: true,
isSpeedUp: false,
isRetry: false,
txId: 34,
isMainnet: true,
maxModeOn: false,
sendToken: null,
tokenBalance: '0x0',
transaction: {
id: 34,
},
value: '0x640000000000000',
};
const baseMockOwnProps = { transaction: { id: 34 } };
const tests = [
{
mockState: baseMockState,
expectedResult: baseExpectedResult,
mockOwnProps: baseMockOwnProps,
},
{
mockState: {
...baseMockState,
metamask: {
...baseMockState.metamask,
balance: '0xfffffffffffffffffffff',
},
},
expectedResult: {
...baseExpectedResult,
balance: '0xfffffffffffffffffffff',
insufficientBalance: false,
},
mockOwnProps: baseMockOwnProps,
},
{
mockState: baseMockState,
mockOwnProps: {
...baseMockOwnProps,
transaction: { id: 34, status: TRANSACTION_STATUSES.SUBMITTED },
},
expectedResult: {
...baseExpectedResult,
isSpeedUp: true,
transaction: { id: 34 },
},
},
{
mockState: {
...baseMockState,
metamask: {
...baseMockState.metamask,
preferences: {
...baseMockState.metamask.preferences,
showFiatInTestnets: false,
},
provider: {
...baseMockState.metamask.provider,
type: 'rinkeby',
chainId: '0x4',
},
},
},
mockOwnProps: baseMockOwnProps,
expectedResult: {
...baseExpectedResult,
infoRowProps: {
...baseExpectedResult.infoRowProps,
newTotalFiat: '',
},
isMainnet: false,
},
},
{
mockState: {
...baseMockState,
metamask: {
...baseMockState.metamask,
preferences: {
...baseMockState.metamask.preferences,
showFiatInTestnets: true,
},
provider: {
...baseMockState.metamask.provider,
type: 'rinkeby',
chainId: '0x4',
},
},
},
mockOwnProps: baseMockOwnProps,
expectedResult: {
...baseExpectedResult,
isMainnet: false,
},
},
{
mockState: {
...baseMockState,
metamask: {
...baseMockState.metamask,
preferences: {
...baseMockState.metamask.preferences,
showFiatInTestnets: true,
},
provider: {
...baseMockState.metamask.provider,
type: 'mainnet',
chainId: '0x1',
},
},
},
mockOwnProps: baseMockOwnProps,
expectedResult: baseExpectedResult,
},
];
let result;
tests.forEach(({ mockState, mockOwnProps, expectedResult }) => {
result = mapStateToProps(mockState, mockOwnProps);
assert.deepStrictEqual(result, expectedResult);
});
});
});
describe('mapDispatchToProps()', function () {
let dispatchSpy;
let mapDispatchToPropsObject;

View File

@ -1,47 +1,40 @@
import { connect } from 'react-redux';
import { compose } from 'redux';
import { multiplyCurrencies } from '../../../../helpers/utils/conversion-util';
import withModalProps from '../../../../helpers/higher-order-components/with-modal-props';
import { showModal, createCancelTransaction } from '../../../../store/actions';
import { getHexGasTotal } from '../../../../helpers/utils/confirm-tx.util';
import { addHexPrefix } from '../../../../../../app/scripts/lib/util';
import CancelTransaction from './cancel-transaction.component';
const mapStateToProps = (state, ownProps) => {
const { metamask } = state;
const { transactionId, originalGasPrice } = ownProps;
const {
transactionId,
originalGasPrice,
newGasFee,
defaultNewGasPrice,
gasLimit,
} = ownProps;
const { currentNetworkTxList } = metamask;
const transaction = currentNetworkTxList.find(
({ id }) => id === transactionId,
);
const transactionStatus = transaction ? transaction.status : '';
const defaultNewGasPrice = addHexPrefix(
multiplyCurrencies(originalGasPrice, 1.1, {
toNumericBase: 'hex',
multiplicandBase: 16,
multiplierBase: 10,
}),
);
const newGasFee = getHexGasTotal({
gasPrice: defaultNewGasPrice,
gasLimit: '0x5208',
});
return {
transactionId,
transactionStatus,
originalGasPrice,
defaultNewGasPrice,
newGasFee,
gasLimit,
};
};
const mapDispatchToProps = (dispatch) => {
return {
createCancelTransaction: (txId, customGasPrice) => {
return dispatch(createCancelTransaction(txId, customGasPrice));
createCancelTransaction: (txId, customGasPrice, customGasLimit) => {
return dispatch(
createCancelTransaction(txId, customGasPrice, customGasLimit),
);
},
showTransactionConfirmedModal: () =>
dispatch(showModal({ name: 'TRANSACTION_CONFIRMED' })),
@ -49,7 +42,12 @@ const mapDispatchToProps = (dispatch) => {
};
const mergeProps = (stateProps, dispatchProps, ownProps) => {
const { transactionId, defaultNewGasPrice, ...restStateProps } = stateProps;
const {
transactionId,
defaultNewGasPrice,
gasLimit,
...restStateProps
} = stateProps;
// eslint-disable-next-line no-shadow
const { createCancelTransaction, ...restDispatchProps } = dispatchProps;
@ -58,7 +56,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
...restDispatchProps,
...ownProps,
createCancelTransaction: () =>
createCancelTransaction(transactionId, defaultNewGasPrice),
createCancelTransaction(transactionId, defaultNewGasPrice, gasLimit),
};
};

View File

@ -30,12 +30,12 @@ export default class Sidebar extends Component {
renderSidebarContent() {
const { type, sidebarProps = {} } = this.props;
const { transaction = {} } = sidebarProps;
const { transaction = {}, onSubmit } = sidebarProps;
switch (type) {
case 'customize-gas':
return (
<div className="sidebar-left">
<CustomizeGas transaction={transaction} />
<CustomizeGas transaction={transaction} onSubmit={onSubmit} />
</div>
);
default:

View File

@ -118,13 +118,21 @@ export default function TransactionListItem({
<Button
type="secondary"
rounded
onClick={retryTransaction}
className="transaction-list-item-details__header-button"
onClick={hasCancelled ? cancelTransaction : retryTransaction}
style={hasCancelled ? { width: 'auto' } : null}
>
{t('speedUp')}
{hasCancelled ? t('speedUpCancellation') : t('speedUp')}
</Button>
);
}, [shouldShowSpeedUp, isUnapproved, t, isPending, retryTransaction]);
}, [
shouldShowSpeedUp,
isUnapproved,
t,
isPending,
retryTransaction,
hasCancelled,
cancelTransaction,
]);
return (
<>

View File

@ -7,6 +7,7 @@ import { getConversionRate, getSelectedAccount } from '../../selectors';
import { useCancelTransaction } from '../useCancelTransaction';
import { showModal } from '../../store/actions';
import { increaseLastGasPrice } from '../../helpers/utils/confirm-tx.util';
import * as actionConstants from '../../store/actionConstants';
describe('useCancelTransaction', function () {
let useSelector;
@ -46,7 +47,7 @@ describe('useCancelTransaction', function () {
);
assert.strictEqual(result.current[0], false);
});
it(`should return a function that kicks off cancellation for id ${transactionId}`, function () {
it(`should return a function that opens the gas sidebar onsubmit kicks off cancellation for id ${transactionId}`, function () {
const { result } = renderHook(() =>
useCancelTransaction(transactionGroup),
);
@ -55,12 +56,35 @@ describe('useCancelTransaction', function () {
preventDefault: () => undefined,
stopPropagation: () => undefined,
});
const dispatchAction = dispatch.args;
// calls customize-gas sidebar
// also check type= customize-gas
assert.strictEqual(
dispatchAction[dispatchAction.length - 1][0].type,
actionConstants.SIDEBAR_OPEN,
);
assert.strictEqual(
dispatchAction[dispatchAction.length - 1][0].value.props.transaction
.id,
transactionId,
);
// call onSubmit myself
dispatchAction[dispatchAction.length - 1][0].value.props.onSubmit(
'0x5208',
'0x1',
);
assert.strictEqual(
dispatch.calledWith(
showModal({
name: 'CANCEL_TRANSACTION',
transactionId,
originalGasPrice,
newGasFee: '0x5208',
defaultNewGasPrice: '0x1',
gasLimit: '0x5208',
}),
),
true,
@ -98,7 +122,7 @@ describe('useCancelTransaction', function () {
);
assert.strictEqual(result.current[0], true);
});
it(`should return a function that kicks off cancellation for id ${transactionId}`, function () {
it(`should return a function that opens the gas sidebar onsubmit kicks off cancellation for id ${transactionId}`, function () {
const { result } = renderHook(() =>
useCancelTransaction(transactionGroup),
);
@ -107,12 +131,31 @@ describe('useCancelTransaction', function () {
preventDefault: () => undefined,
stopPropagation: () => undefined,
});
const dispatchAction = dispatch.args;
assert.strictEqual(
dispatchAction[dispatchAction.length - 1][0].type,
actionConstants.SIDEBAR_OPEN,
);
assert.strictEqual(
dispatchAction[dispatchAction.length - 1][0].value.props.transaction
.id,
transactionId,
);
dispatchAction[dispatchAction.length - 1][0].value.props.onSubmit(
'0x5208',
'0x1',
);
assert.strictEqual(
dispatch.calledWith(
showModal({
name: 'CANCEL_TRANSACTION',
transactionId,
originalGasPrice,
newGasFee: '0x5208',
defaultNewGasPrice: '0x1',
gasLimit: '0x5208',
}),
),
true,

View File

@ -64,6 +64,50 @@ describe('useRetryTransaction', function () {
);
});
it('should handle cancelled or multiple speedup transactions', async function () {
const cancelledTransaction = {
initialTransaction: {
...transactions[0].initialTransaction,
txParams: {
...transactions[0].initialTransaction.txParams,
},
},
primaryTransaction: {
...transactions[0].primaryTransaction,
txParams: {
from: '0xee014609ef9e09776ac5fe00bdbfef57bcdefebb',
gas: '0x5308',
gasPrice: '0x77359400',
nonce: '0x3',
to: '0xabca64466f257793eaa52fcfff5066894b76a149',
value: '0x0',
},
},
transactions: [
{
submittedTime: new Date() - 5001,
},
],
hasRetried: false,
};
const { result } = renderHook(() =>
useRetryTransaction(cancelledTransaction, true),
);
const retry = result.current;
await retry(event);
assert.strictEqual(
dispatch.calledWith(
showSidebar({
transitionName: 'sidebar-left',
type: 'customize-gas',
props: { transaction: cancelledTransaction.primaryTransaction },
}),
),
true,
);
});
after(function () {
sinon.restore();
});

View File

@ -1,12 +1,18 @@
import { useDispatch, useSelector } from 'react-redux';
import { useCallback } from 'react';
import { showModal } from '../store/actions';
import { addHexPrefix } from 'ethereumjs-util';
import { showModal, showSidebar } from '../store/actions';
import { isBalanceSufficient } from '../pages/send/send.utils';
import {
getHexGasTotal,
increaseLastGasPrice,
} from '../helpers/utils/confirm-tx.util';
import { getConversionRate, getSelectedAccount } from '../selectors';
import {
setCustomGasLimit,
setCustomGasPriceForRetry,
} from '../ducks/gas/gas.duck';
import { multiplyCurrencies } from '../helpers/utils/conversion-util';
/**
* Determine whether a transaction can be cancelled and provide a method to
@ -19,27 +25,61 @@ import { getConversionRate, getSelectedAccount } from '../selectors';
* @return {[boolean, Function]}
*/
export function useCancelTransaction(transactionGroup) {
const { primaryTransaction, initialTransaction } = transactionGroup;
const { primaryTransaction } = transactionGroup;
const gasPrice = primaryTransaction.txParams?.gasPrice?.startsWith('-')
? '0x0'
: primaryTransaction.txParams?.gasPrice;
const { id } = initialTransaction;
const transaction = primaryTransaction;
const dispatch = useDispatch();
const selectedAccount = useSelector(getSelectedAccount);
const conversionRate = useSelector(getConversionRate);
const defaultNewGasPrice = addHexPrefix(
multiplyCurrencies(gasPrice, 1.1, {
toNumericBase: 'hex',
multiplicandBase: 16,
multiplierBase: 10,
}),
);
const cancelTransaction = useCallback(
(event) => {
event.stopPropagation();
dispatch(setCustomGasLimit('0x5208'));
dispatch(setCustomGasPriceForRetry(defaultNewGasPrice));
const tx = {
...transaction,
txParams: {
...transaction.txParams,
gas: '0x5208',
value: '0x0',
},
};
return dispatch(
showModal({
name: 'CANCEL_TRANSACTION',
transactionId: id,
originalGasPrice: gasPrice,
showSidebar({
transitionName: 'sidebar-left',
type: 'customize-gas',
props: {
transaction: tx,
onSubmit: (newGasLimit, newGasPrice) => {
const userCustomizedGasTotal = getHexGasTotal({
gasPrice: newGasPrice,
gasLimit: newGasLimit,
});
dispatch(
showModal({
name: 'CANCEL_TRANSACTION',
newGasFee: userCustomizedGasTotal,
transactionId: transaction.id,
defaultNewGasPrice: newGasPrice,
gasLimit: newGasLimit,
}),
);
},
},
}),
);
},
[dispatch, id, gasPrice],
[dispatch, transaction, defaultNewGasPrice],
);
const hasEnoughCancelGas =

View File

@ -16,7 +16,7 @@ import { useMetricEvent } from './useMetricEvent';
* @return {Function}
*/
export function useRetryTransaction(transactionGroup) {
const { primaryTransaction, initialTransaction } = transactionGroup;
const { primaryTransaction } = transactionGroup;
// Signature requests do not have a txParams, but this hook is called indiscriminately
const gasPrice = primaryTransaction.txParams?.gasPrice;
const trackMetricsEvent = useMetricEvent({
@ -34,7 +34,7 @@ export function useRetryTransaction(transactionGroup) {
trackMetricsEvent();
await dispatch(fetchBasicGasEstimates);
const transaction = initialTransaction;
const transaction = primaryTransaction;
const increasedGasPrice = increaseLastGasPrice(gasPrice);
await dispatch(
setCustomGasPriceForRetry(
@ -50,7 +50,7 @@ export function useRetryTransaction(transactionGroup) {
}),
);
},
[dispatch, trackMetricsEvent, initialTransaction, gasPrice],
[dispatch, trackMetricsEvent, gasPrice, primaryTransaction],
);
return retryTransaction;

View File

@ -1483,7 +1483,7 @@ export function clearPendingTokens() {
};
}
export function createCancelTransaction(txId, customGasPrice) {
export function createCancelTransaction(txId, customGasPrice, customGasLimit) {
log.debug('background.cancelTransaction');
let newTxId;
@ -1492,6 +1492,7 @@ export function createCancelTransaction(txId, customGasPrice) {
background.createCancelTransaction(
txId,
customGasPrice,
customGasLimit,
(err, newState) => {
if (err) {
dispatch(displayWarning(err.message));