rpc generate: print useful help and error message #19455

pull jonatack wants to merge 3 commits into bitcoin:master from jonatack:rpc-generate-helpful-error_message changing 3 files +52 −2
  1. jonatack commented at 6:31 AM on July 6, 2020: member

    This was a requested follow-up to #19133 and #17700 to alleviate confusion and head-scratching by people following tutorials that use generate. See #19455 (comment) below, #19133 (comment) and #17700 (comment).

    before

    $ bitcoin-cli help generate
help: unknown command: generate

$ bitcoin-cli generate
error code: -32601
error message:
Method not found

    after

    $ bitcoin-cli help generate
generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information.

$ bitcoin-cli generate
error code: -32601
error message:
generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information.

    In the general help it remains hidden, as requested by laanwj.

    $ bitcoin-cli help

== Generating ==
generateblock "output" ["rawtx/txid",...]
generatetoaddress nblocks "address" ( maxtries )
generatetodescriptor num_blocks "descriptor" ( maxtries )
  2. jonatack commented at 6:33 AM on July 6, 2020: member

    Tagging @adamjonas and @MarcoFalke who suggested this feature.

  3. fanquake added the label RPC/REST/ZMQ on Jul 6, 2020
  4. DrahtBot commented at 12:07 PM on July 6, 2020: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)

    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.

  5. adamjonas commented at 11:59 PM on July 6, 2020: member

    Concept ACK. Thanks for circling back on this.

    When looking at the help options (bitcoin-cli -regtest help), this explanation looks out of place. I'd suggest rewording to at least start with the word generate to keep it alphabetical and consistent.

    <img width="704" alt="Screen Shot 2020-07-06 at 7 51 59 PM" src="https://user-images.githubusercontent.com/755825/86670029-36130c00-bfc2-11ea-87bb-7cfae65a5fc3.png">

  6. jonatack force-pushed on Jul 7, 2020
  7. jonatack commented at 4:00 AM on July 7, 2020: member

    When looking at the help options (bitcoin-cli -regtest help), this explanation looks out of place. I'd suggest rewording to at least start with the word generate to keep it alphabetical and consistent.

    Thanks Jonas! Good point -- updated.

    EDIT: per review feedback below, changed it to a hidden RPC command, which won't shown up in the general help.

  8. jonatack force-pushed on Jul 7, 2020
  9. jonatack force-pushed on Jul 7, 2020
  10. jonatack force-pushed on Jul 7, 2020
  11. jonatack force-pushed on Jul 7, 2020
  12. jonatack force-pushed on Jul 7, 2020
  13. in src/rpc/mining.cpp:1208 in 68fe377d4b outdated 
    1204 | @@ -1194,6 +1205,7 @@ static const CRPCCommand commands[] =
1205 |      { "generating",         "generatetoaddress",      &generatetoaddress,      {"nblocks","address","maxtries"} },
1206 |      { "generating",         "generatetodescriptor",   &generatetodescriptor,   {"num_blocks","descriptor","maxtries"} },
1207 |      { "generating",         "generateblock",          &generateblock,          {"output","transactions"} },
1208 | +    { "generating",         "generate",               &generate,               {} },
    laanwj commented at 2:21 PM on July 9, 2020:

    Why not make this a hidden RPC? It's a non-functional command only returning an error message, after all, there's no need for discoverability.

    jonatack commented at 3:30 PM on July 9, 2020:

    Yes, that's better. Done.

  14. laanwj commented at 2:22 PM on July 9, 2020: member

    I'm not sure on this. On one hand this is useful, on the other hand we did remove the similar message for getinfo a long time ago in b2f23c41538eaadd71c373ada75dd3a982eeb8bf. generate, though shorter ago, was removed more than a year ago (in 8bb3e4c487500a580e3e18791b1f4e7dcdd35442), so it seems to be a bit late to add this. But no strong opposition.

  15. test: consider generate covered in _get_uncovered_rpc_commands() 8d32d2011d
  16. jonatack force-pushed on Jul 9, 2020
  17. rpc: print useful help and error message for generate 92d94ffb8d
  18. test: add rpc_generate functional test f0aa8aeea5
  19. jonatack force-pushed on Jul 11, 2020
  20. jonatack commented at 5:14 PM on July 23, 2020: member

    No ACKs. Closing.

  21. jonatack closed this on Jul 23, 2020
  22. adamjonas commented at 6:24 PM on August 3, 2020: member

    This is my fault for the slow review, but I still think it's a useful addition to reduce confusion for all the outdated tutorials out there [1, 2, 3, 4].

    tACK f0aa8aeea5a183ea44a877255d12db7732f2e0a8

    Tried help generate, which returns a helpful prompt:

    $ bitcoin-cli help generate
generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information.

    This is much more instructive than the method not found error.

    $ bitcoin-cli generate
error code: -32601
error message:
generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information.

    And verified that it is a hidden RPC command <img width="455" alt="Screen Shot 2020-08-03 at 2 15 02 PM" src="https://user-images.githubusercontent.com/755825/89213859-c2015f00-d593-11ea-91ce-18b0ee2da7c9.png">

    I'm still an advocate for including this for the poor, unfortunate souls out there just trying to get started on regtest.

  23. jonatack commented at 10:57 AM on August 4, 2020: member

    Thanks for the review @adamjonas. Re-opening to see if this gets traction.

  24. jonatack reopened this on Aug 4, 2020
  25. kallewoof commented at 9:10 AM on August 12, 2020: member

    Concept / code review ACK. LGTM

    No strong opinion, but an alternative might be to PR an update on those outdated docs, and/or ping the respective authors. Perhaps both; ping and merge.

  26. jonatack commented at 10:20 AM on August 12, 2020: member

    Thanks @adamjonas and @kallewoof :heart:

    Good idea; I notified on the bitcoin.org github a couple months ago that generate is back as a cli option, though following up on all the tuturials would be helpful for people.

    If the maintainers agree on this change, ISTM the 2 ACKs and the test coverage here may enough to merge this no-risk user-facing aid.

  27. in test/functional/test_runner.py:721 in f0aa8aeea5 
     718 |          coverage_filenames = set()
 719 |          all_cmds = set()
 720 | -        covered_cmds = set()
 721 | +        # Consider RPC generate covered, because it is overloaded in
 722 | +        # test_framework/test_node.py and not seen by the coverage check.
 723 | +        covered_cmds = set({'generate'})
    pinheadmz commented at 1:38 PM on August 12, 2020:

    I don't quite understand this. generate is not returned by help and so I don't think its even added to the coverage list:

    https://github.com/bitcoin/bitcoin/blob/bd00d3b1f2036893419d1e8c514a8af2c4e4b1fb/test/functional/test_framework/coverage.py#L96

    Both with and without this change, my coverage results were the same:

    Uncovered RPC commands:
  - pruneblockchain
    jonatack commented at 3:59 PM on August 12, 2020:

    @pinheadmz Yes, it's a hidden command (not in the help, see the added test) as suggested by @laanwj. IIRC this was needed to appease a failing check in the Travis CI that despite the added test it thought there was no coverage.

  28. pinheadmz commented at 1:40 PM on August 12, 2020: member

    Concept ACK

    Built and tested locally, all behavior is as expected trying different combinations:

    src/bitcoin-cli -regtest -generate
