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

feat: add more recursion endpoints #144

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

janniks
Copy link
Contributor

@janniks janniks commented Jul 11, 2023

Adding these endpoints covered by ord
https://docs.ordinals.com/inscriptions.html?highlight=rec#recursion

A couple other endpoints that inscriptions may access are the following:

/blockheight: latest block height.
/blockhash: latest block hash.
/blockhash/<HEIGHT>: block hash at given block height.
/blocktime: UNIX time stamp of latest block.

todo

  • cacheing: what's your preferred etag/cacheing option? we could add a stats/inscriptioncountperblock cache for most, and a custom new cache for the /:blockheight show endpoint.

@janniks janniks added the feature request New feature or request label Jul 11, 2023
@janniks janniks requested a review from rafaelcr July 11, 2023 21:26
@janniks janniks self-assigned this Jul 11, 2023
@janniks janniks temporarily deployed to Preview July 11, 2023 21:26 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Jul 11, 2023

Vercel deployment URL: https://ordinals-5ut3rkhrx-blockstack.vercel.app 🚀

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #144 (a3c3add) into develop (a7674a9) will decrease coverage by 4.25%.
The diff coverage is 100.00%.

❗ Current head a3c3add differs from pull request most recent head 389ec23. Consider uploading reports for the commit 389ec23 to get more accurate results

@@             Coverage Diff             @@
##           develop     #144      +/-   ##
===========================================
- Coverage    91.98%   87.74%   -4.25%     
===========================================
  Files           26       34       +8     
  Lines         2970     3622     +652     
  Branches       239      294      +55     
===========================================
+ Hits          2732     3178     +446     
- Misses         236      440     +204     
- Partials         2        4       +2     
Impacted Files Coverage Δ
src/api/init.ts 70.49% <100.00%> (+1.52%) ⬆️
src/api/routes/recursion.ts 100.00% <100.00%> (ø)
src/api/schemas.ts 100.00% <100.00%> (ø)
src/pg/pg-store.ts 99.27% <100.00%> (+1.79%) ⬆️

... and 19 files with indirect coverage changes

@smcclellan smcclellan marked this pull request as draft July 17, 2023 14:51
@janniks janniks temporarily deployed to Preview July 17, 2023 15:42 — with GitHub Actions Inactive
@janniks janniks force-pushed the fix/add-more-recursion-endpoints branch from bf66c92 to acf5e54 Compare July 17, 2023 15:43
@janniks janniks temporarily deployed to Preview July 17, 2023 15:43 — with GitHub Actions Inactive
@janniks janniks force-pushed the fix/add-more-recursion-endpoints branch from acf5e54 to 389ec23 Compare July 18, 2023 08:36
@janniks janniks temporarily deployed to Preview July 18, 2023 08:36 — with GitHub Actions Inactive
Copy link
Collaborator

@rafaelcr rafaelcr left a comment

Choose a reason for hiding this comment

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

Looking good! A few comments

},
},
async (request, reply) => {
const blockHeight = (await fastify.db.getChainTipBlockHeight()) ?? 'blockheight';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the word blockheight the right thing to return if we don't have a proper block height here? Not familiar with the ord docs on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's to match the current ord implementation. https://sourcegraph.com/search?q=context:global+repo:ordinals/ord+and+%7C%7C+%22

src/api/routes/recursion.ts Show resolved Hide resolved
`;

const result = await this.sql<{ block_hash: string }[]>`
SELECT block_hash FROM inscriptions_per_block
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could make these queries more efficient if we modified the chain_tip materialized view to keep also the block hash and timestamp. That way it would be a O(1) retrieval from the DB always.

@janniks janniks added this to the Q3-2023 milestone Sep 18, 2023
@smcclellan smcclellan modified the milestones: Q3-2023, Q1-2024 Jan 16, 2024
@janniks
Copy link
Contributor Author

janniks commented Jan 29, 2024

Forgot about this one. A lot of recursive viewing doesn't really work in our explorer due to security concerns, so it might not be too important to have these endpoints right now..

@smcclellan smcclellan removed this from the Q1-2024 milestone Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
Status: 📋 Backlog
Development

Successfully merging this pull request may close these issues.

3 participants