[Bug]: MV3 - Transactions - Perf. Degradation whenever triggering batch transactions and Confirm/Reject, MM stays loading for unusual time #24583
Assignees
Labels
release-11.18.0
Issue or pull request that will be included in release 11.18.0
Sev2-normal
Normal severity; minor loss of service or inconvenience.
team-extension-platform
type-bug
Comments
seaona
added
type-bug
Sev2-normal
7 tasks
danjm
added a commit
that referenced
this issue
May 30, 2024
…4851) ## **Description** #15978 attempted to deal with the following problem: "in MV3 the background process (now a service worker) is constantly (and unpredictably) winding down... there is a small window of time when, if the service worker's lifecycle ends immediately after a user action and the action is executed in the background but not yet persisted, the action will not be added to the replay queue since the action was in fact executed - just not yet persisted." The solution was: "This solution we've landed on is to persist the data that results from background state changes further upstream than previously, in the controller multiplex. This allows us to persist the data before the action is resolved ensuring that the queuing system will work " The problem with the solution is that the metaRPChandler would now have to do a write to store after every single request, and if multiple of these are occuring around the same time, they end up blocking subsequent requests (as currently storage writes can take upwards of 100 ms on some systems) Fortunately, the original problem is no longer a problem, as the MV3 service worker will no longer be stopped if the UI is open. So, we can delete the blocking code, as it is no longer needed. [](https://codespaces.new/MetaMask/metamask-extension/pull/24851?quickstart=1) ## **Related issues** Fixes: #24583 ## **Manual testing steps** On a linux machine: 1. Go to the test dapp 2. Trigger 10 batx tx 3. Accept/Reject multiple times in a row 4. It should be as performant as MV2 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] 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/develop/.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. Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
](https://codespaces.new/MetaMask/metamask-extension/pull/24851?quickstart=1){kind=link}
metamaskbot
added
the
release-11.18.0
danjm
added a commit
that referenced
this issue
May 31, 2024
…4851) #15978 attempted to deal with the following problem: "in MV3 the background process (now a service worker) is constantly (and unpredictably) winding down... there is a small window of time when, if the service worker's lifecycle ends immediately after a user action and the action is executed in the background but not yet persisted, the action will not be added to the replay queue since the action was in fact executed - just not yet persisted." The solution was: "This solution we've landed on is to persist the data that results from background state changes further upstream than previously, in the controller multiplex. This allows us to persist the data before the action is resolved ensuring that the queuing system will work " The problem with the solution is that the metaRPChandler would now have to do a write to store after every single request, and if multiple of these are occuring around the same time, they end up blocking subsequent requests (as currently storage writes can take upwards of 100 ms on some systems) Fortunately, the original problem is no longer a problem, as the MV3 service worker will no longer be stopped if the UI is open. So, we can delete the blocking code, as it is no longer needed. [](https://codespaces.new/MetaMask/metamask-extension/pull/24851?quickstart=1) Fixes: #24583 On a linux machine: 1. Go to the test dapp 2. Trigger 10 batx tx 3. Accept/Reject multiple times in a row 4. It should be as performant as MV2 - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] 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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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. Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
danjm
added a commit
that referenced
this issue
May 31, 2024
…4851) #15978 attempted to deal with the following problem: "in MV3 the background process (now a service worker) is constantly (and unpredictably) winding down... there is a small window of time when, if the service worker's lifecycle ends immediately after a user action and the action is executed in the background but not yet persisted, the action will not be added to the replay queue since the action was in fact executed - just not yet persisted." The solution was: "This solution we've landed on is to persist the data that results from background state changes further upstream than previously, in the controller multiplex. This allows us to persist the data before the action is resolved ensuring that the queuing system will work " The problem with the solution is that the metaRPChandler would now have to do a write to store after every single request, and if multiple of these are occuring around the same time, they end up blocking subsequent requests (as currently storage writes can take upwards of 100 ms on some systems) Fortunately, the original problem is no longer a problem, as the MV3 service worker will no longer be stopped if the UI is open. So, we can delete the blocking code, as it is no longer needed. [](https://codespaces.new/MetaMask/metamask-extension/pull/24851?quickstart=1) Fixes: #24583 On a linux machine: 1. Go to the test dapp 2. Trigger 10 batx tx 3. Accept/Reject multiple times in a row 4. It should be as performant as MV2 - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] 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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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. Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
danjm
added a commit
that referenced
this issue
May 31, 2024
…4851) #15978 attempted to deal with the following problem: "in MV3 the background process (now a service worker) is constantly (and unpredictably) winding down... there is a small window of time when, if the service worker's lifecycle ends immediately after a user action and the action is executed in the background but not yet persisted, the action will not be added to the replay queue since the action was in fact executed - just not yet persisted." The solution was: "This solution we've landed on is to persist the data that results from background state changes further upstream than previously, in the controller multiplex. This allows us to persist the data before the action is resolved ensuring that the queuing system will work " The problem with the solution is that the metaRPChandler would now have to do a write to store after every single request, and if multiple of these are occuring around the same time, they end up blocking subsequent requests (as currently storage writes can take upwards of 100 ms on some systems) Fortunately, the original problem is no longer a problem, as the MV3 service worker will no longer be stopped if the UI is open. So, we can delete the blocking code, as it is no longer needed. [](https://codespaces.new/MetaMask/metamask-extension/pull/24851?quickstart=1) Fixes: #24583 On a linux machine: 1. Go to the test dapp 2. Trigger 10 batx tx 3. Accept/Reject multiple times in a row 4. It should be as performant as MV2 - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] 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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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. Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
danjm
added a commit
that referenced
this issue
May 31, 2024
…4851) #15978 attempted to deal with the following problem: "in MV3 the background process (now a service worker) is constantly (and unpredictably) winding down... there is a small window of time when, if the service worker's lifecycle ends immediately after a user action and the action is executed in the background but not yet persisted, the action will not be added to the replay queue since the action was in fact executed - just not yet persisted." The solution was: "This solution we've landed on is to persist the data that results from background state changes further upstream than previously, in the controller multiplex. This allows us to persist the data before the action is resolved ensuring that the queuing system will work " The problem with the solution is that the metaRPChandler would now have to do a write to store after every single request, and if multiple of these are occuring around the same time, they end up blocking subsequent requests (as currently storage writes can take upwards of 100 ms on some systems) Fortunately, the original problem is no longer a problem, as the MV3 service worker will no longer be stopped if the UI is open. So, we can delete the blocking code, as it is no longer needed. [](https://codespaces.new/MetaMask/metamask-extension/pull/24851?quickstart=1) Fixes: #24583 On a linux machine: 1. Go to the test dapp 2. Trigger 10 batx tx 3. Accept/Reject multiple times in a row 4. It should be as performant as MV2 - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] 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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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. Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
danjm
added a commit
that referenced
this issue
May 31, 2024
…4851) #15978 attempted to deal with the following problem: "in MV3 the background process (now a service worker) is constantly (and unpredictably) winding down... there is a small window of time when, if the service worker's lifecycle ends immediately after a user action and the action is executed in the background but not yet persisted, the action will not be added to the replay queue since the action was in fact executed - just not yet persisted." The solution was: "This solution we've landed on is to persist the data that results from background state changes further upstream than previously, in the controller multiplex. This allows us to persist the data before the action is resolved ensuring that the queuing system will work " The problem with the solution is that the metaRPChandler would now have to do a write to store after every single request, and if multiple of these are occuring around the same time, they end up blocking subsequent requests (as currently storage writes can take upwards of 100 ms on some systems) Fortunately, the original problem is no longer a problem, as the MV3 service worker will no longer be stopped if the UI is open. So, we can delete the blocking code, as it is no longer needed. [](https://codespaces.new/MetaMask/metamask-extension/pull/24851?quickstart=1) Fixes: #24583 On a linux machine: 1. Go to the test dapp 2. Trigger 10 batx tx 3. Accept/Reject multiple times in a row 4. It should be as performant as MV2 - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] 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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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. Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Whenever I trigger several transactions (ie 10 batch tx's) when I reject/accept one I see MM loading for an unusual amount of time. The same happens if I try to Reject all at once.
This is also a bit noticeable already with 1 transaction, but it's way evident if we trigger them in batch
Expected behavior
Similar loading time as in prod (see vids)
Screenshots/Recordings
batch-loading-indefinetly.mp4
reject-all-loading.mp4
Compare to Production version
prod-fast-loading.mp4
Steps to reproduce
Error messages or log output
No response
Version
develop MV3 build
Build type
None
Browser
Chrome
Operating system
Linux
Hardware wallet
No response
Additional context
No response
Severity
No response
The text was updated successfully, but these errors were encountered: