init: Remove dead code in LoadChainTip #15999

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:1905-initNoDead changing 1 files +1 −11

MarcoFalke commented at 9:55 PM on May 9, 2019: member

LoadChainTip sets ::ChainActive() based on pcoinsTip's best block. LoadChainTip is never called when that block is null, so we can remove all code from within that method that is only executed when that block is null.

Fixes #15967 Inconsistent locking behavior in LoadChainTip
MarcoFalke added the label Refactoring on May 9, 2019
MarcoFalke added the label Validation on May 9, 2019
MarcoFalke force-pushed on May 9, 2019
DrahtBot commented at 12:16 AM on May 10, 2019: member


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #15921 (Tidy up ValidationState interface by jnewbery)
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.
gmaxwell commented at 2:51 AM on May 10, 2019: contributor

This is essentially just reverting b0f32497b873cd1eaf0be86f8e265355aa86174f PR doesn't seem to address why this prior change was inapplicable.
MarcoFalke commented at 11:48 AM on May 10, 2019: member

See the immediate follow-up commit 13ab353829bfcf928a0dd88cc9841e6515eea111 that disabled the code. So the code was never executed and tested as part of a release. (And if it ever were executed it would immediately crash with an assert failure anyway.)

If you want to enable that code, it needs to be properly reviewed and tested.
practicalswift commented at 7:51 PM on May 12, 2019: contributor

Concept ACK
in src/validation.cpp:3957 in fa32fd052b outdated
```
3963 | -            LogPrintf("%s: failed to activate chain (%s)\n", __func__, FormatStateMessage(state));
3964 | -            return false;
3965 | -        }
3966 | -    }
3967 | -
3968 | +    assert(!pcoinsTip->GetBestBlock().IsNull());
```
promag commented at 3:34 PM on May 13, 2019:

Could move this up, before above condition?

MarcoFalke commented at 3:54 PM on May 13, 2019:

Done and added comment

promag commented at 3:59 PM on May 13, 2019:

Cool.
init: Remove dead code in LoadChainTip fa86c8aec6
MarcoFalke force-pushed on May 13, 2019
promag commented at 3:59 PM on May 13, 2019: member

utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d.
practicalswift commented at 4:33 PM on May 13, 2019: contributor

utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d
ryanofsky approved
ryanofsky commented at 7:17 PM on May 15, 2019: member

utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d. LoadChainTip isn't called currently when pcoinsTip best block is null due to this line: https://github.com/bitcoin/bitcoin/blob/2d16fb7a2b6a9e5a2535295d2de03e27c2438d1f/src/init.cpp#L1573, and the assert should prevent it from being erroneously called in the future.
jamesob approved
jamesob commented at 7:31 PM on May 15, 2019: member

utACK https://github.com/bitcoin/bitcoin/pull/15999/commits/fa86c8aec611a9b9d2f53960c92419cf2a8bb92d

Single use of LoadChainTip() is here, and is only called when pcoinsTip has contents; verify with git grep LoadChainTip\(.
Empact commented at 3:15 AM on May 17, 2019: member

utACK https://github.com/bitcoin/bitcoin/pull/15999/commits/fa86c8aec611a9b9d2f53960c92419cf2a8bb92d
laanwj commented at 10:53 AM on May 17, 2019: member

utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d It's only confusing to have this if it isn't ever reached—the code can always be re-introduced later when necessary.
MarcoFalke merged this on May 17, 2019
MarcoFalke closed this on May 17, 2019
MarcoFalke referenced this in commit a822a0e4f6 on May 17, 2019
MarcoFalke deleted the branch on May 17, 2019
deadalnix referenced this in commit 094184ba43 on Jun 19, 2020
ftrader referenced this in commit d225244b90 on Aug 17, 2020
kittywhiskers referenced this in commit 73ccadda7a on Oct 10, 2021
kittywhiskers referenced this in commit d82c1b700e on Oct 10, 2021
kittywhiskers referenced this in commit 07110eeaeb on Oct 10, 2021
kittywhiskers referenced this in commit b3bc1e04db on Oct 16, 2021
kittywhiskers referenced this in commit 993a65ef0d on Oct 16, 2021
kittywhiskers referenced this in commit 31b2b53b9b on Oct 16, 2021
Munkybooty referenced this in commit 9a7b4cab65 on Oct 17, 2021
kittywhiskers referenced this in commit 52390ef58e on Oct 21, 2021
kittywhiskers referenced this in commit 51f73b558f on Oct 22, 2021
Munkybooty referenced this in commit 08710599bf on Oct 22, 2021
Munkybooty referenced this in commit 083b301262 on Oct 22, 2021
Munkybooty referenced this in commit a8807d2dce on Oct 23, 2021
Munkybooty referenced this in commit 5a319a15ce on Oct 26, 2021
Munkybooty referenced this in commit 69c9144d01 on Oct 28, 2021
pravblockc referenced this in commit ec20de16ab on Nov 18, 2021
DrahtBot locked this on Dec 16, 2021