pindexBestHeader, fHavePruned
#24909
Split off from #22564 per Marco’s suggestion: #22564 (comment)
This is basically the move-mostly parts of #22564. The overall intent is to move mutable globals manually reset by ::UnloadBlockIndex into appropriate structs such that they are cleared at the appropriate times. Please read #22564’s description for more rationale.
In summary , this PR moves:
pindexBestHeader -> ChainstateManager::m_best_headerfHavePruned -> BlockManager::m_have_pruned
Now BlockManager::LoadBlockIndex() will ACTUALLY only load BlockMan
members.
[META] In a later commit, pindexBestHeader will be moved to ChainMan as
a member
-----
Code Reviewer Notes
Call graph of relevant functions:
ChainstateManager::LoadBlockIndex() <-- Moved to
calls BlockManager::LoadBlockIndexDB()
which calls BlockManager::LoadBlockIndex() <-- Moved from
There is only one call to each of inner functions, meaning that no
behavior is changing.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
105@@ -106,8 +106,8 @@ CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block)
106 pindexNew->nTimeMax = (pindexNew->pprev ? std::max(pindexNew->pprev->nTimeMax, pindexNew->nTime) : pindexNew->nTime);
107 pindexNew->nChainWork = (pindexNew->pprev ? pindexNew->pprev->nChainWork : 0) + GetBlockProof(*pindexNew);
108 pindexNew->RaiseValidity(BLOCK_VALID_TREE);
109- if (pindexBestHeader == nullptr || pindexBestHeader->nChainWork < pindexNew->nChainWork)
110- pindexBestHeader = pindexNew;
111+ if (best_header == nullptr || best_header->nChainWork < pindexNew->nChainWork)
112+ best_header = pindexNew;
{ }
ACK 7b434e7d6bc5ea92b14694375ac4d429e644e61d ; code review only
BlockManager::LoadBlockIndex privateLoadBlockIndex does not need to be a friend of ChainstateManager ?fHavePruned should someday be private to blockman and the pruning logic should move from CChainState::FlushStateToDisk to blockman4669@@ -4670,28 +4670,28 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
4670 CNodeState &state = *State(pto->GetId());
4671
4672 // Start block sync
4673- if (pindexBestHeader == nullptr)
4674- pindexBestHeader = m_chainman.ActiveChain().Tip();
4675+ if (m_chainman.pindexBestHeader == nullptr)
nit in c51e6112624e3f4c6282be5f4c78ee3e95aef797:
add {}
278@@ -280,7 +279,7 @@ static bool IsCurrentForFeeEstimation(CChainState& active_chainstate) EXCLUSIVE_
279 return false;
280 if (active_chainstate.m_chain.Tip()->GetBlockTime() < count_seconds(GetTime<std::chrono::seconds>() - MAX_FEE_ESTIMATION_TIP_AGE))
281 return false;
282- if (active_chainstate.m_chain.Height() < pindexBestHeader->nHeight - 1)
283+ if (active_chainstate.m_chain.Height() < active_chainstate.m_chainman.pindexBestHeader->nHeight - 1)
review ACK 7b434e7d6bc5ea92b14694375ac4d429e644e61d 🌜
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3review ACK 7b434e7d6bc5ea92b14694375ac4d429e644e61d 🌜
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUhK5AwAnnxOI4tcYCP4IleWGvPN536u+3aq40UauEOEzMSOcfqOzRjfZqpBAqwC
8GBvAzOjwaTAkG6S6H77mLAPxDGZZB2GdUWFT1RqMd1GRJmhyeer9wtqQ/NATvETg
9qKUoJiCn5o1xZEOkTMgkAJyUJ8vkVF2UxjL/AbeAu3l79/Qzh527vUQ+s7xsR0Pe
10fSfJQlrxqaB1p3yrwdsZwEYN2g+nzDIz3xEKG7P2sGoY9Vboh8FqggJW1+LOJfYE
11/STQmMjcWGLFzQzUa5lUD64WT5dHruXrLgoJQ+jlOFfNd89he07ybmN8mP91Iu2I
12ZNucJcKDoDQVJHIz/vSNWltIeTNfc/Xxj54gCW6N6IUSyRr4uzs9aVkToacWO5kP
13e+1hT8EK5gn0an0NWxQmS2PBmsgrAZ/AeenrDuaMkYl5DjBe70v7rX6IG7/kbHu3
14d3/pqVWzugeA+LH6XUyt1a0TdH1JEX6IgQo6N06a1i2WzVOpG/Ol4TY4jonziIHy
155/TITCQv
16=Uk9X
17-----END PGP SIGNATURE-----
In reply to #24909#pullrequestreview-945162779 private and friend:
Seems unrelated to the commits here, see https://github.com/bitcoin/bitcoin/pull/24917
[META] In the next commit, we move the clearing of pindexBestHeader to
ChainstateManager::Unload()
...of touched lines and surrounding
-----
Code Reviewer Notes
Call graph of relevant functions:
UnloadBlockIndex() <-- Moved from
calls ChainstateManager::Unload() <-- Moved to
Safe because ChainstateManager::Unload() is called only by
UnloadBlockIndex() and no other callers.
[META] In the next commit, we move the clearing of fHavePruned to
BlockManager::Unload()
-----
Code Reviewer Notes
Call graph of relevant functions:
UnloadBlockIndex() <-- Moved from
calls ChainstateManager::Unload()
which calls BlockManager::Unload() <-- Moved to
So calling UnloadBlockIndex() would still run this moved code. The code
will also now run when ~BlockManager gets called, which makes sense.
...to m_best_header and m_have_pruned
-BEGIN VERIFY SCRIPT-
find_regex="\bpindexBestHeader\b" \
&& git grep -l -E "$find_regex" -- src \
| xargs sed -i -E "s@$find_regex@m_best_header@g"
find_regex="\bfHavePruned\b" \
&& git grep -l -E "$find_regex" -- src \
| xargs sed -i -E "s@$find_regex@m_have_pruned@g"
-END VERIFY SCRIPT-Pushed 7b434e7d6bc5ea92b14694375ac4d429e644e61d -> f0a2fb3c5dbf3c4bec7faf934baff3e723734b3f
if {} comments
kirby ACK f0a2fb3c5dbf3c4bec7faf934baff3e723734b3f 😋
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3kirby ACK f0a2fb3c5dbf3c4bec7faf934baff3e723734b3f 😋
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUjhaAwAmXnXA0Who3Rfgotbw/O9rKr6Yf2XxjGSxmW8JYWxbC0HsT72v/PjMOnQ
8ofXjbI3dkZjTL4fE4jsyenMpSLTMbghvirhnSgVYpTY/pUKHeF4s/5ZXp7rpItaf
93ZcMmluRI7e2xzcuSIRRrGkvcpNQEjfoqfPrIyTnpABYbOyeV7pzWq7+QbM5OBgg
10xyiBXPsDf+T1jHjRAsbAbH5/eQdA1B+VdqCjcEn/A27kfmHlSJKx7CNzP4vwTODK
11YYi2hu3cGLdjLGZlfYu2dX4YQbpXxImGJ94+tQhZIZZN9f+8nMFd+L0Qeeka5voQ
12ZImElHgcs38dVQ+pJSgjZtN3it2KIl1OA/kU1xMxyHQWG+qhzKvawmrd8QCRxGYj
133a2GQAOK+2MJa3q6Jq31+Dd46zXm23tgvQdzY2zeySPJz+4IS+Yibu8yWWJjSo/Z
14RB66PlDNW8Exyx57mPsVEbO2gJvZi7kI52lgncDnNi0aQ+s/Q3MIUar9OCTcNmv6
15LdqFVdTh
16=MEJR
17-----END PGP SIGNATURE-----
Labels
Refactoring