-
Notifications
You must be signed in to change notification settings - Fork 118
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/tokens metadata #576
Feat/tokens metadata #576
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #576 +/- ##
===========================================
- Coverage 65.52% 64.60% -0.92%
===========================================
Files 79 87 +8
Lines 8049 8759 +710
Branches 1257 1346 +89
===========================================
+ Hits 5274 5659 +385
- Misses 2770 3095 +325
Partials 5 5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far so good. I think next steps are ingesting token metadata from the event server.
@asimm241 there is a team (Swapr) that would rely on the completion of this PR by the end of May. would this timeline work for you? |
Expanding on this, this PR would enable our wallets to support the FT (SIP 010) and NFT (SIP 009) standards in that we'd be able to properly fetch metadata associated with a given token. This would allow us to display the ticker, name, decimals (for fungible tokens) and the asset image. We have some options on how we could do this ourselves via contract calls in the wallets, but an API based approach would be much better in the long term. Related: leather-io/extension#1070, leather-io/extension#1028 |
I have the same questions as @aulneau, and not clear how this would get used (i.e. queried) to help wallets and explorers. for example, there is no event generated when a new sip-011 token is created (besides a contract deployment), so how would the api node know that it needs to call a bunch of functions on a contract to retrieve things like symbol, name, decimals, and metadata (which requires 2 rpc call), and the sip-010 (and sip-009) meta data schema is different (albeit "compatible", so far). And with the experience building |
I think so. |
The API can check the ABI in the contract-deploy tx handler. If the ABI "contains" the expected sip-10 or sip-09 elements it can be passed to another component responsible for retrieving the initial metadata.
The API can/will expose differences here where necessary?
Can you elaborate or give some examples here? |
That will not work consistently, see for example https://github.com/tokensoft/tokensoft_token_stacks/blob/main/contracts/tokensoft-token.clar#L110-L119, where the meta data url can be changed at will, and of course, the meta data file itself can be changed even more often (not that would know any token that rebranded, ever, 😉 ) I'm also considering doing something similar where I have some generic swapr token that are pre-deployed (simplifies the ui), but gets configured with the correct data, when creating a new pair for example.
I'm experimenting with adding one more image format (explicitly vector) for one, and for a second, swapr will need some extra meta data to help generate the proper post conditions (find the token name, and some token may require special case, so we need something fairly fluid that would store whatever is in the metadata file (as long as it is compliant with the base schema). Rather than storing information in the database at deployment, it may be better to consider querying the token functions on a regular basis (less than daily after the first time?), so you can provide some kind of caching, but can also guarantee it won't get stale. |
This was discussed in the original RFC that requested this API feature. For now we are considering the values as static. If there's a strong desire to have dynamic metadata then I'd suggest an amendment to SIP-10 and SIP-09 to specify a contract print event indicating when metadata has been changed. |
I just checked the tokens from Arkadiko, and they are also using a variable for the token uri, so this seems pretty universal amongst the major projects working on fungible tokens, at least so far, that they assume their metadata is dynamic. Seems correct to me. |
I'd still suggest a new SIP/amendment that specifies contract events should be emitted when metadata is updated. The API could deal with TTLs / cache-invalidation for this potentially in the future, but I would not expect that to be an initial feature. |
@zone117x question for you, is it possible scope this PR such that in addition to fetching the token_uri data, it would be able to expose the |
currently, swapr is using |
@asimm241 I need some help to deblur the next steps on this PR:
@zone117x If this requires an amendment to SIP-10 and SIP-09, should we involved the authors? Is it a blocker to move on with this PR? |
Yes. When SIP-09/SIP-10 contracts are deployed, the API should retrieve and store these values.
Supporting dynamic metadata changes is not a blocker for this PR, it's a feature request. In order for the API to support that feature, it involves SIP amendments. |
sure
aren't you creating more work if we know this is not going work for most tokens, as opposed to try to make something that works out of the gate?
can't seem be convinced this is the solution. No event will be generated if the metadata json file is updated, and that metadata file was purposefully created as something outside the contract so it could be modified easily, while the contract can not. So in order to not forever present stale data, the api node would need to query that file on a regular basis anyway (weekly or less is probably fine, it is probably not going to change that often...), and if you need to do this, you might as well get the up to date uri value before checking the file content for changes. |
I'm not sure if that's true or meaningful with the current sample size of tokens.
A contract-call can be made to signal that metadata should be updated. If you're aiming to have remote JSON file rapidly changing such that an "expiry" event via contract-call is too much too ask for, then setting an arbitrary TTL of a week or a month or whatever won't solve that problem anyway. The clear solution here is for the API to be notified when token metadata is updated, for example via contract events. You're asking the API to implement a kind of statefulness that does not yet exist in the codebase. Different API instances can easily end up with different views/responses for the same requested resource. Additionally, attempting to cache with absolutely zero indication of a TTL will result in either scalability/wastefulness problems on one end, or long-expired data on the other end. Trying to solve that by picking an interval "in the middle" like a week doesn't solve that problem, rather it just introduces a bit of both problems. That said, I'm not absolutely against implementing a generic cache refreshing mechanism like you're suggesting, but it is a kludge and should not be treated as the solution for dynamically updating metadata. |
Thank you for the dialogue, @zone117x, it helps better understand where you are coming from and what your concerns are. You make valid points. Getting a bit late for me here, so I'll sleep on it and keep on the conversation tomorrow. |
74144ab
to
2574a84
Compare
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/blockstack/stacks-blockchain-api/FFzWCh6d97ptqwJ9saUrBUeDaX5c |
…g and serving token metadata images
…essing after event-replay import
23f4faa
to
8127607
Compare
@zone117x Does this fully support NFT metadata in addition to FT metadata? |
@markmhx it doesn't support NFTs |
🤔 good catch |
No. NFT |
🎉 This PR is included in version 0.65.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@markmhx see new issue #742 for completing NFT class metadata support -- however, the last I heard was that NFT class support wasn't very important, and that individual NFT token metadata support is what we want, which requires some discussion. I'm setting up a meeting with a few of the stakeholders. In the meantime, it would be helpful if UX could start a doc or gh issue/discussion to capture initial requirements? |
Sounds good. On the UX side, we're looking to display individual names, individual images and class names for particular NFTs as seen here: leather-io/extension#1028 (comment) I could see us dropping the need for class metadata entirely if we can move forward with just token metadata to start. Please do indicate whether @jasperjansz needs to update these designs given limitations to what data can be made available. |
I have in .env the following: STACKS_API_ENABLE_NFT_METADATA=1 When I connect to: http://localhost:3999/extended/v1/tokens/nft/metadata I get the following error: "error": "NFT metadata processing is not enabled on this server" What gives? Little help here. Thanks guys / gals! |
Description
This PR adds the support for Fungible token metadata and Non Fungible token metadata. It checks the deployed contract's compliance with sip-9 and sip-10.
It checks the standards by comparing contract abi. If it is compliance, It fetches data from the contract by making read-only call to:
For FT:
For NFT:
Using the token uri it fetches the metdata from the web, and store the relevent data in the database.
It exposes two APIs
v1/tokens/:contractId/ft/metadata
v1/tokens/:contractId/nft/metadata
Type of Change
Are documentation updates required?
@zone117x do we need to add documentation
Testing information
npm run test:integration:tokens
npm run test:token
Checklist
npm run test
passes