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

Implement additional JSON endpoints (/inscription, /inscriptions) and bugfixes on /sat #2323

Merged
merged 14 commits into from
Aug 14, 2023

Conversation

veryordinally
Copy link
Collaborator

Implement JSON endpoint for /inscription

Move check if JSON API is enabled into AcceptJson. This allows returning StatusCode::NOT_ACCEPTABLE early, and from a single place.

Ordinally added 3 commits August 11, 2023 19:31
…ing StatusCode::NOT_ACCEPTABLE early, and from a single place.

Implement JSON endpoint for /inscription
- Added corresponding test inscribing on a common sat.
- Change timestamp format to actual (i64) timestamp for their
  universality, compactness, ease of computation and language and
  platform agnosticism.
@veryordinally veryordinally changed the title Implement JSON endpoint for /inscription Implement additional JSON endpoints (/inscription, /inscriptions) and bugfixes on /sat Aug 12, 2023
@veryordinally veryordinally marked this pull request as ready for review August 13, 2023 17:35
@veryordinally veryordinally requested a review from raphjaph August 13, 2023 17:35
@veryordinally
Copy link
Collaborator Author

A topic to think about is the order of inscriptions returned by the various /inscriptions endpoints. In the current implementation, this works exactly like the HTML based version (based on get_latest_inscriptions), which returns inscriptions starting from the highest number, going backwards. This is consistently implemented, but provides for somewhat counterintuitive pagination, especially in combination with negative inscription numbers.

src/subcommand/server.rs Outdated Show resolved Hide resolved
src/subcommand/server.rs Outdated Show resolved Hide resolved
src/subcommand/server.rs Outdated Show resolved Hide resolved
tests/json_api.rs Outdated Show resolved Hide resolved
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) August 14, 2023 12:47
@raphjaph raphjaph merged commit 40e4807 into ordinals:master Aug 14, 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.

2 participants