Skip to content

fix: restore isCurrentlyOwned when re-adding an NFT - #9814

Open
SnowingFox wants to merge 1 commit into
MetaMask:mainfrom
SnowingFox:fix/restore-iscurrentlyowned-addnft
Open

fix: restore isCurrentlyOwned when re-adding an NFT#9814
SnowingFox wants to merge 1 commit into
MetaMask:mainfrom
SnowingFox:fix/restore-iscurrentlyowned-addnft

Conversation

@SnowingFox

@SnowingFox SnowingFox commented Aug 10, 2026

Copy link
Copy Markdown

Description

Fixes #9787

When an NFT leaves a wallet and is later re-acquired, the manual import path
reports success but the NFT stays invisible. #addMultipleNfts refreshes an
existing state entry with { ...existingEntry, ...nftMetadata }, and because
nftMetadata never carries isCurrentlyOwned, the stale isCurrentlyOwned: false written earlier by checkAndUpdateSingleNftOwnershipStatus survives the
merge.

Ownership is already proven on the manual import path: addNftVerifyOwnership
throws unless the user owns the token on-chain, and addNft/addNfts default
to Source.Custom when invoked for manual imports. This PR restores
isCurrentlyOwned: true when refreshing an existing entry on the
Source.Custom path, so re-acquired NFTs reappear. Other sources (autodetection
Source.Detected, dapp-suggested Source.Dapp) are untouched.

Changes

  • packages/assets-controllers/src/NftController.ts: restore isCurrentlyOwned
    to true in #addMultipleNfts when source === Source.Custom.
  • packages/assets-controllers/src/NftController.test.ts: add a regression test
    that re-adds a previously-unowned NFT via addNftVerifyOwnership and asserts
    isCurrentlyOwned flips back to true.
  • packages/assets-controllers/CHANGELOG.md: add [Unreleased] Fixed entry.

Test plan

cd packages/assets-controllers
NODE_OPTIONS=--experimental-vm-modules yarn jest src/NftController.test.ts src/NftDetectionController.test.ts --coverage=false
  • New regression test fails on main (Expected: true, Received: false) and
    passes with the fix.
  • NftController.test.ts: 117 passed, 4 snapshots.
  • NftDetectionController.test.ts: 20 passed.
  • eslint clean on changed files; changelog:validate passes.

Note

Low Risk
Narrow fix to manual NFT import merge logic with a targeted regression test; no auth, security, or broad behavioral changes outside Custom source updates.

Overview
Fixes re-acquired NFTs staying hidden after a successful manual import when state still had isCurrentlyOwned: false from an earlier ownership check.

In #addMultipleNfts, updating an existing entry now sets isCurrentlyOwned: true when source === Source.Custom (manual import / addNftVerifyOwnership), instead of leaving the stale flag from the spread merge. Autodetection and dapp-suggested sources are unchanged.

Adds a regression test for addNftVerifyOwnership and an [Unreleased] changelog entry.

Reviewed by Cursor Bugbot for commit 13aba0e. Bugbot is set up for automated code reviews on this repo. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

addNft never restores isCurrentlyOwned, making re-acquired NFTs permanently invisible

1 participant