Skip to content

Commit

Permalink
Enhance checkAndUpdateSingleCollectibleOwnershipStatus to use use pas…
Browse files Browse the repository at this point in the history 
…sed accountParams for selectedAddress and chainId (#672)

* Enhance checkAndUpdateSingleCollectibleOwnershipStatus to use use passed accountParams for selectedAddress and chainId
  • Loading branch information
@adonesky1 @MajorLift
adonesky1 authored and MajorLift committed Oct 11, 2023
1 parent c80ca76 commit d4e5108
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 27 deletions.
65 changes: 65 additions & 0 deletions src/assets/CollectiblesController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
});
});
});
});
55 changes: 28 additions & 27 deletions src/assets/CollectiblesController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export interface CollectibleMetadata {
lastSale?: ApiCollectibleLastSale;
}

interface DetectionParams {
interface AccountParams {
userAddress: string;
chainId: string;
}
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -540,7 +536,7 @@ export class CollectiblesController extends BaseController<
address: string,
tokenId: string,
collectibleMetadata: CollectibleMetadata,
detection?: DetectionParams,
detection?: AccountParams,
): Promise<Collectible[]> {
// TODO: Remove unused return
const releaseLock = await this.mutex.acquire();
Expand Down Expand Up @@ -598,7 +594,7 @@ export class CollectiblesController extends BaseController<
this.updateNestedCollectibleState(
newCollectibles,
ALL_COLLECTIBLES_STATE_KEY,
{ chainId, selectedAddress },
{ chainId, userAddress: selectedAddress },
);

return newCollectibles;
Expand All @@ -616,7 +612,7 @@ export class CollectiblesController extends BaseController<
*/
private async addCollectibleContract(
address: string,
detection?: DetectionParams,
detection?: AccountParams,
): Promise<CollectibleContract[]> {
const releaseLock = await this.mutex.acquire();
try {
Expand Down Expand Up @@ -686,7 +682,7 @@ export class CollectiblesController extends BaseController<
this.updateNestedCollectibleState(
newCollectibleContracts,
ALL_COLLECTIBLES_CONTRACTS_STATE_KEY,
{ chainId, selectedAddress },
{ chainId, userAddress: selectedAddress },
);

return newCollectibleContracts;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit d4e5108

Please sign in to comment.