In RESTful APIs, typically path
parameters (e.g. /some/unique/resource/) are used to represent resources, and query parameters
(e.g. ?sort=asc) are used to control how these resources are being loaded through e.g. sorting,
pagination, filtering, etc.
As first discussed in #17631,
the current REST api
contains two endpoints /headers/ and /blockfilterheaders/ that rather unexpectedly use path
parameters to control how many (filter) headers are returned in the response. While this is no
critical issue, it is unintuitive and we are still early enough to easily phase this behaviour out
and ensure new endpoints (if any) do not have to stick to non-standard behaviour just for internal
consistency.
In this PR, a new HTTPRequest::GetQueryParameter method is introduced to easily parse query
parameters, as well as two new /headers/ and /blockfilterheaders/ endpoints that use a count
query parameter are introduced. The old path parameter-based endpoints are kept without too much
overhead, but the documentation now points to the new query parameter-based endpoints as the default
interface to encourage standardness.
The COUNT parameter controls how many
headers
or
blockfilterheaders
are returned for a single API request, and defaults to 5 if unspecified.
Using the REST API
We’ll test the REST
API on
signet. To do so, first start a bitcoind instance with the -rest flag enabled. To use the
blockfilterheaders endpoint, you’ll also need to set -blockfilterindex=1: ./bitcoind -signet
-rest -blockfilterindex=1
As soon as bitcoind is fully up and running, you should be able to query the API, for example by
using curl on the command line: curl "127.0.0.1:38332/rest/chaininfo.json" As a response, you
should get:
{"chain":"signet","blocks":78071,"headers":78071,"bestblockhash":"000000b3e98c0de440154f42819b56586ed36bad0baa2db8ba5d0950e416dcad","difficulty":0.002873067874458486,"time":1645188967,"mediantime":1645188099,"verificationprogress":0.9999934105995945,"initialblockdownload":false,"chainwork":"000000000000000000000000000000000000000000000000000000dc187f6fa0","size_on_disk":373918704,"pruned":false,"softforks":{"bip34":{"type":"buried","active":true,"height":1},"bip66":{"type":"buried","active":true,"height":1},"bip65":{"type":"buried","active":true,"height":1},"csv":{"type":"buried","active":true,"height":1},"segwit":{"type":"buried","active":true,"height":1},"taproot":{"type":"bip9","bip9":{"status":"active","start_time":-1,"timeout":9223372036854775807,"since":0,"min_activation_height":0},"height":0,"active":true}},"warnings":"This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"}
In HTTP requests, what is the difference between a path and a query parameter?
What are the benefits of changing the COUNT parameter from a path parameter to a query
parameter?
What are the drawbacks of changing the COUNT parameter from a path parameter to a query
parameter?
(general discussion) If the effort of implementing a change is already done, what could be some
good reasons for rejecting an (unharmful) code change anyway? Do you (dis)agree with this
comment?
Does this PR change any of the existing function signatures? If so, why? Can this cause any
behaviour change?
Can you list all the commits that introduce behaviour change(s)? Do you feel comfortable about
these behaviour change(s)?
Consider the request (signet) GET
/rest/blockfilterheaders/basic/2/0000004c6aad0c89c1c060e8e116dcd849e0554935cd78ff9c6a398abeac6eda.json?count=1.
What would the response be prior to this PR? What would the response be after this PR? Which
(modified) function is responsible for this behaviour change? Try reasoning about it before
verifying experimentally.
One reviewer raises the
view that /rest/headers/ is a collection endpoint instead of a single resource endpoint. Do you
agree? If so, would you change the PR to implement this? What would be the drawbacks of doing that?
<stickies-v> Welcome everyone! Today we're looking at #24098 (https://bitcoincore.reviews/24098) which aims to improve the endpoint logic of the REST API.
<stickies-v> As you may have deduced from some of the later questions, there are still some open discussion topics in this PR. Please don't feel shy to chip in with your thoughts, it helps to know what the community consensus is.
<stickies-v> Who got the chance to read review the PR or read the notes? Can I get a quick y/n? If you tried, did you run into any issues running and testing the REST API?
<galv> I heard about it through someone at a local meetup in the bay area (not crypto-related, but effective altruism related) from someone who works on bitcoin futures contracts.
<svav> Was this PR done because people using the functionality were complaining that it was non-standard? Or did the developer just decide it would be better if updated?
<stickies-v> svav I've not heard of any complaints. My main motivation to do it now is to minimize the overhaul needed if/when we have more endpoints in the future. It makes it easier for devs to start using the API the more standard it is.
<stickies-v> Easy one to get started, just to make sure everyone understands the concept of what we're changing here. In HTTP requests, what is the difference between a `path` and a `query` parameter?
<stickies-v> svav very good, although to be pedantic the `query string` appears to the right of the `?`, and the `query string` can consist of multiple `query parameters` separated by an `&`
<jaonoctus> The query parameters are used to sort/filter resources. On the other hand, path parameters are used to identify a specific resource or resources.
<OliverOffing> A URL/URI is composed of different parts: <protocol>:<domain>/<path>?<query>, e.g. https://github.com/bitcoin/bitcoin?page=5. The path is generally used to represent objects/resources whereas the query usually represent filters or arguments that shape which of those objects are returned, in which order, and containing which fields
<stickies-v> michaelfolkson I'm not sure if I understand your question 100% right, but REST and RPC are different paradigms on how to organize your API, they don't strictly specify the communication protocol. REST is almost always done over HTTP(S), but this is not required
<stickies-v> Well actually just one haha, everything else is covered already, but path parameters are positional, query parameter are key-val structures
<stickies-v> bitplebpaul well each API is free to implement its logic however it wants to, but that sounds like what you'd expect (assuming a GET request to this endpoint). This is a collection endpoint.
<svav> The COUNT parameter is best described as a query parameter, and therefore the intuitive and conventional implementation would be to have it as a query parameter.
<stickies-v> bitplebpaul theStack yes exactly, and this is also the main benefit for the current endpoints. There are additional general functional advantages that we can benefit from in the future too, though
<stickies-v> willcl_ark path parameters are indeed required, unless of course you construct multiple endpoints where some path parameters are omitted, but this can become really confusing both for users and for developers as there will often be ambiguity
<svav> For my understanding, are the headers we are referring to Version, Previous Block Hash, Merkle Root, Timestamp, Difficulty Target and Nonce? So e.g. a COUNT of 2, would return the first two of these?
<stickies-v> svav to my knowledge there is no data collection whatsoever, including on usage, so this is difficult to answer. My gut feeling says it's much less used than the RPC, also because it's unauthenticated and only contains a subset of the functionality. Main purpose over RPC is that it's much easier to use (less overhead), so if you don't need any of the RPC endpoints REST is probably the best choice. Also in general
<larryruane> svav: "... REST interface is being used..." I thinkthere are general-purpose monitoring mechanisms like Grafana that can easily be plugged into a REST interface, while it would take more work (if even possible) to use the RPC interface
<stickies-v> Alright let's dive into the code. Does this PR change any of the existing function signatures? If so, why? Can this cause any behaviour change?
<stickies-v> theStack in general, it's much less overhead to interface with a HTTP JSON REST API because requests and (de)serialization are super straightforward. Imo, if the REST API has the endpoints you need for your purpose, pretty much any (programmatic) use should be easier instead of RPC I think.
<larryruane> stickies-v: "...should be easier instead of RPC..." But isn't the REST interface more restrictive? You can't for example `sendmany` using it, right? I thought REST was in effect readonly
<larryruane> Not sure if this counts as a behavior change, but when a function is `static`, it can be inlined, but otherwise it can't (unless it's implemented in a header file)
<kouloumos> If it's static is only visible to functions in the same file right? so I understand that you did that to be able to use that function in test/rest_test.cpp
<stickies-v> kouloumos technically it's only visible within the same translation unit, which in practice should mean same file but I think there are exceptions
<Kaizen_Kintsugi_> from the reference "static -- this function can only be seen in this file? No, this means that this function can be called without an instantiated object, as normally member functions (methods) must be called using an instantiation of the class, though with this keyword, you don't need it."
<larryruane> and confusingly, a static method within a class means something different, it means you don't need an instance of that object to call the method
<stickies-v> Behaviour change is always something to be extra careful with. Can you list all the commits that introduce behaviour change(s)? Do you feel comfortable about these behaviour change(s)?
<stickies-v> larryruane yes exactly those two introduce behaviour change, even though those should be commits 3 and 5 so Kaizen_Kintsugi_ yes you're also right on 5
<stickies-v> but have a closer look at 833803e9a Handle query string when parsing data format - it modifies ParseDataFormat to return `param` without the query string, so effectively this changes how the API responds. Even though previously we weren't *expecting* people to add query parameters, they still could
<stickies-v> Alright we've already partly covered the next question, but just to see if anyone has issues running/testing this I'll quickly cover it anyway
<stickies-v> Consider the request (signet) `GET /rest/blockfilterheaders/basic/2/0000004c6aad0c89c1c060e8e116dcd849e0554935cd78ff9c6a398abeac6eda.json?count=1`. What would the response be prior to this PR? What would the response be after this PR? Which (modified) function is responsible for this behaviour change? Try reasoning about it before verifying experimentally.
<stickies-v> Kaizen_Kintsugi_ yes! Prior, the ParseDataFormat would consider the query string as part of the format suffix (.json?count=5), which is an unrecognized. Moreover, it would try to look up the blockhash suffixed with the formatt suffix and query parameter (<somehash>.json?count=5), which of course doesn't exist, so the API would return
<stickies-v> svav larryruane nope, prior to this PR the API does not handle the query string at all, it is just considered as part of the path, so it would fail most requests
<stickies-v> One reviewer raises (https://github.com/bitcoin/bitcoin/pull/24098#issuecomment-1027755825) the view that `/rest/headers/` is a collection endpoint instead of a single resource endpoint. Do you agree? If so, would you change the PR to implement this? What would be the drawbacks of doing that?
<stickies-v> larryruane exactly - but as I've described in the Behaviour Change section of the PR, I can't anticipate any situations where this would be undesirable or unexpected, until someone proves me wrong...
<stickies-v> Kaizen_Kintsugi_ well basically you could e.g. query GET /users/ to get all users, and that would be a collection endpoint. When querying GET /users/5/, you would be querying a single resource and expect to get details on user 5