Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions packages/assets-controllers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
102 changes: 102 additions & 0 deletions packages/assets-controllers/src/NftController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
62 changes: 62 additions & 0 deletions packages/assets-controllers/src/NftController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1858,6 +1858,68 @@ 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<Nft> {
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.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, {
userAddress: addressToSearch,
chainId,
});
Comment thread
cursor[bot] marked this conversation as resolved.
}

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.
Expand Down
Loading