Deprecate REST Api and Replace with Python share script

JeremyRubin commented at 4:11 am on October 12, 2021: contributor

The REST API introduces functionality that is redundant with the JSON RPCs, but it’s designed to be an unauthenticated connection (but still trusted). This is a vauge security model and introduces additional code into bitcoin core.

Instead, it might make sense to deprecate the REST Interface and replace it with a share python script that gives a similar interface. This server connection can receive a whitelistrpc’d credential that only enables the rest API queries. The python script can detect if e.g. twisted is available and run with that or use python3 built in http.server.

One justification for maintaining it’s existence in present form is that I have heard is that performance is better than the JSON RPCs.

In terms of deprecation, one path would be (over a few releases):

Phase 1: Marking Deprecated + Adding Python Script Server Phase 2: Making compilation of rest optional, default disabled Phase 3: Total Removal

irc log discussion from #bitcoin-core-dev more generally about moving http outside of core

 0[2021-09-22T15:55:55.479Z] <jeremyrubin> I think what we'd want is to remove the http support from Bitcoin core and then build a separate binary with a simple http proxy to it
 1[2021-09-22T15:56:31.719Z] <jeremyrubin> That doesn't even have to live in core.
 2[2021-09-22T15:56:45.905Z] <sipa> that's also a possibility, but probably not a short-term one
 3[2021-09-22T15:56:55.548Z] <sipa> (also, unix domain sockets don't exist on windows...)
 4[2021-09-22T15:56:59.655Z] <jeremyrubin> It would be a breaking change so would be a maj release
 5[2021-09-22T15:57:15.533Z] <jeremyrubin> I think they have something that can be used the same way these days
 6[2021-09-22T15:57:24.404Z] <sipa> right, but we'd need that too :)
 7[2021-09-22T15:57:58.032Z] <jeremyrubin> I think it's basically the exact same
 8[2021-09-22T15:58:41.309Z] <darosior> Windows does support Unix Domain Sockets since W10
 9[2021-09-22T15:58:45.306Z] <jeremyrubin> Just different headers and negative FDs
10[2021-09-22T16:06:49.684Z] <jeremyrubin> I think over the span of a few releases could reasonably do deprecated, not built by default, removed.
11[2021-09-22T16:30:01.771Z] <laanwj> yes, modern windows has them too
12[2021-09-22T16:36:51.958Z] <laanwj> i might give 'simple line protocol over UNIX socket' a try, if there is really enthousiasm for adding such a thing; at least i don't have to fight libevent's http client then, that's what i lost interest in
13[2021-09-22T16:43:05.850Z] <laanwj> i guess the only context information from HTTP that is used inside RPC calls is the wallet name from the path, in case of wallet RPC calls
14[2021-09-22T16:44:50.909Z] <laanwj> but would be straightforward to wrap in an object that provides the context otherwise, e.g. { 'wallet': 'name', 'params': {...} }
15[2021-09-22T16:57:06.615Z] <jeremyrubin> i guess the only other question is the rest API thing?
16[2021-09-22T16:58:39.695Z] <jeremyrubin> that also seems like stuff that should just be a proxy over core tho
17[2021-09-22T16:58:58.789Z] <jeremyrubin> (or just removed and see if anyone complains?)
18[2021-09-22T17:01:57.578Z] <laanwj> rest is very closely bound to http, how i imagine this would purely be for RPC calls (all information on rest is also available through RPC it's 'extra')
19[2021-09-22T17:02:59.063Z] <laanwj> i think that could very well be an optional thing that people can enable if they want it
20[2021-09-22T17:03:56.020Z] <laanwj> sharing the http server between rest and the json-rpc interface was always a bit questionable, they are separate things
21[2021-09-22T17:05:22.061Z] <jeremyrubin> laanwj does anyone use the rest interface in practice?
22[2021-09-22T17:05:56.144Z] <laanwj> i have no idea
23[2021-09-22T17:06:39.211Z] <jeremyrubin> i've never heard of anyone using it either
24[2021-09-22T17:06:45.038Z] <laanwj> it's much faster than JSON-RPC for some things as it saves on formatting and the info can (basically) directly be dumped to the socket
25[2021-09-22T17:10:11.984Z] <laanwj> though it's not as great as it sounds, i wanted to add a rest call for streaming utxo data once (without buffering it in memory first), but libevent's http server wouldn't let me due to not really being thread safe
26[2021-09-22T17:12:58.334Z] <laanwj> mostly it's not even issues with http itself we run against, but issues with libevent's http, but adding another dependency for http would be not nice
27[2021-09-22T17:17:50.893Z] <jeremyrubin> laanwj i think given that there are few users for rest (most likely) we could ship a /share script with a python http proxy for rest, altho performance might suffer a bit (maybe we add a raw response mode for the json rpcs if it's a concern?)
28[2021-09-22T17:20:19.972Z] <laanwj> yes...
29[2021-09-22T17:22:52.016Z] <laanwj> in the case where we directly 'own' the socket, and the protocol is up to us to define, in principle it could reply in a different way than a line with JSON on it if so requested, this does make clients that want to support that more complex, so everything should at least support JSON line in JSON line out
30[2021-09-22T17:24:11.086Z] <laanwj> honestly that kind of thing seems far enough away, i'm not sure it's worth spendng time on, it works as it does now, not sure about the added benefit of doing things slightly different
31[2021-09-22T17:25:23.066Z] <laanwj> i think my reply on twitter was more hypothetical, in a perfect clean-slate world where you don't have to balance so many different concerns and backwards compatibility, it would have been nice

JeremyRubin added the label Feature on Oct 12, 2021

JeremyRubin commented at 4:14 am on October 12, 2021: contributor

see https://github.com/bitcoin/bitcoin/pull/17631

TheBlueMatt commented at 4:22 am on October 12, 2021: contributor

There’s another reason that it exists that is missed here - JSON-RPC is basically a Bitcoin Core-only thing, and is pretty hard to work with. There isn’t standard tooling for JSON-RPC available anywhere, there’s just Bitcoin Core stuff. REST is the standard for RPC calls at this point, so ripping out the one standardized way to get data out of Core is kinda obnoxious.

JeremyRubin commented at 4:30 am on October 12, 2021: contributor

@TheBlueMatt fwiw i’d also concept ACK a solid plan to eliminate the json rpc and make everything rest based and provide the json rpc through a proxy, but that seems less likely since there are far more apis / users / uses of the json rpcs and there’s only 9 REST APIs presently.

JeremyRubin commented at 4:36 am on October 12, 2021: contributor

responding to #17631 (comment)

You can sanitize in NGINX too, its probably easier than some python script. But, seriously, that notion is absurd - “here, you have a running production daemon, run this hacked-together python script to make its API usable” is not the same as “we don’t want to vouch for the security of this thing, so make sure you pay close attention if you want to expose calls to the outside”.

as a general principle i think we should be very careful about shipping core with any sort of feature / service that might lead to inadvertent security issues, especially because exploits running in the same memory space as core are much more concerning. Rather a hacked together python script proxy than a hacked together c++ http server in core. And nothing says the script has to be hacked together :)

JeremyRubin commented at 5:46 am on October 12, 2021: contributor

This can be improved (e.g. to use tornado’s http client and to use gen.task/async), but this is an example of how one might implement one of the proxy function handlers based on the AuthServiceProxy in the rpc tests and tornado library.

 0import tornado.ioloop
 1import sys
 2import tornado.web
 3node = AuthServiceProxy(sys.argv[1])
 4class rest_tx(tornado.web.RequestHandler):
 5    def get(self, txhash, encoding):
 6        try:
 7            result = node.getrawtransaction(txhash, 1 if encoding == ".json" else 0)
 8        except JSONRPCException as e:
 9            raise tornado.web.HTTPError(status_code=404, log_message="Transaction Not Found")
10        if encoding == ".json":
11            self.set_header("Content-Type", 'application/json')
12            self.write(json.dumps(result, default=EncodeDecimal))
13        if encoding == ".bin":
14            self.set_header("Content-Type", 'application/octet-stream')
15            self.write(bytes.fromhex(result))
16        if encoding == ".hex":
17            self.set_header("Content-Type", 'application/plain')
18            self.write(result)
19
20def make_app():
21    format_hash = "([0-9a-fA-F]{64})(\.json|\.hex|\.bin)"
22    print(f"/rest/tx/{format_hash}")
23    return tornado.web.Application([
24        (f"/rest/tx/{format_hash}", rest_tx),
25        ])
26
27if __name__ == "__main__":
28    app = make_app()
29    app.listen(8888)
30    tornado.ioloop.IOLoop.current().start()

fanquake added the label RPC/REST/ZMQ on Oct 12, 2021

fanquake removed the label Feature on Oct 12, 2021

practicalswift commented at 8:02 am on October 12, 2021: contributor

Concept ACK on doing something about the vague/confusing security model (“trusted but also not trusted”) of the REST API.

maflcko added the label Brainstorming on Oct 12, 2021

ghost commented at 2:58 pm on October 12, 2021: none

i’d also concept ACK a solid plan to eliminate the json rpc and make everything rest based and provide the json rpc through a proxy, but that seems less likely since there are far more apis / users / uses of the json rpcs and there’s only 9 REST APIs presently

Yes removing JSON-RPC doesn’t make sense IMO. Infact it is the only thing I liked in Bitcoin Core because GUI feels like Windows 95. Although UI/UX is not a problem anymore since lot of projects have better GUI with option to use Bitcoin Core with RPC credentials.

It the goal is to remove unnecessary things and focus on important things, improve them etc. based on below tweet:

https://twitter.com/JeremyRubin/status/1440402791676014595

I think we should keep both JSON-RPC and REST. Maybe remove bitcoin-qt. There are trade-offs and few developers might prefer REST. If security is a concern, it can be improved and some of the things are already mentioned in doc/REST-interface.md and doc/JSON-RPC-interface.md

sipa commented at 3:07 pm on October 12, 2021: member

I don’t see the problem with having both REST and JSON-RPC (it’s pretty straightforward code), nor with their security models.

tylerchambers commented at 1:52 pm on October 13, 2021: contributor

Every time I’ve wanted to expose any information from my node over REST I always wound up spinning up a “middleware” in Flask or similar that took the request from the client -> JSON-RPC to core -> response back to client.

As it stands right now I would like to see the REST API drastically improved or removed entirely. Improving it would greatly increase accessibility for people new to the space to build on top of core, which is obviously a net good long term. Would be a pretty big project to get (secure) feature parity with JSON-RPC, + maintenance costs down the line.

I don’t see much of a downside to removing it (but I’m not sure how utilized it is).

Keeping the REST API in its current state and doing nothing has potential security implications long term, without much utility gained in the best case.

Removing it and providing something in Python or similar as a REST bridge to the JSON-RPC functionality (or at least giving some examples on how to do so? an official python JSON-RPC library for core?) may be a happy medium.

JeremyRubin commented at 6:59 pm on October 19, 2021: contributor

one thing i’ve noticed is the utxos rest API does not seem to be a valid rest API (it should use query parameters for >1 utxo?)

sipa commented at 7:02 pm on October 19, 2021: member

Would be a pretty big project to get (secure) feature parity with JSON-RPC, + maintenance costs down the line.

The REST interface is designed for fast, low-overhead access to public data only. It’s not intended to be an alternative to RPC.

brunoerg commented at 11:03 am on October 25, 2021: contributor

@JeremyRubin Is there a specific reason to use tornado, instead of using flask or any other alternative?

JeremyRubin commented at 5:28 pm on October 25, 2021: contributor

tornado is async / non blocking and so it should be simpler to use Tornado to e.g. extend the script to work with a pool of nodes that respond to the rest queries async.

The main reason to not use the builtin http server is that IIRC it’s not recommended for production use (see https://docs.python.org/3/library/http.server.html#module-http.server)

brunoerg commented at 5:45 pm on October 25, 2021: contributor

tornado is async / non blocking and so it should be simpler to use Tornado to e.g. extend the script to work with a pool of nodes that respond to the rest queries async.

The main reason to not use the builtin http server is that IIRC it’s not recommended for production use (see https://docs.python.org/3/library/http.server.html#module-http.server)

Cool, thank you!

fanquake commented at 8:37 am on August 8, 2022: member

@stickies-v you might have some opinion / something to comment here?

michaelfolkson commented at 9:02 am on August 8, 2022: contributor

As it stands right now I would like to see the REST API drastically improved or removed entirely. Improving it would greatly increase accessibility for people new to the space to build on top of core, which is obviously a net good long term. Would be a pretty big project to get (secure) feature parity with JSON-RPC, + maintenance costs down the line.

I agree with this. If @stickies-v and others plan to work on improving the REST API that would great.

stickies-v commented at 3:35 pm on August 8, 2022: contributor

From reading the initial PR (https://github.com/bitcoin/bitcoin/pull/2844), it seems ease-of-use/accessibility was the main concern when introducing the REST API (and interestingly, it had a lot of the same discussion we have here, including suggesting a Python script approach). I think having a fully mature, functional and easy-to-use REST API would be a significant driver in having developers (especially for smaller projects) build directly on Bitcoin Core instead of on third-party APIs, or even not building at all.

With e.g. #25752 I was going for a more iterative approach, but I’m starting to see more benefit in having a separate service on top of the JSON-RPC as suggested here. It would obviously have a performance impact, but I think that’s okay, since imo the first thing to optimize for is ease-of-use, and then if we notice sufficient adoption, performance bottlenecks can be fixed if and where necessary. Extending and improving/overhauling the current REST API is probably too much of a burden without visible demand, realistically.

A couple of questions:

In this proposal, I don’t see any more reason to limit the REST API to unauthenticated endpoints, but rather should go for full feature parity. Does anyone see that differently?
Would Rust or Go be a better alternative instead of Python? I think that might be less controversial here for production services (I’m a Python developer), and they both seem to fit all the requirements (especially Go if async is important, which I’m not 100% convinced it is - the overhead of spinning up more workers is low)

JeremyRubin commented at 7:08 pm on August 8, 2022: contributor

i dont have any preferences, however, i’ll note that adding a new compiled artifact might go beyond what project maintainers want in Core v.s. a simple contrib script.

JeremyRubin closed this on Dec 16, 2022

bitcoin locked this on Dec 16, 2023

Deprecate REST Api and Replace with Python share script #23259