highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-06-29 10:19:26 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#24098: rest: Use query parameters to control resource loading

Browse Source 
54b39cfb34 Add release notes (stickies-v)
f959fc0397 Update /<count>/ endpoints to use a '?count=' query parameter instead (stickies-v)
a09497614e Add GetQueryParameter helper function (stickies-v)
fff771ee86 Handle query string when parsing data format (stickies-v)
c1aad1b3b9 scripted-diff: rename RetFormat to RESTResponseFormat (stickies-v)
9f1c54787c Refactoring: move declarations to rest.h (stickies-v)

Pull request description:

  In RESTful APIs, [typically](https://rapidapi.com/blog/api-glossary/parameters/query/) 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, ...

  As first [discussed in #17631](https://github.com/bitcoin/bitcoin/pull/17631#discussion_r733031180), the [current REST api](https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md) 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.

  ## Behaviour change
  ### New endpoints and default values
  `/headers/` and `/blockfilterheaders/` now have 2 new endpoints that contain query parameters (`?count=<count>`) instead of path parameters (`/<count>/`), as described in REST-interface.md. Since query parameters can easily have default values, I have set this at 5 for both endpoints.

  **headers**
  `GET /rest/headers/<BLOCK-HASH>.<bin|hex|json>?count=<COUNT=5>`
  should now be used instead of
  `GET /rest/headers/<COUNT>/<BLOCK-HASH>.<bin|hex|json>`

  **blockfilterheaders**
  `GET /rest/blockfilterheaders/<FILTERTYPE>/<BLOCK-HASH>.<bin|hex|json>?count=<COUNT=5>`
  should now be used instead of
  `GET /rest/blockfilterheaders/<FILTERTYPE>/<COUNT>/<BLOCK-HASH>.<bin|hex|json>`

  ### Some previously invalid API calls are now valid
  API calls that contained query strings in the URI could not be parsed prior to this PR. This PR changes behaviour in that previously invalid calls (e.g. `GET /rest/headers/5/somehash.json?someunusedparam=foo`) would now become valid, as the query parameters are properly parsed, and discarded if unused.
  For example, prior to this PR, adding an irrelevant `someparam` parameter would be illegal:
  ```
  GET /rest/headers/5/0000004c6aad0c89c1c060e8e116dcd849e0554935cd78ff9c6a398abeac6eda.json?someparam=true
  ->
  Invalid hash: 0000004c6aad0c89c1c060e8e116dcd849e0554935cd78ff9c6a398abeac6eda.json?someparam=true
  ```
  **This behaviour change affects all rest endpoints, not just the 2 new ones introduced here.**

  *(Note: I'd be open to implementing additional logic to refuse requests containing unrecognized query parameters to minimize behaviour change, but for the endpoints that we currently have I don't really see the point for that added complexity. E.g. I don't see any scenarios where misspelling a parameter could lead to harmful outcomes)*

  ## Using the REST API

  To run the API HTTP server, 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"```.
  To more easily parse the JSON output, you can also use tools like 'jq' or `json_pp`, e.g.:
  ```
  curl -s "localhost:38332/rest/blockfilterheaders/basic/0000004c6aad0c89c1c060e8e116dcd849e0554935cd78ff9c6a398abeac6eda.json?count=2" | json_pp .
  ```

  ## To do
  - [x] update `doc/release-notes`

  ## Feedback
  This is my first PR (hooray!). Please don't hold back on any feedback/comments/nits/... you may have, big or small, whether they are code, process, language, ... related. I welcome private messages too if there's anything you don't want to clutter the PR with. I'm here to learn and am grateful for everyone's input.

ACKs for top commit:
  stickies-v:
    I've had to push a tiny doc update to `REST-interface.md` (`git range-diff 219d728 9aac438 54b39cf`) since this was not merged for v23, but since there are no significant changes beyond theStack and jnewbery's ACKs I think this PR is now ready to be considered for merging? @MarcoFalke
  jnewbery:
    ACK 54b39cfb34
  theStack:
    re-ACK 54b39cfb34

Tree-SHA512: 3b393ffde34f25605ca12c0b1300799a19684b816a1d03aed38b0f5439df47bfe6a589ffbcd7b83fd2def6c9d00a1bae5e45b1d18df4ae998c617c709990f83f
This commit is contained in:
MarcoFalke
2022-04-06 09:25:49 +02:00
parent 70c522004f 54b39cfb34
commit 27cfaeed1e
11 changed files with 334 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
test/functional/interface_rest.py
View File

@ -10,6 +10,7 @@ import http.client
from io import BytesIO
import json
from struct import pack, unpack
import typing
import urllib.parse
@ -57,14 +58,21 @@ class RESTTest (BitcoinTestFramework):
args.append("-whitelist=noban@127.0.0.1")
self.supports_cli = False
def test_rest_request(self, uri, http_method='GET', req_type=ReqType.JSON, body='', status=200, ret_type=RetType.JSON):
def test_rest_request(
self,
uri: str,
http_method: str = 'GET',
req_type: ReqType = ReqType.JSON,
body: str = '',
status: int = 200,
ret_type: RetType = RetType.JSON,
query_params: typing.Dict[str, typing.Any] = None,
) -> typing.Union[http.client.HTTPResponse, bytes, str, None]:
rest_uri = '/rest' + uri
if req_type == ReqType.JSON:
rest_uri += '.json'
elif req_type == ReqType.BIN:
rest_uri += '.bin'
elif req_type == ReqType.HEX:
rest_uri += '.hex'
if req_type in ReqType:
rest_uri += f'.{req_type.name.lower()}'
if query_params:
rest_uri += f'?{urllib.parse.urlencode(query_params)}'
conn = http.client.HTTPConnection(self.url.hostname, self.url.port)
self.log.debug(f'{http_method} {rest_uri} {body}')
@ -83,6 +91,8 @@ class RESTTest (BitcoinTestFramework):
elif ret_type == RetType.JSON:
return json.loads(resp.read().decode('utf-8'), parse_float=Decimal)
return None
def run_test(self):
self.url = urllib.parse.urlparse(self.nodes[0].url)
self.wallet = MiniWallet(self.nodes[0])
@ -213,12 +223,12 @@ class RESTTest (BitcoinTestFramework):
bb_hash = self.nodes[0].getbestblockhash()
# Check result if block does not exists
assert_equal(self.test_rest_request(f"/headers/1/{UNKNOWN_PARAM}"), [])
assert_equal(self.test_rest_request(f"/headers/{UNKNOWN_PARAM}", query_params={"count": 1}), [])
self.test_rest_request(f"/block/{UNKNOWN_PARAM}", status=404, ret_type=RetType.OBJ)
# Check result if block is not in the active chain
self.nodes[0].invalidateblock(bb_hash)
assert_equal(self.test_rest_request(f'/headers/1/{bb_hash}'), [])
assert_equal(self.test_rest_request(f'/headers/{bb_hash}', query_params={'count': 1}), [])
self.test_rest_request(f'/block/{bb_hash}')
self.nodes[0].reconsiderblock(bb_hash)
@ -228,7 +238,7 @@ class RESTTest (BitcoinTestFramework):
response_bytes = response.read()
# Compare with block header
response_header = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ)
response_header = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ, query_params={"count": 1})
assert_equal(int(response_header.getheader('content-length')), BLOCK_HEADER_SIZE)
response_header_bytes = response_header.read()
assert_equal(response_bytes[:BLOCK_HEADER_SIZE], response_header_bytes)
@ -240,7 +250,7 @@ class RESTTest (BitcoinTestFramework):
assert_equal(response_bytes.hex().encode(), response_hex_bytes)
# Compare with hex block header
response_header_hex = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ)
response_header_hex = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ, query_params={"count": 1})
assert_greater_than(int(response_header_hex.getheader('content-length')), BLOCK_HEADER_SIZE*2)
response_header_hex_bytes = response_header_hex.read(BLOCK_HEADER_SIZE*2)
assert_equal(response_bytes[:BLOCK_HEADER_SIZE].hex().encode(), response_header_hex_bytes)
@ -267,7 +277,7 @@ class RESTTest (BitcoinTestFramework):
self.test_rest_request("/blockhashbyheight/", ret_type=RetType.OBJ, status=400)
# Compare with json block header
json_obj = self.test_rest_request(f"/headers/1/{bb_hash}")
json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1})
assert_equal(len(json_obj), 1) # ensure that there is one header in the json response
assert_equal(json_obj[0]['hash'], bb_hash) # request/response hash should be the same
@ -278,9 +288,9 @@ class RESTTest (BitcoinTestFramework):
# See if we can get 5 headers in one response
self.generate(self.nodes[1], 5)
json_obj = self.test_rest_request(f"/headers/5/{bb_hash}")
json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 5})
assert_equal(len(json_obj), 5) # now we should have 5 header objects
json_obj = self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}")
json_obj = self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 5})
first_filter_header = json_obj[0]
assert_equal(len(json_obj), 5) # now we should have 5 filter header objects
json_obj = self.test_rest_request(f"/blockfilter/basic/{bb_hash}")
@ -294,7 +304,7 @@ class RESTTest (BitcoinTestFramework):
for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']:
assert_equal(
bytes(f'Header count is invalid or out of acceptable range (1-2000): {num}\r\n', 'ascii'),
self.test_rest_request(f"/headers/{num}/{bb_hash}", ret_type=RetType.BYTES, status=400),
self.test_rest_request(f"/headers/{bb_hash}", ret_type=RetType.BYTES, status=400, query_params={"count": num}),
)
self.log.info("Test tx inclusion in the /mempool and /block URIs")
@ -351,6 +361,11 @@ class RESTTest (BitcoinTestFramework):
json_obj = self.test_rest_request("/chaininfo")
assert_equal(json_obj['bestblockhash'], bb_hash)
# Test compatibility of deprecated and newer endpoints
self.log.info("Test compatibility of deprecated and newer endpoints")
assert_equal(self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/headers/1/{bb_hash}"))
assert_equal(self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}"))
if __name__ == '__main__':
RESTTest().main()