leather-io / extension

Leather browser extension
https://leather.io
MIT License
293 stars 140 forks source link

Show minimal listings under collectibles for Stacks NFTs without metadata available from API #3290

Open 314159265359879 opened 1 year ago

314159265359879 commented 1 year ago

Previously the wallet would show a list of names and amounts of non fungible tokens available in the wallet.

With the new gallery view not all are shown. Perhaps some of the images are not yet available in the api used?

I would expect the wallet to show all collectables available on the address. If there are no images then show the list of names and amounts only for those. Or have a toggle option gallery/list.

For this address the collectables in the wallet showed one NFT. SP1CM0AWD5FCT7RMNCQJ29XTX7ANXH4HMDDY3QW9H

Whereas this is shown on the explorer: image

314159265359879 commented 1 year ago

A user noted that the original Megapont series is not showing up in the collectables. Lets make sure one of the most popular NFT series is visible in the wallet.

markmhendrickson commented 1 year ago

Can we request a screenshot of this user's collectibles area to cross-check?

@fbwoolf can you think of any way we might be inadvertently filtering these out? Perhaps if they lack a certain attribute?

r0zar commented 1 year ago

Also seeing a similar problem with the bitgear NFT collection.

Screen Shot 2023-03-08 at 4 41 04 PM
r0zar commented 1 year ago

Here's the metadata file and contract address, if that's helpful.

markmhendrickson commented 1 year ago

@rafaelcr thoughts on whether this could be server-side with the new metadata API?

314159265359879 commented 1 year ago

https://explorer.stacks.co/address/SP1SCEXE6PMGPAC6B4N5P2MDKX8V4GF9QDE1FNNGJ.bitcoin-degens?chain=mainnet

Another collection not showing on the wallet yet. Reported by BowTiedDeployer, on gamma.io and xverse it does get displayed alright.

fbwoolf commented 1 year ago

@rafaelcr thoughts on whether this could be server-side with the new metadata API?

Yeah, typically when I check the new endpoint for missing NFTs it is returning Token not found. @Edu opened an issue in the api repo so I'll link this issue there: https://github.com/hirosystems/token-metadata-api/issues/142

EDIT: I do also see an odd filter on token_uri here so I'll look into that at the same time.

fbwoolf commented 1 year ago

Seems relevant to: https://github.com/hirosystems/wallet/issues/3452 so I'm assigning to myself.

rafaelcr commented 1 year ago

Sorry for the late response @markmhx @fbwoolf I'm looking into it today

rafaelcr commented 1 year ago

I found the underlying issue, it seems to be something with the clarity contracts... see https://github.com/hirosystems/token-metadata-api/issues/140#issuecomment-1474111451

rafaelcr commented 1 year ago

For now, I've manually refreshed the contracts mentioned in this thread so they should be showing up in the next few hours. The underlying issue will be tracked here: https://github.com/hirosystems/token-metadata-api/issues/140

markmhendrickson commented 1 year ago

I'm modifying this issue so we can handle the case wallet-side wherein the wallet knows the user has a particular Stacks NFT but isn't able to retrieve metadata for it for whatever reason from the API. In this case, we can show a listing under the collectibles area with minimal data (i.e. whatever we have aside from the metadata API) so the user can at least see that the wallet recognizes it (because currently the NFT simply appears missing).

cc @mica000 might we already have such a minimal listing design handy or would this be a new case?

mica000 commented 1 year ago

@markmhx This would be a new use case but not sure I follow why we would need to show a list under existing cards of collectibles. Do we know which information we are able to display?

markmhendrickson commented 1 year ago

By "listing" I mean the unit inside the grid (not sure what we're calling these exactly):

Screenshot 2023-04-17 at 12 47 23

Which is to say, we'd show a minimal version of this unit (i.e. without image, title) and include just what we know directly from the contract (e.g. the type of info shown here by default: https://github.com/hirosystems/wallet/issues/3551#issue-1670124716).

@fbwoolf do you call what data points exactly we get back from the contract vs. rely on the metadata API? To help with design guidance here re: what we have on hand to include sans metadata 🙏

fbwoolf commented 1 year ago

@fbwoolf do you call what data points exactly we get back from the contract vs. rely on the metadata API? To help with design guidance here re: what we have on hand to include sans metadata 🙏

Here, we are just using the metadata query, which you can always find docs on what is returned in the api docs: https://docs.hiro.so/metadata

markmhendrickson commented 1 year ago

@fbwoolf Oh I mean, in the scenario in which we don't find any data from the metadata API, we have other on-chain attributes for the asset, yea? Like the contract name and much else to work with?

fbwoolf commented 1 year ago

@fbwoolf Oh I mean, in the scenario in which we don't find any data from the metadata API, we have other on-chain attributes for the asset, yea? Like the contract name and much else to work with?

No, in the component when we render the collectible tile we only have the metadata query info. We do use the nft holdings endpoint to map thru getting that data, but that only has the asset_identifier on it ...so we would have principal. Code would have to be reworked thought to have access to it in the component.