Skip to content

Commit

Permalink
add+cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@adonesky1
adonesky1 committed Nov 13, 2023
1 parent 5999f4a commit 4a9ea83
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 63 deletions.
235 changes: 184 additions & 51 deletions packages/assets-controllers/src/NftController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,18 @@ describe('NftController', () => {
);
});

it('should error if tokenId is not a valid stringified decimal number', async function () {
const { nftController } = setupController();
const assetWithNumericTokenId = {
address: ERC721_NFT_ADDRESS,
tokenId: '123abc',
};

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore-next-line
const result = nftController.watchNft(assetWithNumericTokenId, ERC721);
await expect(result).rejects.toThrow('Invalid tokenId');
});
it('should error if address is invalid', async function () {
const { nftController } = setupController();
const assetWithInvalidAddress = {
Expand Down Expand Up @@ -2141,7 +2153,7 @@ describe('NftController', () => {
}),
);

const { nftController, getNetworkClientByIdSpy } = setupController({
const { nftController } = setupController({
getERC721TokenURIStub: jest.fn().mockImplementation((tokenAddress) => {
switch (tokenAddress) {
case '0x01':
Expand All @@ -2162,31 +2174,6 @@ describe('NftController', () => {
}),
});

getNetworkClientByIdSpy.mockImplementation((networkClientId) => {
switch (networkClientId) {
case 'sepolia':
return {
configuration: {
chainId: SEPOLIA.chainId,
},
};
case 'goerli':
return {
configuration: {
chainId: GOERLI.chainId,
},
};
case 'customNetworkClientId-1':
return {
configuration: {
chainId: '0xa',
},
};
default:
throw new Error('Invalid network client id');
}
});

await nftController.addNft('0x01', '1234', {
networkClientId: 'sepolia',
});
Expand Down Expand Up @@ -2243,6 +2230,128 @@ describe('NftController', () => {
},
]);
});

it('should add an NFT with the correct chainId/userAddress and metadata when passed a userAddress', async () => {
const userAddress = '0x123ABC';
nock('https://testtokenuri-1.com')
.get('/')
.reply(
200,
JSON.stringify({
image: 'test-image-1',
name: 'test-name-1',
description: 'test-description-1',
}),
);

nock('https://testtokenuri-2.com')
.get('/')
.reply(
200,
JSON.stringify({
image: 'test-image-2',
name: 'test-name-2',
description: 'test-description-2',
}),
);

nock('https://testtokenuri-3.com')
.get('/')
.reply(
200,
JSON.stringify({
image: 'test-image-3',
name: 'test-name-3',
description: 'test-description-3',
}),
);

const { nftController, preferences, changeNetwork } = setupController({
getERC721TokenURIStub: jest.fn().mockImplementation((tokenAddress) => {
switch (tokenAddress) {
case '0x01':
return 'https://testtokenuri-1.com';
case '0x02':
return 'https://testtokenuri-2.com';
default:
throw new Error('Not an ERC721 token');
}
}),
getERC1155TokenURIStub: jest.fn().mockImplementation((tokenAddress) => {
switch (tokenAddress) {
case '0x03':
return 'https://testtokenuri-3.com';
default:
throw new Error('Not an ERC1155 token');
}
}),
});

await nftController.addNft('0x01', '1234', {
userAddress,
});

changeNetwork(GOERLI);

await nftController.addNft('0x02', '4321', {
userAddress,
});
changeNetwork(SEPOLIA);

await nftController.addNft('0x03', '5678', {
userAddress,
});

// check that the currently selected address is not the same as userAddress
// to which the NFTs were added
expect(preferences.state.selectedAddress).toStrictEqual(OWNER_ADDRESS);

expect(nftController.state.allNfts[userAddress]['0x1']).toStrictEqual([
{
address: '0x01',
description: 'test-description-1',
image: 'test-image-1',
name: 'test-name-1',
tokenId: '1234',
favorite: false,
standard: ERC721,
tokenURI: 'https://testtokenuri-1.com',
isCurrentlyOwned: true,
},
]);

expect(
nftController.state.allNfts[userAddress][GOERLI.chainId],
).toStrictEqual([
{
address: '0x02',
description: 'test-description-2',
image: 'test-image-2',
name: 'test-name-2',
tokenId: '4321',
favorite: false,
standard: ERC721,
tokenURI: 'https://testtokenuri-2.com',
isCurrentlyOwned: true,
},
]);

expect(
nftController.state.allNfts[userAddress][SEPOLIA.chainId],
).toStrictEqual([
{
address: '0x03',
description: 'test-description-3',
image: 'test-image-3',
name: 'test-name-3',
tokenId: '5678',
favorite: false,
standard: ERC1155,
tokenURI: 'https://testtokenuri-3.com',
isCurrentlyOwned: true,
},
]);
});
});

describe('addNftVerifyOwnership', () => {
Expand Down Expand Up @@ -2287,31 +2396,8 @@ describe('NftController', () => {
});

it('should verify ownership by selected address and add NFT by the correct chainId when passed networkClientId', async () => {
const { nftController, preferences, getNetworkClientByIdSpy } =
setupController();

getNetworkClientByIdSpy.mockImplementation((networkClientId) => {
switch (networkClientId) {
case 'sepolia':
return {
configuration: {
chainId: SEPOLIA.chainId,
},
};
case 'goerli':
return {
configuration: {
chainId: GOERLI.chainId,
},
};
default:
return {
configuration: {
chainId: '0x1',
},
};
}
});
const { nftController, preferences } = setupController();

const firstAddress = '0x123';
const secondAddress = '0x321';

Expand Down Expand Up @@ -2352,6 +2438,53 @@ describe('NftController', () => {
isCurrentlyOwned: true,
});
});

it('should verify ownership by selected address and add NFT by the correct userAddress when passed userAddress', async () => {
const { nftController, changeNetwork, preferences } = setupController();

const firstAddress = '0x123';
const secondAddress = '0x321';

sinon.stub(nftController, 'isNftOwner' as any).returns(true);

sinon
.stub(nftController, 'getNftInformation' as any)
.returns({ name: 'name', image: 'url', description: 'description' });
changeNetwork(SEPOLIA);
await nftController.addNftVerifyOwnership('0x01', '1234', {
userAddress: firstAddress,
});
changeNetwork(GOERLI);
await nftController.addNftVerifyOwnership('0x02', '4321', {
userAddress: secondAddress,
});

// check that the currently selected address is not the same as either of the userAddresses
expect(preferences.state.selectedAddress).toStrictEqual(OWNER_ADDRESS);

expect(
nftController.state.allNfts[firstAddress][SEPOLIA.chainId][0],
).toStrictEqual({
address: '0x01',
description: 'description',
image: 'url',
name: 'name',
tokenId: '1234',
favorite: false,
isCurrentlyOwned: true,
});
expect(
nftController.state.allNfts[secondAddress][GOERLI.chainId][0],
).toStrictEqual({
address: '0x02',
description: 'description',
image: 'url',
name: 'name',
tokenId: '4321',
favorite: false,
isCurrentlyOwned: true,
});
});
});

describe('removeNft', () => {
Expand Down
19 changes: 7 additions & 12 deletions packages/assets-controllers/src/NftController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,22 +754,19 @@ export class NftController extends BaseController<NftConfig, NftState> {
*
* @param options - options.
* @param options.tokenAddress - Hex address of the NFT contract.
* @param options.chainId - The chainId of the network where the NFT is being added.
* @param options.userAddress - The address of the account where the NFT is being added.
* @param options.networkClientId - The networkClientId that can be used to identify the network client to use for this request.
* @param options.source - Whether the NFT was detected, added manually or suggested by a dapp.
* @returns Promise resolving to the current NFT contracts list.
*/
private async addNftContract({
tokenAddress,
chainId,
userAddress = this.config.selectedAddress,
userAddress,
networkClientId,
source,
}: {
tokenAddress: string;
chainId?: Hex;
userAddress?: string;
userAddress: string;
networkClientId?: NetworkClientId;
source?: Source;
}): Promise<NftContract[]> {
Expand All @@ -778,7 +775,6 @@ export class NftController extends BaseController<NftConfig, NftState> {
tokenAddress = toChecksumHexAddress(tokenAddress);
const { allNftContracts } = this.state;
const currentChainId = this.getCorrectChainId({
chainId,
networkClientId,
});

Expand Down Expand Up @@ -1162,16 +1158,12 @@ export class NftController extends BaseController<NftConfig, NftState> {
// temporary method to get the correct chainId until we remove chainId from the config & the chainId arg from the detection logic
// Just a helper method to prefer the networkClient chainId first then the chainId argument and then finally the config chainId
private getCorrectChainId({
chainId,
networkClientId,
}: {
chainId?: Hex;
networkClientId?: NetworkClientId;
}) {
if (networkClientId) {
return this.getNetworkClientById(networkClientId).configuration.chainId;
} else if (chainId) {
return chainId;
}
return this.config.chainId;
}
Expand Down Expand Up @@ -1337,7 +1329,11 @@ export class NftController extends BaseController<NftConfig, NftState> {
) {
throw new Error('This NFT is not owned by the user');
}
await this.addNft(address, tokenId, { networkClientId, source });
await this.addNft(address, tokenId, {
networkClientId,
userAddress,
source,
});
}

/**
Expand Down Expand Up @@ -1373,7 +1369,6 @@ export class NftController extends BaseController<NftConfig, NftState> {

const newNftContracts = await this.addNftContract({
tokenAddress,
chainId,
userAddress,
networkClientId,
source,
Expand Down

0 comments on commit 4a9ea83

Please sign in to comment.