node/http: get block header by hash/height

[tynes]

Add an HTTP endpoint to the Node for fetching block headers.

• GET /header/:block

This endpoint makes 1 less database call than fetching a block by hash or height. The view is not necessary for header.getJSON.

It also returns far less data and should always be consistent with the amount of data that it returns.

[pinheadmz]

I think this is available as an RPC command already no?

[braydonf]

I think this is available as an RPC command already no?

Yeah, however it doesn't look like it can be queried by height currently there though.

[pinheadmz]

@braydonf true, I actually make this a compound call like:

$ bcoin-cli rpc getblockheader `bcoin-cli rpc getblockhash <int height>`

[braydonf]

Cool, that could be made into one call similar to getblockbyheight, a possible new RPC command to add (not necessary in this PR).

[braydonf]

So I think this is just missing a test?

[tynes]

@braydonf Will update this PR with a test

[tynes]

Corresponding PR in bclient can be found here: bcoin-org/bclient#21

[tynes]

Added a test to assert that the same header is fetched when querying by hash and by height to cover the get block header by hash codepath.

[braydonf]

It could be worth it to use nclient.get(`/block/${height}\`); here instead, as to be similar to the header call with await nclient.get(`/header/${height}`);.

[tynes]

I'm only using nclient.get('/header/${height}') here because there is no nclient.getBlockHeader. There is a PR to add it here: https://github.com/bcoin-org/bclient/pull/21/files

I feel like the raw get request should be turned into using the wrapper once its merged into bclient.

[braydonf]

That's a reason why I think it's better to use nclient.get in most cases. I think there is also case to have a minimal RPC/HTTP client for testing, instead of depending on bclient. Meanwhile bclient does not have any tests.

[tynes]

I think there is also case to have a minimal RPC/HTTP client for testing, instead of depending on bclient.

I get this perspective and its generally not the best practice to couple tests for different repos together. bclient is partially tested in bcoin (which is better than no tests at all). Since bclient is used in production and is already a dependency of bcoin, I feel like using it in tests is a net positive, unless you want bcoin as a devDependency of bclient so that a full test suite can be written there.

[braydonf]

Should be useful for light wallet clients an as alternative means to syncing a header chain.

[tynes]

@braydonf Will push up fixes for the nits

[tynes]

Looks like existing tests need to be updated to generate an initial state in a before hook. Also looking more at the http-test.js and could probably be separated similar to node-rpc-test.js and wallet-rpc-test.js has been, it doesn't look like /block/${hash} is tested, aside from the additional getBlock call added below.

I agree that an additional PR can break up test/http-test.js