[Bug]: Simulation Details reports incorrect ERC20 amount if an NFT is minted #24586
Labels
release-11.18.0
Issue or pull request that will be included in release 11.18.0
Sev1-high
High severity; partial loss of service with severe impact upon users, with no workaround.
team-transactions
Transactions team
type-bug
Comments
3 tasks
gauthierpetetin
added
the
Sev1-high
dbrans
added a commit
to MetaMask/core
that referenced
this issue
May 21, 2024
…ns that include an NFT mint (#4290) ## Explanation When simulating a transaction resulting in an NFT mint, we skip the prior owner check for the NFT since the token does not exist yet. However, for subsequent tokens, we were using the incorrect index for the prior balance transaction. The fix is to keep a separate prevBalanceIndex, which we only increment if a prior balance transaction was included in the simulation. ## References * Related to MetaMask/metamask-extension#24586 ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/transaction-controller` - **FIXED**: Incorrect token balance changes for simulations of multiple tokens that include an NFT mint. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
dbrans
added a commit
to MetaMask/core
that referenced
this issue
May 21, 2024
## Explanation Patch release of TransactionController with #4290 ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? For example: * Fixes #12345 * Related to #67890 --> * Related to MetaMask/metamask-extension#24586
7 tasks
|
Fixed - when NFT is minted the simulation displays correct amount: |
dbrans
added a commit
that referenced
this issue
May 22, 2024
## **Description** Bump transaction controller to get a couple fixes in. [](https://codespaces.new/MetaMask/metamask-extension/pull/24701?quickstart=1) ## **Related issues** Fixes: #24586 Fixes: #24183 Related: #24359 @keithchew to verify ## **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 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.
metamaskbot
added
the
release-11.18.0
dbrans
added a commit
that referenced
this issue
May 22, 2024
## **Description** Bump transaction controller to get a couple fixes in. [](https://codespaces.new/MetaMask/metamask-extension/pull/24701?quickstart=1) ## **Related issues** Fixes: #24586 Fixes: #24183 Related: #24359 @keithchew to verify ## **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 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.
dbrans
added a commit
that referenced
this issue
May 23, 2024
Bump transaction controller to get a couple fixes in. [](https://codespaces.new/MetaMask/metamask-extension/pull/24701?quickstart=1) Fixes: #24586 Fixes: #24183 Related: #24359 @keithchew to verify 1. Go to this page... 2. 3. <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] --> <!-- [screenshots/recordings] --> - [ ] I’ve followed [MetaMask 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. - [ ] 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
May 23, 2024
Bump transaction controller to get a couple fixes in. [](https://codespaces.new/MetaMask/metamask-extension/pull/24701?quickstart=1) Fixes: #24586 Fixes: #24183 Related: #24359 @keithchew to verify 1. Go to this page... 2. 3. <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] --> <!-- [screenshots/recordings] --> - [ ] I’ve followed [MetaMask 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. - [ ] 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.
](https://codespaces.new/MetaMask/metamask-extension/pull/24701?quickstart=1){kind=link}
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Simulation Details reports incorrect ERC20 amount if an NFT is minted.
Reported here: https://community.metamask.io/t/unlock-protocol-support/28955
Expected behavior
No response
Screenshots/Recordings
No response
Steps to reproduce
Have 66 USDC on Optimism and go through purchase flow on https://app.unlock-protocol.com/event/mcon-iii
Error messages or log output
No response
Version
Develop
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: