jonasschnelli added the label
Scripts and tools
on Oct 30, 2020
jonasschnelli
commented at 4:19 pm on October 30, 2020:
contributor
The first commit might help to avoid reviewing moved code.
jonasschnelli force-pushed
on Oct 30, 2020
kristapsk
commented at 6:26 pm on October 30, 2020:
contributor
Tests are failing for me.
0rpc_deriveaddresses.py --usecli | ✖ Failed | 2 s
1wallet_createwallet.py --usecli | ✖ Failed | 2 s
2wallet_multiwallet.py --usecli | ✖ Failed | 2 s
3wallet_watchonly.py --usecli | ✖ Failed | 1 s
Failing with test_framework.authproxy.JSONRPCException: Requested wallet does not exist or is not loaded (-18).
jonasschnelli force-pushed
on Oct 31, 2020
jonasschnelli force-pushed
on Oct 31, 2020
jonasschnelli
commented at 4:22 pm on October 31, 2020:
contributor
@kristapsk: should be fixed now.
What currently is not working are nested commands in conjunction with descriptors. I’ll work on a fix soon.
jonasschnelli force-pushed
on Nov 1, 2020
jonasschnelli force-pushed
on Nov 1, 2020
DrahtBot
commented at 11:15 am on November 3, 2020:
member
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#21430 (build: Add -Werror=implicit-fallthrough compile flag by hebasto)
#21415 (refactor: remove Optional & nullopt by fanquake)
#20452 (util: Replace use of locale dependent atoi(…) with locale-independent std::from_chars(…) (C++17) by practicalswift)
#16439 (cli/gui: support “@height” in place of blockhash for getblock on client side by ajtowns)
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.
ajtowns
commented at 4:44 am on November 9, 2020:
member
The command line already lets you do nested calls natively, eg:
Personally, the only time it makes a difference to me is when I want to quickly query blocks around a particular height (eg for a in {100..200}; do bitcoin-cli getblockheader @$a; done | grep versionHex), so the approach in #16439 is quicker and easier (less typing and don’t have to deal with brackets). Otherwise I’m pretty much writing a script, and the extra typing/aliases is no big deal.
jonasschnelli
commented at 8:46 am on November 9, 2020:
contributor
@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 power-commands (nested options plus subelement access) we have in the GUI to the CLI which I think is desirable (also for further extensions).
ajtowns
commented at 12:27 pm on November 9, 2020:
member
@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.
More or less: the disadvantage of a shell alias (eg bitcoin-cli getblockheader $(bcbest)) is that the alias can’t inherit -conf, -datadir, -testnet, -signet, or -regtest from the “parent” bitcoin-cli command, while getblockheader(getbestblockhash) or @best will automatically adapt to the right context.
For me, that matters when I’m doing a quick oneliner, but not when I’m doing anything more complicated. YMMV obviously.
jonasschnelli
commented at 7:26 am on November 10, 2020:
contributor
More or less: the disadvantage of a shell alias (eg bitcoin-cli getblockheader $(bcbest)) is that the alias can’t inherit -conf, -datadir, -testnet, -signet, or -regtest from the “parent” bitcoin-cli command, while getblockheader(getbestblockhash) or @best will automatically adapt to the right context.
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 @best or @H(400000) that just translates into getbestblockhash() or getblockhash(400000).
jonatack
commented at 3:15 pm on December 17, 2020:
member
Concept ACK
theStack
commented at 4:43 pm on January 2, 2021:
member
Concept ACK
DrahtBot added the label
Needs rebase
on Jan 27, 2021
MOVEONLY: move RPCParseCommandLine to nested.cpp/.h (won't compile)d52fc07e6a
Factor out RPC consoles ParseCommandLine. Add callbacks for flexibilitye147b44f3a
Add support for command nesting in bitcoin-cli951e1b3f42
avoid atoi in nested.cpp28f3a40d41
Use CHECK_NONFATAL() instead of assert() in nested.cpp65ef219e6b
Extend bitcoin-cli test to cover nested calls027ff50331
jonasschnelli force-pushed
on Mar 10, 2021
jonasschnelli
commented at 8:40 am on March 10, 2021:
contributor
Rebased
DrahtBot removed the label
Needs rebase
on Mar 10, 2021
ghost
commented at 0:12 am on March 11, 2021:
none
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:
0satoshi@ubuntu:~$ bitcoin-cli getblock(getblock(getbestblockhash())[previousblockhash])[tx][0]
12bash: syntax error near unexpected token `('
JeremyRubin
commented at 1:30 am on March 11, 2021:
contributor
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.
ghost
commented at 2:53 am on March 11, 2021:
none
including a mini programming langauge seems to me to be asking for trouble
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?
kristapsk
commented at 3:02 pm on March 11, 2021:
contributor
I am not sure about the correct syntax for nesting so got error without quotes.
@prayank23, that’s issue of shell, nothing we can do about here, just echo ( in bash will fail too.
ghost
commented at 6:44 pm on March 11, 2021:
none
that’s issue of shell, nothing we can do about here, just echo ( in bash will fail too.
Okay. I was interested in this PR because nesting helps in creating scripts for specific tasks. I have used cmdlets in PowerShell, the pipe symbol is used when one cmdlet pipes a result to another cmdlet. For example,
Get-MsolUser | Set-MsolUser -PasswordNeverExpires $true
Will perform a task (Set one parameter true) on all the results returned in the first cmdlet.
DrahtBot
commented at 12:45 pm on March 17, 2021:
member
🐙 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”.
DrahtBot added the label
Needs rebase
on Mar 17, 2021
DrahtBot
commented at 11:21 am on December 15, 2021:
member
Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.
MarcoFalke added the label
Up for grabs
on Dec 15, 2021
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-12-23 00:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me