src/bitcoin-cli -regtest generate
src/bitcoin-cli -regtest help generate
src/bitcoin-cli -regtest help -generate
src/bitcoin-cli -regtest -help -generate

    Only thing that caught me as far as interface is the fact that there is help and -help. The extra help menu is a bit of an easter egg? But that's out of scope for this PR I think.

  29. laanwj commented at 2:03 PM on August 12, 2020: member

    Only thing that caught me as far as interface is the fact that there is help and -help. The extra help menu is a bit of an easter egg? But that's out of scope for this PR I think.

    They are principally different on purpose. Like for any UNIX-ish application, -help or --help shows the help message for the local binary. This includes the different options how to connect to the server.

    On the other hand, help is a command for the remote server. It shows the help for the specific RPC server it connects to, and fails if there is none.

  30. in test/functional/rpc_generate.py:20 in f0aa8aeea5 
      15 | +    def set_test_params(self):
  16 | +        self.num_nodes = 1
  17 | +
  18 | +    def run_test(self):
  19 | +        message = (
  20 | +            "generate ( nblocks maxtries ) has been replaced by the -generate "
    laanwj commented at 2:42 PM on August 12, 2020:

    Isn't a whole new test for what is essentially a stub maybe a bit overkill? :smile:

    jonatack commented at 2:47 PM on August 12, 2020:

    I usually write tests before code; it saved my deliverables so many times and habits die hard :D

  31. pinheadmz approved
  32. pinheadmz commented at 3:59 PM on August 12, 2020: member

    ACK f0aa8aeea5a183ea44a877255d12db7732f2e0a8

    Thanks for your answers to my questions @jonatack and @laanwj

    <details><summary>Show Signature</summary>

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK f0aa8aeea5a183ea44a877255d12db7732f2e0a8
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAl80EaUACgkQ5+KYS2KJ
yTrkBw//dEwEnhDeko2ScTnzl/4mZSJLR0KG4D//BhWq+bhsSKKslWUQDMdJDYE/
HljPiuButxN01vJOVowqYV6ZGIPEVLiILu/R7OYkU81KLUPtZwsaouT9b0/8opmZ
/uXysTqvUa9UUfrk01vxkU63+viAQdpe5WBaf6dURdBjodRdoxL41r60GhMlacoe
wS75MuzR+pojiyr/Wf9s+t2BKDVO79rhoClhdOo9xiud4kqomdpmyQh2/86T5moe
LgTOjufZGAl/0haI1EAlWmtw0+AjMl6hRnrVUcgblElOJ201HZoE2Qj0L8bkykU1
I890Cu28PFWFeeK6xX3ev/ja6v2iLzrq1VsKmH1WRkEYaOkL8GvH4MJHwuQSlsFS
/h+dxMFjPvmigZMQTuaHDtDrsRGLqMTd6WEYoIC/NKRK+7UjgpGjzSiiRBS4g7iB
pVTTpei9/zjvXwlvr427+ji1kbWt2YKQdqrDeMIHHc5gcsFY9E1vgSKDw94TjVyt
YwOHlYG+ezhULsxNbW/D40ox1veFDkK/I0DkHN0AcUP7RuKTTdAiZ4B6+Fturk0f
3eFNIxf3qilCy5LPfRBcqtQ8GkpVgvcDpsO83cEeffyTIxrtRrPDULRc0upI81Jd
mi6DrhAultLW1xFvAjB8EBTTEVrdg9doVD5vMqHM+CFGm+wnyvM=
=wdk/
-----END PGP SIGNATURE-----

    pinheadmz's public key is on keybase

    </details>

  33. jonatack commented at 4:02 PM on August 12, 2020: member

    Thanks @pinheadmz!

  34. laanwj merged this on Aug 14, 2020
  35. laanwj closed this on Aug 14, 2020
  36. jonatack deleted the branch on Aug 14, 2020
  37. sidhujag referenced this in commit ff1cb84f45 on Aug 16, 2020
  38. deadalnix referenced this in commit 803956cc16 on Sep 14, 2021
  39. DrahtBot locked this on Feb 15, 2022
Contributors
jonatackDrahtBotadamjonaslaanwjkallewoofpinheadmz
Labels
RPC/REST/ZMQ
Linked (view graph)
#17700 cli: add -generate option#19133 rpc, cli, test: add bitcoin-cli -generate command
github-metadata-mirror

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: 2026-05-02 21:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me