/bitcoin

nCheckDepth chain height fix #10775

Merged
merged 1 commit into from Jul 20, 2017
+1 −3

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
Validation
Projects
None yet
Milestone
No milestone
7 participants
@romanornr @promag @sipa @TheBlueMatt @btcdrak @fanquake @laanwj
@romanornr
Contributor

romanornr commented Jul 8, 2017 • edited 

if (nCheckDepth <= 0)
    nCheckDepth = 1000000000; // suffices until the year 19000
if (nCheckDepth > chainActive.Height())
    nCheckDepth = chainActive.Height();

These lines confuse me.
Correct me if I am wrong, but we can't check any more blocks than we have right?
If someone requests <= 0 it get set it into some huge number and then immediately limit it to the chain height in the following statement.

if (nCheckDepth > chainActive.Height())
    nCheckDepth = chainActive.Height();

when using --checkblocks=Z When Z is 0 or any other negative number, it will check all blocks.

I think it should be changed to this maybe.

if (nCheckDepth <= 0 || nCheckDepth > chainActive.Height())
    nCheckDepth = chainActive.Height();

Which gets rid of that huge number which is confusing for any other altcoins that have a different block time.
@romanornr romanornr nCheckDepth chain height fix
This commit was signed with a verified signature.
@romanornr romanornr Romano
GPG key ID: 3F92368F0D21A206 Learn about signing commits
d9d1bd3
@promag
Contributor

promag commented Jul 8, 2017

That code was added in #972 by @sipa.

utACK d9d1bd3.
@sipa
Owner

sipa commented Jul 8, 2017

utACK

fanquake added the Validation label Jul 8, 2017

@TheBlueMatt
Contributor

TheBlueMatt commented Jul 9, 2017

utACK
@btcdrak
Member

btcdrak commented Jul 9, 2017

utACK

@laanwj laanwj merged commit d9d1bd3 into bitcoin:master Jul 20, 2017
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@laanwj laanwj added a commit that referenced this pull request Jul 20, 2017

@laanwj laanwj Merge #10775: nCheckDepth chain height fix 
d9d1bd3 nCheckDepth chain height fix (romanornr)

Pull request description:

  ````
  if (nCheckDepth <= 0)
      nCheckDepth = 1000000000; // suffices until the year 19000
  if (nCheckDepth > chainActive.Height())
      nCheckDepth = chainActive.Height();
  ````

  These lines confuse me.
  Correct me if I am wrong, but we can't check any more blocks than we have right?
  If someone requests <= 0 it get set it into some huge number and then immediately limit it to the chain height in the following statement.
  ````
  if (nCheckDepth > chainActive.Height())
      nCheckDepth = chainActive.Height();
  ````
  when using ````--checkblocks=Z```` When Z is ````0```` or any other negative number, it will check all blocks.

  I think it should be changed to this maybe.
  ````
  if (nCheckDepth <= 0 || nCheckDepth > chainActive.Height())
      nCheckDepth = chainActive.Height();
  ````
  Which gets rid of that huge number which is confusing for any other altcoins that have a different block time.

Tree-SHA512: 8ee0ae5f33b399fa74dc16926709694ccfe1fc8a043cba2f5d00884220ac1b9b13f2df4588041f4133be634e5c7b14f4eebe24294028dafe91581a97dbe627f3
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 1E4AED62986CD25D Learn about signing commits
7c2400c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment