Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

ERC721 NFTs with different token identifiers may show incorrect balance #6954

Closed
StephenHeaps opened this issue Feb 16, 2023 · 2 comments · Fixed by #6983
Closed

ERC721 NFTs with different token identifiers may show incorrect balance #6954

StephenHeaps opened this issue Feb 16, 2023 · 2 comments · Fixed by #6983
Assignees
@StephenHeaps
Labels
Epic: Brave Wallet QA Pass - iPad QA Pass - iPhone X QA/Yes release-notes/include
Milestone
1.49

Comments

@StephenHeaps
Copy link
Contributor

StephenHeaps commented Feb 16, 2023 •
edited
Loading

Description:

On Portfolio, the balance calculations are not taking into account the token id so two ERC721 NFTs from the same collection (same contract address) but with different token ids might report the same balance

ref: https://bravesoftware.slack.com/archives/C023VS4HJ6Q/p1676555545832879

Steps to Reproduce

pre-requisite: Own 1 ERC721 NFT that belongs to a collection with an NFT you don't own. Same contract address, symbol and chain but different token id.

  1. Add 2 ERC721 NFTs with same contract address, symbol and chain but different token ids (you must have a different balance for the two NFTs)
    e.g. NFT1 and NFT2
  2. Observe balance is displayed the same for both, despite only owning one.

h/t @Pavneet-Sing
@jamesmudgett jamesmudgett moved this to Backlog in Web3 Feb 17, 2023
@Pavneet-Sing
Copy link

Pavneet-Sing commented Feb 21, 2023 •
edited
Loading

I think the steps to produce this issue on iOS would be:

  1. Buy two NFTs with same contract addressed and symbol. e.g. NFT1 and NFT2
  2. Make sure the balance is accurate on Portfolio screen

@StephenHeaps StephenHeaps self-assigned this Feb 21, 2023
@StephenHeaps StephenHeaps moved this from Backlog to In Progress in Web3 Feb 21, 2023
@StephenHeaps StephenHeaps mentioned this issue Feb 21, 2023
Merged
11 tasks
@StephenHeaps StephenHeaps added this to the 1.49 milestone Feb 22, 2023
StephenHeaps added a commit that referenced this issue Feb 23, 2023
@StephenHeaps
Fix #6954: Balance of ERC721 when 2 added with different token identi…
dacdd14 
…fiers (#6983)

Include the token's `tokenId` in `assetBalanceId` so two different ERC721 NFTs with the same contract address, token and chain id can be added and have balance displayed correctly.
@github-project-automation github-project-automation bot moved this from In Progress to Done in Web3 Feb 23, 2023
@srirambv
Copy link
Contributor

srirambv commented Mar 8, 2023

Verification passed on the following devices running 1.49 (23.3.3.16)

  • Verified steps from #6983
  • Verified correct balance is shown for NFT's with the same contract address but with different token id's
iPhone XR (iOS 16.3.1) iPad Pro (iOS 16.4 Beta 2)
6954-iPhone.XR.MP4
6954-iPad.Pro.MP4

@Pavneet-Sing Pavneet-Sing mentioned this issue Mar 16, 2023
Closed
@Uni-verse Uni-verse mentioned this issue Apr 13, 2023
Closed
arthuredelstein pushed a commit to brave/brave-core that referenced this issue Feb 13, 2024
@StephenHeaps
Fix brave/brave-ios#6954: Balance of ERC721 when 2 added with differe…
250014e 
…nt token identifiers (brave/brave-ios#6983)

Include the token's `tokenId` in `assetBalanceId` so two different ERC721 NFTs with the same contract address, token and chain id can be added and have balance displayed correctly.
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Assignees

@StephenHeaps StephenHeaps

Labels
Epic: Brave Wallet QA Pass - iPad QA Pass - iPhone X QA/Yes release-notes/include
Projects
Web3
Archived in project
Milestone
1.49
Development

Successfully merging a pull request may close this issue.

Fix #6954: Balance of ERC721 when 2 added with different token identifiers brave/brave-ios
3 participants
@StephenHeaps @Pavneet-Sing @srirambv