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

Add nifty asset pages #294

Merged
merged 6 commits into from
Apr 16, 2024
Merged

Add nifty asset pages #294

merged 6 commits into from
Apr 16, 2024

Conversation

febo
Copy link
Contributor

@febo febo commented Apr 10, 2024

This PR adds a page to show the details of Asset accounts from the Nifty Asset program, a lightweight open-source standard for non-fungible assets on Solana. Asset accounts can store a variable amount of information on-chain in the form of extensions – the page shows all the information on the account.

Details of the program can be found here: https://github.com/nifty-oss/asset

Here is the address of an asset on devnet: DKejcqEdGRz6D5WupdC1GT6RmG4vH8qnnweXSasgxPiM

And here one on mainnet: 3WBnQbQwqe1NBNAgQUZXLv64b1uncBATWpthHyHgqgNP

image

Copy link

vercel bot commented Apr 10, 2024

@febo is attempting to deploy a commit to the helius Team on Vercel.

A member of the Team first needs to authorize it.

@febo febo marked this pull request as ready for review April 10, 2024 22:20
Copy link

vercel bot commented Apr 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
xray-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 16, 2024 5:34am

@0xIchigo
Copy link
Contributor

gm @febo, and thank you for the PR!

I've gone ahead and deployed the branch to test it in my browser, and unfortunately, I'm getting a bunch of errors. When querying 3WBnQbQwqe1NBNAgQUZXLv64b1uncBATWpthHyHgqgNP on mainnet, it gets stuck loading due to a number of uncaught type errors. And, when querying DKejcqEdGRz6D5WupdC1GT6RmG4vH8qnnweXSasgxPiM on devnet, it tries to route to it as a token and nothing shows up.

Can you please have a look into these errors? Everything else looks solid code-wise

@febo
Copy link
Contributor Author

febo commented Apr 11, 2024

gm @febo, and thank you for the PR!

I've gone ahead and deployed the branch to test it in my browser, and unfortunately, I'm getting a bunch of errors. When querying 3WBnQbQwqe1NBNAgQUZXLv64b1uncBATWpthHyHgqgNP on mainnet, it gets stuck loading due to a number of uncaught type errors. And, when querying DKejcqEdGRz6D5WupdC1GT6RmG4vH8qnnweXSasgxPiM on devnet, it tries to route to it as a token and nothing shows up.

Can you please have a look into these errors? Everything else looks solid code-wise

Hi @0xIchigo, thanks for having a look at it. On the first issue, I can't see any errors either on the server or client console. Is there a chance that you can post some of the type errors that you see?

On the second issue, I noticed that the routing defaults to a "token" when it is not possible to determine the owner of the account here:

https://github.com/helius-labs/xray/blob/dev/src/lib/xray/lib/search.ts#L64-L73

I did not want to change that logic, but perhaps the default should be to route to an "account"? In any case, I have had the routing issue and the cause that I found is when it is not possible to determine the program owner of the account. I put it down as my endpoint not being too reliable.

@0xIchigo
Copy link
Contributor

gm @febo!

This is the error when I get searching for 3WBnQbQwqe1NBNAgQUZXLv64b1uncBATWpthHyHgqgNP on mainnet and the account doesn't load properly:
image

For the second issue, feel free to change it to default to an account

@samuelvanderwaal
Copy link

samuelvanderwaal commented Apr 13, 2024

gm @febo!

This is the error when I get searching for 3WBnQbQwqe1NBNAgQUZXLv64b1uncBATWpthHyHgqgNP on mainnet and the account doesn't load properly: image

For the second issue, feel free to change it to default to an account

This is strange; can you provide the steps for duplication as I also don't see errors. What I'm doing:

  • Cloning the fork
  • Checking out the branch: febo/nifty-asset
  • Adding api keys to .env file
  • npm install and npm run dev
  • Searching for asset 3WBnQbQwqe1NBNAgQUZXLv64b1uncBATWpthHyHgqgNP on mainnet

I see some 429 errors but then it loads:

Selection_039
Selection_040

@febo
Copy link
Contributor Author

febo commented Apr 13, 2024

gm @febo!

This is the error when I get searching for 3WBnQbQwqe1NBNAgQUZXLv64b1uncBATWpthHyHgqgNP on mainnet and the account doesn't load properly: image

For the second issue, feel free to change it to default to an account

Hi @0xIchigo

Changed the default routing to "account" as suggested.

For the type issues, we were not able to reproduce so not sure what could be the reason. The PR adds a new dependency @nifty-oss/asset to package.json, which has the type that you see in your error.

@0xIchigo
Copy link
Contributor

gm @febo @samuelvanderwaal ! Thank you for your contributions! It seems like Vercel isn't building the branch correctly with the new dependency for the preview build, which is why the errors are being thrown. I'm going to go ahead and merge this PR and then test things out before merging to main. If I have any questions, I'll reach out!

Copy link
Contributor

@0xIchigo 0xIchigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🔥

@0xIchigo 0xIchigo added this pull request to the merge queue Apr 16, 2024
Merged via the queue into helius-labs:dev with commit 967f689 Apr 16, 2024
2 of 3 checks passed
@samuelvanderwaal samuelvanderwaal deleted the febo/nifty-asset branch April 17, 2024 07:36
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.

3 participants