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

Prevent automatic rejection of confirmations (#13194)

* Prevent automatic rejection of confirmations

Confirmations are now only automatically rejected if a user explicitly
closes the notification window. If we close the window programmatically
because there are no notifications left to show, nothing gets rejected.

This partially avoids a race condition where a confirmation gets
rejected automatically without the user having seen the confirmation
first. This could happen if the confirmation was processed just as the
notification window was being closed.

It's still possible for a confirmation that the user has never seen to
get rejected as a result of the user closing the window. But at least
now it's no longer possible for a confirmation to get rejected in this
manner after the user resolves the last confirmation in the queue.

* Fix bug that prevented automatic closure detection

All windows were being detected as explicit window closures,
essentially just as they were previously, because this variable was
cleared too soon.

* Re-open popup when necessary

After the window is automatically closed, a confirmation may have been
queued up while the window was closing. If so, the popup is now re-
opened.
This commit is contained in:
Mark Stacey 2022-01-05 13:39:19 -03:30 committed by GitHub
parent 139a67fca4
commit 22f931e6b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 91 additions and 42 deletions

View File

@ -218,6 +218,7 @@ function setupController(initState, initLangCode) {
initLangCode,
// platform specific api
platform,
notificationManager,
extension,
getRequestAccountTabIds: () => {
return requestAccountTabIds;
@ -455,6 +456,15 @@ function setupController(initState, initLangCode) {
*/
function updateBadge() {
let label = '';
const count = getUnapprovedTransactionCount();
if (count) {
label = String(count);
}
extension.browserAction.setBadgeText({ text: label });
extension.browserAction.setBadgeBackgroundColor({ color: '#037DD6' });
}
function getUnapprovedTransactionCount() {
const unapprovedTxCount = controller.txController.getUnapprovedTxCount();
const { unapprovedMsgCount } = controller.messageManager;
const { unapprovedPersonalMsgCount } = controller.personalMessageManager;
@ -466,7 +476,7 @@ function setupController(initState, initLangCode) {
const pendingApprovalCount = controller.approvalController.getTotalApprovalCount();
const waitingForUnlockCount =
controller.appStateController.waitingForUnlock.length;
const count =
return (
unapprovedTxCount +
unapprovedMsgCount +
unapprovedPersonalMsgCount +
@ -474,17 +484,19 @@ function setupController(initState, initLangCode) {
unapprovedEncryptionPublicKeyMsgCount +
unapprovedTypedMessagesCount +
pendingApprovalCount +
waitingForUnlockCount;
if (count) {
label = String(count);
}
extension.browserAction.setBadgeText({ text: label });
extension.browserAction.setBadgeBackgroundColor({ color: '#037DD6' });
waitingForUnlockCount
);
}
notificationManager.on(
NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED,
rejectUnapprovedNotifications,
({ automaticallyClosed }) => {
if (!automaticallyClosed) {
rejectUnapprovedNotifications();
} else if (getUnapprovedTransactionCount() > 0) {
triggerUi();
}
},
);
function rejectUnapprovedNotifications() {

View File

@ -22,6 +22,16 @@ export default class NotificationManager extends EventEmitter {
this.platform.addOnRemovedListener(this._onWindowClosed.bind(this));
}
/**
* Mark the notification popup as having been automatically closed.
*
* This lets us differentiate between the cases where we close the
* notification popup v.s. when the user closes the popup window directly.
*/
markAsAutomaticallyClosed() {
this._popupAutomaticallyClosed = true;
}
/**
* Either brings an existing MetaMask notification window into focus, or creates a new notification window. New
* notification windows are given a 'popup' type.
@ -72,7 +82,10 @@ export default class NotificationManager extends EventEmitter {
_onWindowClosed(windowId) {
if (windowId === this._popupId) {
this._popupId = undefined;
this.emit(NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED);
this.emit(NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED, {
automaticallyClosed: this._popupAutomaticallyClosed,
});
this._popupAutomaticallyClosed = undefined;
}
}

View File

@ -131,6 +131,7 @@ export default class MetamaskController extends EventEmitter {
this.opts = opts;
this.extension = opts.extension;
this.platform = opts.platform;
this.notificationManager = opts.notificationManager;
const initState = opts.initState || {};
const version = this.platform.getVersion();
this.recordFirstTimeInfo(initState);
@ -1053,6 +1054,8 @@ export default class MetamaskController extends EventEmitter {
safelistPhishingDomain: this.safelistPhishingDomain.bind(this),
getRequestAccountTabIds: this.getRequestAccountTabIds,
getOpenMetamaskTabsIds: this.getOpenMetamaskTabsIds,
markNotificationPopupAsAutomaticallyClosed: () =>
this.notificationManager.markAsAutomaticallyClosed(),
// primary HD keyring management
addNewAccount: this.addNewAccount.bind(this),

View File

@ -3,8 +3,6 @@ import PropTypes from 'prop-types';
import Button from '../../components/ui/button';
import Identicon from '../../components/ui/identicon';
import TokenBalance from '../../components/ui/token-balance';
import { getEnvironmentType } from '../../../app/scripts/lib/util';
import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../shared/constants/app';
import { isEqualCaseInsensitive } from '../../helpers/utils/util';
export default class ConfirmAddSuggestedToken extends Component {
@ -40,11 +38,8 @@ export default class ConfirmAddSuggestedToken extends Component {
if (suggestedAssets.length > 0) {
return;
}
if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
global.platform.closeCurrentWindow();
} else {
history.push(mostRecentOverviewPage);
}
history.push(mostRecentOverviewPage);
}
getTokenName(name, symbol) {

View File

@ -46,6 +46,18 @@ const LEGACY_WEB3_URL =
const INFURA_BLOCKAGE_URL =
'https://metamask.zendesk.com/hc/en-us/articles/360059386712';
function shouldCloseNotificationPopup({
isNotification,
totalUnapprovedCount,
isSigningQRHardwareTransaction,
}) {
return (
isNotification &&
totalUnapprovedCount === 0 &&
!isSigningQRHardwareTransaction
);
}
export default class Home extends PureComponent {
static contextTypes = {
t: PropTypes.func,
@ -68,6 +80,8 @@ export default class Home extends PureComponent {
setShowRestorePromptToFalse: PropTypes.func,
threeBoxLastUpdated: PropTypes.number,
firstPermissionsRequestId: PropTypes.string,
// This prop is used in the `shouldCloseNotificationPopup` function
// eslint-disable-next-line react/no-unused-prop-types
totalUnapprovedCount: PropTypes.number.isRequired,
setConnectedStatusPopoverHasBeenShown: PropTypes.func,
connectedStatusPopoverHasBeenShown: PropTypes.bool,
@ -91,14 +105,17 @@ export default class Home extends PureComponent {
seedPhraseBackedUp: PropTypes.bool.isRequired,
newNetworkAdded: PropTypes.string,
setNewNetworkAdded: PropTypes.func.isRequired,
// This prop is used in the `shouldCloseNotificationPopup` function
// eslint-disable-next-line react/no-unused-prop-types
isSigningQRHardwareTransaction: PropTypes.bool.isRequired,
newCollectibleAddedMessage: PropTypes.string,
setNewCollectibleAddedMessage: PropTypes.func.isRequired,
closeNotificationPopup: PropTypes.func.isRequired,
};
state = {
canShowBlockageNotification: true,
closing: false,
notificationClosing: false,
redirecting: false,
};
@ -106,24 +123,19 @@ export default class Home extends PureComponent {
super(props);
const {
closeNotificationPopup,
firstPermissionsRequestId,
haveSwapsQuotes,
isNotification,
isSigningQRHardwareTransaction,
showAwaitingSwapScreen,
suggestedAssets = [],
swapsFetchParams,
totalUnapprovedCount,
unconfirmedTransactionsCount,
} = this.props;
if (
isNotification &&
totalUnapprovedCount === 0 &&
!isSigningQRHardwareTransaction
) {
this.state.closing = true;
global.platform.closeCurrentWindow();
if (shouldCloseNotificationPopup(props)) {
this.state.notificationClosing = true;
closeNotificationPopup();
} else if (
firstPermissionsRequestId ||
unconfirmedTransactionsCount > 0 ||
@ -141,21 +153,13 @@ export default class Home extends PureComponent {
history,
isNotification,
suggestedAssets = [],
totalUnapprovedCount,
unconfirmedTransactionsCount,
haveSwapsQuotes,
showAwaitingSwapScreen,
swapsFetchParams,
pendingConfirmations,
isSigningQRHardwareTransaction,
} = this.props;
if (
isNotification &&
totalUnapprovedCount === 0 &&
!isSigningQRHardwareTransaction
) {
global.platform.closeCurrentWindow();
} else if (!isNotification && showAwaitingSwapScreen) {
if (!isNotification && showAwaitingSwapScreen) {
history.push(AWAITING_SWAP_ROUTE);
} else if (!isNotification && haveSwapsQuotes) {
history.push(VIEW_QUOTE_ROUTE);
@ -176,18 +180,33 @@ export default class Home extends PureComponent {
this.checkStatusAndNavigate();
}
componentDidUpdate() {
static getDerivedStateFromProps(props) {
if (shouldCloseNotificationPopup(props)) {
return { notificationClosing: true };
}
return null;
}
componentDidUpdate(_prevProps, prevState) {
const {
closeNotificationPopup,
setupThreeBox,
showRestorePrompt,
threeBoxLastUpdated,
threeBoxSynced,
isNotification,
} = this.props;
const { notificationClosing } = this.state;
isNotification && this.checkStatusAndNavigate();
if (threeBoxSynced && showRestorePrompt && threeBoxLastUpdated === null) {
if (notificationClosing && !prevState.notificationClosing) {
closeNotificationPopup();
} else if (isNotification) {
this.checkStatusAndNavigate();
} else if (
threeBoxSynced &&
showRestorePrompt &&
threeBoxLastUpdated === null
) {
setupThreeBox();
}
}
@ -428,7 +447,7 @@ export default class Home extends PureComponent {
if (forgottenPassword) {
return <Redirect to={{ pathname: RESTORE_VAULT_ROUTE }} />;
} else if (this.state.closing || this.state.redirecting) {
} else if (this.state.notificationClosing || this.state.redirecting) {
return null;
}

View File

@ -22,6 +22,7 @@ import {
} from '../../selectors';
import {
closeNotificationPopup,
restoreFromThreeBox,
turnThreeBoxSyncingOn,
getThreeBoxLastUpdated,
@ -127,6 +128,7 @@ const mapStateToProps = (state) => {
};
const mapDispatchToProps = (dispatch) => ({
closeNotificationPopup: () => closeNotificationPopup(),
turnThreeBoxSyncingOn: () => dispatch(turnThreeBoxSyncingOn()),
setupThreeBox: () => {
dispatch(getThreeBoxLastUpdated()).then((lastUpdated) => {

View File

@ -955,7 +955,7 @@ export function cancelTxs(txDataList) {
});
} finally {
if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
global.platform.closeCurrentWindow();
closeNotificationPopup();
} else {
dispatch(hideLoadingIndication());
}
@ -1775,7 +1775,7 @@ export function closeCurrentNotificationWindow() {
getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION &&
!hasUnconfirmedTransactions(getState())
) {
global.platform.closeCurrentWindow();
closeNotificationPopup();
}
};
}
@ -3007,6 +3007,11 @@ export function getGasFeeTimeEstimate(maxPriorityFeePerGas, maxFeePerGas) {
);
}
export async function closeNotificationPopup() {
await promisifiedBackground.markNotificationPopupAsAutomaticallyClosed();
global.platform.closeCurrentWindow();
}
// MetaMetrics
/**
* @typedef {import('../../shared/constants/metametrics').MetaMetricsEventPayload} MetaMetricsEventPayload