validation: default initialize and guard chainman members #24857

1. 
   ajtowns commented at 10:12 pm on April 14, 2022: member

   #23785 moved pindexBestInvalid from a global to become ChainstateManager.m_best_invalid but dropped the explcit initalization to nullptr. Replace that, and annotate it as guarded by cs_main.
2. validation: default initialize and guard chainman members fdd0ca541a
3. ajtowns added the label Needs backport (23.x) on Apr 14, 2022
4. 
   DrahtBot commented at 9:12 am on April 15, 2022: 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:

   • #24232 (assumeutxo: add init and completion logic by jamesob)

   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.

5. MarcoFalke added this to the milestone 23.1 on Apr 15, 2022
6. MarcoFalke added the label Refactoring on Apr 15, 2022
7. 
   MarcoFalke commented at 9:38 am on April 15, 2022: member

   Initialization is done by UnloadBlockIndex for all code paths at least as of current master. Not calling UnloadBlockIndex would be a bug, as it leaves some globals untouched, letting them point to potentially deleted memory.

   So it looks like this is refactoring and not a bug fix or something that can be exploited?

   ACK on the changes either way, I am just trying to figure out if this is a blocker for 23.0, as you tagged this for backport.

8. ajtowns removed the label Needs backport (23.x) on Apr 15, 2022
9. 
   ajtowns commented at 9:55 pm on April 15, 2022: member

   @MarcoFalke No, I think you’re right and this only becomes a bug when the call to UnloadBlockIndex gets removed in #22564. cc @dongcarl – maybe you want to just take ths into your PR
10. MarcoFalke removed this from the milestone 23.1 on Apr 16, 2022
11. 
    dongcarl commented at 8:22 pm on April 16, 2022: member

    @ajtowns Good point! Will do. I will also check in my PR that everything that’s reset/initialized by the removed UnloadBlockIndex are explicitly initialized.
12. 
    dongcarl commented at 5:25 pm on April 18, 2022: member

    cherry-picked here! https://github.com/bitcoin/bitcoin/pull/22564/commits/f56f4fe7dbd705d61909c51b9ecde617a6f45dcb
13. 
    ajtowns commented at 11:21 pm on April 18, 2022: member

    See #22564
14. ajtowns closed this on Apr 18, 2022

    ---

15. DrahtBot locked this on Apr 18, 2023

Contributors
ajtowns DrahtBot MarcoFalke dongcarl

Labels
Refactoring