This is an alternate approach to #16345 . It may solve #15412 and remove the desire to drop client-side arg validation per #15448 . Previous discussion (as pointed out in 16345) is in #16317, #14858 and #8457.

There's a bunch of other ways of doing this:

• just keep requiring a call to getblockhash N first -- this is less convenient and slower
• use the type of the parameter to distinguish heights and hashes -- this is what getblockstats does, but it means the type of the parameter is ambiguous which goes back to RPC getblockstats first argument inconsistency #15412
• add an additional height parameter to the RPC functions and make blockhash or height be alternatives -- I tried this at ajtowns@cf08687 but it's more code and not very convenient to use
• duplicate the RPC calls that take a blockhash and add "byheight" variants -- this is what 16345 does, but it's a fair chunk of duplicated code, which has received NACKs (see rpc: Add getblockbyheight method #16345 (comment) and rpc: Add getblockbyheight method #16345 (comment) )
• MarcoFalke suggested that aliases would cover the convenience factor and be useful for other things like "generate", but notes this isn't really possible in bitcoin-qt without a lot of work
• laanwj suggested that we could add pipelining to json, so a single request would call two RPCs passing the results of the first into the second. This seems pretty complicated, and the cli and gui would need to define some sort of shortcut mechanism to make that accessible as far as I can see.

This way isn't much code, particularly for supporting more generally, and while having to turn the number into a string is a bit annoying in code, it's not that bad, and very easy from the command line or the GUI. So up for consideration

(edited to add json pipelining suggestion)

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #19949 (cli: Parse and allow hash value by fjahr)
• #19762 (rpc: Allow named and positional arguments to be used together by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

I still don't like overloading the meaning of the parameter, but I'll stop repeating myself …

(also theoretically this could mess with software that assumes that only valid block hashes would be accepted by the RPC, though passing unchecked user input directly to RPC is arguably a bad idea…)

I still don't like overloading the meaning of the parameter, but I'll stop repeating myself …

So leave things as-is including the exception for getblockstats hash_or_height and consider this a wontfix, or do any of the other approaches seem appealing? Could be plausible to teach bitcon-cli to accept a 64-byte hex string or a number and make the hash_or_height str-vs-num work a bit better, I think, but everything else seems a lot of hassle to me.

@ajtowns do you see advantages of these approaches beside being faster to write or slightly fast execution (compared to also calling getblockhash)?

If this goes forward, wouldn't it be better to prefix with # instead? Or maybe just check the input because an hash can never be an height.

@ajtowns do you see advantages of these approaches beside being faster to write or slightly fast execution (compared to also calling getblockhash)?

It's a fair bit less complexity for the person asking for something -- you don't have to cut and paste a blockhash, or have a $( .. ) subcommand in shell, or remember what the name of the getblockhash rpc is. The speed difference is really just a side-effect of that, if you're making lots of getblock calls, you can lookup the blockhash once, and use the nextblockhash or previousblockhash to avoid extra calls after that, but again it's more complexity.

If this goes forward, wouldn't it be better to prefix with # instead?

Did think about that, but # is a comment character in shell, so bitcoin-cli getblock #500000 would need quotes which seems annoying. I think "getblock at 500000" reads pretty well anyway though.

Or maybe just check the input because an hash can never be an height.

Using a non-hex prefix makes it pretty unambiguous you don't mean a block hash. At least in theory you could have a block with hash 0000000000000000000000000000000000000000000000000000000000512345 eg, and misinterpret it. Also, see #8457 (comment)

Weak concept ACK.

I understand @laanwj's point of view: ideally, the RPC server code implements simple unambiguous primitive operations, and complexity of how to make them interact is offloaded to the client. In places where user friendliness for CLI users conflicts with that, it can be implemented as extra functionality in bitcoin-cli.

On the other hand, this approach seems actually simpler in total code complexity than having various hooks in bitcoin-cli for intercepting heights and requesting their hashes, and this is a frequently requested feature.

Perhaps an analogy is git's syntax for specifying commits, where hashes can be provided, but also branch names, and even simple operators on other commits like ~.

