[Bug]: When a user has a Smart transaction in a Pending state and initiates a transaction through a dApp on a different network, the dApp transaction is generated with an empty nonce. Pressing the 'Confirm' button then leads to an endless loading spinner.

[sleepytanya]

Describe the bug

v12.0.0 build [https://github.com/MetaMask/metamask-extension/commit/f5631fba1e83899f3e8d8a1bf2b1fc0de9fb87c6]

When a user has a transaction (STX) in a Pending state and initiates a transaction through a dApp on a different network, the dApp transaction is generated with an empty nonce. Pressing the 'Confirm' button then leads to an endless loading spinner. After reloading the extension, the second transaction's popup becomes accessible, and the transaction is then displayed with the correct nonce.

Expected behavior

Screenshots/Recordings

https://github.com/MetaMask/metamask-extension/assets/104780023/a24fda33-25a3-4ad0-b1c0-1dc422065279

Steps to reproduce

1. Turn on nonce customization in the SEttings->Advanced
2. Connect to Ethereum
3. Send STX (set low gas fee so the STX stays in 'Pending' state for awhile)
4. Connect to dapp (Pancake Swap, Uniswap) to BNB or other network
5. Start a dapp transaction
6. Notice that nonce field is empty
7. Confirm the transaction, see the loading spinner

Error messages or log output

Version

v12.0.0

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response

[sleepytanya]

1. First STX on mainnet from Uniswap, in 'Pending' state
2. Second transaction on BNB from Pancake Swaps
3. The popup for the second tx shows empty Activity tab
4. Second transaction appears on the Activity tab after ~30-45 seconds in 'Unapproved' state

https://github.com/MetaMask/metamask-extension/assets/104780023/6f7613b9-f5b7-4e65-88f8-4c514df2e06f

[sleepytanya]

Present on the latest v12.0.0 build

https://github.com/MetaMask/metamask-extension/assets/104780023/95defedb-ed65-4d7c-b99a-a86f21696193

[benjisclowder]

Collaborative Effort Required for Root Cause Analysis (RCA) on Critical Issues

We are quickly approaching the end of the quarter and we encourage you once again to take some moments and perform RCA on this critical issue. You may do so by answering the questions below:

1. What PR fixed the issue? 2. Can you pinpoint the commit from which the issue originated? 3. Write a short explanation of the technical cause of the bug 4. How could we have avoided merging this bug? What would have had to be different about our code, tests or processes? 4.1. Were there any missing unit, e2e or manual tests that could have preempted this issue? 4.2. Were there any other elements lacking, such as typed code, comprehensive documentation, well-architected APIs, etc., that might have prevented this issue? 4.3. If your answer to a and b is no, then is there anything at all that you can think of that, if it had been different before this bug was introduced, would have prevented it from being merged?

Please provide your answers as a reply to this comment and tag me as well.

You can read more about the initiative here. Thank you!

Tagging eng. who added the fix: @vinistevam

[vinistevam]

Collaborative Effort Required for Root Cause Analysis (RCA) on Critical Issues

We are quickly approaching the end of the quarter and we encourage you once again to take some moments and perform RCA on this critical issue. You may do so by answering the questions below:

1. What PR fixed the issue? 2. Can you pinpoint the commit from which the issue originated? 3. Write a short explanation of the technical cause of the bug 4. How could we have avoided merging this bug? What would have had to be different about our code, tests or processes? 4.1. Were there any missing unit, e2e or manual tests that could have preempted this issue? 4.2. Were there any other elements lacking, such as typed code, comprehensive documentation, well-architected APIs, etc., that might have prevented this issue? 4.3. If your answer to a and b is no, then is there anything at all that you can think of that, if it had been different before this bug was introduced, would have prevented it from being merged?

Please provide your answers as a reply to this comment and tag me as well.

You can read more about the initiative here. Thank you!

Tagging eng. who added the fix: @vinistevam

1. What PR fixed the issue? The PR that fixed the issue is the one that fetches transactions across all networks to address the nonce management limitation.

2. Can you pinpoint the commit from which the issue originated? Not sure about the specific commit, because this is a new scenario created by the evolving effort implementing multichain.

3. Write a short explanation of the technical cause of the bug. The bug was caused by a limitation in the Transaction Controller's nonce management, which locked the nonce for a pending transaction on one network, preventing the generation of a valid nonce for a new transaction on a different network. This resulted in the dApp transaction being generated with an empty nonce. The update extends in the client the nonce validation across different networks to resolve this issue.

4. How could we have avoided merging this bug? What would have had to be different about our code, tests or processes? E2E or integration test case covering this scenario.

5. Were there any missing unit, e2e or manual tests that could have preempted this issue? Yes, E2E for a more robust coverage of the flow. Now I've added unit tests and documentation so if something changes the tests will break.

6. If your answer to a and b is no, then is there anything at all that you can think of that, if it had been different before this bug was introduced, would have prevented it from being merged? It's hard to say, now with multichain we will see new scenarios coming and we can think of the most common and cover it with E2E and integration tests.

cc. @benjisclowder Let me know if you need any extra info. :pray: