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
  1. HowHsu commented at 10:03 am on July 4, 2025: none
    The comment here overlooked a specific case where the block is the tip of m_chain, fix it.
  2. DrahtBot added the label UTXO Db and Indexes on Jul 4, 2025
  3. 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.

  4. 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 
    
  5. 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.

  6. HowHsu force-pushed on Jul 4, 2025
  7. 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?

  8. 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>
    cf2551883b
  9. HowHsu force-pushed on Jul 4, 2025
  10. 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    }
    

github-metadata-mirror

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

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me