Concept ACK. I didn't follow previous discussions, but I like the @ syntax, think it is simpler and more convenient than the other duplicating / aliasing / pipelining suggestions, and don't see a real potential for misuse. (Apologies if I missed any other practical objections in previous discussion, they mostly seemed like aesthetic complaints.)

Weak concept ACK, for exactly the same reasons that sipa gives here: #16439 (comment)

I could live with that concept,... though I still think it's client functionality.

What if the client (bitcoin-cli) does an extra call for @<height> which would be getblockhash<height>?

For a getblock @<height>, bitcoin-cli would acctually do getblock(getblockhash(<height>))?

I think it's worth to extent bitcoin-cli for the hight-shortcut but I kida think it's wasted effort and unnecesarry risks for the daemon.

I understand @laanwj's point of view: ideally, the RPC server code implements simple unambiguous primitive operations,

Thanks… you're formulating it better than me

To be clear: it's not that I dislike the syntax. I'm not NACKing this. I think this would be useful from the command line. But I think doing this on the server side is the wrong way to go. I think this is the wrong direction.

Please also look at it from the perspective of someone writing a RPC wrapper in a static language, say, rust. After this change, the parameter needs to be an enum() which allows for multiple kinds of data, instead of a BlockHash.

One problem with this is that the height can refer to a different block in a sequence of calls:

foo @height
# a wild reorg
bar @height
# bar operates on a different block than foo

One problem with this is that the height can refer to a different block in a sequence of calls:
foo @height; reorg; bar @height

Isn't that to be expected and not particularly surprising? You get the same behaviour now if you call foo $(getblockhash N); reorg; bar $(getblockhash N).

What if the client (bitcoin-cli) does an extra call for @ which would be getblockhash?

I think you'd have to define a new table "convert height to blockhash params" in rpc/client.cpp as otherwise you'd do weird things like convert a wallet label from "@123" into a block 123's hash. That in turn means you have to keep your client in sync with the server, more than you would if it was just parsed on the server.

I've added in a quick hack to bitcoin-cli to support getblock %height purely on the client side, which actually doesn't look too bad to me. But taking a quick look at qt/rpcconsole.cpp made my eyes bleed, so will leave trying to make that work until later... The disadvantage of this approach is that every client needs code to support the @ shortcut; so if you're writing an RPC browser in rust, you'll have more code to write than if it was just supported on the server side directly. The advantage is it works as soon as you have the updated client, no daemon upgrade/restart needed, which is pretty nice.

Please also look at it from the perspective of someone writing a RPC wrapper in a static language, say, rust. After this change, the parameter needs to be an enum() which allows for multiple kinds of data, instead of a BlockHash.

Note this is already the case for hash_or_height in getblockstats, except that it's "string or numeric" not just two different types of string.

Are there any other shortcuts like this that seem useful? git comparisons like "foo^" for a parent or ancestor don't seem like they'd be that useful for blocks if you're already able to query by height easily, and short-references (ie, just specifying 8 digits if that's enough to reference a unique id) don't seem better than the height, and aren't convenient when the first digits are all zero...

Please also look at it from the perspective of someone writing a RPC wrapper in a static language, say, rust. After this change, the parameter needs to be an enum() which allows for multiple kinds of data, instead of a BlockHash.

It doesn't have to be an enum, it can also be a plain blockhash (hex string) and not support the user-facing @123 syntax.

Isn't that to be expected and not particularly surprising? You get the same behaviour

No, you would call getblockhash once and then all other calls would just use that hash.

re: #16439 (comment)

Please also look at it from the perspective of someone writing a RPC wrapper in a static language, say, rust. After this change, the parameter needs to be an enum() which allows for multiple kinds of data, instead of a BlockHash.

Could someone expand on this more? I don't understand this comment, and don't understand in general why people seem to like the idea of moving logic from the server to the client. Can someone be specific about an actual advantage that would come from moving logic from server to client, or be clear about what types of logic should be moved? Should every type of logic that it's possible to move be moved, or is there a different criteria for deciding what belongs in bitcoind vs bitcoin-cli?

If I'm writing a rust client or any other type of client, it would seem like the more logic there is in bitcoin-cli, the more work I have to do in rust to get access to the same functionality, and the greater chance I have of introducing bugs or inconsistencies.

