validation: check the specified number of blocks (off-by-one) #13428

pull kallewoof wants to merge 1 commits into bitcoin:master from kallewoof:validation-off-by-one changing 1 files +1 −1

kallewoof commented at 5:28 AM on June 11, 2018: member

echeveria | 2018-06-11 02:03:03.384975 Verifying last 3 blocks at level 3
echeveria | 2018-06-11 02:03:23.676793 No coin database inconsistencies in last 4 blocks (6564 transactions)
echeveria | off by one?
sipa      | echeveria: possibly!
kallewoof | Looks like it checks one more block than suggested. `if (pindex->nHeight < chainActive.Height()-nCheckDepth) break;` should probably be `<=`.
sipa      | kallewoof: agree

Post-commit:

2018-06-11T05:24:02Z Verifying last 6 blocks at level 3
2018-06-11T05:24:02Z [0%]...[16%]...[33%]...[50%]...[66%]...[83%]...[99%]...[DONE].
2018-06-11T05:25:07Z No coin database inconsistencies in last 6 blocks (7258 transactions)

Pre-commit:

2018-06-11T05:27:11Z Verifying last 6 blocks at level 3
2018-06-11T05:27:11Z [0%]...[16%]...[33%]...[50%]...[66%]...[83%]...[99%]...[DONE].
2018-06-11T05:27:12Z No coin database inconsistencies in last 7 blocks (9832 transactions)

validation: check the specified number of blocks (off-by-one) f6f8026e40
fanquake added the label Validation on Jun 11, 2018
Empact commented at 5:50 AM on June 11, 2018: member

utACK f6f8026 - at this point pindexState == pindex, if e.g. chainActive.Height() == 10 and nCheckDepth == 1, pindex should break at height 9 (<=) rather than 8 (<).

One thing throwing me off about this method: looks like reporting doesn't work if nCheckLevel < 3, because pindexState is always chainActive.Tip(). Is that right?
kallewoof commented at 6:05 AM on June 11, 2018: member

Yeah, that clearly looks wrong. The report at the end will say 'last 0 blocks'... hm.

Edit: see #13431 for fix.
jonasschnelli approved
jonasschnelli commented at 9:06 AM on June 12, 2018: contributor

utACK f6f8026e40326e74293dc8ecc270a7e3b4850727
MarcoFalke commented at 1:21 PM on June 12, 2018: member

utACK f6f8026e40326e74293dc8ecc270a7e3b4850727
laanwj commented at 1:42 PM on June 13, 2018: member

utACK f6f8026e40326e74293dc8ecc270a7e3b4850727
laanwj merged this on Jun 13, 2018
laanwj closed this on Jun 13, 2018
laanwj referenced this in commit caabdea627 on Jun 13, 2018
kallewoof deleted the branch on Jun 15, 2018
MarcoFalke referenced this in commit 954f4a9c7c on Jul 1, 2018
jasonbcox referenced this in commit b925970984 on Sep 27, 2019
jonspock referenced this in commit c77e011d9c on Dec 24, 2019
jonspock referenced this in commit acd6561a03 on Dec 24, 2019
jonspock referenced this in commit 207d21d5da on Dec 24, 2019
jonspock referenced this in commit 266d6d9816 on Dec 24, 2019
jonspock referenced this in commit a78d9dcdcb on Dec 24, 2019
jonspock referenced this in commit 3f4b54ae76 on Dec 26, 2019
PastaPastaPasta referenced this in commit eea8da81c5 on Jul 7, 2020
PastaPastaPasta referenced this in commit ae293d581b on Jul 7, 2020
PastaPastaPasta referenced this in commit c32d781d75 on Jul 7, 2020
PastaPastaPasta referenced this in commit 55d74a6de2 on Jul 8, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#13431 validation: count blocks correctly for check level < 3