Skip to content

Conversation

@runway-github
Copy link
Contributor

@runway-github runway-github bot commented Oct 27, 2025

Description

This fixes an issue where Snaps may try to call APIs that are only
available when the client is unlocked in the onActive handler. After
this change, onActive is only called when the client is being
unlocked, or when the client is already in the unlocked state and is
opened. onInactive is only called when the client is in the unlocked
state and closed, or when the client is being locked.

Open in GitHub Codespaces

Changelog

CHANGELOG entry: Only trigger onActive and onInactive Snap lifecycle
hooks when client is unlocked

Related issues

Fixes #37179.

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

Note

Only notify Snaps of client active/inactive when the wallet is unlocked, wiring KeyringController lock/unlock events and adding tests.

  • Snaps:
    • Wire KeyringController:lock/unlock events to SnapController:setClientActive via SnapControllerInit.
    • Extend SnapControllerInit messenger to allow SnapController:setClientActive action and subscribe to lock/unlock events.
    • Update isClientOpen setter to call SnapController:setClientActive only if KeyringController is unlocked.
  • Tests:
    • Add tests verifying SnapController:setClientActive is called with false on lock and true on unlock.

Written by Cursor Bugbot for commit a69dbd5. This will update automatically on new commits. Configure here.

89eb2a2

…`onInactive` when client is unlocked (cp-13.7.0) (#37222)

<!--
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**

<!--
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?
-->

This fixes an issue where Snaps may try to call APIs that are only
available when the client is unlocked in the `onActive` handler. After
this change, `onActive` is only called when the client is being
unlocked, or when the client is already in the unlocked state and is
opened. `onInactive` is only called when the client is in the unlocked
state and closed, or when the client is being locked.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37222?quickstart=1)

## **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: Only trigger `onActive` and `onInactive` Snap lifecycle
hooks when client is unlocked

## **Related issues**

Fixes #37179.

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- 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**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/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]
> Gate Snap lifecycle activation to the unlocked state and hook
KeyringController lock/unlock to update Snap client activity.
> 
> - **Snaps Init/Messenger**:
> - Allow `SnapController:setClientActive` action and subscribe to
`KeyringController:lock`/`unlock` in `SnapControllerInit`, calling
`setClientActive(false|true)` accordingly.
> - Extend init messenger types/permissions to include `SetClientActive`
and `KeyringControllerUnlockEvent`.
> - **MetaMask Controller**:
> - In `isClientOpen` setter, call `SnapController:setClientActive` only
if `KeyringController` is unlocked.
> - **Tests**:
> - Add tests verifying `setClientActive` is called on lock/unlock
events and refactor test setup to accept a base messenger.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
468cadd. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@runway-github runway-github bot requested a review from a team as a code owner October 27, 2025 12:38
@metamaskbot metamaskbot added the team-bots Bot team (for MetaMask Bot, Runway Bot, etc.) label Oct 27, 2025
@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

