Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ERC721/ERC1155 resolver #28

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat: add ERC721/ERC1155 resolver #28

wants to merge 6 commits into from

Conversation

Sekhmet
Copy link
Contributor

@Sekhmet Sekhmet commented Jul 9, 2022

Closes: #25

It adds NFT resolver for ERC721/ERC1155.
One issue I encountered is that OpenSea metadata URL always return 404 for ERC1155s and I'm not sure why - you can check test case for more details. fixed, was due to loss of precision when converting tokenIds.

@Sekhmet Sekhmet requested a review from bonustrack July 9, 2022 13:59
@Sekhmet Sekhmet self-assigned this Jul 9, 2022
@bonustrack
Copy link
Contributor

@Sekhmet The 2 examples you share are working well for me. One issue I found is with resizing, when using different width and height it crop the image, for example with this NFT:
https://opensea.io/assets/ethereum/0xedac702337b70f176423175d2f30fdf9c7a613a4/4858
I tried to render an image with same ratio but got this:
image
See here: http://localhost:3000/nft/0xedac702337b70f176423175d2f30fdf9c7a613a4/4858?h=444&w=305
We should instead make that the image contain the most of the origin image.

@Sekhmet
Copy link
Contributor Author

Sekhmet commented Jul 13, 2022

@bonustrack fixed issue with cropping as well with OpenSea ERC1155.

@bonustrack
Copy link
Contributor

I think we still need to resize the image to avoid storing too large file and make resizing later faster using the stored original file. The original image resize could work with the same max of 500 but keeping the aspect ratio

@Sekhmet
Copy link
Contributor Author

Sekhmet commented Jul 24, 2022

This should be ready, tests are failing due to our Pinata instance not being able to resolve SelfIDs IPFS avatars:
https://snapshot.mypinata.cloud/ipfs/QmYxazQj8wHersDbYqr1MYRmayoPwvmnRJrJbfgxn9odXb
https://ipfs.infura.io/ipfs/QmYxazQj8wHersDbYqr1MYRmayoPwvmnRJrJbfgxn9odXb

Can we do something about it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add resolver for NFT images
2 participants