-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Conversation
Oh nice! @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? |
src/subcommand/server.rs
Outdated
Extension(index): Extension<Arc<Index>>, | ||
Extension(config): Extension<Arc<Config>>, | ||
Path(inscription_id): Path<InscriptionId>, | ||
) -> ServerResult<Response> { |
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.
Response should be type Json?
See
https://github.com/ordinals/ord/pull/2493/files#diff-8e4ad094cf68664c9ac22638126698f549875d0e0f2dbf6b4af48554f1670f85R794
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. |
Hilariously I originally just returned cbor, but then i saw raph was trying to jsonify all the things so changed it. Im happy to change it back and make deserialzation someone else’s problem.
Sent from [Proton Mail](https://proton.me/mail/home) for iOS
…On Mon, Oct 30, 2023 at 4:54 PM, Casey Rodarmor ***@***.***(mailto:On Mon, Oct 30, 2023 at 4:54 PM, Casey Rodarmor <<a href=)> wrote:
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.
—
Reply to this email directly, [view it on GitHub](#2604 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/A3UR6LUHIYSPTCW4AWMTFTTYCAH2FAVCNFSM6AAAAAA6TIZ4AWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBWGAZDINJQGY).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
f9dc015
to
60d28ad
Compare
60d28ad
to
a8a9272
Compare
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. |
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.
LGTM!
closes #2603
quick little endpoint. open to change the path, or whatever other feedback folks have :)