Putting more logic in bitcoin-cli would also seem bad for testing and usability. Right now bitcoin-cli is a minimal wrapper around the RPC interface, so it's very easy to test things on command line, and then translate command line calls to real code (definitely in python and javascript, and I'm pretty sure it should be similar with rust).

Putting more logic in bitcoin-cli also has some practical problems right now (though these are solvable):

• bitcoin-cli has less knowledge of rpc methods and parameters than bitcoind does. So a lot of the transformations you could imagine implementing client side aren't really possible with current information.
• bitcoind and bitcoin-cli aren't currently doing any version checking, and there aren't currently any consequences from using an old bitcoin-cli with a new bitcoind or vice versa. If we start adding more logic to bitcoin-cli, interaction across versions will start being something that we'll want to think about (or prevent).

re: #16439 (comment)

Isn't that to be expected and not particularly surprising? You get the same behaviour

No, you would call getblockhash once and then all other calls would just use that hash.

This seems like an argument not only for requiring hashes server side, but also for requiring hashes client side, and against both this PR and #16345. I don't have much opinion on it, but just wanted to be clear about the implications. Maybe it would be a big inconvenience, or maybe it would prevent mistakes by users who forget about the possibilities of reorgs.

This seems like an argument not only for requiring hashes server side, but also for requiring hashes client side, and against both this PR and #16345.

Yes yes for this PR, not much for the other. This one promotes calls by height which don't take into account reorgs.

User should be assumed to know what a reorg is or be educated about them in the rpc help text, imo.

Could someone expand on this more? I don't understand this comment, and don't understand in general why people seem to like the idea of moving logic from the server to the client

The point is that use of the interface will usually be automatic/programmatic. Manual use (though a cli, in whatever programming language) is only a small subset of uses of the RPC interface.

Features that are only useful for manual use (such as this one) should go into the client. Every programmatic use can just as easily call getblockhash to get the hash first and go on with that. That's even more stable with regard to reorgs (@promag's scenario).

It seems it needs to be said often enough: the RPC interface does not exist for bitcoin-cli, or for end users. It's an API for applications to use bitcoin core. If you want to make things friendlier for end-users, the place to do so is not usually the server side.

Concept NACK. The RPC interface is for software, not human interaction. The decision to not support heights in places of hashes was intentional.

Concept ACK. This seems like a nice convenience that doesn't introduce any ambiguity or much complexity.

Honestly I don't understand what is so inconvenient.

Here's some more problems/inconsistencies:

-blocknotify and ZMQ messages notify the block hash, not height, so I'd say most of the time you end up using the hash.

Some (at least getchaintxstats) have the parameter named as blockhash and IMO blockhash=@1000 is inconsistent.

Also, consider this:

Would be nice if the gui supported this, even when it is not added to bitcoind

@MarcoFalke GUI console supports getblock(getblockhash(1)). Are you suggesting getblock(@1) to result in the previous expression?

Restating my Concept ACK here. I prefer the @height on server approach to the %height in client approach.

I've skimmed the code and it looks good. Will fully review once commits are cleaned up and this is no longer a draft.

Rebased

See also #20273.
Nested commands allow similar functionality with more flexibility.

• Getting a block by height bitcoin-cli "getblock(getblockhash(10000))"
• Getting the first transaction hash of the previous block by height bitcoin-cli "getblock(getblock(getblockhash(10000))[previousblockhash])[tx][0]"

I think #20273 is better solution for a problem, server code should not be complicated if that can be avoided, so Concept NACK.

@kristapsk This doesn't touch server code...

@ajtowns You're right, my bad, didn't look at the actual code here. In any case, still think #20273 is superior to this approach.

Rebased on top of #20012

Bumped to add new rpcs...

ACK, modulo may as well squash the new commit into the first one.

I reread the discussion at the top to reswap the context into memory and agree this is better on the client side for the reasons stated in #16439 (comment).

I haven't had any good ideas about how to document this -- it's client side, so updating the RPC help doesn't make sense.

Since it needs to be documented somewhere, maybe add a reusable line to the RPC helps mentioning that it's a client-side shortcut. Human users consume these helps for CLI use in addition to for writing client-side software.

This isn't seeing any progress so closing.