index: Fix missing case in the comment in NextSyncBlock() #32875
pull HowHsu wants to merge 1 commits into bitcoin:master from HowHsu:fix-comment changing 1 files +1 −1-
HowHsu commented at 10:03 am on July 4, 2025: noneThe comment here overlooked a specific case where the block is the tip of m_chain, fix it.
-
DrahtBot added the label UTXO Db and Indexes on Jul 4, 2025
-
DrahtBot commented at 10:03 am on July 4, 2025: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32875.
Reviews
See the guideline for information on the review process. A summary of reviews will appear here.
-
stickies-v commented at 2:41 pm on July 4, 2025: contributor
You’re correct that the current docstring is incorrect. I personally don’t find your suggestion very clear either, as the two cases talk about different parts of the return statement.
An alternative would be to keep the docstring as-is and just make the chaintip case more explicit in code:
0 const CBlockIndex* pindex = chain.Next(pindex_prev); 1 if (pindex || pindex_prev == chain.Tip()) { 2 return pindex; 3 }
0diff --git a/src/index/base.cpp b/src/index/base.cpp 1index 8c958ac5f6..506ee73d2e 100644 2--- a/src/index/base.cpp 3+++ b/src/index/base.cpp 4@@ -140,15 +140,12 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& 5 } 6 7 const CBlockIndex* pindex = chain.Next(pindex_prev); 8- if (pindex) { 9+ if (pindex || pindex_prev == chain.Tip()) { 10 return pindex; 11 } 12 13- // Two cases goes here: 14- // 1. pindex_prev is the tip: chain.FindFork(pindex_pre) returns pindex_prev itself 15- // 2. pindex_prev is not in m_chain: since block is not in the chain, return the next 16- // block in the chain AFTER the last common ancestor. Caller will be responsible for 17- // rewinding back to the common ancestor. 18+ // If block is not in the chain, return the next block in the chain AFTER the last common ancestor. 19+ // Caller will be responsible for rewinding back to the common ancestor. 20 return chain.Next(chain.FindFork(pindex_prev)); 21 } 22
-
HowHsu commented at 3:15 pm on July 4, 2025: none
You’re correct that the current docstring is incorrect. I personally don’t find your suggestion very clear either, as the two cases talk about different parts of the return statement.
An alternative would be to keep the docstring as-is and just make the chaintip case more explicit in code:
0 const CBlockIndex* pindex = chain.Next(pindex_prev); 1 if (pindex || pindex_prev == chain.Tip()) { 2 return pindex; 3 }
git diff on d6bf3b1
True, I’ll update it.
-
HowHsu force-pushed on Jul 4, 2025
-
HowHsu commented at 3:41 pm on July 4, 2025: none
You’re correct that the current docstring is incorrect. I personally don’t find your suggestion very clear either, as the two cases talk about different parts of the return statement.
An alternative would be to keep the docstring as-is and just make the chaintip case more explicit in code:
0 const CBlockIndex* pindex = chain.Next(pindex_prev); 1 if (pindex || pindex_prev == chain.Tip()) { 2 return pindex; 3 }
git diff on d6bf3b1
Hey stickies-v, do you mind me add Suggested-by tag of you?
-
index: handle case where pindex_prev equals chain tip in NextSyncBlock()
When pindex_prev is the chain tip, return early rather than mixing it with the reorg case. Suggested-by: stickies-v Signed-off-by: Hao Xu <hao.xu@linux.dev>
-
HowHsu force-pushed on Jul 4, 2025
-
luke-jr commented at 8:57 pm on July 7, 2025: member
0 CBlockIndex* NextInclRewind(const CBlockIndex* pindex) const 1 { 2 if (!Contains(Assert(pindex))) { 3 pindex = Assert(FindFork(pindex)); 4 } 5 return (*this)[pindex->nHeight + 1]; 6 }
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-07 21:13 UTC
More mirrored repositories can be found on mirror.b10c.me