doc: Explain in the generate* RPC docs that they are only for testing #24028

sipa commented at 12:06 AM on January 11, 2022: member

No description provided.

Explain in the generate* RPC docs that they are only for testing e187e0ef6e

fanquake added the label Docs on Jan 11, 2022

fjahr commented at 12:24 AM on January 11, 2022: member

ACK e187e0ef6e0cafa6236aa26f7b033d21264bb044

w0xlt approved

w0xlt commented at 12:33 AM on January 11, 2022: contributor

ACK e187e0e

DrahtBot commented at 2:04 AM on January 11, 2022: 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:

#24137 (doc: Rework generate* doc by MarcoFalke)
#18933 (rpc: Add submit option to generateblock by MarcoFalke)

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.

unknown approved

unknown commented at 5:53 AM on January 11, 2022: none

ACK https://github.com/bitcoin/bitcoin/pull/24028/commits/e187e0ef6e0cafa6236aa26f7b033d21264bb044

This PR improves documentation for generateblock generatetoaddress generatetodescriptor RPC and fixes https://github.com/bitcoin/bitcoin/issues/24027

MarcoFalke renamed this:
~~Explain in the generate* RPC docs that they are only for testing~~
doc: Explain in the generate* RPC docs that they are only for testing
on Jan 11, 2022

in src/rpc/mining.cpp:213 in e187e0ef6e

 209 | @@ -210,7 +210,10 @@ static RPCHelpMan generatetodescriptor()
 210 |  {
 211 |      return RPCHelpMan{
 212 |          "generatetodescriptor",
 213 | -        "\nMine blocks immediately to a specified descriptor (before the RPC call returns)\n",
 214 | +        "\nMine blocks immediately to a specified descriptor (before the RPC call returns)\n"

MarcoFalke commented at 7:47 AM on January 11, 2022:

        "Mine blocks immediately to a specified descriptor (before the RPC call returns)\n"

While touching this, there is no need to keep the \ns in the beginning and end. See https://github.com/bitcoin/bitcoin/pull/23714/files

jonatack commented at 9:40 AM on January 11, 2022:

suggestions for each of these helps

period at end of sentences
shorten the longer line
maybe wrap maxtries with backticks
omit newlines at start and end (as mentioned by Marco)

-        "\nMine blocks immediately to a specified descriptor (before the RPC call returns)\n"
-        "Only maxtries hash attempts will be performed. If no valid block can be found among those,\n"
-        "an error will be returned. This RPC is only intended for testing purposes, as it is highly inefficient and extremely\n"
-        "unlikely to succeed on mainnet\n",
+        "Mine blocks immediately to a specified descriptor (before the RPC call returns).\n"
+        "Only `maxtries` hash attempts will be performed. If no valid block can be found among those,\n"
+        "an error will be returned. This RPC is only intended for testing purposes, as it is highly\n"
+        "inefficient and extremely unlikely to succeed on mainnet.",

output

Mine blocks immediately to a specified descriptor (before the RPC call returns).
Only `maxtries` hash attempts will be performed. If no valid block can be found among those,
an error will be returned. This RPC is only intended for testing purposes, as it is highly
inefficient and extremely unlikely to succeed on mainnet.

MarcoFalke commented at 9:59 AM on January 11, 2022:

If this turns into a pull to rework the docs, the "immediately" and "(before the RPC call returns)" can be removed. It should be clear from the return value (the block hash) that the block was generated before the has was returned.

MarcoFalke commented at 7:51 AM on January 11, 2022: member

No objection to updating the docs, but I think the real issue is that the docs aren't hidden. For some reason no one ever removed them from the rpc help overview. I'd suggest to do that instead.

And once the RPCs are hidden, I think we can assume that anyone reading their help text understand POW. So a very brief "testing only because ridiculously inefficient" is enough.

laanwj commented at 11:33 AM on January 13, 2022: member

Concept ACK

theStack commented at 9:53 PM on January 22, 2022: member

Concept ACK

in src/rpc/mining.cpp:215 in e187e0ef6e

 209 | @@ -210,7 +210,10 @@ static RPCHelpMan generatetodescriptor()
 210 |  {
 211 |      return RPCHelpMan{
 212 |          "generatetodescriptor",
 213 | -        "\nMine blocks immediately to a specified descriptor (before the RPC call returns)\n",
 214 | +        "\nMine blocks immediately to a specified descriptor (before the RPC call returns)\n"
 215 | +        "Only maxtries hash attempts will be performed. If no valid block can be found among those,\n"
 216 | +        "an error will be returned. This RPC is only intended for testing purposes, as it is highly inefficient and extremely\n"

MarcoFalke commented at 10:50 AM on January 24, 2022:

nit: This is wrong

        "an empty array will be returned. This RPC is only intended for testing purposes, as it is highly inefficient and extremely\n"

MarcoFalke commented at 10:53 AM on January 24, 2022: member

The docs in this pull aren't exactly right (no error will be returned). I've created a conflicting pull in #24137 due to inactivity here. Feel free to cherry-pick or rebase on that one.

DrahtBot added the label Needs rebase on Feb 21, 2022

DrahtBot commented at 12:31 PM on February 21, 2022: member

🐙 This pull request conflicts with the target branch and needs rebase.

<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>

MarcoFalke commented at 12:56 PM on February 21, 2022: member

@sipa Are you still working on this?

MarcoFalke commented at 2:00 PM on February 25, 2022: member

Closing for now, but this can be reopened anytime (or a new pull can be created).

MarcoFalke closed this on Feb 25, 2022

DrahtBot locked this on Feb 25, 2023