🫰 @MetaMask/core-platform (3 files, +56 -6)
  • 📁 app/
    • 📁 scripts/
      • 📁 controller-init/
        • 📁 messengers/
          • 📁 snaps/
            • 📄 snap-controller-messenger.ts +9 -3
        • 📁 snaps/
          • 📄 snap-controller-init.test.ts +39 -3
          • 📄 snap-controller-init.ts +8 -0

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: a69dbd5 | Date: 10/27/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.05s (±38ms) 🟡 | historical mean value: 1.06s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 734ms (±36ms) 🟢 | historical mean value: 740ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 76ms (±12ms) 🟢 | historical mean value: 77ms ⬇️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.05s 38ms 1.02s 1.33s 1.08s 1.33s
domContentLoaded 734ms 36ms 710ms 1.00s 759ms 1.00s
firstPaint 76ms 12ms 60ms 168ms 88ms 168ms
firstContentfulPaint 76ms 12ms 60ms 168ms 88ms 168ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [a69dbd5]
UI Startup Metrics (1291 ± 84 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyStandard HomeuiStartup1291113014958413351461
load110695813198511431285
domContentLoaded109895613148511371278
domInteractive19145481941
firstPaint59772128944010881239
backgroundConnect23522037215237246
firstReactRender29208193344
getState1984982137
initialActions717310714
loadScripts8717331083859181051
setupStore1072741119
BrowserifyPower User HomeuiStartup22401758376161229163761
load1202945209034415942090
domContentLoaded1195941208134315872081
domInteractive32161102552110
firstPaint53897159143710071591
backgroundConnect26722462292258622
firstReactRender23202932629
getState18514823117195231
initialActions1139622696
loadScripts945719162330213271623
setupStore1081411114
WebpackStandard HomeuiStartup8407241090718591015
load63758289568641824
domContentLoaded62957688468634818
domInteractive15114571436
firstPaint17857825150193590
backgroundConnect22114262634
firstReactRender25164583135
getState1162441321
initialActions30143410
loadScripts62657487466632807
setupStore1042331115
WebpackPower User HomeuiStartup18411491271046425332710
load79061514612648321461
domContentLoaded75160013222248131322
domInteractive18123782237
firstPaint48110613254128161325
backgroundConnect12827428143299428
firstReactRender22202822228
getState17911239259181392
initialActions120130323130
loadScripts74759713092218001309
setupStore2161673824167
FirefoxBrowserifyStandard HomeuiStartup1477130620079815321611
load1258109514567113061386
domContentLoaded1258109514567113051385
domInteractive1163535946129202
firstPaint------
backgroundConnect3926104134661
firstReactRender25214752539
getState74435716
initialActions3126336
loadScripts1234107514266812811357
setupStore1376181230
BrowserifyPower User HomeuiStartup28302359349636832183496
load14781311167211916181672
domContentLoaded14781311167111916181671
domInteractive1415126063219260
firstPaint------
backgroundConnect24139764208349764
firstReactRender41291011745101
getState1205820538136205
initialActions12152151452
loadScripts14141277163212015511632
setupStore35989263489
WebpackStandard HomeuiStartup15731398209712016161892
load1355121716469114141529
domContentLoaded1355121616469114141528
domInteractive1042940960105248
firstPaint------
backgroundConnect4520143205082
firstReactRender27217262736
getState73566715
initialActions3139438
loadScripts1325119916228713781497
setupStore1365981233
WebpackPower User HomeuiStartup30782542441453233794414
load17561428241526819652415
domContentLoaded17561427241426819642414
domInteractive1555634988235349
firstPaint------
backgroundConnect24933887278406887
firstReactRender402863115063
getState1437618629163186
initialActions14176191576
loadScripts16731405212520418642125
setupStore27578254378

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: d77d957 | Date: 10/27/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 986ms (±43ms) 🟢 | historical mean value: 1.05s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 686ms (±59ms) 🟢 | historical mean value: 738ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 87ms (±120ms) 🟢 | historical mean value: 79ms ⬆️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 986ms 43ms 964ms 1.32s 1.02s 1.32s
domContentLoaded 686ms 59ms 663ms 1.22s 713ms 1.22s
firstPaint 87ms 120ms 64ms 1.28s 84ms 1.28s
firstContentfulPaint 87ms 120ms 64ms 1.28s 84ms 1.28s
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [d77d957]
UI Startup Metrics (1279 ± 85 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyStandard HomeuiStartup1279111714448513461419
load109994612738711671239
domContentLoaded109294112628611601230
domInteractive19145081846
firstPaint69572127144811181239
backgroundConnect2312182708235250
firstReactRender27195373043
getState1785372130
initialActions51657615
loadScripts8677261034859381007
setupStore1061621014
BrowserifyPower User HomeuiStartup22721859361557528603615
load1190957186634016661866
domContentLoaded1181947185033816551850
domInteractive311689225889
firstPaint757157185459510131854
backgroundConnect283224731119299731
firstReactRender23202822328
getState19012429935205299
initialActions123102245102
loadScripts931719150730213651507
setupStore1081211012
WebpackStandard HomeuiStartup8517331128748651052
load63758891968641827
domContentLoaded63058190566635800
domInteractive15115281435
firstPaint19257922182188605
backgroundConnect24124972938
firstReactRender281783103343
getState1252841420
initialActions3015358
loadScripts62657989264633787
setupStore1052441218
WebpackPower User HomeuiStartup16611367228231620802282
load70659310281478991028
domContentLoaded667579912119793912
domInteractive20133793537
firstPaint46659971328796971
backgroundConnect11826444135127444
firstReactRender22202722327
getState1479317420156174
initialActions1117121671
loadScripts662576901116784901
setupStore20681234281
FirefoxBrowserifyStandard HomeuiStartup1438126017829214911585
load1229106413866712741332
domContentLoaded1229106413856712741332
domInteractive1103628340124208
firstPaint------
backgroundConnect4024150154660
firstReactRender25204862543
getState74183913
initialActions4167936
loadScripts1205104413606712541312
setupStore137103111321
BrowserifyPower User HomeuiStartup34552929463547738394635
load18871568259524719722595
domContentLoaded18871567259424719722594
domInteractive293119780179375780
firstPaint------
backgroundConnect27160729224561729
firstReactRender55341272652127
getState17510526141187261
initialActions3022216216221
loadScripts18151532250824319202508
setupStore4592826452282
WebpackStandard HomeuiStartup1510135018999615461741
load1303114615917613391444
domContentLoaded1303114515917613381444
domInteractive963222035105188
firstPaint------
backgroundConnect402381144566
firstReactRender272169102859
getState74647914
initialActions4140638
loadScripts1278113015667513151425
setupStore145114141348
WebpackPower User HomeuiStartup29402542353135334063531
load16821483202216017762022
domContentLoaded16821481202116017762021
domInteractive1577530565212305
firstPaint------
backgroundConnect24846717229442717
firstReactRender37275174451
getState1605624948193249
initialActions10133112233
loadScripts16041438184713917311847
setupStore34669215769

@davidrentc davidrentc merged commit e4ca203 into release/13.7.0 Oct 27, 2025
168 of 170 checks passed
@davidrentc davidrentc deleted the runway-cherry-pick-13.7.0-1761568699 branch October 27, 2025 17:19
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2025
@metamaskbot metamaskbot added the release-13.7.0 Issue or pull request that will be included in release 13.7.0 label Oct 27, 2025
@metamaskbot
Copy link
Collaborator

No release label on PR. Adding release label release-13.7.0 on PR, as PR was cherry-picked in branch 13.7.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-13.7.0 Issue or pull request that will be included in release 13.7.0 team-bots Bot team (for MetaMask Bot, Runway Bot, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants