Skip to content

Conversation

ChainQuery
Copy link

The generate RPC has no default numblocks and a numeric value is required.

The `generate` RPC has no default `numblocks` and a numeric value is required.
@ChainQuery
Copy link
Author

Re-submission of #6609 - sorry for the double request.

@paveljanik
Copy link
Contributor

@laanwj Continuing from #6609. Being explicit is nice goal. But in this case, setting default to 1 is much better because regtest/testnet is being used by people who should know what they are doing.

When calling generate, they expect to generate some blocks. In almost all cases (at least this is my usage), one block. So after calling generate without arguments, they should reasonably expect at least one block generated (and they see its hash in the returned array).

Setting the default to one can speed their work (or at least mine), as usually one block is enough.

But if you really NACK setting the default to 1, my ACK here, of course. No problem with such decision.

@ChainQuery
Copy link
Author

@paveljanik I agree that when calling generate the expectation would be blocks generated, however there are varying expected default values: some folks may expect 1 block , a wallet developer might be expecting 6 blocks, and a miner might be expecting 100.

I like the idea of being explicit in this case, just my 0.02 bits...

@paveljanik
Copy link
Contributor

@ChainQuery after forgetting that miner won't probably mine his blocks using generate on regtest but on testnet, even miner has to test other counts of blocks being generated. But the common case in every case you mentioned is one. Generating one block is a typical use-case for every type of user you mentioned.
A miner (see above...) will test 99 blocks and then one to see a difference.
Wallet developer will test 5 blocks and then one (if he is testing displaying the number of confirmations, he will even generate one by one...).

But as I said above, I can live with being explicit. But the UX lecture told me to do something when you can expect something to happen and you can't broke anything with it.

@laanwj
Copy link
Member

laanwj commented Sep 4, 2015

@paveljanik For a user-facing UI I'd probably have agreed. But this is an API. Let's not have a bikeshedding discussion about a call that's only used for testing. It's not like adding '1' in a test script is a big drain on anyone's time.

@laanwj laanwj merged commit e83df07 into bitcoin:master Sep 4, 2015
laanwj added a commit that referenced this pull request Sep 4, 2015
e83df07 Update RPC generate help for numblocks to include required (Ian T)
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Jan 9, 2016
The `generate` RPC has no default `numblocks` and a numeric value is required.
Github-Pull: bitcoin#6631
Rebased-From: e83df07
zkbot added a commit to zcash/zcash that referenced this pull request Dec 18, 2019
Bitcoin 0.12 cleanup PRs 2

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6631
- bitcoin/bitcoin#6664
  - Only the first commit (we already had the second through bitcoin/bitcoin#6825).
- bitcoin/bitcoin#6669
- bitcoin/bitcoin#6887
  - Only the non-QT parts.
- bitcoin/bitcoin#6962
- bitcoin/bitcoin#6822
  - Only first and third commits (we already had the second through an earlier PR).
- bitcoin/bitcoin#7136
  - Excludes Travis CI changes, and fixes to documents we don't have anymore.
- bitcoin/bitcoin#7084
- bitcoin/bitcoin#7509
- bitcoin/bitcoin#7617
- bitcoin/bitcoin#7726

Part of #2074.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants