refactor: Move GuessVerificationProgress into ChainstateManager #31393

1. 
   maflcko commented at 9:40 AM on November 29, 2024: member

   Currently the function is standalone, which means any passed-in data like TxData or the block pointer needs to be taken from the ChainstateManager and passed in. This is currently verbose and may become even more verbose if the function is reworked in the future. As the function can not be called without a ChainstateManager in production code anyway, make it a member function on the class.

2. 
   DrahtBot commented at 9:40 AM on November 29, 2024: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31393.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	ryanofsky, TheCharlatan, danielabrozzoni
   Concept ACK	mzumsande

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
   • #29039 (versionbits refactoring by ajtowns)

   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.

3. DrahtBot added the label Refactoring on Nov 29, 2024
4. danielabrozzoni approved
5. 
   danielabrozzoni commented at 7:07 PM on December 2, 2024: member

   light tACK fa5ad4ed2b36fa483385670a210eb8564bc53c9b - i'm not too familiar with the code, but the change is reasonable. Tests are passing locally.

6. in src/validation.cpp:2969 in fa5ad4ed2b outdated

   2962 | @@ -2963,6 +2963,7 @@ void Chainstate::PruneAndFlush()
   2963 |  }
   2964 |  
   2965 |  static void UpdateTipLog(
   2966 | +    const ChainstateManager& chainman,
   2967 |      const CCoinsViewCache& coins_tip,
   2968 |      const CBlockIndex* tip,
   2969 |      const CChainParams& params,
   

   ---

   ---

   

   mzumsande commented at 8:52 PM on December 2, 2024:

   can remove params, it's no longer used

   ---

   

   maflcko commented at 9:37 AM on December 3, 2024:

   Thanks, done

7. 
   mzumsande commented at 11:10 PM on December 2, 2024: contributor

   Concept ACK

8. maflcko force-pushed on Dec 3, 2024
9. in src/node/interfaces.cpp:411 in 5555ed0c3b outdated

    405 | @@ -406,9 +406,9 @@ class NodeImpl : public Node
    406 |      }
    407 |      std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) override
    408 |      {
    409 | -        return MakeSignalHandler(::uiInterface.NotifyBlockTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) {
    410 | +        return MakeSignalHandler(::uiInterface.NotifyBlockTip_connect([fn, this](SynchronizationState sync_state, const CBlockIndex* block) {
    411 |              fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()},
    412 | -                GuessVerificationProgress(Params().TxData(), block));
    413 | +               chainman().GuessVerificationProgress(block));
   

   ---

   ---

   

   danielabrozzoni commented at 10:24 AM on December 4, 2024:

   nit: you also deleted a whitespace here, and now the line is indented with only 3 whitespaces, is this on purpose?

   ---

   

   maflcko commented at 11:00 AM on December 4, 2024:

   It is done by clang-format

10. danielabrozzoni approved
11. 
    danielabrozzoni commented at 10:57 AM on December 4, 2024: member

    re-ACK 5555ed0c3b8d1beecb2785dc8cbe10c1693ca290

12. DrahtBot requested review from mzumsande on Dec 4, 2024
13. DrahtBot added the label Needs rebase on Dec 13, 2024
14. refactor: Move GuessVerificationProgress into ChainstateManager facb4d010c
15. maflcko force-pushed on Dec 13, 2024
16. DrahtBot removed the label Needs rebase on Dec 13, 2024
17. ryanofsky approved
18. 
    ryanofsky commented at 4:28 PM on December 17, 2024: contributor

    Code review ACK facb4d010ca5c898756bedee46069509900ebea0. Nice cleanup, that should make this code less awkward to work with

19. DrahtBot requested review from danielabrozzoni on Dec 17, 2024
20. sedited approved
21. 
    sedited commented at 8:22 PM on December 17, 2024: contributor

    ACK facb4d010ca5c898756bedee46069509900ebea0

22. danielabrozzoni approved
23. 
    danielabrozzoni commented at 2:34 PM on December 18, 2024: member

    reACK facb4d010ca5c898756bedee46069509900ebea0

24. ryanofsky merged this on Dec 18, 2024
25. ryanofsky closed this on Dec 18, 2024

    ---

26. maflcko deleted the branch on Dec 19, 2024
27. sedited referenced this in commit 230a439a4a on Jan 17, 2025
28. stickies-v referenced this in commit d760fd3dda on Mar 17, 2025
29. stickies-v referenced this in commit cc83553352 on Mar 17, 2025
30. stickies-v referenced this in commit 2614933f06 on Mar 17, 2025
31. stickies-v referenced this in commit b70418c5fc on Mar 17, 2025
32. stickies-v referenced this in commit 69f8a1fe50 on Mar 17, 2025
33. bug-castercv502 referenced this in commit 44b075fe71 on Sep 28, 2025
34. bitcoin locked this on Dec 19, 2025

Contributors

maflckoDrahtBotdanielabrozzonimzumsanderyanofskysedited

Review Requested
mzumsande