From 804bd4160ef8156158b5c3833e79d279073aa36e Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 13 Apr 2026 09:42:54 +0200 Subject: [PATCH 1/3] fix: revert change on extension checking single NFT ownership --- packages/assets-controllers/CHANGELOG.md | 10 +- .../src/NftController.test.ts | 102 ++++++++++++++++++ .../assets-controllers/src/NftController.ts | 71 ++++++++++++ 3 files changed, 178 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 0d4588da169..f02c94f3f8f 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -10,12 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** `NftController` no longer uses the `AssetsContractController:getERC721OwnerOf` and `AssetsContractController:getERC1155BalanceOf` messenger actions for ownership checks; these have been removed from `AllowedActions` ([#8281](https://github.com/MetaMask/core/pull/8281)) - - Consumers that construct the `NftController` messenger and register handlers for these two actions must remove them from their allowed actions list. -- **BREAKING:** Removed the `checkAndUpdateSingleNftOwnershipStatus` method from `NftController` ([#8281](https://github.com/MetaMask/core/pull/8281)) - - Use `checkAndUpdateAllNftsOwnershipStatus` instead, which now batches all ownership checks via Multicall3 in a single RPC request. -- `checkAndUpdateAllNftsOwnershipStatus` now removes NFTs confirmed as unowned from state instead of setting `isCurrentlyOwned: false` ([#8281](https://github.com/MetaMask/core/pull/8281)) + - Consumers that construct the `NftController` messenger must remove these two actions from their allowed actions list. +- **BREAKING:** `checkAndUpdateSingleNftOwnershipStatus` no longer accepts a `batch` boolean as its second argument; the signature is now `(nft, networkClientId, { userAddress }?)` ([#8281](https://github.com/MetaMask/core/pull/8281)) + - The method now always writes the updated NFT to state and returns it. Remove the `batch` argument from all call sites. +- **BREAKING:** `checkAndUpdateAllNftsOwnershipStatus` now removes NFTs confirmed as unowned from state instead of setting `isCurrentlyOwned: false` on them ([#8281](https://github.com/MetaMask/core/pull/8281)) - The `isCurrentlyOwned: false` flag was originally used to power a "Previously Owned" NFTs section in MetaMask, which is no longer supported. NFTs that are confirmed as no longer owned are now removed from state immediately rather than being retained with a stale flag. -- `NftController` NFT ownership checks (`isNftOwner`, `checkAndUpdateAllNftsOwnershipStatus`) now use Multicall3 to batch ERC-721 `ownerOf` and ERC-1155 `balanceOf` calls into fewer RPC requests, falling back to individual calls on unsupported chains ([#8281](https://github.com/MetaMask/core/pull/8281)) +- `NftController` NFT ownership checks (`isNftOwner`, `checkAndUpdateSingleNftOwnershipStatus`, `checkAndUpdateAllNftsOwnershipStatus`) now use Multicall3 to batch ERC-721 `ownerOf` and ERC-1155 `balanceOf` calls into fewer RPC requests, falling back to individual calls on unsupported chains ([#8281](https://github.com/MetaMask/core/pull/8281)) - Bump `@metamask/accounts-controller` from `^37.1.1` to `^37.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363)) - Bump `@metamask/keyring-controller` from `^25.1.1` to `^25.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363)) - Bump `@metamask/messenger` from `^1.0.0` to `^1.1.1` ([#8364](https://github.com/MetaMask/core/pull/8364), [#8373](https://github.com/MetaMask/core/pull/8373)) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 7cc9953a55c..e5f4d693c78 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -4363,6 +4363,108 @@ describe('NftController', () => { ).toBe(true); }); }); + + describe('checkAndUpdateSingleNftOwnershipStatus', () => { + it('should check whether the passed NFT is still owned by the current selectedAccount/chainId combination and update its isCurrentlyOwned property in state when isNftOwner returns false', async () => { + const { nftController } = setupController({ + defaultSelectedAccount: OWNER_ACCOUNT, + }); + + const nft = { + address: '0x02', + tokenId: '1', + name: 'name', + image: 'image', + description: 'description', + standard: 'standard', + favorite: false, + }; + + await nftController.addNft(nft.address, nft.tokenId, 'mainnet', { + nftMetadata: nft, + }); + + expect( + nftController.state.allNfts[OWNER_ACCOUNT.address][ChainId.mainnet][0] + .isCurrentlyOwned, + ).toBe(true); + + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); + + await nftController.checkAndUpdateSingleNftOwnershipStatus( + nft, + 'mainnet', + ); + + expect( + nftController.state.allNfts[OWNER_ACCOUNT.address][ChainId.mainnet][0] + .isCurrentlyOwned, + ).toBe(false); + }); + + it('should check whether the passed NFT is still owned by the selectedAddress/chainId combination passed in the accountParams argument and update its isCurrentlyOwned property in state, when the currently configured selectedAddress/chainId are different from those passed', async () => { + const firstSelectedAddress = OWNER_ACCOUNT.address; + const { + nftController, + triggerPreferencesStateChange, + triggerSelectedAccountChange, + } = setupController(); + + triggerSelectedAccountChange(OWNER_ACCOUNT); + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + displayNftMedia: true, + }); + + const nft = { + address: '0x02', + tokenId: '1', + name: 'name', + image: 'image', + description: 'description', + standard: 'standard', + favorite: false, + }; + + await nftController.addNft(nft.address, nft.tokenId, 'sepolia', { + nftMetadata: nft, + }); + + expect( + nftController.state.allNfts[firstSelectedAddress][ChainId.sepolia][0] + .isCurrentlyOwned, + ).toBe(true); + + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); + const secondAccount = createMockInternalAccount({ + address: SECOND_OWNER_ADDRESS, + }); + triggerSelectedAccountChange(secondAccount); + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + displayNftMedia: true, + }); + + const updatedNft = + await nftController.checkAndUpdateSingleNftOwnershipStatus( + nft, + 'sepolia', + { + userAddress: OWNER_ADDRESS, + }, + ); + + expect(updatedNft).toStrictEqual({ + ...nft, + isCurrentlyOwned: false, + }); + + expect( + nftController.state.allNfts[OWNER_ADDRESS][SEPOLIA.chainId][0] + .isCurrentlyOwned, + ).toBe(false); + }); + }); }); describe('findNftByAddressAndTokenId', () => { diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 16becb507a2..702e5e734b2 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1858,6 +1858,77 @@ export class NftController extends BaseController< }); } + /** + * Checks whether input NFT is still owned by the user + * and updates the isCurrentlyOwned value on the NFT object accordingly. + * + * @param nft - The NFT object to check and update. + * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. + * @param accountParams - The userAddress to check ownership against. + * @param accountParams.userAddress - the address passed through the confirmed transaction flow to ensure assets are stored to the correct account + * @returns the NFT with the updated isCurrentlyOwned value + */ + async checkAndUpdateSingleNftOwnershipStatus( + nft: Nft, + networkClientId: NetworkClientId, + { userAddress }: { userAddress?: string } = {}, + ): Promise { + const addressToSearch = this.#getAddressOrSelectedAddress(userAddress); + const { + configuration: { chainId }, + } = this.messenger.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); + const { address, tokenId } = nft; + let isOwned = nft.isCurrentlyOwned; + try { + isOwned = await this.isNftOwner( + addressToSearch, + address, + tokenId, + networkClientId, + { standard: nft.standard }, + ); + } catch { + // ignore error + // this will only throw an error 'Unable to verify ownership' in which case + // we want to keep the current value of isCurrentlyOwned for this flow. + } + + const updatedNft = { + ...nft, + isCurrentlyOwned: isOwned, + }; + + const { allNfts } = this.state; + const nfts = [...(allNfts[addressToSearch]?.[chainId] ?? [])]; + const indexToUpdate = nfts.findIndex( + (item) => + item.tokenId === tokenId && + item.address.toLowerCase() === address.toLowerCase(), + ); + + if (indexToUpdate !== -1) { + nfts[indexToUpdate] = updatedNft; + this.update((state) => { + state.allNfts[addressToSearch] = Object.assign( + {}, + state.allNfts[addressToSearch], + { + [chainId]: nfts, + }, + ); + }); + this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { + userAddress: addressToSearch, + chainId, + }); + } + + return updatedNft; + } + /** * Checks whether NFTs associated with current selectedAddress/chainId combination are still owned by the user * And updates the isCurrentlyOwned value on each accordingly. From 18a705c4a0468f1c175a2669a89ec40e5b364589 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 13 Apr 2026 11:29:39 +0200 Subject: [PATCH 2/3] chore:r evert changelog --- packages/assets-controllers/CHANGELOG.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index f02c94f3f8f..e36d7a3ef56 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -10,15 +10,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** `NftController` no longer uses the `AssetsContractController:getERC721OwnerOf` and `AssetsContractController:getERC1155BalanceOf` messenger actions for ownership checks; these have been removed from `AllowedActions` ([#8281](https://github.com/MetaMask/core/pull/8281)) - - Consumers that construct the `NftController` messenger must remove these two actions from their allowed actions list. -- **BREAKING:** `checkAndUpdateSingleNftOwnershipStatus` no longer accepts a `batch` boolean as its second argument; the signature is now `(nft, networkClientId, { userAddress }?)` ([#8281](https://github.com/MetaMask/core/pull/8281)) - - The method now always writes the updated NFT to state and returns it. Remove the `batch` argument from all call sites. -- **BREAKING:** `checkAndUpdateAllNftsOwnershipStatus` now removes NFTs confirmed as unowned from state instead of setting `isCurrentlyOwned: false` on them ([#8281](https://github.com/MetaMask/core/pull/8281)) + - Consumers that construct the `NftController` messenger and register handlers for these two actions must remove them from their allowed actions list. +- **BREAKING:** Removed the `checkAndUpdateSingleNftOwnershipStatus` method from `NftController` ([#8281](https://github.com/MetaMask/core/pull/8281)) + - Use `checkAndUpdateAllNftsOwnershipStatus` instead, which now batches all ownership checks via Multicall3 in a single RPC request. +- `checkAndUpdateAllNftsOwnershipStatus` now removes NFTs confirmed as unowned from state instead of setting `isCurrentlyOwned: false` ([#8281](https://github.com/MetaMask/core/pull/8281)) - The `isCurrentlyOwned: false` flag was originally used to power a "Previously Owned" NFTs section in MetaMask, which is no longer supported. NFTs that are confirmed as no longer owned are now removed from state immediately rather than being retained with a stale flag. -- `NftController` NFT ownership checks (`isNftOwner`, `checkAndUpdateSingleNftOwnershipStatus`, `checkAndUpdateAllNftsOwnershipStatus`) now use Multicall3 to batch ERC-721 `ownerOf` and ERC-1155 `balanceOf` calls into fewer RPC requests, falling back to individual calls on unsupported chains ([#8281](https://github.com/MetaMask/core/pull/8281)) +- `NftController` NFT ownership checks (`isNftOwner`, `checkAndUpdateAllNftsOwnershipStatus`) now use Multicall3 to batch ERC-721 `ownerOf` and ERC-1155 `balanceOf` calls into fewer RPC requests, falling back to individual calls on unsupported chains ([#8281](https://github.com/MetaMask/core/pull/8281)) - Bump `@metamask/accounts-controller` from `^37.1.1` to `^37.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363)) - Bump `@metamask/keyring-controller` from `^25.1.1` to `^25.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363)) - Bump `@metamask/messenger` from `^1.0.0` to `^1.1.1` ([#8364](https://github.com/MetaMask/core/pull/8364), [#8373](https://github.com/MetaMask/core/pull/8373)) +- Bump `@metamask/transaction-controller` from `^64.0.0` to `^64.1.0` ([#8432](https://github.com/MetaMask/core/pull/8432)) ## [103.1.1] From 55939764db3ba69a70e8a557e8752fcedafbf7bb Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 13 Apr 2026 11:49:55 +0200 Subject: [PATCH 3/3] chore: minor update --- packages/assets-controllers/CHANGELOG.md | 12 ++++++++---- packages/assets-controllers/src/NftController.ts | 9 --------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index e36d7a3ef56..5201641bd24 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -11,16 +11,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** `NftController` no longer uses the `AssetsContractController:getERC721OwnerOf` and `AssetsContractController:getERC1155BalanceOf` messenger actions for ownership checks; these have been removed from `AllowedActions` ([#8281](https://github.com/MetaMask/core/pull/8281)) - Consumers that construct the `NftController` messenger and register handlers for these two actions must remove them from their allowed actions list. -- **BREAKING:** Removed the `checkAndUpdateSingleNftOwnershipStatus` method from `NftController` ([#8281](https://github.com/MetaMask/core/pull/8281)) - - Use `checkAndUpdateAllNftsOwnershipStatus` instead, which now batches all ownership checks via Multicall3 in a single RPC request. -- `checkAndUpdateAllNftsOwnershipStatus` now removes NFTs confirmed as unowned from state instead of setting `isCurrentlyOwned: false` ([#8281](https://github.com/MetaMask/core/pull/8281)) +- **BREAKING:** `checkAndUpdateSingleNftOwnershipStatus` no longer accepts a `batch` boolean as its second argument; the signature is now `(nft, networkClientId, { userAddress }?)` ([#8281](https://github.com/MetaMask/core/pull/8281), [#8435](https://github.com/MetaMask/core/pull/8435)) + - The method now always writes the updated NFT to state and returns it. Remove the `batch` argument from all call sites. +- **BREAKING:** `checkAndUpdateAllNftsOwnershipStatus` now removes NFTs confirmed as unowned from state instead of setting `isCurrentlyOwned: false` on them ([#8281](https://github.com/MetaMask/core/pull/8281)) - The `isCurrentlyOwned: false` flag was originally used to power a "Previously Owned" NFTs section in MetaMask, which is no longer supported. NFTs that are confirmed as no longer owned are now removed from state immediately rather than being retained with a stale flag. -- `NftController` NFT ownership checks (`isNftOwner`, `checkAndUpdateAllNftsOwnershipStatus`) now use Multicall3 to batch ERC-721 `ownerOf` and ERC-1155 `balanceOf` calls into fewer RPC requests, falling back to individual calls on unsupported chains ([#8281](https://github.com/MetaMask/core/pull/8281)) +- `NftController` NFT ownership checks (`isNftOwner`, `checkAndUpdateSingleNftOwnershipStatus`, `checkAndUpdateAllNftsOwnershipStatus`) now use Multicall3 to batch ERC-721 `ownerOf` and ERC-1155 `balanceOf` calls into fewer RPC requests, falling back to individual calls on unsupported chains ([#8281](https://github.com/MetaMask/core/pull/8281)) - Bump `@metamask/accounts-controller` from `^37.1.1` to `^37.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363)) - Bump `@metamask/keyring-controller` from `^25.1.1` to `^25.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363)) - Bump `@metamask/messenger` from `^1.0.0` to `^1.1.1` ([#8364](https://github.com/MetaMask/core/pull/8364), [#8373](https://github.com/MetaMask/core/pull/8373)) - Bump `@metamask/transaction-controller` from `^64.0.0` to `^64.1.0` ([#8432](https://github.com/MetaMask/core/pull/8432)) +### Fixed + +- Restore `checkAndUpdateSingleNftOwnershipStatus` to `NftController` to fix a regression where consumers (e.g. the MetaMask extension) that call this method individually were broken by its removal in [#8281](https://github.com/MetaMask/core/pull/8281) ([#8435](https://github.com/MetaMask/core/pull/8435)) + ## [103.1.1] ### Changed diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 702e5e734b2..5c4e9649b75 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1911,15 +1911,6 @@ export class NftController extends BaseController< if (indexToUpdate !== -1) { nfts[indexToUpdate] = updatedNft; - this.update((state) => { - state.allNfts[addressToSearch] = Object.assign( - {}, - state.allNfts[addressToSearch], - { - [chainId]: nfts, - }, - ); - }); this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { userAddress: addressToSearch, chainId,