cli: add -generate option by brakmic · Pull Request #17700 · bitcoin/bitcoin

brakmic · 2019-12-09T09:14:25Z

This PR is related to #16000 and should "bring back" the generate call.

bitcoin-cli would get a new option -generate that mimics the original API.

The allowed (optional) parameters are nblocks and maxtries.
For example:

bitcoin-cli -generate
bitcoin-cli -generate 2
bitcoin-cli -generate 3 10000

laanwj · 2019-12-09T09:35:49Z

Concept ACK, seems useful for testing
(vaguely related to #17314 as for improving -cli user experience)

promag · 2019-12-09T11:25:56Z

Mind adding some test to test/functional/interface_bitcoin_cli.py?

brakmic · 2019-12-09T11:28:36Z

Mind adding some test to test/functional/interface_bitcoin_cli.py?

Yes, I spoke about it in the original posting, but I am not sure how to do it the correct way.

But I'll look into it.

promag · 2019-12-09T11:31:37Z

@brakmic something like call bitcoin-cli -generate ... and check the response format and maybe also check the new tip against getblockchaininfo.

src/bitcoin-cli.cpp

brakmic · 2019-12-09T13:57:49Z

@brakmic something like call bitcoin-cli -generate ... and check the response format and maybe also check the new tip against getblockchaininfo.

I've added tests in test/functional/rpc_generate.py
However, I am not sure if it should be prefixed with "rpc", because under the hood it uses CLI to mimic the old generate API. So maybe it should rather be called "cli_generate" or something along the line.

The current test checks for:

block generation (default, with or without args nblocks & maxtries)
errors (when args invalid)

maflcko

ACK

src/bitcoin-cli.cpp

Co-Authored-By: MarcoFalke <[email protected]>

src/bitcoin-cli.cpp

Co-Authored-By: MarcoFalke <[email protected]>

src/bitcoin-cli.cpp

jonasschnelli · 2019-12-16T06:02:28Z

Nice.
Although I would prefer something that is also possible through the GUI console (maybe server side) instead of in both clients?

laanwj · 2019-12-16T10:20:01Z

Although I would prefer something that is also possible through the GUI console (maybe server side) instead of in both clients?

I'm definitely opposed to adding more mining functionality back server-side.

But I guess in the longer term we could have a new library that's shared between the GUI console and cli, for these kind of features.

I think GUI is out of scope here though.

adamjonas · 2019-12-16T17:24:47Z

Concept ACK. I did a little playing around with this and had two main pieces of feedback:

One of the reasons I'm such a big supporter for bringing back generate is because many of the historical tutorials were broken when it was removed (for example https://btcinformation.org/en/developer-examples#regtest-mode). Unfortunately, this isn't corrected here. I'm aware that adding the dash is based on the -getinfo precedent, but at the very least, catching generate client side and providing user feedback, even if it refers to adding the dash or gives fuller instructions on how to use generatetoaddress, would reduce a lot of head scratching for first-time users.
The JSON that's returned could be improved.

I think returning a blockhash string similar to generatetoaddress would be ideal. But if we are going to include key/values, I'd argue that both the blockhashes and the newly generated address should be displayed. This would educate the user as to what is going on under the hood and allow testers to access the address that would be otherwise lost. If we decide to omit the addresses, we can just return the blockhashes in an array like generatetoaddress rather than putting it a level deep inside of "result" ("result" should probably be renamed to "blockhashes"). But regardless, removing the "error" field in the absence of errors and taking out the "id" field would clean it up a lot.

brakmic · 2019-12-17T00:24:21Z

The JSON that's returned could be improved.

I've changed the structure of JSON result and it now looks like this:

./src/bitcoin-cli -regtest -generate 3
{
  "address": "bcrt1qxwysj6war3v4jf6kcderstxsecq534umcalefc",
  "blocks": [
    "6f78253c94c0e5b180ae7002c366811a9f70a32b70fc17a81e42fd69d351ecf9",
    "46f1b2790fe2e973fe67531c1acb7f29298005a5107b80cd56d510050599316b",
    "34d3c018be1455fcfd40a07ba5e55f245af9515e9e9f50116f4948399378ff59"
  ]
}

jonatack

Concept ACK

src/bitcoin-cli.cpp

test/functional/rpc_generate.py

cvengler · 2020-01-29T18:31:54Z

Concept NACK
If this option is for testing, why not make a simple shell script locally to do that?
What's the advantage of having this?

brakmic · 2020-01-29T18:33:42Z

Concept NACK
If this option is for testing, why not make a simple shell script locally to do that?
What's the advantage of having this?

Please, check the original discussion which this PR is based on: #16000

cvengler · 2020-01-29T18:36:51Z

@brakmic I flew over the old discussion but I saw that the old point wasn't pointed out somewhere.
Maybe I oversaw could you point me to thing you think I may missed?

brakmic · 2020-01-29T18:42:47Z

@brakmic I flew over the old discussion but I saw that the old point wasn't pointed out somewhere.
Maybe I oversaw could you point me to thing you think I may missed?

Old point? Not sure what you mean with it.

All in all, the reason for "resurrecting" this retired API call is to create something that behaves similarly to -getinfo. As we all know, In the past getinfo was one of the RPC calls that later got removed (and subsequently reintroduced as a CLI-only -getinfo option). Now, the same should happen with -generate which is not an RPC call but a purely CLI-thing.

In a way, it already functions like a shell script would do (and could also itself be reused in other shell scripts too, in my opinion).

I hope my explanation helps you further.

DrahtBot · 2020-04-13T01:54:29Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

cli: display multiwallet balances in -getinfo #18594 (cli: display multiwallet balances in -getinfo by jonatack)

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.

src/bitcoin-cli.cpp

jonatack · 2020-05-28T20:20:08Z

Hi @brakmic, would you have any objection if I relaunch and update this, building on some of your commits?

brakmic · 2020-05-28T20:22:57Z

Hi @brakmic, would you have any objection if I relaunch and update this, building on some of your commits?

No problem. Take anything that could be of use to you.

jonatack · 2020-06-01T12:39:06Z

Thanks @brakmic! Continued the work in #19133.

22cb303 rpc: add missing space in JSON parsing error message, update test (Jon Atack) bf53ebe test: add multiwallet tests for bitcoin-cli -generate (Jon Atack) 4b859cf cli: add multiwallet capability to GetNewAddress and -generate (Jon Atack) 18f9354 test: add tests for bitcoin-cli -generate (Jon Atack) 4818124 cli: create bitcoin-cli -generate command (Jon Atack) ff41a36 cli: extract ParseResult() and ParseError() (Jon Atack) f4185b2 cli: create GenerateToAddressRequestHandler class (Harris) f7c65a3 cli: create GetNewAddress() (Jon Atack) 9be7fd3 rpc: make generatetoaddress locals const (Jon Atack) cb00510 rpc: create rpc/mining.h, hoist default max tries values to constant (Jon Atack) Pull request description: This PR continues and completes the work begun in #17700 working on issue #16000 to create a client-side version of RPC `generate`. Basically, `bitcoin-cli -generate` wraps calling `generatenewaddress` followed by `generatetoaddress [nblocks] [maxtries]` and prints the following: ``` $ bitcoin-cli -generate { "address": "bcrt1qn4aszr2y2xvpa70y675a76wsu70wlkwvdyyln6" "blocks": [ "01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5", ] } $ bitcoin-cli -rpcwallet=wallet-name -generate 3 100 { "address": "bcrt1q4cunfw0gnsj7g7e6mk0v0uuvvau9mwr09dj45l", "blocks": [ "7a6650ca5e0c614992ee64fb148a7e5e022af842e4b6003f81abd8baf1e75136", "01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5", "3f8795ec40b1ad812b818c177680841be319a3f6753d4e32dc7dfb5bafe5d00e" ] } ``` Help doc: ``` $ bitcoin-cli -h | grep -A5 "\-generate" -generate Generate blocks immediately, equivalent to RPC generatenewaddress followed by RPC generatetoaddress. Optional positional arguments are number of blocks to generate (default: 1) and maximum iterations to try (default: 1000000), equivalent to RPC generatetoaddress nblocks and maxtries arguments. Example: bitcoin-cli -generate 4 1000 ``` Quite a bit of test coverage turned out to be needed to cover the change and the different cases (arguments, multiwallet mode) and error-handling. This PR also improves some things that working on these changes brought to light. Credit to Harris Brakmić for the initial work in #17700. ACKs for top commit: adamjonas: utACK 22cb303 meshcollider: utACK 22cb303 Tree-SHA512: 94f67f632fe093d076f614e0ecff09ce7342ac6e424579200d5211a6615260e438d857861767fb788950ec6da0b26ef56dc8268c430012a3b3d4822b24ca6fbf

22cb303 rpc: add missing space in JSON parsing error message, update test (Jon Atack) bf53ebe test: add multiwallet tests for bitcoin-cli -generate (Jon Atack) 4b859cf cli: add multiwallet capability to GetNewAddress and -generate (Jon Atack) 18f9354 test: add tests for bitcoin-cli -generate (Jon Atack) 4818124 cli: create bitcoin-cli -generate command (Jon Atack) ff41a36 cli: extract ParseResult() and ParseError() (Jon Atack) f4185b2 cli: create GenerateToAddressRequestHandler class (Harris) f7c65a3 cli: create GetNewAddress() (Jon Atack) 9be7fd3 rpc: make generatetoaddress locals const (Jon Atack) cb00510 rpc: create rpc/mining.h, hoist default max tries values to constant (Jon Atack) Pull request description: This PR continues and completes the work begun in bitcoin#17700 working on issue bitcoin#16000 to create a client-side version of RPC `generate`. Basically, `bitcoin-cli -generate` wraps calling `generatenewaddress` followed by `generatetoaddress [nblocks] [maxtries]` and prints the following: ``` $ bitcoin-cli -generate { "address": "bcrt1qn4aszr2y2xvpa70y675a76wsu70wlkwvdyyln6" "blocks": [ "01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5", ] } $ bitcoin-cli -rpcwallet=wallet-name -generate 3 100 { "address": "bcrt1q4cunfw0gnsj7g7e6mk0v0uuvvau9mwr09dj45l", "blocks": [ "7a6650ca5e0c614992ee64fb148a7e5e022af842e4b6003f81abd8baf1e75136", "01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5", "3f8795ec40b1ad812b818c177680841be319a3f6753d4e32dc7dfb5bafe5d00e" ] } ``` Help doc: ``` $ bitcoin-cli -h | grep -A5 "\-generate" -generate Generate blocks immediately, equivalent to RPC generatenewaddress followed by RPC generatetoaddress. Optional positional arguments are number of blocks to generate (default: 1) and maximum iterations to try (default: 1000000), equivalent to RPC generatetoaddress nblocks and maxtries arguments. Example: bitcoin-cli -generate 4 1000 ``` Quite a bit of test coverage turned out to be needed to cover the change and the different cases (arguments, multiwallet mode) and error-handling. This PR also improves some things that working on these changes brought to light. Credit to Harris Brakmić for the initial work in bitcoin#17700. ACKs for top commit: adamjonas: utACK 22cb303 meshcollider: utACK 22cb303 Tree-SHA512: 94f67f632fe093d076f614e0ecff09ce7342ac6e424579200d5211a6615260e438d857861767fb788950ec6da0b26ef56dc8268c430012a3b3d4822b24ca6fbf

f0aa8ae test: add rpc_generate functional test (Jon Atack) 92d94ff rpc: print useful help and error message for generate (Jon Atack) 8d32d20 test: consider generate covered in _get_uncovered_rpc_commands() (Jon Atack) Pull request description: 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 ) ``` ACKs for top commit: adamjonas: utACK f0aa8ae pinheadmz: ACK f0aa8ae Tree-SHA512: d083652589ad3e8228c733455245001db22397559c3946e7e573cf9bd01c46e9e88b72d934728ec7f4361436ae4c74adb8f579670b09f479011924357e729af5

f0aa8ae test: add rpc_generate functional test (Jon Atack) 92d94ff rpc: print useful help and error message for generate (Jon Atack) 8d32d20 test: consider generate covered in _get_uncovered_rpc_commands() (Jon Atack) Pull request description: This was a requested follow-up to bitcoin#19133 and bitcoin#17700 to alleviate confusion and head-scratching by people following tutorials that use `generate`. See bitcoin#19455 (comment) below, bitcoin#19133 (comment) and bitcoin#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 ) ``` ACKs for top commit: adamjonas: utACK f0aa8ae pinheadmz: ACK f0aa8ae Tree-SHA512: d083652589ad3e8228c733455245001db22397559c3946e7e573cf9bd01c46e9e88b72d934728ec7f4361436ae4c74adb8f579670b09f479011924357e729af5

