Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Rework generate* doc #24137

Merged
merged 1 commit into from Feb 21, 2022
Merged

doc: Rework generate* doc #24137

merged 1 commit into from Feb 21, 2022

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jan 24, 2022

Hide the test-only calls and clarify the short description

@maflcko
Copy link
Member Author

maflcko commented Jan 24, 2022

Rendered diff:

diff --git a/generateblock b/generateblock
index 55aca89..1fcd557 100644
--- a/generateblock
+++ b/generateblock
@@ -1,6 +1,6 @@
 generateblock "output" ["rawtx/txid",...]
 
-Mine a block with a set of ordered transactions immediately to a specified address or descriptor (before the RPC call returns)
+Mine a set of ordered transactions to a specified address or descriptor and return the block hash.
 
 Arguments:
 1. output               (string, required) The address or descriptor to send the newly generated bitcoin to.
diff --git a/generatetoaddress b/generatetoaddress
index 04c2092..6abb868 100644
--- a/generatetoaddress
+++ b/generatetoaddress
@@ -1,9 +1,9 @@
 generatetoaddress nblocks "address" ( maxtries )
 
-Mine blocks immediately to a specified address (before the RPC call returns)
+Mine to a specified address and return the block hashes.
 
 Arguments:
-1. nblocks     (numeric, required) How many blocks are generated immediately.
+1. nblocks     (numeric, required) How many blocks are generated.
 2. address     (string, required) The address to send the newly generated bitcoin to.
 3. maxtries    (numeric, optional, default=1000000) How many iterations to try.
 
diff --git a/generatetodescriptor b/generatetodescriptor
index cfbcf86..9572971 100644
--- a/generatetodescriptor
+++ b/generatetodescriptor
@@ -1,9 +1,9 @@
 generatetodescriptor num_blocks "descriptor" ( maxtries )
 
-Mine blocks immediately to a specified descriptor (before the RPC call returns)
+Mine to a specified descriptor and return the block hashes.
 
 Arguments:
-1. num_blocks    (numeric, required) How many blocks are generated immediately.
+1. num_blocks    (numeric, required) How many blocks are generated.
 2. descriptor    (string, required) The descriptor to send the newly generated bitcoin to.
 3. maxtries      (numeric, optional, default=1000000) How many iterations to try.
 

@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24028 (doc: Explain in the generate* RPC docs that they are only for testing by sipa)
  • #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.

Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK, but I think it should keep the explicit mention of not returning until the blocks are mined.

@maflcko
Copy link
Member Author

maflcko commented Jan 30, 2022

It should be common sense that a hash of something can only be returned after it has been created (mined), so I am leaving this as-is for now

@maflcko
Copy link
Member Author

maflcko commented Feb 14, 2022

Also, the wording "immediately" isn't true. On testnets with difficulty higher than regtest, it may take a few iterations.

src/rpc/mining.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@0xB10C 0xB10C left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review ACK fa3bb58

Only reviewed changes to the RPC docs via #24137 (comment).

bitcoin-cli -help lists -generate and says that it mines the blocks immediately. Could be changed there too.

  -generate
       Generate blocks immediately, equivalent to RPC getnewaddress followed by
       RPC generatetoaddress. Optional positional integer 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

src/rpc/mining.cpp Show resolved Hide resolved
@maflcko
Copy link
Member Author

maflcko commented Feb 21, 2022

Thanks, rebased and fixed up bitcoin-cli.cpp. Should be trivial to re-review with git range-diff bitcoin-core/master fa3bb584dc fa30e62cc6.

Can be reviewed with --word-diff-regex=. --ignore-all-space
@0xB10C
Copy link
Contributor

0xB10C commented Feb 21, 2022

reACK fa30e62. changes since fa3bb58 are: dropping the immediately + formatting the touched line and a rebase

@fanquake fanquake merged commit 85ae549 into bitcoin:master Feb 21, 2022
@maflcko maflcko deleted the 2201-docBlock branch February 21, 2022 12:20
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 22, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Feb 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants