[Bug]: Multichain - Cannot destructure property 'balance' of 'K[Z]' as it is undefined. and Cannot destructure property 'balance' of 'accounts[fromAddress]' as it is undefined.

[seaona]

Describe the bug

Triggering a transaction on one chain, and without approving it, triggering a signature in another chain, whenever I reject/accept the first transaction, MM breaks with the error Cannot destructure property 'balance' of 'K[Z]' as it is undefined.

I've also seen it fail with this other message Cannot destructure property 'balance' of 'accounts[fromAddress]' as it is undefined.

Expected behavior

No response

Screenshots/Recordings

destructure-balance-z.mp4

cannot-destructure-balance-accts.mp4

Steps to reproduce

1. Connect to the test dapp with Sepolia
2. Connect to pancakeswap with Ethereum
3. Trigger a swap in pancakeswap https://pancakeswap.finance/swap?chain=eth
4. Without accepting/rejecting, go to the test dapp and trigger a signature
5. Reject the first tx
6. See how MM breaks

Error messages or log output

No response

Version

12.0.0

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

[darkwing]

Will continue trying to reproduce but I had no issue with this case but will keep trying:

Screen.Recording.2024-06-19.at.12.58.42.PM.mov

[darkwing]

The actual error occurs in: ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.container.js are accepting or rejecting the first swap transaction. The error line is:

// FYI: This line was introduced 3 years ago...
const { balance } = accounts[fromAddress];

The problem is that fromAddress becomes undefined after (in this case) rejecting:

The from address comes from:

// FYI: This line was introduced 5 years ago
const {
    from: fromAddress,
    to: txParamsToAddress,
    gasPrice,
    gas: gasLimit,
    value: amount,
    data,
  } = (transaction && transaction.txParams) || txParams;

[dan437]

@matthewwalsh0 can you or someone from the Confirmations team have a look?

[jiexi]

Was able to reproduce Message: Cannot destructure property 'balance' of 'accounts[fromAddress]' as it is undefined.

[sleepytanya]

Present in the latest build [https://github.com/MetaMask/metamask-extension/commit/eebc4d5b2284024eaeb190cddfa89990f593ca34]

Screen.Recording.2024-06-20.at.17.43.05.mov

[sleepytanya]

I think this is the same bug? With different errors. We could combine them into one.

[sleepytanya]

• Connect to test-dapp
• Click button to start legacy send transaction; popup comes up, I see a "1" in the extension icon, I ignore the popup window
• Open new tab, go to Uniswap
• Click "Connect" button, click MetaMask; I see a "2" in the extension icon, no popup occurs
• Click "Connect" button again, click MetaMask; I see a "3" in the extension icon, no popup occurs
• After the first request is rejected and the subsequent request can be accessed MM can break with an error

[sleepytanya]

Start send+swap txs with low gas
Send another (approve) tx from a dapp
Confirmation is stuck on the spinner
Activity tab has two txs - Pending and Unapproved
Unapprovedtxs can't be rejected
If user rejects first tx MM displays an error Cannot destructure property 'balance' of 'Q[z]' as it is undefined.

[sleepytanya]

[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: @digiwand

[digiwand]

Hi @benjisclowder,

1. What PR fixed the issue?
   fix: Cannot destructure property 'balance' of 'K[Z]' as it is undefined fromAddress error when no transaction is loaded in ConfirmTransactionBase #25506

2. Can you pinpoint the commit from which the issue originated?
   Unknown

3. Write a short explanation of the technical cause of the bug

In confirm-transaction-base.container.js, while the transaction is switching, the component may be in a state without transaction data. It is a race-condition.

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?
   I think we are missing e2e tests.
   4.2. Were there any other elements lacking, such as typed code, comprehensive documentation, well-architected APIs, etc., that might have prevented this issue?
   I don't think we are supporting multiple dapps in our e2e tests.
   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?