Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Nonce - Nonce doesn't update while having 2 pending transactions, and after confirming the first one #27617

Open
seaona opened this issue Oct 4, 2024 · 9 comments · Fixed by #27874
Labels
release-12.6.0 Issue or pull request that will be included in release 12.6.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team type-bug

Comments

@seaona
Copy link
Contributor

seaona commented Oct 4, 2024

Describe the bug

While having the Custom Nonce settings enabled, if I queue 2 transactions and confirm the first one, I can see how the nonce for the 2nd transaction is never updated.

Expected behavior

Same as in prod? The nonce is updated after confirming the first transaction

Screenshots/Recordings

In the first part of the video you see the issue, in the second part, you see how it's the behaviour currently in production 12.3.1

nonce-not-updated-unapproved-tx.mp4

Steps to reproduce

  1. Go to Advanced Settings
  2. Enable Custom nonce
  3. Go to the test dapp
  4. Trigger 2 transactions
  5. Confirm the first one
  6. See how the second one still has the old nonce
  7. Confirming the second one leads to nonce too low error

Error messages or log output

No response

Detection stage

During release testing

Version

12.5.0

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

@seaona seaona added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. release-blocker This bug is blocking the next release labels Oct 4, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Oct 4, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Oct 4, 2024
@bschorchit bschorchit added the team-confirmations Push issues to confirmations team label Oct 4, 2024
@jpuri
Copy link
Contributor

jpuri commented Oct 11, 2024

Hey @seaona : I am not able to replicate this issue in release branch 12.5

Screen.Recording.2024-10-11.at.3.55.32.PM.mov

@seaona
Copy link
Contributor Author

seaona commented Oct 14, 2024

hey @jpuri I'm still able to reproduce it consistently. Could it be something around settings? I have smart transactions disabled.
See how the popup nonce never updates. If I open the full view, I see that the nonce is updated in full view but not in the popup, and sending the transaction from the popup makes it fail with the nonce too low error.

nonce-not-update-batch-tx.mp4

@sleepytanya
Copy link
Contributor

sleepytanya commented Oct 15, 2024

@seaona @jpuri

Probably we are encountering two distinct issues related to nonce management?

  • Subsequent transactions are being generated with identical nonce values;
  • There is a discrepancy in the nonce displayed between the popup and the expanded view

RC 12.5.0, STX disabled.

  1. Two transactions were initiated, both assigned the same nonce value of 65, as indicated in the popup
  2. Following the submission of the first transaction, the nonce value for the pending second transaction does not update
  3. Upon attempting to confirm the second transaction, the correct subsequent nonce value of 66 is momentarily displayed, followed by an 'already known' error message
  4. Only one transaction, with the nonce 65, is submitted / visible in the Activity list
1.mov
  1. The nonce for the last transaction that was submitted (and confirmed on the block explorer) is 65
  2. Three subsequent transactions were initiated, each displaying a nonce of 66 in the popup
  3. However, when viewed in the expanded interface, all three transactions are shown to have a nonce of 65
2.mov

github-merge-queue bot pushed a commit that referenced this issue Oct 15, 2024
…7874)

## **Description**

Fix issue with nonce not updating when there are multiple transaction
created in parallel and once transaction is submitted.

## **Related issues**

Fixes: #27617

## **Manual testing steps**

1. Go to testdapp
2. create 2 transactions and submit first one
3. Nonce for second transaction should update

## **Screenshots/Recordings**
TODO

## **Pre-merge author checklist**

- [X] 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).
- [X] I've completed the PR template to the best of my ability
- [X] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [X] 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.
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Oct 15, 2024
@metamaskbot metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 15, 2024
hjetpoluru pushed a commit that referenced this issue Oct 15, 2024
…multiple parallel transactions (#27852)

## **Description**

Fix issue with nonce not updating when there are multiple transaction
created in parallel and once transaction is submitted.

## **Related issues**

Fixes: #27617

## **Manual testing steps**

1. Go to testdapp
2. create 2 transactions and submit first one
3. Nonce for second transaction should update

## **Screenshots/Recordings**
TODO

## **Pre-merge author checklist**

- [X] 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).
- [X] I've completed the PR template to the best of my ability
- [X] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [X] 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.
@gauthierpetetin gauthierpetetin added release-12.6.0 Issue or pull request that will be included in release 12.6.0 and removed release-12.7.0 Issue or pull request that will be included in release 12.7.0 labels Oct 21, 2024
@sleepytanya sleepytanya reopened this Oct 26, 2024
@github-project-automation github-project-automation bot moved this from Fixed to To be fixed in Bugs by team Oct 26, 2024
@sleepytanya
Copy link
Contributor

The issue is present in the latest 12.6.0

On the legacy confirmations nonce value is not updated for second transaction after the first one is submitted (the second transaction is created with the incorrect nonce value):

legacySend.mov
legacyTxRequest.mov

On redesigned confirmations nonce is updated after first transaction is submitted:

redesignedTxRequest.mov

@sleepytanya sleepytanya added regression-RC-12.6.0 Regression bug that was found in release candidate (RC) for release 12.6.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 labels Oct 26, 2024
@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Oct 26, 2024
@metamaskbot
Copy link
Collaborator

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

@bschorchit bschorchit removed the release-blocker This bug is blocking the next release label Oct 29, 2024
@bschorchit bschorchit removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 regression-RC-12.5.0 regression-RC-12.6.0 Regression bug that was found in release candidate (RC) for release 12.6.0 labels Oct 29, 2024
@bschorchit
Copy link

bschorchit commented Oct 29, 2024

Re-posting this here for visibility. This is not a regression and we're not considering this issue no longer a release blocker. We'll go forward with the path of making the redesigned confirmations the default ones (on v12.8) as the main fix for this. There's also a task being planned to refactor the nonce implementation in the wallet to prevent similar issues in the future.

Slack message

hey all, I was testing the nonce one issue, and I am not sure it should be considered a release blocker. Actual issue:
trigger 2 transactions with the custom nonce option enable and see that when one transaction is submitted, the nonce does not automatically updates on the other transaction

Behaviour on 12.5:
fixed on new confirmations (so contract interactions + contract deployments)
old confirmations the issue highlighted above persists

Behaviour on 12.4.1 (downloaded from the Chrome store):
old confirmations - the custom nonce on the 2nd confirmation is cleared.
Bottom line:
issue occurs on old confirmations, for users that have custom nonce enabled and have multiple transactions to confirm at the same time.

behaviour on 12.4.1 is doubtfully better, as the user needs to remember which is the next sequential nonce and input it manually, while on 12.5.0 the user needs to bump that value by 1 (if he does not bump it and keep the same value, we will not submit the transaction and will return an error to the dapp for that transaction)

For the reasons above I would not consider this a release blocker and would even question if it makes sense for us to fix this ahead of enabling the new confirmations by default for all users.

@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Oct 29, 2024
@metamaskbot
Copy link
Collaborator

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

@seaona
Copy link
Contributor Author

seaona commented Nov 4, 2024

could be the same as these users are experiencing?
cc @bschorchit

#28239

@sleepytanya
Copy link
Contributor

Possibly fixed by #28272

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-12.6.0 Issue or pull request that will be included in release 12.6.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team type-bug
Projects
Archived in project
Status: To be fixed
Development

Successfully merging a pull request may close this issue.

6 participants