laanwj added the RPC/REST/ZMQ label Dec 9, 2019

maflcko reviewed Dec 9, 2019

View reviewed changes

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

cli: add -generate option

99011af

maflcko approved these changes Dec 9, 2019

View reviewed changes

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

brakmic and others added 3 commits December 9, 2019 20:05

Update src/bitcoin-cli.cpp

a9faef5

Co-Authored-By: MarcoFalke <[email protected]>

cleanups

70543e9

adapt command help-info

7081ab2

maflcko reviewed Dec 9, 2019

View reviewed changes

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

Update src/bitcoin-cli.cpp

03de7ae

Co-Authored-By: MarcoFalke <[email protected]>

maflcko reviewed Dec 9, 2019

View reviewed changes

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

remove redundant DEFAULT_GENERATED_MAXTRIES

781fa33

changed JSON result structure, +updated tests

1b815f3

jonatack reviewed Jan 18, 2020

View reviewed changes

src/bitcoin-cli.cpp Show resolved Hide resolved

test/functional/rpc_generate.py Outdated Show resolved Hide resolved

test/functional/rpc_generate.py Outdated Show resolved Hide resolved

test/functional/rpc_generate.py Outdated Show resolved Hide resolved

brakmic added 3 commits January 18, 2020 20:47

put -generate before -getinfo in bitcoin-cli.cpp

