Skip to content

Commit 7b4433e

Browse files
authored
fix: reject pending confirmations when app locks (#26905)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** If the user is on a send confirmation and the app goes idle until the device and MetaMask lock, after unlocking they no longer see that confirmation. If they then start a **new** send, the UI can show the **previous** confirmation instead of the new one, because the old approval was never rejected and remains first in the pending list. **Solution** When the app locks, reject all pending approvals by calling `ApprovalController.clear(providerErrors.userRejectedRequest())` in the lock saga, before navigating to the lock screen. That way there are no stale confirmations after unlock, and any new send shows the correct confirmation. **Changes** - **`app/store/sagas/index.ts`**: In `appLockStateMachine`, after handling `LOCKED_APP`, clear pending approvals via `Engine.context.ApprovalController.clear(...)` inside try/catch, then navigate to `LOCK_SCREEN`. Log and ignore errors so navigation still runs. - **`app/store/sagas/sagas.test.ts`**: Add `ApprovalController` with `clear` to the Engine mock; add tests that clear is called with `userRejectedRequest()` when the app locks and that navigation to `LOCK_SCREEN` still happens when `clear` throws. - <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Fixed issue of confirmation not rejecting when app locks ## **Related issues** Fixes: #26320 ## **Manual testing steps** ```gherkin Feature: Transaction Confirmation Persistence After Lock Scenario: Stale confirmation displayed after device lock timeout and new transaction Given the user has MetaMask open and unlocked on the home screen # First transaction When user initiates a send transaction And user reaches the confirmation screen # Lock timeout And user allows the phone to idle until device and MetaMask lock And user unlocks the phone And user unlocks MetaMask Then the confirmation screen should no longer be open # Second transaction - bug occurs When user initiates a different send transaction And user reaches the confirmation screen Then the confirmation shown should be for the previous transaction instead of the current one ``` ## **Screenshots/Recordings** [reject-approval-app-locks.webm](https://github.com/user-attachments/assets/ed331559-bf7a-452b-8688-7014dd4bff34) <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes approval/confirmation lifecycle by clearing all pending approvals on app lock, which could inadvertently reject legitimate in-flight requests if triggered unexpectedly. Guarded with try/catch and covered by new saga tests, but behavior impacts transaction confirmations. > > **Overview** > Prevents stale transaction/permission confirmations after unlocking by clearing any pending approvals when `UserActionType.LOCKED_APP` fires, rejecting them with `providerErrors.userRejectedRequest()` before navigating to `Routes.LOCK_SCREEN`. > > Updates saga tests to mock `ApprovalController.clear` and assert it is invoked on lock, and that navigation to the lock screen still occurs even if clearing approvals throws. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2f1c2d3. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 5eeba88 commit 7b4433e

2 files changed

Lines changed: 45 additions & 0 deletions

File tree

app/store/sagas/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { rewardsBulkLinkSaga } from './rewardsBulkLinkAccountGroups';
3434
import Authentication from '../../core/Authentication';
3535
import { AppState, AppStateStatus } from 'react-native';
3636
import trackErrorAsAnalytics from '../../util/metrics/TrackError/trackErrorAsAnalytics';
37+
import { providerErrors } from '@metamask/rpc-errors';
3738

3839
/**
3940
* Creates a channel to listen to app state changes.
@@ -109,6 +110,19 @@ export function* appLockStateMachine() {
109110
while (true) {
110111
yield take(UserActionType.LOCKED_APP);
111112

113+
// Reject any pending confirmations so the user doesn't see a stale confirmation after unlock.
114+
try {
115+
const { ApprovalController } = Engine.context;
116+
if (ApprovalController) {
117+
ApprovalController.clear(providerErrors.userRejectedRequest());
118+
}
119+
} catch (error) {
120+
Logger.error(
121+
error as Error,
122+
'Failed to reject pending approvals on app lock',
123+
);
124+
}
125+
112126
// Navigate to lock screen.
113127
NavigationService.navigation?.navigate(Routes.LOCK_SCREEN);
114128

app/store/sagas/sagas.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import WC2Manager from '../../core/WalletConnect/WalletConnectV2';
2525
import Authentication from '../../core/Authentication';
2626
import AppConstants from '../../core/AppConstants';
2727
import trackErrorAsAnalytics from '../../util/metrics/TrackError/trackErrorAsAnalytics';
28+
import { providerErrors } from '@metamask/rpc-errors';
2829

2930
const mockNavigate = jest.fn();
3031
const mockReset = jest.fn();
@@ -75,6 +76,9 @@ jest.mock('../../core/Engine', () => ({
7576
AccountsController: {
7677
updateAccounts: jest.fn(),
7778
},
79+
ApprovalController: {
80+
clear: jest.fn(),
81+
},
7882
RemoteFeatureFlagController: {
7983
state: {
8084
remoteFeatureFlags: {
@@ -344,9 +348,13 @@ describe('appStateListenerTask', () => {
344348
});
345349

346350
describe('appLockStateMachine', () => {
351+
const mockApprovalControllerClear = Engine.context.ApprovalController
352+
.clear as jest.Mock;
353+
347354
beforeEach(() => {
348355
mockNavigate.mockClear();
349356
mockReset.mockClear();
357+
mockApprovalControllerClear.mockClear();
350358
});
351359

352360
it('forks appStateListenerTask and navigates to LockScreen when app is locked', async () => {
@@ -359,6 +367,29 @@ describe('appLockStateMachine', () => {
359367
// Verify navigation to LockScreen
360368
expect(mockNavigate).toHaveBeenCalledWith(Routes.LOCK_SCREEN);
361369
});
370+
371+
it('clears pending approvals via ApprovalController.clear when app is locked', async () => {
372+
await expectSaga(appLockStateMachine)
373+
.dispatch({ type: UserActionType.LOCKED_APP })
374+
.run();
375+
376+
expect(mockApprovalControllerClear).toHaveBeenCalledWith(
377+
providerErrors.userRejectedRequest(),
378+
);
379+
expect(mockNavigate).toHaveBeenCalledWith(Routes.LOCK_SCREEN);
380+
});
381+
382+
it('navigates to LockScreen even when ApprovalController.clear throws', async () => {
383+
mockApprovalControllerClear.mockImplementationOnce(() => {
384+
throw new Error('clear failed');
385+
});
386+
387+
await expectSaga(appLockStateMachine)
388+
.dispatch({ type: UserActionType.LOCKED_APP })
389+
.run();
390+
391+
expect(mockNavigate).toHaveBeenCalledWith(Routes.LOCK_SCREEN);
392+
});
362393
});
363394

364395
// TODO: Update all saga tests to use expectSaga (more intuitive and easier to read)

0 commit comments

Comments
 (0)