[Bug]: Multichain - when there are three unapproved transactions across different networks, confirming the first one causes MetaMask to break with an error: 'Message: Minified React error #185; ...' #25596
Labels
regression-RC-12.0.0
release-blocker
This bug is blocking the next release
Sev2-normal
Normal severity; minor loss of service or inconvenience.
team-wallet-ux
type-bug
Comments
sleepytanya
changed the title
|
Another scenario where the
twoOnSameNet.movFirst tx: 
Second tx: 
Third tx: 
|
|
The bug is present in this v12.0.0 build as well: #25098 (comment) Only two txs submitted: 3tx.mov |
gauthierpetetin
added
the
Sev2-normal
|
I merged a PR for this exact scenario last week. 🤔. Will track this down ASAP #25536 |
darkwing
added a commit
that referenced
this issue
Jul 2, 2024
…dRequestCount (#25614) ## **Description** There's a case where there could be a race condition between the `QueuedRequestController`'s network switching and the Routes' network switching. The root issue is that the use of `getUnapprovedTransactions` only gets transactions on the `current` chain, thus triggering Routes to switch networks once the last transaction of the current chain is done, but there could be more transactions on other chains which we should allow QRC to handle network switching for. Thus, this PR lets QRC control network switching until there are absolutely no transactions or queued requests left. [This PR includes an **E2E](#25536 for the scenario where there are multiple queued transactions but since this is a race issue, it's difficult to reproduce in a provable E2E [](https://codespaces.new/MetaMask/metamask-extension/pull/25614?quickstart=1) ## **Related issues** Possibly related: #25596 ## **Manual testing steps** 1. Open `Tab 1`, connect to `Uniswap` on `Ethereum Mainnet` 2. Open `Tab 2`, connect to `PancakeSwap` on `BNB Chain` 3. Open `Tab 3`, connect to `Test Dapp` on `Sepolia` 4. Initiate a swap on `Tab 1` and `Tab 2` *BUT DO NOT CONFIRM IT, JUST MOVE ON TO THE NEXT TAB* 5. Initiate a send on `Tab 3` *BUT DO NOT CONFIRM IT* 6. On the confirmation screen, you should still see the first confirmation from `Tab 1` (`Uniswap`) on `Ethereum Mainnet`; confirm or reject it. See the confirmation window close 7. A new confirmation popup should come up with the `PancakeSwap`/ `Tab 2` confirmation on `BNB` chain; confirm or reject it. See the confirmation window close 8. See one last confirmation screen pop up for the `Tab 3` / `Test Dapp` send on `Sepolia`. Confirm or reject it. Create transactions on different networks, reject and/or confirm them, click around between popup and notification window, ensure nothing unexpected occurs ## **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/develop/.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/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.
darkwing
added a commit
that referenced
this issue
Jul 2, 2024
…dRequestCount (#25614) ## **Description** There's a case where there could be a race condition between the `QueuedRequestController`'s network switching and the Routes' network switching. The root issue is that the use of `getUnapprovedTransactions` only gets transactions on the `current` chain, thus triggering Routes to switch networks once the last transaction of the current chain is done, but there could be more transactions on other chains which we should allow QRC to handle network switching for. Thus, this PR lets QRC control network switching until there are absolutely no transactions or queued requests left. [This PR includes an **E2E](#25536 for the scenario where there are multiple queued transactions but since this is a race issue, it's difficult to reproduce in a provable E2E [](https://codespaces.new/MetaMask/metamask-extension/pull/25614?quickstart=1) ## **Related issues** Possibly related: #25596 ## **Manual testing steps** 1. Open `Tab 1`, connect to `Uniswap` on `Ethereum Mainnet` 2. Open `Tab 2`, connect to `PancakeSwap` on `BNB Chain` 3. Open `Tab 3`, connect to `Test Dapp` on `Sepolia` 4. Initiate a swap on `Tab 1` and `Tab 2` *BUT DO NOT CONFIRM IT, JUST MOVE ON TO THE NEXT TAB* 5. Initiate a send on `Tab 3` *BUT DO NOT CONFIRM IT* 6. On the confirmation screen, you should still see the first confirmation from `Tab 1` (`Uniswap`) on `Ethereum Mainnet`; confirm or reject it. See the confirmation window close 7. A new confirmation popup should come up with the `PancakeSwap`/ `Tab 2` confirmation on `BNB` chain; confirm or reject it. See the confirmation window close 8. See one last confirmation screen pop up for the `Tab 3` / `Test Dapp` send on `Sepolia`. Confirm or reject it. Create transactions on different networks, reject and/or confirm them, click around between popup and notification window, ensure nothing unexpected occurs ## **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/develop/.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/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.
14 tasks
|
I can no longer reproduce any of these issues after this PR.#25641 |
darkwing
added a commit
that referenced
this issue
Jul 3, 2024
…hainId doesn't match current chainId (#25634) ## **Description** This PR adds a safety rail to the confirmation screen such that an error is thrown if the user is seeing a confirmation for a transaction triggered on another chain. [](https://codespaces.new/MetaMask/metamask-extension/pull/25634?quickstart=1) ## **Related issues** Related: #25596 ## **Manual testing steps** 1. This state should be impossible to get to via the UI, thus no reasonable manual testing steps. Unit test included. ## **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/develop/.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/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.
darkwing
added a commit
that referenced
this issue
Jul 3, 2024
…hainId doesn't match current chainId (#25634) ## **Description** This PR adds a safety rail to the confirmation screen such that an error is thrown if the user is seeing a confirmation for a transaction triggered on another chain. [](https://codespaces.new/MetaMask/metamask-extension/pull/25634?quickstart=1) ## **Related issues** Related: #25596 ## **Manual testing steps** 1. This state should be impossible to get to via the UI, thus no reasonable manual testing steps. Unit test included. ## **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/develop/.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/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.
14 tasks
|
I think popup behavior looks acceptable for now? (correct me if I'm wrong). I don't see any errors in all scenarios I've tried. multipleUnapproved.mov |
|
Popup behavior above is being addressed by this PR: #25619 |
|
Should we close this issue since it was reported to be fixed by this PR: #25641 ? |
|
@bschorchit I think so! |
|
Closing with Tanya's recommendation. |
danjm
pushed a commit
that referenced
this issue
Jul 9, 2024
…orks and QueuedRequestCount (#25614) (#25641) ## **Description** There's a case where there could be a race condition between the `QueuedRequestController`'s network switching and the Routes' network switching. The root issue is that the use of `getUnapprovedTransactions` only gets transactions on the `current` chain, thus triggering Routes to switch networks once the last transaction of the current chain is done, but there could be more transactions on other chains which we should allow QRC to handle network switching for. Thus, this PR lets QRC control network switching until there are absolutely no transactions or queued requests left. [This PR includes an **E2E](#25536 for the scenario where there are multiple queued transactions but since this is a race issue, it's difficult to reproduce in a provable E2E [](https://codespaces.new/MetaMask/metamask-extension/pull/25614?quickstart=1) ## **Related issues** Possibly related: #25596 ## **Manual testing steps** 1. Open `Tab 1`, connect to `Uniswap` on `Ethereum Mainnet` 2. Open `Tab 2`, connect to `PancakeSwap` on `BNB Chain` 3. Open `Tab 3`, connect to `Test Dapp` on `Sepolia` 4. Initiate a swap on `Tab 1` and `Tab 2` *BUT DO NOT CONFIRM IT, JUST MOVE ON TO THE NEXT TAB* 5. Initiate a send on `Tab 3` *BUT DO NOT CONFIRM IT* 6. On the confirmation screen, you should still see the first confirmation from `Tab 1` (`Uniswap`) on `Ethereum Mainnet`; confirm or reject it. See the confirmation window close 7. A new confirmation popup should come up with the `PancakeSwap`/ `Tab 2` confirmation on `BNB` chain; confirm or reject it. See the confirmation window close 8. See one last confirmation screen pop up for the `Tab 3` / `Test Dapp` send on `Sepolia`. Confirm or reject it. Create transactions on different networks, reject and/or confirm them, click around between popup and notification window, ensure nothing unexpected occurs ## **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/develop/.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/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. <!-- 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? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/25641?quickstart=1) ## **Related issues** Fixes: ## **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/develop/.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/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.
danjm
pushed a commit
that referenced
this issue
Jul 10, 2024
](https://codespaces.new/MetaMask/metamask-extension/pull/25614?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/25634?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/25641?quickstart=1){kind=link}
…hainId doesn't match current chainId (#25634) ## **Description** This PR adds a safety rail to the confirmation screen such that an error is thrown if the user is seeing a confirmation for a transaction triggered on another chain. [](https://codespaces.new/MetaMask/metamask-extension/pull/25634?quickstart=1) ## **Related issues** Related: #25596 ## **Manual testing steps** 1. This state should be impossible to get to via the UI, thus no reasonable manual testing steps. Unit test included. ## **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/develop/.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/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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
When there are three unapproved transactions across different networks, confirming the first one causes MetaMask to break with an error:
Message: Minified React error #185; visit https://reactjs.org/docs/error-decoder.html?invariant=185 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.STX toggle is OFF.
Latest v12.0.0 build
Expected behavior
Screenshots/Recordings
err.mov
Steps to reproduce
Error messages or log output
No response
Version
12.0.0
Build type
None
Browser
Chrome
Operating system
MacOS
Hardware wallet
No response
Additional context
No response
Severity
No response
The text was updated successfully, but these errors were encountered: