This is a simple PR: add support for getting a block by its height via RPC call getblock.
Instead of 2 calls bitcoin-cli getblockhash 0 + bitcoin-cli getblock <hash>
we call directly bitcoin-cli getblock 0.
Note: it will continue to support via block hash.
getbockbyheight or so.
Though even then I’m not convinced it’s a worthwhile addition.
This would need to be a new RPC call getbockbyheight or so.
Given that getblock is not called getblockbyhash was the reason that I’ve added the height support.
Though even then I’m not convinced it’s a worthwhile addition.
For a new RPC I also share your opinion, about the getblock IMHO I think is useful for developers.
getbockbyheight could be handy. Concept ACK.
I don’t like overloading the argument, making it ambiguous.
Disagree, there is a place for overloading in APIs. In this case blocks are usually referenced by height or hash.
There is also overloading in the protocol: 4 lock_time uint32_t A time (Unix epoch time) or block number. See the locktime parsing rules.
It seems messy, and we don’t do that anywhere else on the API.
From help importaddress: 1. "script" (string, required) The hex-encoded script (or address) either.
E.g. a long hash with no hexadecimal characters could be interpreted as a long number.
Unlikely? Still it’s simple to distinguish that block hash from a number.
Though even then I’m not convinced it’s a worthwhile addition.
From the API point of view, I think it’s fair to fetch a block from it’s height. From a technical point of view, making 2 calls causing locks on cs_main etc on a heavy loaded daemon is resource wasting.
So, +1 for getblock hash|height (verbose).
I like the concept of one RPC call. But unfortunately there is one big caveat. In the future (and because of probability even in not so distant one), we can have a block whose hash is a hexadecimal number parseable as a decimal one resulting in the uncertain block referenced. We can’t afford to have uncertain result…
So NACK.
So if we want to have an API to get block by hash, we can have one more RPC call or to live with
0btc getblock `btc getblockhash 423787`
I’m fine even without the new RPC call…
FWIW: as of block 423792 we do not have any block whose hash written in hex is decimal number.
we can have a block whose hash is a hexadecimal number parseable as a decimal one
IMO it is possible to detect if a given input is a block hash, as it comes as string, and, given that block hashes have fixed length, I don’t think that if we want get the block 5125122 we call like this:
0getblock 0000000000000000000000000000000000000000000000000000000005125122
So it will push a fix to this special case.
So if we want to have an API to get block by hash, we can have one more RPC call or to live with
Imagine that you need to scan the whole blockchain, given that getblockhash RPC call takes 1 second to process (can be more or less):
So, IMO, a call like btc getblock 'btc getblockhash 423787' is not very optimized.
Compare results of
0btc getblock 00c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09
1btc getblock c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09
@laanwj @promag @paveljanik WDYT of supporting something like this:
0bitcoin-cli getblock hash (verbose)
0bitcoin-cli getblock '{ "hash": "0x123", "verbose": true}'
1bitcoin-cli getblock '{ "block": 1, "verbose": false}'
It’s better. But still this is special casing to work around one specific round-trip. This opens the flood-gates to many more.
To solve this more generally I plan on working on a cute little extension RFC to the JSONRPC 2.0 spec. See discussion in #7783 (comment) .
This will add a simple form of (in-batch) promise pipelining to JSON-RPC batching, which will allow doing this kind of composition in a more flexible way. Eg ext_store and ext_load below:
0[
1{
2 "version": "x.x",
3 "method": "getblockhash",
4 "id": 0,
5 "params": ["123456"],
6 "ext_store": True // Store the result of this request for later use
7},
8{
9 "version": "x.x",
10 "method": "getblock",
11 "id": 1,
12 "params": [
13 null // Placeholder for argument 0, will be replaced by ext_load below
14 ],
15 "ext_load": [
16 {
17 "src": [0, "result"], // Load the output of request id 0, ["result"]
18 "dst": [0] // into into params[0]
19 }
20 ],
21}
22]
The effect of which will be:
0tmp0 = getblockhash(123456);
1tmp1 = getblock(tmp0);
In contrast to simple nesting of calls, which would be the alternative extension, this also allows for more complex composition, and re-use of values.
Returning:
0[
1{
2 "id": 0,
3 "error": null,
4 "result": "0000000000002917ed80650c6174aac8dfc46f5fe36480aaef682ff6cd83c3ca"
5},
6{
7 "id": 1,
8 "error": null,
9 "result": {
10 "hash": ...,
11 "confirmations": ...,
12 ...
13 }
14}
15]
In contrast to simple nesting of calls, which would be the alternative extension, this also allows for more complex composition, and re-use of values.
Nice JSONRPC feature, delegating the responsibility of forwarding the output from the RPC client to the RPC server, but in our case it will still have the wallet locks and performance issues.
but in our case it will still have the wallet locks and performance issues.
That’s an implementation detail and will likely go away in the future, and we should not design the RPC protocol around that. In any case what use-case to you have that requires shaving a few milliseconds from a getblock call? Have you benchmarked? I’m fairly sure most of the bottleneck is reading and parsing the block and formatting JSON [hopefully done without holding the cs_main lock] - if the latter, have you looked at the REST interface?
Edit: If you need a lot of getblock calls you can pipeline without any RPC extensions: in the first batch, run getblockhash on block number 0..X - this is what our linearize.py script does too. In the second batch you can then request all the actual block data using getblock.
I think it’s a security risk to support getblock by height directly. People shouldn’t use height to refer to specific blocks, since reorgs can change the block at a given height.
Interesting point. It’s safe for old blocks, but for working at the tip, using it probably means you’re not handling reorgs properly.