-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Extend support for nested commands to bitcoin-cli #20273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extend support for nested commands to bitcoin-cli #20273
Conversation
The first commit might help to avoid reviewing moved code. |
7d392d0
to
7587eed
Compare
Tests are failing for me.
Failing with |
a614f7f
to
f48000e
Compare
@kristapsk: should be fixed now. |
8d3fa27
to
c5053ff
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
The command line already lets you do nested calls natively, eg: $ bitcoin-cli getblock $(bitcoin-cli getblockheader $(bitcoin-cli getbestblockhash) | jq -r .previousblockhash) | jq -r .size You could use shell aliases to reduce typing: $ bc () { bitcoin-cli "$@"; }
$ bcfield () { local F="$1"; shift; bitcoin-cli "$@" | jq -r "$F"; }
$ bcfield size getblock $(bcfield previousblockhash getblockheader $(bc getbestblockhash)) Personally, the only time it makes a difference to me is when I want to quickly query blocks around a particular height (eg |
@ajtowns: Good point. Yes. You can also do it with shell magic. Though you could also argue that writing a shell alias (or short script) could replace bitcoin-cli entirely (use cURL instead). Also for #16439, a shell alias/script would be trivial. I guess bitcoin-cli is a utility. Something that should allow one to perform a task quick and fast (to not make you use cURl or other low level http/RPC clients). This PR is not a replacement for #16439. It goes in a similar direction and may allow solve the same issues. IMO this PR extends the |
More or less: the disadvantage of a shell alias (eg For me, that matters when I'm doing a quick oneliner, but not when I'm doing anything more complicated. YMMV obviously. |
Indeed. That would also speak for this PR over shell aliases. I think #16439 could be integrated into this PR. There could be special commands like |
Concept ACK |
1 similar comment
Concept ACK |
c5053ff
to
027ff50
Compare
Rebased |
Concept ACK Compiled successfully on Ubuntu 20.04.2 LTS. Tests passed. I am not sure about the correct syntax for nesting so got error without quotes. Error:
With quotes:
|
mixed feelings on this, general concept NACK -- had no idea you could do this from the GUI, and I generally think the cli should be for dumb rpc calls, including a mini programming langauge seems to me to be asking for trouble. OTOH I do really like the concept of being able to atomically make RPC queries. |
Security? Interesting point. Although GUI console has limited access and usage, can we try few things in it? Or enable this only for testnet, regtest etc. and experiment for few weeks/months? |
@prayank23, that's issue of shell, nothing we can do about here, just |
Okay. I was interested in this PR because nesting helps in creating scripts for specific tasks. I have used
A similar thing in Bitcoin Core can be:
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
Currently, only the GUI supports nested commands like
getblock(getblock(getbestblockhash())[previousblockhash])[size]
(just an example) (see #7783).This PR extends the support for nested commands to bitcoin-cli (entirely client side).
See https://github.com/bitcoin/bitcoin/blob/0.20/src/qt/rpcconsole.cpp#L397 for more information.