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 metadata recursive endpoint #2604

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

rot13maxi
Copy link
Contributor

closes #2603

quick little endpoint. open to change the path, or whatever other feedback folks have :)

@elocremarc
Copy link
Contributor

elocremarc commented Oct 27, 2023

Oh nice!
@raphjaph lets just yeet into /-/ and get the endpoints moving.

@rot13maxi can you test out if this works on regtest via recursion by adding the CSP-Headers like Raphs pr? I usually make a small inscription for recursive endpoints see if it works correctly client side. Wonder if we should all rebase off his?

#2493

Extension(index): Extension<Arc<Index>>,
Extension(config): Extension<Arc<Config>>,
Path(inscription_id): Path<InscriptionId>,
) -> ServerResult<Response> {
Copy link
Contributor

@elocremarc elocremarc Oct 27, 2023

Choose a reason for hiding this comment

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

@casey
Copy link
Collaborator

casey commented Oct 30, 2023

I think for metadata, we should return the CBOR. Converting the CBOR to JSON makes the details of that conversion part of the contract of the endpoint. The mapping from CBOR to JSON isn't well defined, and I'm not sure how serde would handle types in the CBOR which have no JSON equivalent. It sucks to make inscriptions deserialize CBOR, but someone can always inscribe a CBOR parser, and everyone can use that, and when something goes wrong, we can blame the library author instead of us 😂

For the response, I would just return a string with the hex-encoded CBOR, since all recursive endpoint responses must be JSON.

@rot13maxi
Copy link
Contributor Author

rot13maxi commented Oct 30, 2023 via email

@lifofifoX
Copy link
Collaborator

Instead of metadata specific endpoint, how about a recursive endpoint for inscription that includes metadata among other things like current owner, sat location etc? Basically, whatever we can include without needing a full sat index.

Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

LGTM!

@raphjaph raphjaph enabled auto-merge (squash) November 1, 2023 20:48
@raphjaph raphjaph changed the title add endpoint for metadata Add metadata recursive endpoint Nov 1, 2023
@raphjaph raphjaph merged commit 0a90581 into ordinals:master Nov 1, 2023
6 checks passed
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.

Feature Request: recursive endpoint to read metadata
5 participants