Skip to content

fix: conditionally display delete account confirmation cp-13.20.1#40502

Open
gantunesr wants to merge 8 commits intomainfrom
gar/fix/delete-snap-account-confirmation
Open

fix: conditionally display delete account confirmation cp-13.20.1#40502
gantunesr wants to merge 8 commits intomainfrom
gar/fix/delete-snap-account-confirmation

Conversation

@gantunesr
Copy link
Member

@gantunesr gantunesr commented Feb 27, 2026

Description

As part of a solution for the #38464, the resyncAccounts method now requests the snaps to delete any un-synced account that have created a state discrepancy between the snap and wallet state. This action triggers a confirmation out of nowhere that requires user approval to proceed, and without any context, the user will most likely dismiss this confirmation and keep the inconsistent state.

The solution introduce in this PR is to ignore the confirmation dialog for all accounts that are not part of a snap wallet. This way we avoid any confirmation trigger as a result of the execution of resyncAccounts.

Open in GitHub Codespaces

Changelog

CHANGELOG entry: conditionally display delete account confirmation

Related issues

Fixes: None

Manual testing steps

  1. Using Flask
  2. Go to https://metamask.github.io/snap-simple-keyring/latest/
  3. Connect and create a few accounts with Create Account, copy the ID of each of this accounts
  4. Scroll to the method Remove Account and try to remove the newly created accounts
  5. The confirmation screen should be displayed

Screenshots/Recordings

Before

After

Screen.Recording.2026-02-27.at.5.34.32.PM.mov

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

Medium Risk
Changes the snap account removal flow to bypass user confirmation based on AccountTreeController state, which could affect when users are prompted during account deletion. Risk is moderate because it touches account-removal UX and state persistence logic but is scoped and covered by new tests.

Overview
Prevents unexpected “delete snap account” confirmation prompts by only showing the removal confirmation dialog when the target account is actually in a Snap wallet per AccountTreeController state; otherwise removal proceeds immediately.

This adds AccountTreeController:getState to the snap keyring messenger/type allowlists, refactors removal into a shared helper (#performAccountRemoval) plus a wallet-type check, exports SnapKeyringImpl for testing, and adds unit tests covering snap-vs-non-snap wallet removal behavior (including deny/error paths).

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

@metamaskbotv2
Copy link
Contributor

metamaskbotv2 bot commented Feb 27, 2026

✨ Files requiring CODEOWNER review ✨

🔑 @MetaMask/accounts-engineers (3 files, +294 -7)
  • 📁 app/
    • 📁 scripts/
      • 📁 lib/
        • 📁 snap-keyring/
          • 📄 snap-keyring.test.ts +224 -2
          • 📄 snap-keyring.ts +67 -4
          • 📄 types.ts +3 -1

@gantunesr gantunesr marked this pull request as ready for review February 27, 2026 20:51
@gantunesr gantunesr requested a review from a team as a code owner February 27, 2026 20:51
@HowardBraham HowardBraham changed the title fix: conditionally display delete account confirmation fix: conditionally display delete account confirmation cp-13-20-1 Feb 27, 2026
@gantunesr
Copy link
Member Author

@metamaskbot update-policies

@metamaskbotv2
Copy link
Contributor

metamaskbotv2 bot commented Feb 27, 2026

Builds ready [2ba2ee6]
⚡ Performance Benchmarks
👆 Interaction Benchmarks
BenchmarkMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P75 (ms)P95 (ms)
Load New Accountload_new_account27126029112277291
total27126029112277291
Confirm Txconfirm_tx601760076031960176031
total601760076031960176031
Bridge User Actionsbridge_load_page23421025216244252
bridge_load_asset_picker1591531738155173
bridge_search_token7056987167703716
total1143108612215712051221
🔌 Startup Benchmarks
BenchmarkMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P75 (ms)P95 (ms)
Standard HomeuiStartup1318109315709413631497
load110792013148111521247
domContentLoaded110091412838211471240
domInteractive271688172474
firstPaint175581081114209292
backgroundConnect18717022311190212
firstReactRender18113651927
initialActions107124
loadScripts9297391115819781066
setupStore1274451320
numNetworkReqs312296212293
Power User HomeuiStartup17351420229215017742065
load11651004181316111491628
domContentLoaded1148991180315511261596
domInteractive36191572733102
firstPaint194741660173223394
backgroundConnect29826244131301358
firstReactRender24155992543
initialActions103113
loadScripts93279915901539111347
setupStore1565161723
numNetworkReqs58351652953143
🧭 User Journey Benchmarks
BenchmarkMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P75 (ms)P95 (ms)
Onboarding Import WalletimportWalletToSocialScreen2202182232220223
srpButtonToSrpForm96959709697
confirmSrpToPwForm22212202222
pwFormToMetricsScreen15151601516
metricsToWalletReadyScreen16151601616
doneButtonToHomeScreen1046822135922812781359
openAccountMenuToAccountListLoaded73977237764616874547646
total88678643896913189498969
Onboarding New WalletcreateWalletToSocialScreen2182162191219219
srpButtonToPwForm1051031072106107
createPwToRecoveryScreen888088
skipBackupToMetricsScreen36363703737
agreeButtonToOnboardingSuccess16161601616
doneButtonToAssetList779476130232810211302
total1168859168733514401687
Asset DetailsassetClickToPriceChart573877146777
total573877146777
Solana Asset DetailsassetClickToPriceChart47474704747
total47474704747
Import Srp HomeloginToHomeScreen20431810227615821392276
openAccountMenuAfterLogin45415034650
homeAfterImportWithNewWallet24502253266516126052665
total4511443446077246074607
Send TransactionsopenSendPageFromHome18172011820
selectTokenToSendFormLoaded19182112021
reviewTransactionToConfirmationPage8628538748864874
total90589092011911920
SwapopenSwapPageFromHome1328817534170175
fetchAndDisplaySwapQuotes4636456847315946294731
total4779474048484347844848
🌐 Dapp Page Load Benchmarks

Current Commit: 2ba2ee6 | Date: 2/27/2026

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 972ms (±38ms) 🟢 | historical mean value: 1.05s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 682ms (±35ms) 🟢 | historical mean value: 740ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 77ms (±12ms) 🟢 | historical mean value: 83ms ⬇️ (historical data)

📈 Detailed Results

Metric Mean Std Dev Min Max P95 P99
pageLoadTime 972ms 38ms 946ms 1.26s 1.00s 1.26s
domContentLoaded 682ms 35ms 663ms 947ms 709ms 947ms
firstPaint 77ms 12ms 64ms 188ms 84ms 188ms
firstContentfulPaint 77ms 12ms 64ms 188ms 84ms 188ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms
Bundle size diffs
  • background: 670 Bytes (0.01%)
  • ui: 5 Bytes (0%)
  • common: 20 Bytes (0%)

@HowardBraham HowardBraham changed the title fix: conditionally display delete account confirmation cp-13-20-1 fix: conditionally display delete account confirmation cp-13.20.1 Feb 27, 2026
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 1, 2026

@metamaskbotv2
Copy link
Contributor

metamaskbotv2 bot commented Mar 1, 2026

Builds ready [bf840c9]
⚡ Performance Benchmarks
👆 Interaction Benchmarks
BenchmarkMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P75 (ms)P95 (ms)
Load New Accountload_new_account29927633426328334
total29927633426328334
Confirm Txconfirm_tx609260876098560976098
total609260876098560976098
Bridge User Actionsbridge_load_page21519024121240241
bridge_load_asset_picker20517023722216237
bridge_search_token71770573211729732
total1137111511651911461165
🔌 Startup Benchmarks
BenchmarkMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P75 (ms)P95 (ms)
Standard HomeuiStartup1445121416959914891645
load1212102614298812571377
domContentLoaded1206102114168712511372
domInteractive2917115192673
firstPaint1497043181204301
backgroundConnect21619925711221235
firstReactRender19134551928
initialActions107124
loadScripts100681812168710421173
setupStore1474261626
numNetworkReqs312293192281
Power User HomeuiStartup17801420237415618302030
load11861077178315911761639
domContentLoaded11711068175115411671628
domInteractive40212223436116
firstPaint1828353288242304
backgroundConnect30126542423312339
firstReactRender25165672838
initialActions105114
loadScripts95185715201529311422
setupStore1784371831
numNetworkReqs60371572856139
🧭 User Journey Benchmarks
BenchmarkMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P75 (ms)P95 (ms)
Onboarding Import WalletimportWalletToSocialScreen2192192190219219
srpButtonToSrpForm94919729597
confirmSrpToPwForm22222202222
pwFormToMetricsScreen16151711517
metricsToWalletReadyScreen17161701717
doneButtonToHomeScreen13571219148710314181487
openAccountMenuToAccountListLoaded7073705470991970997099
total8783864188708988478870
Onboarding New WalletcreateWalletToSocialScreen2212192231222223
srpButtonToPwForm1071061081108108
createPwToRecoveryScreen889099
skipBackupToMetricsScreen36343813738
agreeButtonToOnboardingSuccess16161701617
doneButtonToAssetList936586144539914021445
total1328977184940217901849
Asset DetailsassetClickToPriceChart553890217090
total553890217090
Solana Asset DetailsassetClickToPriceChart47464814748
total47464814748
Import Srp HomeloginToHomeScreen1980189920817720662081
openAccountMenuAfterLogin41374844148
homeAfterImportWithNewWallet22962035257119523522571
total43364148452913543464529
Send TransactionsopenSendPageFromHome19182111821
selectTokenToSendFormLoaded20182322123
reviewTransactionToConfirmationPage8608508719867871
total91088693116911931
SwapopenSwapPageFromHome110991228116122
fetchAndDisplaySwapQuotes53034613632179462256321
total54134725642079363476420
🌐 Dapp Page Load Benchmarks

Current Commit: bf840c9 | Date: 3/1/2026

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.04s (±42ms) 🟡 | historical mean value: 1.05s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 732ms (±38ms) 🟢 | historical mean value: 739ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 81ms (±14ms) 🟢 | historical mean value: 83ms ⬇️ (historical data)

📈 Detailed Results

Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.04s 42ms 1.02s 1.35s 1.05s 1.35s
domContentLoaded 732ms 38ms 712ms 1.01s 744ms 1.01s
firstPaint 81ms 14ms 64ms 208ms 88ms 208ms
firstContentfulPaint 81ms 14ms 64ms 208ms 88ms 208ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 715 Bytes (0.01%)
  • ui: 1.31 KiB (0.02%)
  • common: 1.3 KiB (0.01%)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants