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 recursive directory and make all endpoints JSON #2493

Merged
merged 4 commits into from
Oct 30, 2023

Conversation

raphjaph
Copy link
Collaborator

@raphjaph raphjaph commented Oct 2, 2023

In order to avoid future clashes with other normal endpoints all future recursive endpoints should be under /-/. I copied the existing ones here as well and made sure they all return valid JSON. The existing ones will stay where they are for backwards compatability. Also the /content/<INSCRIPTION_ID> stays where it is.

Tagging everything relevant:
#2476
#2277
#2431
#2452

@elocremarc, @devords, @huuep, @casey, @veryordinally, @stet, @BitTogether, @tansanDOTeth and @rot13maxi what do you think? Is this the right way to proceed?

For example, future endpoints like the sat one would be: /-/sat/<SAT> and return JSON with a list of inscriptions on that sat.

@casey
Copy link
Collaborator

casey commented Oct 2, 2023

I didn't review the code, since we're still discussing the best way to do this.

There's another way we could do this, which is to serve all recursive endpoints under /content/.

So blocktime would be /content/blocktime, for example.

Then, recursive inscriptions could fetch recursive API endpoints with relative paths. For example, with a recursive inscription served at /content/INSCRIPTION_ID, it can reference blocktime like so:

const response = await fetch("blocktime");

This works because relative paths are interpreted by appending the relative path to the current URL with anything after the last slash removed.

The relative path blocktime resolved in a page served at /content/INSCRIPTION_ID is /content/blocktime.

This has the advantage of having shorter URLS in inscriptions, since they can request INSCRIPTION_ID or blocktime directly. It also means that if you want to serve recursive inscriptions, you can do so under a single path, e.g. /foo/bar/ or whatever. (You still need to redirect requests for the old recursive endpoints at /content, etc.)

@tansanDOTeth
Copy link

@raphjaph

In order to avoid future clashes with other normal endpoints all future recursive endpoints should be under /-/.

What are the reasons for selecting /-/? is it primarily to save on characters?

what do you think? Is this the right way to proceed?

General direction, yes. I think it is the right move to separate out inscription IDs with general API functions. I have no issues with using absolute paths which I think is an accepted convention for web.

@casey

The relative path blocktime resolved in a page served at /content/INSCRIPTION_ID is /content/blocktime.

This has the advantage of having shorter URLS in inscriptions, since they can request INSCRIPTION_ID or blocktime directly. It also means that if you want to serve recursive inscriptions, you can do so under a single path, e.g. /foo/bar/ or whatever. (You still need to redirect requests for the old recursive endpoints at /content, etc.)

If single path is a concern, then how about better namespacing?

Example with 3-letter abbreviation, but I imagine you could select much better formats to maximize for other values such as relative paths:

/ord/api/[api-functions]
/ord/ins/[inscription-id]

In terms of character count, its the same as /content. The advantages here are extensibility, and it has some hint of a domain making it more human readable.

More food for thought

If the URL is too long in general, it might make sense to solve this issue separately. Ex, one can hide the URL lengths with a small JS SDK with short function names. This way devs can fetch API functions and inscriptions with less focus on URL length.

Ex,

<script src="/content/[id-to-js-sdk].js" />
<script>
   const { ord } = window;
   ord.ins([content-id]);
   ord.api([function-name]);
</script>

@huuep
Copy link

huuep commented Oct 2, 2023

For inscriptions every byte matters, so I like paths that are as short as possible. @casey's relative path sounds good for keeping the endpoints very short.

Endpoints should also be as easy as possible to support for explorers. Having an endpoint that returns the full list of inscriptions at a given sat may not scale well when there are thousands of inscriptions on a given sat at some point in the future. Pagination/offset may be needed.

@elocremarc
Copy link
Contributor

elocremarc commented Oct 2, 2023

Currently I got #2452
Setup as follows:

  1. /-/content/sat/:sat/:page_index -> Content
  2. /-/sat/:sat/:number -> {id: <InscriptionId>}

I just saw this comment #2277 (comment) I will take out 1.
So we would only be returning the ID then so only endpoint 2?
We could make libraries to lower code footprint so we can get to the content still like the intent of 1.

Change to?
/content/sat/:sat/:number -> JSON: {id: <InscriptionId>}

For inscriptions every byte matters, so I like paths that are as short as possible. casey's relative path sounds good for keeping the endpoints very short.

agreed I think that is a good way to do it as well. To get the most recent inscription on a sat

const recentId = await fetch("/sat/69420/-1");
console.log(recentId.id)

Endpoints should also be as easy as possible to support for explorers. Having an endpoint that returns the full list of inscriptions at a given sat may not scale well when there are thousands of inscriptions on a given sat at some point in the future. Pagination/offset may be needed.

Yeah that is why I have /:number it returns just one at a time. If someone for loops through that however it could put more requests then doing pagination. Could rethink that too.

For inscriptions every byte matters

Something to consider for that is make a shorter inscriptionId based on block and index. See #2489

Question: Will we be able to resolve /content/sat/:sat/:index using the CSP headers we currently have for /content/?

@raphjaph
Copy link
Collaborator Author

raphjaph commented Oct 2, 2023

What are the reasons for selecting /-/? is it primarily to save on characters?

Yes, and that it is not used by anything else.

Question: Will we be able to resolve /content/sat/:sat/:index using the CSP headers we currently have for /content/?

Yes, the CSP header allows requests to all paths underneath.

Having an endpoint that returns the full list of inscriptions at a given sat may not scale well when there are thousands of inscriptions on a given sat at some point in the future. Pagination/offset may be needed.

At the moment I'm not too concerned about this but the /:number or /:offset makes sense to add.

@casey's approach is very elegant although it makes caching a bit more difficult since we now have dynamic content under /content. I imagine the endpoints looking like this under that scenario:

/content/sat/-1
/content/<INSCRIPTION_ID>
/content/blocktime

The only weird thing would be that some would return binary data + MIME type and others JSON.

@lifofifoX
Copy link
Collaborator

lifofifoX commented Oct 2, 2023

+1 on name-spacing. Not sold on using /- over using more descriptive single characters like /c (when we're returning content) or /s (for any sat specific endpoints). Endpoint paths exposed via recursion should get a priority over non-recursive ones when it comings to naming, as we'd probably never be able to remove them. But updating non-recursive endpoints is more doable.

Around the same topic should consider introducing a namespace for all the non-recursive endpoints too, especially for APIs. Maybe, even add a version prefix: /api/v1/...

Re: the approach proposed by @casey - I think the space savings by that would be miniscule and we risk making inscriptions brittle as relative paths would break when those inscriptions are loaded from other namespaces (i.e /content/sat/-1) in the future.

@elocremarc
Copy link
Contributor

although it makes caching a bit more difficult since we now have dynamic content

If /content/ would cause caching issues I think /-/ works pretty well currently in my sat endpoint.

/- over using more descriptive single characters like /c (when we're returning content) or /s

Do we think its a good idea to save on bytes by using shorter notations? That wouldn't be as clear for new people learning the endpoints without referencing the mapping and could cause collisions in future if a endpoint has similar spelling.

/-/sat -> /-/s
/-/blockheight -> /-/bh
we get a collision already
/-/blockhash -> /-/bh

I am gonna reference the old recursion discussion because we originally had it /-/content/ #2167
In hindsight maybe we should have left it. #2167 (comment)

@lifofifoX
Copy link
Collaborator

If /content/ would cause caching issues I think /-/ works pretty well currently in my sat endpoint.

I believe the caching issues are due to the same endpoint (i.e /content/sat/1234/-1) possibly returning different content based on what's been inscribed, making it harder to cache without a more complex cache invalidation method. So, /-/ would have the same issues.

@elocremarc
Copy link
Contributor

casey's approach is very elegant although it makes caching a bit more difficult since we now have dynamic content under /content.

"A programmer's job, fundamentally, is to drag his (or her!) balls through miles of broken glass if it benefits the user."

Get the neosporin out

@tansanDOTeth
Copy link

casey's approach is very elegant although it makes caching a bit more difficult since we now have dynamic content under /content.

"A programmer's job, fundamentally, is to drag his (or her!) balls through miles of broken glass if it benefits the user."

Get the neosporin out

Maybe they can be a sponsor

@tansanDOTeth
Copy link

tansanDOTeth commented Oct 3, 2023

Just thinking out loud here: Would it be a bad idea to have two sets of endpoints with the same functionality?

One human readable for the inscriptions that aren't sensitive to bytes. Link shortened version for byte sensitive inscriptions.

Most of the inscriptions I've seen don't have too many references to the URL and ones with many are often interpolating the URL path via a loop.

// Seen this a lot for many recursive inscriptions
let url = "/content/" + i

Personally, not really convinced 1-character URL path prefixes really make a huge difference in the examples I have seen so far.

@casey
Copy link
Collaborator

casey commented Oct 3, 2023

Another option is to make future recursive endpoints all single characters. This would make them very short, and set them apart from and avoid conflict with non-recursive endpoints.

@BitTogether
Copy link

I love it, it looks like /-/ will be my preferred method. Trying to consider all the avenues of future proofing and how everything on the roadmap might affect these endpoints going forward so we can avoid any future changes.
I dont see any obvious conflicts but lets make sure this will still works well in cases where inscription numbers are longer or the inscription id is longer or in the case of a quantum upgrade that lengthens keys, addresses, or txs. We need to picture the shortest useful recursive ordinal and then make it even smaller without breaking or complicating the endpoint, that might help to better measure the economics of our endpoints vs other solutions.

@elocremarc
Copy link
Contributor

elocremarc commented Oct 3, 2023

Another option is to make future recursive endpoints all single characters. This would make them very short, and set them apart from and avoid conflict with non-recursive endpoints.

/sat -> /s
/blockheight -> /h
/blockhash -> /bh
/blockhash/:height -> /bh/:height
/children -> /c

@casey We would still have to add all these to CSP headers by having /-/ we would only have to do one. Lets say we add even more like TX or output/input. We could run into collisions eventually.

/-/s
/-/h
/-/bh
/-/o output
/-/tx/:txid transaction
/-/i input
/-/i/:height Inscriptions in block

idk if we would add all these just trying to think of some more.

@zmeyer44
Copy link
Contributor

zmeyer44 commented Oct 4, 2023

I don't think we should be returing the json of the inscription id on the /sat/:sat/:number endpoint in an effort to make things more extensible in the future. These endpoints should be created with the expectation that they will be set-in-stone, and I think just returning the inscriptionId as a string rather than {id: inscriptionId} is a cleaner approach.

@elocremarc
Copy link
Contributor

and I think just returning the inscriptionId as a string rather than {id: inscriptionId} is a cleaner approach.

Look at this PR everything is to be Json going forward we are extending the prior block ones to be json.

These endpoints should be created with the expectation that they will be set-in-stone.

They would be set in stone id: would never change it would just add more if we ever want to put in things like rarity etc. We don't want to create a ton of endpoints if we can just just add a new key to the json response. Prior inscriptions will always be able to get response.id and newer ones would be able to key off of anything added like response.rarity.

In addition ordinals.com might not ever do that but other explorers are more than welcome to add all these keys. If we standardize to JSON then other explorers could do things first and then ordinals could eventually support it if it becomes popular.

You can make as many endpoints as you want and run your own explorers nobody should live and die by this repo. However coming together and standardizing things is important.

@elocremarc
Copy link
Contributor

elocremarc commented Oct 4, 2023

Summary

+ Pros
- Cons

Everything in JSON for future recursion

Pagination

Example /children and /sat routes with multiple inscriptions

Option A: 1 inscription at a time. -1 for most recent 1 for earliest. Returns object key id

+ Simple, Smaller request size
- requires more requests for many inscriptions, Doesn't have 0 index

Option B: 10-100 inscriptions at a time offset and reverse params. Returns array [ids]

+ Returns more inscriptions per request
- Needs params `offset` `reverse` bigger request size, Requires more code to work with array.

Routing

Option 1: Recursive Directory under /-/

Example: /-/sat/<SAT>/<Index> 
+ One CSP header, Easier caching
- Extra bytes. 

Option 2: Recursive Directory under /content/

Example: /content/blocktime would only need fetch(blocktime)
+ Shorter URLs, Relative paths
- Caching issues

Option 3: Single Character Endpoints /Letter

Example: /s for sat, /h for blockheight.
+ Shorter URLs, No collision with html routes
- Letter collisions. multiple CSP headers.

Option 4: Single Character Endpoints under /content/

Example: /content/s
+ Shortest possible bytes, 1 CSP header, Relative path.
- Letter collisions. Caching issues.

Option 5: Single Charcter Endpoints under /-/

Example: /-/s
+ 1 CSP header, segregated routes, Short route names, Easier caching
- Letter collisions, Extra bytes for /-/

Option 6: Single Character Endpoints under /0/ versioned

Example: /0/s
+ Shortest possible bytes, Relative path, Version controlled
- Letter collisions, Needs extra CSP headers each version

Option 7: Single Charcter Endpoints under /0/ version full name

Example: /0/sat
+ 1 CSP header, segregated routes, Easier caching, Version controlled
- Extra bytes for /0/ and full name, Needs extra CSP headers each version

@casey
Copy link
Collaborator

casey commented Oct 13, 2023

An option other than /-/ that might make sense would be to use an integer, so that recursive endpoints can be versioned. /0/ would be the first version, and if we needed to add or remove fields in a way that could break recursive inscriptions, we publish the modified endpoints under /1/.

@zmeyer44
Copy link
Contributor

An option other than /-/ that might make sense would be to use an integer, so that recursive endpoints can be versioned. /0/ would be the first version, and if we needed to add or remove fields in a way that could break recursive inscriptions, we publish the modified endpoints under /1/.

I like this a lot. Should make maintaining things in the future a lot cleaner.

@tansanDOTeth
Copy link

An option other than /-/ that might make sense would be to use an integer, so that recursive endpoints can be versioned. /0/ would be the first version, and if we needed to add or remove fields in a way that could break recursive inscriptions, we publish the modified endpoints under /1/.

Great idea on the versioning. You can even use /-/ to always point to the latest.

@lifofifoX
Copy link
Collaborator

One thing to keep in mind here is that if we have the same inscriptions accessible via different routes, it could break inscriptions that rely on fetching inscription ID from the URL. I have advocated for having inscription content accessible via indexed routes (i.e /content/sat/XXXX/-1) but I hadn't considered the scenario re: fetching inscription ID from the URL. I don't have any additional thoughts, but wanted to throw it out here.

@elocremarc
Copy link
Contributor

elocremarc commented Oct 23, 2023

An option other than /-/ that might make sense would be to use an integer

I updated summary with the two new options. @casey wouldn't the version need new CSP headers each time we add a new version?

@elocremarc
Copy link
Contributor

elocremarc commented Oct 23, 2023

I think we should just go with /-/ like this PR is initially shows. Would adding new keys to the response break any inscription prior? Because they won't be aware of any new keys we add say if we add rarity to the sat endpoint it won't break any inscriptions from my understanding. The current sat endpoint PR is using /-/ so people can build that branch and play around with it in regtest.

@owenstrevor
Copy link

Love seeing all the debate here. It's very hard to choose between the different options for me. I think they're all good and unlock huge value for Ordinals. Would err on the side of whichever gives the most optionality for the future.

@casey
Copy link
Collaborator

casey commented Oct 30, 2023

We went with /r/ENDPOINT, with no version number. We can easily add a version number later, and by the time we need a new version of an endpoint, we'll have a better idea of how versioning should work, exactly, so best to wait until then.

@raphjaph raphjaph enabled auto-merge (squash) October 30, 2023 20:35
@raphjaph raphjaph merged commit 8409ce9 into master Oct 30, 2023
6 checks passed
@raphjaph raphjaph deleted the recursive-directory branch October 30, 2023 20:39
hans-crypto added a commit to ordpool-space/ordpool that referenced this pull request Oct 31, 2023
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.

9 participants