Skip to content

Commit

Permalink
V12 cherrypick fix infinite loop confirmation notification hardware w…
Browse files Browse the repository at this point in the history
…allet (#25475)

Cherrypicks #25454

Fixes: #25412
Fixes: MetaMask/MetaMask-planning#2677

Merge conflict (veryminor) in:
* `app/scripts/lib/setupSentry.js`
* `ui/pages/home/home.container.js`

Co-authored-by: Alex Donesky <[email protected]>
  • Loading branch information
jiexi and adonesky1 authored Jun 21, 2024
1 parent 1eaeeb0 commit 8c105a0
Show file tree
Hide file tree
Showing 14 changed files with 42 additions and 21 deletions.
3 changes: 3 additions & 0 deletions app/scripts/lib/setupSentry.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,9 @@ export const SENTRY_BACKGROUND_STATE = {
PushPlatformNotificationsController: {
fcmToken: false,
},
QueuedRequestController: {
queuedRequestCount: true,
},
SelectedNetworkController: { domains: false },
SignatureController: {
unapprovedMsgCount: true,
Expand Down
1 change: 1 addition & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2276,6 +2276,7 @@ export default class MetamaskController extends EventEmitter {
AuthenticationController: this.authenticationController,
UserStorageController: this.userStorageController,
MetamaskNotificationsController: this.metamaskNotificationsController,
QueuedRequestController: this.queuedRequestController,
PushPlatformNotificationsController:
this.pushPlatformNotificationsController,
...resetOnRestartStore,
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/default-fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ function defaultFixture(inputChainId = CHAIN_IDS.LOCALHOST) {
useMultiAccountBalanceChecker: true,
useRequestQueue: true,
},
QueuedRequestController: {
queuedRequestCount: 0,
},
SelectedNetworkController: {
domains: {},
},
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ function onboardingFixture() {
useMultiAccountBalanceChecker: true,
useRequestQueue: true,
},
QueuedRequestController: {
queuedRequestCount: 0,
},
SelectedNetworkController: {
domains: {},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@
"selectedAddress": "string"
},
"PushPlatformNotificationsController": { "fcmToken": "string" },
"QueuedRequestController": {
"queuedRequestCount": 0
},
"SelectedNetworkController": { "domains": "object" },
"SignatureController": {
"unapprovedMsgs": "object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"nftsDropdownState": {},
"termsOfUseLastAgreed": "number",
"qrHardware": {},
"queuedRequestCount": 0,
"usedNetworks": { "0x1": true, "0x5": true, "0x539": true },
"snapsInstallPrivacyWarningShown": true,
"surveyLinkLastClickedOrClosed": "object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@
"useMultiAccountBalanceChecker": true,
"useRequestQueue": true
},
"QueuedRequestController": {
"queuedRequestCount": 0
},
"SelectedNetworkController": { "domains": "object" },
"SmartTransactionsController": {
"smartTransactionsState": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@
"useMultiAccountBalanceChecker": true,
"useRequestQueue": true
},
"QueuedRequestController": {
"queuedRequestCount": 0
},
"SelectedNetworkController": { "domains": "object" },
"SmartTransactionsController": {
"smartTransactionsState": {
Expand Down
6 changes: 4 additions & 2 deletions ui/pages/home/home.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,19 @@ import FlaskHomeFooter from './flask/flask-home-footer.component';

function shouldCloseNotificationPopup({
isNotification,
totalUnapprovedCount,
totalUnapprovedAndQueuedRequestCount,
hasApprovalFlows,
isSigningQRHardwareTransaction,
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
waitForConfirmDeepLinkDialog,
institutionalConnectRequests,
///: END:ONLY_INCLUDE_IF
}) {
// we can't use totalUnapproved because there are also queued requests

let shouldClose =
isNotification &&
totalUnapprovedCount === 0 &&
totalUnapprovedAndQueuedRequestCount === 0 &&
!hasApprovalFlows &&
!isSigningQRHardwareTransaction;

Expand Down
5 changes: 5 additions & 0 deletions ui/pages/home/home.container.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
getNewTokensImportedError,
hasPendingApprovals,
getSelectedInternalAccount,
getQueuedRequestCount,
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
getAccountType,
///: END:ONLY_INCLUDE_IF
Expand Down Expand Up @@ -112,6 +113,9 @@ const mapStateToProps = (state) => {
const { address: selectedAddress } = getSelectedInternalAccount(state);
const { forgottenPassword } = metamask;
const totalUnapprovedCount = getTotalUnapprovedCount(state);
const queuedRequestCount = getQueuedRequestCount(state);
const totalUnapprovedAndQueuedRequestCount =
totalUnapprovedCount + queuedRequestCount;
const swapsEnabled = getSwapsFeatureIsLive(state);
const pendingConfirmations = getUnapprovedTemplatedConfirmations(state);
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
Expand Down Expand Up @@ -173,6 +177,7 @@ const mapStateToProps = (state) => {
selectedAddress,
firstPermissionsRequestId,
totalUnapprovedCount,
totalUnapprovedAndQueuedRequestCount,
participateInMetaMetrics,
hasApprovalFlows: getApprovalFlows(state)?.length > 0,
connectedStatusPopoverHasBeenShown,
Expand Down
2 changes: 2 additions & 0 deletions ui/pages/routes/routes.component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ describe('toast display', () => {
approvalFlows: [],
completedOnboarding: true,
usedNetworks: [],
pendingApprovals: {},
pendingApprovalCount: 0,
swapsState: { swapsFeatureIsLive: true },
newPrivacyPolicyToastShownDate: date,
},
Expand Down
4 changes: 4 additions & 0 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,10 @@ export function getTotalUnapprovedCount(state) {
return state.metamask.pendingApprovalCount ?? 0;
}

export function getQueuedRequestCount(state) {
return state.metamask.queuedRequestCount ?? 0;
}

export function getTotalUnapprovedMessagesCount(state) {
const {
unapprovedMsgCount = 0,
Expand Down
22 changes: 5 additions & 17 deletions ui/selectors/transactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -597,22 +597,6 @@ export const submittedPendingTransactionsSelector = createSelector(
),
);

const hasUnapprovedTransactionsInCurrentNetwork = (state) => {
const unapprovedTxs = getUnapprovedTransactions(state);
const unapprovedTxRequests = getApprovalRequestsByType(
state,
ApprovalType.Transaction,
);

const chainId = getCurrentChainId(state);

const filteredUnapprovedTxInCurrentNetwork = unapprovedTxRequests.filter(
({ id }) => unapprovedTxs[id] && unapprovedTxs[id].chainId === chainId,
);

return filteredUnapprovedTxInCurrentNetwork.length > 0;
};

const TRANSACTION_APPROVAL_TYPES = [
ApprovalType.EthDecrypt,
ApprovalType.EthGetEncryptionPublicKey,
Expand All @@ -622,8 +606,12 @@ const TRANSACTION_APPROVAL_TYPES = [
];

export function hasTransactionPendingApprovals(state) {
const unapprovedTxRequests = getApprovalRequestsByType(
state,
ApprovalType.Transaction,
);
return (
hasUnapprovedTransactionsInCurrentNetwork(state) ||
unapprovedTxRequests.length > 0 ||
hasPendingApprovals(state, TRANSACTION_APPROVAL_TYPES)
);
}
4 changes: 2 additions & 2 deletions ui/selectors/transactions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,10 @@ describe('Transaction Selectors', () => {
expect(result).toBe(true);
});

it('should return false if there is a pending transaction on different network', () => {
it('should return true if there is a pending transaction on different network', () => {
mockedState.metamask.transactions[0].chainId = 'differentChainId';
const result = hasTransactionPendingApprovals(mockedState);
expect(result).toBe(false);
expect(result).toBe(true);
});

it.each([
Expand Down

0 comments on commit 8c105a0

Please sign in to comment.