37321af

don't ask for clean chain in test

200c0ba

execute tests in run_test directly

ca02d36

laanwj added the Feature label Mar 19, 2020

DrahtBot mentioned this pull request Apr 12, 2020

cli: display multiwallet balances in -getinfo #18594

Merged

jonatack reviewed Apr 13, 2020

View reviewed changes

src/bitcoin-cli.cpp Show resolved Hide resolved

brakmic closed this May 10, 2020

jonatack mentioned this pull request Jun 1, 2020

rpc, cli, test: add bitcoin-cli -generate command #19133

Merged

jonatack mentioned this pull request Jun 23, 2020

Newsletters: add 103 (2020-06-24) bitcoinops/bitcoinops.github.io#425

Merged

4 tasks

jonatack mentioned this pull request Jul 6, 2020

rpc generate: print useful help and error message #19455

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Conversation

brakmic commented Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Dec 9, 2019

Uh oh!

brakmic commented Dec 9, 2019

Uh oh!

promag commented Dec 9, 2019

Uh oh!

Uh oh!

brakmic commented Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonasschnelli commented Dec 16, 2019

Uh oh!

laanwj commented Dec 16, 2019

Uh oh!

adamjonas commented Dec 16, 2019

Uh oh!

brakmic commented Dec 17, 2019

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cvengler commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brakmic commented Jan 29, 2020

Uh oh!

cvengler commented Jan 29, 2020

Uh oh!

brakmic commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Apr 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Uh oh!

jonatack commented May 28, 2020

Uh oh!

brakmic commented May 28, 2020

Uh oh!

jonatack commented Jun 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

brakmic commented Dec 9, 2019 •

edited

Loading

laanwj commented Dec 9, 2019 •

edited

Loading

brakmic commented Dec 9, 2019 •

edited

Loading

cvengler commented Jan 29, 2020 •

edited

Loading

brakmic commented Jan 29, 2020 •

edited

Loading

DrahtBot commented Apr 13, 2020 •

edited

Loading