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
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.
514@@ -474,6 +515,48 @@ static int CommandLineRPC(int argc, char *argv[])
515 if (gArgs.GetBoolArg("-getinfo", false)) {
516 rh.reset(new GetinfoRequestHandler());
517 method = "";
518+ } else if (gArgs.GetBoolArg("-generate", false)) {
519+ //-generate is special as it executes two RPC calls:
520+ // 1) "getnewaddress" without any parameters, which returns a bech32 address
521+ // 2) "generatetoaddress" with three params: numblocks, address and maxtries
522+ // If no args are given the args-vector will be extended with DEFAULT_GENERATED_NBLOCKS
523+ // and DEFAULT_GENERATED_MAXTRIES.
So I tried this diff:
0diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
1index ef05c910d4..2fff98c05c 100644
2--- a/src/bitcoin-cli.cpp
3+++ b/src/bitcoin-cli.cpp
4@@ -535,11 +535,11 @@ static int CommandLineRPC(int argc, char *argv[])
5 if (args.size() == 0) {
6 args.push_back(std::to_string(DEFAULT_GENERATED_NBLOCKS));
7 args.push_back(std::move(address));
8- args.push_back(std::to_string(DEFAULT_GENERATED_MAXTRIES));
9 }
10 if (args.size() < 2) {
11 args.push_back(std::move(address));
12- args.push_back(std::to_string(DEFAULT_GENERATED_MAXTRIES));
13 }
14 if (args.size() == 2) {
15 const std::string nblocks = args[0];
And I get
0$ ./src/bitcoin-cli -generate 1
1error: Error parsing JSON:bcrt1qyqfgcaddym58h6py4rrsz4g428gl5tmw84rt36
Not sure why that is. I might take another look later.
Oh, I forgot to replace the if with else if. This works:
0diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
1index ef05c910d4..4179520cc1 100644
2--- a/src/bitcoin-cli.cpp
3+++ b/src/bitcoin-cli.cpp
4@@ -229,7 +229,7 @@ class GetNewAddressRequestHandler: public BaseRequestHandler
5 public:
6 const int ID_GETNEWADDRESS = 0;
7
8- UniValue PrepareRequest(const std::string& method, const std::vector<std::string>& args) override
9+ UniValue PrepareRequest(const std::string& method, const std::vector<std::string>& /* args */) override
10 {
11 const UniValue& request = JSONRPCRequestObj("getnewaddress", NullUniValue, ID_GETNEWADDRESS);
12 return request;
13@@ -248,11 +248,8 @@ public:
14
15 UniValue PrepareRequest(const std::string& method, const std::vector<std::string>& args) override
16 {
17- std::vector<std::string> params = {};
18- params.push_back(args[0]);
19- params.push_back(args[1]);
20- params.push_back(args[2]);
21- UniValue paramsObj = RPCConvertValues("generatetoaddress", params);
22+ UniValue paramsObj = RPCConvertValues("generatetoaddress", args);
23 return JSONRPCRequestObj("generatetoaddress", paramsObj, ID_GENERATETOADDRESS);
24 }
25
26@@ -535,13 +532,11 @@ static int CommandLineRPC(int argc, char *argv[])
27 if (args.size() == 0) {
28 args.push_back(std::to_string(DEFAULT_GENERATED_NBLOCKS));
29 args.push_back(std::move(address));
30- args.push_back(std::to_string(DEFAULT_GENERATED_MAXTRIES));
31 }
32- if (args.size() < 2) {
33+ else if (args.size() < 2) {
34 args.push_back(std::move(address));
35- args.push_back(std::to_string(DEFAULT_GENERATED_MAXTRIES));
36 }
37- if (args.size() == 2) {
38+ else if (args.size() == 2) {
39 const std::string nblocks = args[0];
40 const std::string maxtries = args[1];
41 args.clear();
@brakmic something like call
bitcoin-cli -generate ...and check the response format and maybe also check the new tip againstgetblockchaininfo.
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:
48@@ -47,6 +49,7 @@ static void SetupCliArgs()
49 gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
50 gArgs.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
51 gArgs.AddArg("-getinfo", "Get general information from the remote server. Note that unlike server-side RPC calls, the results of -getinfo is the result of multiple non-atomic requests. Some entries in the result may represent results from different states (e.g. wallet balance may be as of a different block from the chain state reported)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
52+ gArgs.AddArg("-generate=[nblocks] [maxtries]", strprintf("Generate blocks immediately (default nblocks: %s, maxtries: %s)", DEFAULT_GENERATED_NBLOCKS, DEFAULT_GENERATED_MAXTRIES), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION);
-generate=...?
Ok, the simplest solution is the best one. :)
I have removed the whole stuff =nblocks maxtries and only left -generate in command help info.
510@@ -474,6 +511,46 @@ static int CommandLineRPC(int argc, char *argv[])
511 if (gArgs.GetBoolArg("-getinfo", false)) {
512 rh.reset(new GetinfoRequestHandler());
513 method = "";
514+ } else if (gArgs.GetBoolArg("-generate", false)) {
515+ //-generate is special as it executes two RPC calls:
516+ // 1) "getnewaddress" without any parameters, which returns a bech32 address
517+ // 2) "generatetoaddress" with three params: numblocks, address and maxtries
518+ // If no args are given the args-vector will be extended with DEFAULT_GENERATED_NBLOCKS
519+ // and DEFAULT_GENERATED_MAXTRIES.
510@@ -474,6 +511,46 @@ static int CommandLineRPC(int argc, char *argv[])
511 if (gArgs.GetBoolArg("-getinfo", false)) {
512 rh.reset(new GetinfoRequestHandler());
513 method = "";
514+ } else if (gArgs.GetBoolArg("-generate", false)) {
515+ //-generate is special as it executes two RPC calls:
516+ // 1) "getnewaddress" without any parameters, which returns a bech32 address
0 // 1) "getnewaddress" without any parameters
This is the default, but can be modified in the config settings
538+ else if (args.size() < 2) {
539+ args.push_back(std::move(address));
540+ }
541+ else if (args.size() == 2) {
542+ const std::string nblocks = args[0];
543+ const std::string maxtries = args[1];
.at(0) and .at(1). Feel free to ignore.
Co-Authored-By: MarcoFalke <falke.marco@gmail.com>
Co-Authored-By: MarcoFalke <falke.marco@gmail.com>
48@@ -47,6 +49,7 @@ static void SetupCliArgs()
49 gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
50 gArgs.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
51 gArgs.AddArg("-getinfo", "Get general information from the remote server. Note that unlike server-side RPC calls, the results of -getinfo is the result of multiple non-atomic requests. Some entries in the result may represent results from different states (e.g. wallet balance may be as of a different block from the chain state reported)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
52+ gArgs.AddArg("-generate", strprintf("Generate blocks immediately (default nblocks: %s, maxtries: %s)", DEFAULT_GENERATED_NBLOCKS, DEFAULT_GENERATED_MAXTRIES), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION);
0 gArgs.AddArg("-generate", strprintf("[nblocks] [maxtries] Generate blocks immediately (default nblocks: %s, maxtries: Same default as the generatetoaddress RPC)", DEFAULT_GENERATED_NBLOCKS), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION);
32@@ -33,6 +33,8 @@ const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
33 static const char DEFAULT_RPCCONNECT[] = "127.0.0.1";
34 static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900;
35 static const bool DEFAULT_NAMED=false;
36+static const int DEFAULT_GENERATED_NBLOCKS=1;
37+static const int DEFAULT_GENERATED_MAXTRIES=100000;
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.
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.
- The JSON that’s returned could be improved.
I’ve changed the structure of JSON result and it now looks like this:
0./src/bitcoin-cli -regtest -generate 3
1{
2 "address": "bcrt1qxwysj6war3v4jf6kcderstxsecq534umcalefc",
3 "blocks": [
4 "6f78253c94c0e5b180ae7002c366811a9f70a32b70fc17a81e42fd69d351ecf9",
5 "46f1b2790fe2e973fe67531c1acb7f29298005a5107b80cd56d510050599316b",
6 "34d3c018be1455fcfd40a07ba5e55f245af9515e9e9f50116f4948399378ff59"
7 ]
8}
47@@ -47,6 +48,7 @@ static void SetupCliArgs()
48 gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
49 gArgs.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
50 gArgs.AddArg("-getinfo", "Get general information from the remote server. Note that unlike server-side RPC calls, the results of -getinfo is the result of multiple non-atomic requests. Some entries in the result may represent results from different states (e.g. wallet balance may be as of a different block from the chain state reported)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
51+ gArgs.AddArg("-generate", strprintf("[nblocks] [maxtries] Generate blocks immediately (default nblocks: %s, maxtries: Same default as the generatetoaddress RPC)", DEFAULT_GENERATED_NBLOCKS), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION);
9+
10+
11+class GenerateRpcTest(BitcoinTestFramework):
12+ def set_test_params(self):
13+ self.num_nodes = 1
14+ self.setup_clean_chain = True
15+
16+ def skip_test_if_missing_module(self):
17+ self.skip_if_no_wallet()
18+
19+ def run_test(self):
20+ self.generate_blocks()
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: https://github.com/bitcoin/bitcoin/issues/16000
@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.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, 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.
259+ res.pushKV("blocks", result.get_obj()["result"]);
260+ return JSONRPCReplyObj(res, NullUniValue, 1);
261+ }
262+protected:
263+ std::string addr;
264+};
DefaultRequestHandler and put the 2 pushKV lines in your -generate code section below. See 06a1bf6c1e1de3e219d2f8eebe187c1ca2962e9d for a semi-related example.
Many thanks. A good idea. However, my code is based on a pretty “old” version that differs greatly from https://github.com/bitcoin/bitcoin/commit/06a1bf6c1e1de3e219d2f8eebe187c1ca2962e9d
Maybe before changing all that, I should rebase this code and try to re-implement it again.
E.g.
0CallRPC(rh.get(), "getnewaddress", args);
1CallRPC(rh.get(), "generatetoaddress", args);
etc.