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

rpc: Clarify that block count means height excl genesis #16325

Merged
merged 1 commit into from
Jul 3, 2019

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jul 2, 2019

There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in #16292 (comment).

However, it really returns the height, which is 0 for the genesis block.

So clarify that and also remove the misleading "longest blockchain" comment.

Finally, fix the wallet test that incorrectly used this rpc.

@maflcko maflcko force-pushed the 1907-rpcBlockCount branch from fa09371 to fa609ed Compare July 2, 2019 16:26
@maflcko maflcko force-pushed the 1907-rpcBlockCount branch from fa609ed to fab0c82 Compare July 2, 2019 16:28
@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 2, 2019

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

Conflicts

No conflicts as of last run.

@instagibbs
Copy link
Member

utACK fab0c82

@laanwj
Copy link
Member

laanwj commented Jul 2, 2019

However, it really returns the height, which is 0 for the genesis block.

This has always been the case, right? I know of no other block height definition than this.

@maflcko
Copy link
Member Author

maflcko commented Jul 2, 2019

Yes, this has always been the case afaik. The documentation I am changing here was added in 22f721d (2010)

@promag
Copy link
Contributor

promag commented Jul 2, 2019

ACK fab0c82, sorry for the misconception.

@maflcko
Copy link
Member Author

maflcko commented Jul 2, 2019

Eh, no worries. I look it up every time to be sure as well.

@laanwj laanwj merged commit fab0c82 into bitcoin:master Jul 3, 2019
laanwj added a commit that referenced this pull request Jul 3, 2019
fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in #16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
@laanwj
Copy link
Member

laanwj commented Jul 3, 2019

sorry for the misconception.

Yes, no worries, I kind of missed that the RPC call is called getblockcount and not getblockheight. Block count is definitely ambigious.

@maflcko maflcko deleted the 1907-rpcBlockCount branch July 3, 2019 15:58
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 8, 2020
Summary:
Description of PR:
> There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block.
> However, it really returns the height, which is 0 for the genesis block.
> So clarify that and also remove the misleading "longest blockchain" comment.
> Finally, fix the wallet test that incorrectly used this rpc.

Backport of Core [[bitcoin/bitcoin#16325 | PR16325]]

Test Plan:
  ninja && ninja check-all
  src/bitcoin-cli help getbestblockhash
  src/bitcoin-cli help getbockcount

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7822
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Nov 4, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Nov 4, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Dec 7, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Dec 11, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Dec 11, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Dec 13, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Dec 14, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Dec 15, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Dec 15, 2021
…genesis

fab0c82 rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in bitcoin#16292 (comment).

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK bitcoin@fab0c82
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants