Skip to content

fix: dapp receiving two accountsChanged and wallet_sessionChanged events when adding and removing accounts/chainIds in the same edit action #33082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

jiexi
Copy link
Contributor

@jiexi jiexi commented May 20, 2025

Description

This PR adds and uses setPermittedChains and setPermittedAccounts helpers in the ReviewPermissionsPage, replacing usage of addPermittedAccounts, removePermittedAccount, addPermittedChains, and removePermittedChain.

This is necessary because the previous implementation broke up account and chain permission updates into separate add and remove mutations to state. This resulted in two accountsChanged and wallet_sessionChanged events being fired whenever the user added and removed an account / chainId in a single edit action.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Permission a dapp
  2. On the dapp tab, enter window.ethereum.on("accountsChanged", console.log) into console
  3. edit permissions by opening the extension popup, clicking the globe icon, and editing accounts to permit a new account and unpermit an existing account
  4. See that the accountsChanged event only fired once for the dapp

  1. Permission the multichain test dapp
  2. edit permissions by opening the extension popup, clicking the globe icon, and editing accounts to permit a new account and unpermit an existing account
  3. See that the session changed event inpage UI only shows one event
  4. Repeat above for permitted chains

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.

@jiexi jiexi requested a review from a team as a code owner May 20, 2025 18:30
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.

@jiexi jiexi changed the title Fix: add setPermittedChains and setPermittedAccounts background methods fix: add setPermittedChains and setPermittedAccounts background methods May 20, 2025
@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

🖥️ @MetaMask/wallet-ux

  • ui/components/multichain/pages/review-permissions-page/review-permissions-page.tsx

@jiexi jiexi changed the title fix: add setPermittedChains and setPermittedAccounts background methods fix: dapp receiving two accountsChanged and wallet_sessionChanged events when adding and removing accounts/chainIds in the same edit action May 20, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [f13682f]
UI Startup Metrics (1225 ± 63 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1225107913816312611331
load105493612296011011156
domContentLoaded104792912236010911148
domInteractive16134141623
firstPaint795141122939410711145
backgroundConnect84385821
firstReactRender21155062137
getState1463571929
initialActions001001
loadScripts80869298159849915
setupStore86233815
WebpackHomeuiStartup20901584258720922302465
load16211254200715817171894
domContentLoaded16081250197214717051864
domInteractive14114071336
firstPaint1496234957167263
backgroundConnect289338452548
firstReactRender1314236397152324
getState194306441446
initialActions612932935
loadScripts16051249196214517011854
setupStore4073168120297
FirefoxBrowserifyHomeuiStartup12761108191713813071602
load1133973160912611771435
domContentLoaded1133973160912611771434
domInteractive913323228103133
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect19126471934
firstReactRender23205162335
getState10419019827
initialActions001001
loadScripts1117964159312511601416
setupStore6432368
WebpackHomeuiStartup15641362213415416311916
load13591186192813814381647
domContentLoaded13591185192813814381646
domInteractive7552158148393
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2415260242431
firstReactRender37284954044
getState11517617925
initialActions001011
loadScripts13371171191213614151571
setupStore1057611832
Benchmark value 21 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 40 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 2465 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 297 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 27 exceeds gate value 24 for firefox browserify home p95 getState
Benchmark value 32 exceeds gate value 28 for firefox webpack home p95 setupStore
Sum of mean exceeds: 8ms | Sum of p95 exceeds: 253ms
Sum of all benchmark exceeds: 261ms

adonesky1
adonesky1 previously approved these changes May 20, 2025
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and tested and it works great 🙏

@metamaskbot
Copy link
Collaborator

Builds ready [33e02a3]
UI Startup Metrics (1220 ± 78 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1220107714417812671383
load105392612307310931219
domContentLoaded104592012277410881211
domInteractive16138571624
firstPaint761137124939710591138
backgroundConnect94296924
firstReactRender21165272139
getState1564082032
initialActions001001
loadScripts80467998574842966
setupStore85263917
WebpackHomeuiStartup20731654258222022322447
load16141299196515817171882
domContentLoaded16081295196015717111873
domInteractive15114981340
firstPaint1615939165206283
backgroundConnect21875102437
firstReactRender1204237395122335
getState173291401243
initialActions812954035
loadScripts16051293195815517091863
setupStore3363066918275
FirefoxBrowserifyHomeuiStartup13261141182713513741621
load11751019164112312281446
domContentLoaded11751019164112312281446
domInteractive933719127104147
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2213210202136
firstReactRender24205252330
getState9520119810
initialActions002001
loadScripts11561006161311912121382
setupStore84678632
WebpackHomeuiStartup15311338220414416091827
load13301154193413914161556
domContentLoaded13291153193413914161556
domInteractive77521531780112
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2314193182341
firstReactRender37285054144
getState85334913
initialActions002111
loadScripts13101138180213214001539
setupStore10523923820
Benchmark value 16 exceeds gate value 15 for chrome browserify home mean getState
Benchmark value 1383 exceeds gate value 1365 for chrome browserify home p95 uiStartup
Benchmark value 1220 exceeds gate value 1190 for chrome browserify home p95 load
Benchmark value 1212 exceeds gate value 1180 for chrome browserify home p95 domContentLoaded
Benchmark value 24 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 966 exceeds gate value 940 for chrome browserify home p95 loadScripts
Benchmark value 9 exceeds gate value 7 for chrome webpack home mean initialActions
Benchmark value 33 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 275 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 32 exceeds gate value 27 for firefox browserify home p95 setupStore
Sum of mean exceeds: 4ms | Sum of p95 exceeds: 327ms
Sum of all benchmark exceeds: 331ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: -397 Bytes (-0.01%)
  • common: 496 Bytes (0.01%)

ffmcgee725
ffmcgee725 previously approved these changes May 21, 2025
vinnyhoward
vinnyhoward previously approved these changes May 21, 2025
@jiexi jiexi enabled auto-merge May 21, 2025 18:16
@metamaskbot
Copy link
Collaborator

Builds ready [dec7d0f]
UI Startup Metrics (1192 ± 66 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1192107414286612271323
load102692912035610591104
domContentLoaded101992112005610511097
domInteractive16138581524
firstPaint77877116938410441094
backgroundConnect84335823
firstReactRender20155262034
getState1464681929
initialActions001001
loadScripts78167993355813867
setupStore85182813
WebpackHomeuiStartup21251674257920522582492
load16591307199715417451965
domContentLoaded16531304198315217411951
domInteractive15125591341
firstPaint1636536062178301
backgroundConnect3110337532640
firstReactRender15942367116299343
getState164268331426
initialActions317135
loadScripts16471302197214717331888
setupStore3063226019155
FirefoxBrowserifyHomeuiStartup13211132169312613691644
load11681000153711312241412
domContentLoaded11671000153711312241412
domInteractive93383013499168
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2213111132148
firstReactRender24205962429
getState9515415818
initialActions001001
loadScripts1149987151911212071393
setupStore9422923619
WebpackHomeuiStartup15991393225417316842014
load13841207196316214711820
domContentLoaded13831207196216214711820
domInteractive79441521885119
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect23165382342
firstReactRender38305044245
getState105376930
initialActions002111
loadScripts13641186194516214511802
setupStore11521822915
cc: @HowardBraham
Benchmark value 23 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 2492 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 155 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 1384 exceeds gate value 1380 for firefox webpack home mean load
Benchmark value 1384 exceeds gate value 1380 for firefox webpack home mean domContentLoaded
Benchmark value 39 exceeds gate value 38 for firefox webpack home mean firstReactRender
Benchmark value 1364 exceeds gate value 1360 for firefox webpack home mean loadScripts
Benchmark value 2014 exceeds gate value 1935 for firefox webpack home p95 uiStartup
Benchmark value 1820 exceeds gate value 1660 for firefox webpack home p95 load
Benchmark value 1820 exceeds gate value 1660 for firefox webpack home p95 domContentLoaded
Benchmark value 1802 exceeds gate value 1630 for firefox webpack home p95 loadScripts
Sum of mean exceeds: 13ms | Sum of p95 exceeds: 704ms
Sum of all benchmark exceeds: 717ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: -397 Bytes (-0.01%)
  • common: 496 Bytes (0.01%)

@jiexi jiexi dismissed stale reviews from vinnyhoward, adonesky1, and ffmcgee725 via cf2e2e9 May 22, 2025 15:38
adonesky1
adonesky1 previously approved these changes May 22, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [cf2e2e9]
UI Startup Metrics (1271 ± 87 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1271111516268713251409
load109594113898711651224
domContentLoaded108893813818711581218
domInteractive17135551730
firstPaint767150138643711421214
backgroundConnect94335922
firstReactRender21175362135
getState1463981929
initialActions001001
loadScripts844703109785913971
setupStore85243916
WebpackHomeuiStartup21111674253421622492460
load16291299194415717351884
domContentLoaded16221295192915517271875
domInteractive161161101343
firstPaint1586234463180290
backgroundConnect2295892636
firstReactRender14744361103272331
getState144309301327
initialActions317134
loadScripts16191294191815417231865
setupStore2973076019246
FirefoxBrowserifyHomeuiStartup13481127185414313971708
load11941005161113112531549
domContentLoaded11931005161113112531548
domInteractive1003522533107174
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2213181182149
firstReactRender23205232327
getState84416813
initialActions002001
loadScripts1174993159812712351527
setupStore74648611
WebpackHomeuiStartup15611354202515616711905
load13521151182714814611646
domContentLoaded13521151182714814611646
domInteractive81392572687111
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect21145072240
firstReactRender39275654246
getState95346928
initialActions002111
loadScripts13341134180714714441630
setupStore11524725823
cc: @HowardBraham
Benchmark value 1272 exceeds gate value 1234 for chrome browserify home mean uiStartup
Benchmark value 1096 exceeds gate value 1070 for chrome browserify home mean load
Benchmark value 1088 exceeds gate value 1061 for chrome browserify home mean domContentLoaded
Benchmark value 844 exceeds gate value 830 for chrome browserify home mean loadScripts
Benchmark value 1409 exceeds gate value 1365 for chrome browserify home p95 uiStartup
Benchmark value 1225 exceeds gate value 1190 for chrome browserify home p95 load
Benchmark value 1219 exceeds gate value 1180 for chrome browserify home p95 domContentLoaded
Benchmark value 1214 exceeds gate value 1180 for chrome browserify home p95 firstPaint
Benchmark value 23 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 971 exceeds gate value 940 for chrome browserify home p95 loadScripts
Benchmark value 2461 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 246 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 1708 exceeds gate value 1660 for firefox browserify home p95 uiStartup
Benchmark value 1549 exceeds gate value 1495 for firefox browserify home p95 load
Benchmark value 1548 exceeds gate value 1495 for firefox browserify home p95 domContentLoaded
Benchmark value 1527 exceeds gate value 1475 for firefox browserify home p95 loadScripts
Benchmark value 39 exceeds gate value 38 for firefox webpack home mean firstReactRender
Sum of mean exceeds: 106ms | Sum of p95 exceeds: 583ms
Sum of all benchmark exceeds: 689ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: -393 Bytes (-0.01%)
  • common: 464 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [d678edc]
UI Startup Metrics (1259 ± 80 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1259109614818013021452
load108796313157511361240
domContentLoaded108095913067511281233
domInteractive17133651729
firstPaint840149131141011241241
backgroundConnect84375814
firstReactRender21164752235
getState1465481925
initialActions001001
loadScripts835714106674881975
setupStore85152913
WebpackHomeuiStartup21081692262021222572457
load16311318199815317231866
domContentLoaded16261314198715217181857
domInteractive161193121342
firstPaint1686540161192290
backgroundConnect2296492537
firstReactRender15142366107277337
getState124100111328
initialActions317135
loadScripts16221312197615117171847
setupStore3863217819305
FirefoxBrowserifyHomeuiStartup13681162189612914271631
load12101033170912612671462
domContentLoaded12101033170912612671462
domInteractive1003829032116153
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect211487102231
firstReactRender26216292454
getState10520120811
initialActions005101
loadScripts11921020168612412511437
setupStore64373611
WebpackHomeuiStartup15351346189413116051863
load13191176169711813831610
domContentLoaded13191175169711813821610
domInteractive78512682582127
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect23155682544
firstReactRender38294944045
getState1156991030
initialActions102111
loadScripts13001159168111913671590
setupStore105619827
Benchmark value 1260 exceeds gate value 1234 for chrome browserify home mean uiStartup
Benchmark value 1088 exceeds gate value 1070 for chrome browserify home mean load
Benchmark value 1081 exceeds gate value 1061 for chrome browserify home mean domContentLoaded
Benchmark value 841 exceeds gate value 800 for chrome browserify home mean firstPaint
Benchmark value 836 exceeds gate value 830 for chrome browserify home mean loadScripts
Benchmark value 1452 exceeds gate value 1365 for chrome browserify home p95 uiStartup
Benchmark value 1240 exceeds gate value 1190 for chrome browserify home p95 load
Benchmark value 1233 exceeds gate value 1180 for chrome browserify home p95 domContentLoaded
Benchmark value 1242 exceeds gate value 1180 for chrome browserify home p95 firstPaint
Benchmark value 975 exceeds gate value 940 for chrome browserify home p95 loadScripts
Benchmark value 38 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 2457 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 305 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 26 exceeds gate value 25 for firefox browserify home mean firstReactRender
Benchmark value 39 exceeds gate value 38 for firefox webpack home mean firstReactRender
Sum of mean exceeds: 119ms | Sum of p95 exceeds: 530ms
Sum of all benchmark exceeds: 649ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: -393 Bytes (-0.01%)
  • common: 464 Bytes (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.

5 participants