From d7ec98b20cb52c62b776defe1fff4c1d632516b2 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Fri, 7 Jan 2022 11:21:31 -0600 Subject: [PATCH] Enhance checkAndUpdateSingleCollectibleOwnershipStatus to use use passed accountParams for selectedAddress and chainId (#672) * Enhance checkAndUpdateSingleCollectibleOwnershipStatus to use use passed accountParams for selectedAddress and chainId --- src/assets/CollectiblesController.test.ts | 65 +++++++++++++++++++++++ src/assets/CollectiblesController.ts | 55 +++++++++---------- 2 files changed, 93 insertions(+), 27 deletions(-) diff --git a/src/assets/CollectiblesController.test.ts b/src/assets/CollectiblesController.test.ts index 959145da4e..55cedff286 100644 --- a/src/assets/CollectiblesController.test.ts +++ b/src/assets/CollectiblesController.test.ts @@ -25,6 +25,7 @@ const MAINNET_PROVIDER = new HttpProvider( 'https://mainnet.infura.io/v3/ad3a368836ff4596becc3be8e2f137ac', ); const OWNER_ADDRESS = '0x5a3CA5cD63807Ce5e4d7841AB32Ce6B6d9BbBa2D'; +const SECOND_OWNER_ADDRESS = '0x500017171kasdfbou081'; const OPEN_SEA_HOST = 'https://api.opensea.io'; const OPEN_SEA_PATH = '/api/v1'; @@ -1301,6 +1302,70 @@ describe('CollectiblesController', () => { expect(updatedCollectible.isCurrentlyOwned).toBe(false); }); + + it('should check whether the passed collectible is still owned by the 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 firstNetworkType = 'rinkeby'; + const secondNetworkType = 'ropsten'; + + preferences.update({ selectedAddress: OWNER_ADDRESS }); + network.update({ + provider: { + type: firstNetworkType, + chainId: NetworksChainId[firstNetworkType], + }, + }); + + const { selectedAddress, chainId } = collectiblesController.config; + const collectible = { + address: '0x02', + tokenId: '1', + name: 'name', + image: 'image', + description: 'description', + standard: 'standard', + favorite: false, + }; + + await collectiblesController.addCollectible( + collectible.address, + collectible.tokenId, + collectible, + ); + + expect( + collectiblesController.state.allCollectibles[selectedAddress][ + chainId + ][0].isCurrentlyOwned, + ).toBe(true); + + sandbox.restore(); + sandbox + .stub(collectiblesController, 'isCollectibleOwner' as any) + .returns(false); + + preferences.update({ selectedAddress: SECOND_OWNER_ADDRESS }); + network.update({ + provider: { + type: secondNetworkType, + chainId: NetworksChainId[secondNetworkType], + }, + }); + + await collectiblesController.checkAndUpdateSingleCollectibleOwnershipStatus( + collectible, + false, + { + userAddress: OWNER_ADDRESS, + chainId: NetworksChainId[firstNetworkType], + }, + ); + + expect( + collectiblesController.state.allCollectibles[OWNER_ADDRESS][ + NetworksChainId[firstNetworkType] + ][0].isCurrentlyOwned, + ).toBe(false); + }); }); }); }); diff --git a/src/assets/CollectiblesController.ts b/src/assets/CollectiblesController.ts index afbfb20a2b..b7e8ce7746 100644 --- a/src/assets/CollectiblesController.ts +++ b/src/assets/CollectiblesController.ts @@ -118,7 +118,7 @@ export interface CollectibleMetadata { lastSale?: ApiCollectibleLastSale; } -interface DetectionParams { +interface AccountParams { userAddress: string; chainId: string; } @@ -195,31 +195,27 @@ export class CollectiblesController extends BaseController< * @param newCollection - the modified piece of state to update in the controller's store * @param baseStateKey - The root key in the store to update. * @param passedConfig - An object containing the selectedAddress and chainId that are passed through the auto-detection flow. - * @param passedConfig.selectedAddress - the address passed through the collectible detection flow to ensure detected assets are stored to the correct account + * @param passedConfig.userAddress - the address passed through the collectible detection flow to ensure detected assets are stored to the correct account * @param passedConfig.chainId - the chainId passed through the collectible detection flow to ensure detected assets are stored to the correct account */ private updateNestedCollectibleState( newCollection: Collectible[] | CollectibleContract[], baseStateKey: 'allCollectibles' | 'allCollectibleContracts', - passedConfig?: { selectedAddress: string; chainId: string }, + { userAddress, chainId }: AccountParams | undefined = { + userAddress: this.config.selectedAddress, + chainId: this.config.chainId, + }, ) { - // We want to use the passedSelectedAddress and passedChainId when defined and not null - // these values are passed through the collectible detection flow, meaning they may not - // match as the currently configured values (which may be stale for this update) - const address = - passedConfig?.selectedAddress ?? this.config.selectedAddress; - const chain = passedConfig?.chainId ?? this.config.chainId; - const { [baseStateKey]: oldState } = this.state; - const addressState = oldState[address]; + const addressState = oldState[userAddress]; const newAddressState = { ...addressState, - ...{ [chain]: newCollection }, + ...{ [chainId]: newCollection }, }; const newState = { ...oldState, - ...{ [address]: newAddressState }, + ...{ [userAddress]: newAddressState }, }; this.update({ @@ -540,7 +536,7 @@ export class CollectiblesController extends BaseController< address: string, tokenId: string, collectibleMetadata: CollectibleMetadata, - detection?: DetectionParams, + detection?: AccountParams, ): Promise { // TODO: Remove unused return const releaseLock = await this.mutex.acquire(); @@ -598,7 +594,7 @@ export class CollectiblesController extends BaseController< this.updateNestedCollectibleState( newCollectibles, ALL_COLLECTIBLES_STATE_KEY, - { chainId, selectedAddress }, + { chainId, userAddress: selectedAddress }, ); return newCollectibles; @@ -616,7 +612,7 @@ export class CollectiblesController extends BaseController< */ private async addCollectibleContract( address: string, - detection?: DetectionParams, + detection?: AccountParams, ): Promise { const releaseLock = await this.mutex.acquire(); try { @@ -686,7 +682,7 @@ export class CollectiblesController extends BaseController< this.updateNestedCollectibleState( newCollectibleContracts, ALL_COLLECTIBLES_CONTRACTS_STATE_KEY, - { chainId, selectedAddress }, + { chainId, userAddress: selectedAddress }, ); return newCollectibleContracts; @@ -963,7 +959,7 @@ export class CollectiblesController extends BaseController< address: string, tokenId: string, collectibleMetadata?: CollectibleMetadata, - detection?: DetectionParams, + detection?: AccountParams, ) { address = toChecksumHexAddress(address); const newCollectibleContracts = await this.addCollectibleContract( @@ -1045,22 +1041,23 @@ export class CollectiblesController extends BaseController< * * @param collectible - The collectible object to check and update. * @param batch - A boolean indicating whether this method is being called as part of a batch or single update. + * @param accountParams - The userAddress and chainId to check ownership against + * @param accountParams.userAddress - the address passed through the confirmed transaction flow to ensure detected assets are stored to the correct account + * @param accountParams.chainId - the chainId passed through the confirmed transaction flow to ensure detected assets are stored to the correct account * @returns the collectible with the updated isCurrentlyOwned value */ async checkAndUpdateSingleCollectibleOwnershipStatus( collectible: Collectible, batch: boolean, + { userAddress, chainId }: AccountParams | undefined = { + userAddress: this.config.selectedAddress, + chainId: this.config.chainId, + }, ) { - const { allCollectibles } = this.state; - const { selectedAddress, chainId } = this.config; const { address, tokenId } = collectible; let isOwned = collectible.isCurrentlyOwned; try { - isOwned = await this.isCollectibleOwner( - selectedAddress, - address, - tokenId, - ); + isOwned = await this.isCollectibleOwner(userAddress, address, tokenId); } catch (error) { if (!error.message.includes('Unable to verify ownership')) { throw error; @@ -1074,15 +1071,19 @@ export class CollectiblesController extends BaseController< } // if this is not part of a batched update we update this one collectible in state - const collectibles = allCollectibles[selectedAddress]?.[chainId] || []; + const { allCollectibles } = this.state; + const collectibles = allCollectibles[userAddress]?.[chainId] || []; const collectibleToUpdate = collectibles.find( - (item) => item.tokenId === tokenId && item.address === address, + (item) => + item.tokenId === tokenId && + item.address.toLowerCase() === address.toLowerCase(), ); if (collectibleToUpdate) { collectibleToUpdate.isCurrentlyOwned = isOwned; this.updateNestedCollectibleState( collectibles, ALL_COLLECTIBLES_STATE_KEY, + { userAddress, chainId }, ); } return collectible;