metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

Published 20 hours ago • verified publisher icon MetaMask

[Bug]: Attempt to submit another transaction while having transaction in 'Pending' state, Smart Transactions toggle is ON

Open sleepytanya opened this issue 1 year ago • 4 comments

Describe the bug

I have limited knowledge around Smart Transactions feature and expected behavior so I will just write down steps.

When attempting to initiate a new transaction while another is in a 'Pending' state, with the Smart Transactions feature enabled, a series of chaotic behaviors have been observed:

  1. A transaction is set to 'Pending' state using low gas for a direct ETH->ETH send (nonce 1579).
  2. Another direct send ETH->ETH transaction is created.
  3. A red warning on the confirmation screen indicates 'A previous tx is still being signed or submitted,' and the 'Confirm' button is disabled.
  4. After rejection and creation of a new transaction (ETH->USDT), the 'Confirm' button becomes available without warnings.
  5. Clicking 'Confirm' creates 'Unapproved' transaction.
  6. The user can edit recipient and destination token fields for the 'Unapproved' transaction, but the 'Confirm' button is inactive, and rejection does not function.
  7. Modifying the 'Unapproved' transaction activates the 'Confirm' button, but clicking it generates another 'Unapproved' transaction.
  8. I created 1 'Pending' and 3 'Unapproved' transactions. Once the transactions were confirmed, only two Failed transactions can be seen within the Activity tab: nonce 1579 and 1580, marked as ETH->USDC. Popup notifying that the transaction was cancelled was displayed after some time.
  9. Notice that the popup and Smart Transactions screen are only displayed for the first and for the last transaction.

Related bug: https://github.com/MetaMask/metamask-extension/issues/25356

Expected behavior

Screenshots/Recordings

https://github.com/MetaMask/metamask-extension/assets/104780023/e59a7274-5c17-44cf-b242-004bcbf73e7f

https://github.com/MetaMask/metamask-extension/assets/104780023/5845bbc3-5763-426a-9b24-45ba3adeaa71

Screenshot 2024-06-14 at 23 43 53 Screenshot 2024-06-14 at 23 43 25 Screenshot 2024-06-14 at 23 44 10 Screenshot 2024-06-14 at 23 29 23 Screenshot 2024-06-14 at 23 29 09 Screenshot 2024-06-14 at 23 30 05

Steps to reproduce

  1. See the description

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

sleepytanya avatar Jun 15 '24 04:06 sleepytanya

Current behavior

  • Banner 'Transaction still pending to be signed or submitted' does not appear after a transaction has been rejected because a stx was still in progress
  • Confirm button is enabled after a transaction has been rejected because a stx was still in progress

Expected behavior

  • Banner 'Transaction still pending to be signed or submitted' should appear until original (stx) transaction has been submitted. Regardless of whether the user rejected a previous confirmation request
  • Confirm button should be disabled until original transaction is submitted.

hesterbruikman avatar Jun 20 '24 10:06 hesterbruikman

Resolved part of this in #25468; we now prevent subsequent transactions from the send flow when an STX is pending. However, as @sleepytanya pointed out, subsequent transactions are still able to be submitted through dapp, which results in an infinite loading state.

BZahory avatar Jun 21 '24 21:06 BZahory

Missing release label release-12.0.0 on issue. Adding release label release-12.0.0 on issue, as issue is linked to PR #25468 which has this release label.

metamaskbot avatar Jun 24 '24 19:06 metamaskbot

The issue is present in the latest v12.0.0 build

Multiple transactions can be created while the STX mode is ON which results in 'Unapproved' transactions. 'Unapproved' transactions can be modified.

  1. Have STX transaction in 'Pending' state
  2. Start Swap transaction, confirm swap
  3. See 2 transactions within the Activity list
  4. Transaction in 'Unapproved state' can be edited (the token and the recipient address can be changed), but can't be rejected
  5. Select another token, click 'Confirm'
  6. Notice how the original swap tx 'Swap 1 BAT to SUSHI' changed to 'Send 0.1 USDC'

https://github.com/MetaMask/metamask-extension/assets/104780023/59898811-47cc-4f2e-9a2b-da1a0c94ab07

sleepytanya avatar Jun 30 '24 04:06 sleepytanya

v12.0.0 build

The issue is present:

https://github.com/MetaMask/metamask-extension/assets/104780023/366d12a6-acc6-440c-8608-db82fda782d7

sleepytanya avatar Jul 01 '24 03:07 sleepytanya

~@sleepytanya can you test again on the latest build https://github.com/MetaMask/metamask-extension/pull/25098#issuecomment-2198759615~

Never mind, it looks like that is the build you tested with yesterday

danjm avatar Jul 02 '24 11:07 danjm

On the latest v12.0.0 branch queueing is disabled if there is STX in Pending state:

Ethereum + Sepolia testnet

https://github.com/MetaMask/metamask-extension/assets/104780023/0b4c4e98-2a96-4dfd-a90e-a8d67dbca7f1

Ethereum + BNB Chain

https://github.com/MetaMask/metamask-extension/assets/104780023/c975bb9a-155b-44a4-9f4a-d9791a9f43e2

Ethereum + Ethereum

https://github.com/MetaMask/metamask-extension/assets/104780023/bd978710-1b9b-4708-96a0-e23a51c5ad7a

sleepytanya avatar Jul 10 '24 00:07 sleepytanya

Thanks @sleepytanya - this is expected behavior for now 👍

forest-diggs-consensys avatar Jul 10 '24 13:07 forest-diggs-consensys

@danjm I guess we can close this?

sleepytanya avatar Jul 10 '24 14:07 sleepytanya

Fixed by: https://github.com/MetaMask/metamask-extension/pull/25478 https://github.com/MetaMask/metamask-extension/pull/25510

sleepytanya avatar Jul 10 '24 22:07 sleepytanya

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: @BZahory

benjisclowder avatar Aug 12 '24 15:08 benjisclowder