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

Json API for Ord #2036

Closed
wants to merge 21 commits into from
Closed

Json API for Ord #2036

wants to merge 21 commits into from

Conversation

ynohtna92
Copy link
Contributor

Adds several json API endpoints following the API sketch @casey derived in https://github.com/casey/ord/pull/1662.

See docs/src/guides/restapi.md for full list of changes.

Things still to do:

  • Fixes tests so they pass! Failing due to the wallet client having to be injected into server.rs resulting in mismatch of chain type when running tests
  • Figure out how errors will be handled and shown. They need to be wrapped in json but only when accept_json.0 is true.

@ynohtna92 ynohtna92 changed the title Json API for Json API for Ord Apr 21, 2023
@ynohtna92 ynohtna92 mentioned this pull request Apr 22, 2023
@ordinalsbot
Copy link

this is still failing to start for me with --index-sats ?
we have a wallet loaded so I'm not sure what the issue could be.

[2023-05-15T21:43:39Z DEBUG hyper::proto::h1::io] parsed 3 headers [25/1835]
[2023-05-15T21:43:39Z DEBUG hyper::proto::h1::conn] incoming body is content-length (480 bytes)
[2023-05-15T21:43:39Z DEBUG hyper::proto::h1::conn] incoming body completed
[2023-05-15T21:43:39Z DEBUG hyper::client::pool] pooling idle connection for ("http", 127.0.0.1:8332)
error: JSON-RPC error: RPC error response: RpcError { code: -19, message: "Wallet file not specified (must request wal
let RPC through /wallet/ uri-path).", data: None }
0: ord::options::Options::bitcoin_rpc_client_for_wallet_command
1: ord::subcommand::server::Server::run::{{closure}}
2: tokio::runtime::park::CachedParkThread::block_on
3: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
4: ord::subcommand::server::Server::run
5: ord::subcommand::Subcommand::run
6: ord::main
7: std::sys_common::backtrace::__rust_begin_short_backtrace
8: std::rt::lang_start::{{closure}}
9: core::ops::function::impls::<impl core::ops::function::FnOnce for &F>::call_once
at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/ops/function.rs:606:13
std::panicking::try::do_call

@nibty
Copy link

nibty commented May 23, 2023

Looking good. It looks like this will require the wallet RPC to be enabled. Is it possible to make that optional? I run a read-only bitcoind and ord. Also, are there plans to add pagination to the feed endpoint? Thanks.

@ynohtna92
Copy link
Contributor Author

Looking good. It looks like this will require the wallet RPC to be enabled. Is it possible to make that optional? I run a read-only bitcoind and ord. Also, are there plans to add pagination to the feed endpoint? Thanks.

I can try to make the wallet rpc optional, with a flag in the ord.yaml.

No plans for pagination in the feed endpoint at this stage. I do my pagination by using the inscriptions/ endpoint and changed it to output 1000 at a time.

@nibty
Copy link

nibty commented May 24, 2023

No plans for pagination in the feed endpoint at this stage. I do my pagination by using the inscriptions/ endpoint and changed it to output 1000 at a time.

I didn't see the inscription endpoint. That works for pagination.

BTW. Some inscriptions cause 502s.

example

curl -H "Accept: application/json" http://XXXXX.XXX/inscription/994731268d65fa8698dbba9580d7980f7c04d79633d8287baa4de264b915488ei0
<html><body><h1>502 Bad Gateway</h1>
The server returned an invalid or incomplete response.
</body></html>
thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: UnrecognizedScript', src/subcommand/server.rs:1048:81
stack backtrace:
rust_begin_unwind
at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:579:5
core::panicking::panic_fmt
at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/panicking.rs:64:14
core::result::unwrap_failed
at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/result.rs:1750:5
core::result::Result<T,E>::unwrap
<F as axum::handler::Handler<(M,T1,T2,T3,T4),S,B>>::call::{{closure}}

@ynohtna92
Copy link
Contributor Author

994731268d65fa8698dbba9580d7980f7c04d79633d8287baa4de264b915488ei0

I think it is because it should have an address but it doesn't for some reason....

@aguinaldok4 aguinaldok4 linked an issue May 29, 2023 that may be closed by this pull request
@ynohtna92
Copy link
Contributor Author

All tests should now pass.

Added a api_wallet_enable flag to ord.yaml so that the wallet endpoints can be used.

Looking for code reviewers and testers @raphjaph

Not a rust dev normally so there could be some things that are not best practice.

Ie I'm not able to simplify this code snippet. Can't seem to do client.as_ref().unwrap() as it performs a move on Client so had to put it inside of a if let instead.

if let Some(client) = client.as_ref() { ... }

@nibty
Copy link

nibty commented Jun 15, 2023

Found another 502 on this one 994731268d65fa8698dbba9580d7980f7c04d79633d8287baa4de264b915488ei0

Jun 15 16:48:53 xxx ord[276030]: thread 'tokio-runtime-worker' panicked at 'called Result::unwrap() on an Err value: UnrecognizedScript', src/subcommand/server.rs:1086:81
Jun 15 16:48:53 xxx ord[276030]: stack backtrace:
Jun 15 16:48:53 xxx ord[276030]: 0: rust_begin_unwind
Jun 15 16:48:53 xxx ord[276030]: at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:579:5
Jun 15 16:48:53 xxx ord[276030]: 1: core::panicking::panic_fmt
Jun 15 16:48:53xxx ord[276030]: at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/panicking.rs:64:14
Jun 15 16:48:53 xxx ord[276030]: 2: core::result::unwrap_failed
Jun 15 16:48:53 xxx ord[276030]: at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/result.rs:1750:5
Jun 15 16:48:53 xxx ord[276030]: 3: ord::subcommand::server::Server::inscription::{{closure}}

@raphjaph
Copy link
Collaborator

This is a great start! I want to start looking at this but this PR is a bit too large to review. I would also like to start simple with only the endpoints that do not require an API key or use the /wallet endpoint. Additionally I think having an address to inscription numbers table will make the database too large. If you feel like cutting out the stuff that would be great, otherwise I will try to do some slicing and dicing myself on the weekend.

@ynohtna92
Copy link
Contributor Author

This is a great start! I want to start looking at this but this PR is a bit too large to review. I would also like to start simple with only the endpoints that do not require an API key or use the /wallet endpoint. Additionally I think having an address to inscription numbers table will make the database too large. If you feel like cutting out the stuff that would be great, otherwise I will try to do some slicing and dicing myself on the weekend.

Yeah I got carried away with adding things to this pr. The address stuff can be removed or alternatively enabled with a flag similar to index-sats. The address table is quite small compared to the sat ranges table if you are concerned with database size, I think more importantly it could be a performance hit depending on how well it searches and drains though the number arrays when adding or removing an inscription from an address. The wallet API stuff could also be it in its own pr as well. Not sure how to turn this into 3 PRS since two parts depend on the first.

@lnky79
Copy link

lnky79 commented Jun 27, 2023

This is a great start! I want to start looking at this but this PR is a bit too large to review. I would also like to start simple with only the endpoints that do not require an API key or use the /wallet endpoint. Additionally I think having an address to inscription numbers table will make the database too large. If you feel like cutting out the stuff that would be great, otherwise I will try to do some slicing and dicing myself on the weekend.

@raphjaph Great to hear your kind response on the API issue.

Currently, my team is working on a UTXO/PSBT signer library for Ordinals marketplaces, which requires UTXO/cardinal selection based on seller/buyer address before generating unsigned PSBT for users. We have been using the branch created by @ynohtna92 for one month, and it has been working well with positive-number (non-cursed) inscriptions.

In my opinion, the /address endpoint or its official index table is very helpful for ensuring transfer protection for user inscription UTXOs in the ecosystem. I hope it can be preserved in the final version. As @ynohtna92 suggested, we could have an option like --index-address to make this possible.

Thank you for the awesome job you guys have done! @ynohtna92 @raphjaph

@Mathieu-Be
Copy link
Contributor

Hey guys, is anyone still working on this ? Very interested by this so would be glad to contribute and make a smaller PR for non auth endpoints

@Mathieu-Be Mathieu-Be mentioned this pull request Jul 5, 2023
@raphjaph
Copy link
Collaborator

We've started work on the JSON API, feel free to add more endpoints similiar to how we have designed the existing ones :)

@raphjaph raphjaph closed this Aug 22, 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.

https://status.twitterstat.us/pages/564314ae3309c22c3b0002fa/rss
8 participants