Reset setBlockIndexCandidates once block index db loaded #5194

ghost commented at 9:53 PM on November 1, 2014: none

Delete all entries in setBlockIndexCandidates that are worse than our new current block upon completion of the initial LoadBlockIndexDB.

sipa commented at 7:20 AM on November 2, 2014: member

Interesting; we should definitely do that. Can you move the logic for pruning setBlockIndexCandidates to a separate function (as the same code appears in ActivateBestChainStep)>

ghost commented at 2:48 PM on November 2, 2014: none

Updated as requested.

sipa commented at 2:53 PM on November 2, 2014: member

Untested ACK

TheBlueMatt commented at 7:29 PM on November 8, 2014: member

Aside from whitespace bugs, tested ACK only commithash ba5bf8f6ae344bf2e69ff551490b4262e3e9d129: http://bitcoin.ninja/TheBlueMatt-5194.txt

Reset setBlockIndexCandidates once block index db loaded cca48f69b0

in src/main.cpp:None in ba5bf8f6ae outdated

2863 | @@ -2860,6 +2864,9 @@ bool static LoadBlockIndexDB()
2864 |      if (it == mapBlockIndex.end())
2865 |          return true;
2866 |      chainActive.SetTip(it->second);
2867 | +

TheBlueMatt commented at 7:29 PM on November 8, 2014:

Nit: Please remove the trailing whitespace here and on 2869

ghost commented at 6:13 AM on November 12, 2014: none

Removed the trailing whitespace on 2867 and 2869, as suggested.

sipa commented at 2:02 PM on November 18, 2014: member

untested ACK still

sipa added the label Bug on Nov 18, 2014

in src/main.cpp:None in cca48f69b0

1948 | @@ -1949,6 +1949,16 @@ static CBlockIndex* FindMostWorkChain() {
1949 |      } while(true);
1950 |  }
1951 |  
1952 | +// Delete all entries in setBlockIndexCandidates that are worse than the current tip.
1953 | +static void PruneBlockIndexCandidates() {
1954 | +    // Note that we can't delete the current block itself, as we may need to return to it later in case a
1955 | +    // reorganization to a better block fails.
1956 | +    std::set<CBlockIndex*, CBlockIndexWorkComparator>::iterator it = setBlockIndexCandidates.begin();
1957 | +    while (setBlockIndexCandidates.value_comp()(*it, chainActive.Tip())) {

laanwj commented at 10:55 AM on November 20, 2014:

Please check for end before dereferencing *it:

    while (it != setBlockIndexCandidates.end() && setBlockIndexCandidates.value_comp()(*it, chainActive.Tip())) {

sipa commented at 10:59 AM on November 20, 2014:

It's just moved code, and shouldn't be necessary (see the assert that used to follow...). But this shouldn't hurt.

laanwj commented at 11:01 AM on November 20, 2014:

Well I prefer to be of the safe side. Wouldn't want to walk out of the set in case of a bug. But an assertion is fine with me too.

sipa commented at 11:03 AM on November 20, 2014:

No, I agree it feels safer to have the condition. Without it, we wouldn't even reach the assertion before executing code with undefined behaviour (assuming the assertion wouldn't hold).

laanwj commented at 12:23 PM on November 26, 2014: member

Continued in #5321

laanwj closed this on Nov 26, 2014

MarcoFalke locked this on Sep 8, 2021