This moves the chain-related logic in CBlockLocator to CChain (CBlockLocator is a P2P data structure, its implementation shouldn’t depend on validation engine globals), and then moves CBlockLocator itself from main.h to core.h.
This means construction of CBlockLocator objects now happens in O(log nHeight) instead of O(nHeight), as it can use CChain’s height-based index.
Depends on #3077.
untested ACK.
ACK, presuming getblocks and getheaders style downloads from older clients has been tested.
This removes a few unused CBlockLocator methods, and moves the
construction and fork-finding logic to CChain (which can do these
more efficiently, as it has a height-indexable chain available).
It also makes CBlockLocator independent from the validation code.
As CBlockLocator is a P2P data structure, and independent from the
validation logic, it can be moved to core.
199- std::map<uint256, CBlockIndex*>::iterator mi = mapBlockIndex.find(hashBlock);
200- if (mi != mapBlockIndex.end())
201- Set((*mi).second);
202+CBlockIndex *CChain::SetTip(CBlockIndex *pindex) {
203+ if (pindex == NULL) {
204+ std::vector<CBlockIndex*>().swap(vChain);
660@@ -661,4 +661,38 @@ class CBlock : public CBlockHeader
661 void print() const;
662 };
663
664+
665+/** Describes a place in the block chain to another node such that if the
666+ * other node doesn't have the same branch, it can find a recent common trunk.
667+ * The further back it is, the further before the fork it may be.
668+ */
669+struct CBlockLocator
clang warns (repeatedly):
0./core.h:669:1: warning: 'CBlockLocator' defined as a struct here but previously declared as a class [-Wmismatched-tags]
… because db.h contains ‘class CBlockLocator;’
There’s two improvements here.
The first is just a cleanup. The CBlockLocator datastructure is part of the P2P protocol (so something very basic), but its methods directly refer to some of the validation engine’s globals. That’s really ugly, as a user of that class you won’t expect that using some of its methods imply you need to do locking on cs_main, or that it would depend on global data in the first place. In short, it doesn’t belong there - and now that we have a CChain data structure that explicitly encapsulates the block chain, it’s much more natural to ask the chain “give a locator that points to you”, rather than asking locator “make yourself into something that points to… well, you know”. The advantage is really just making the code easier to understand and reuse.
The second is an actual performance improvement. Currently, when constructing a CBlockLocator, we iterate the entire block chain index in memory (just benchmarked that, takes ~20ms), just to find the few ones we need. Since we have CChain that allows looking up entries by height, this becomes trivial (i expect a few microseconds). 20ms may not seem much, but we do this any time we’re asking for blocks from a peer, and it’s really just silly not to use the height-index now that it exists.
EDIT: It also reduces the number of lines of code!
ACK.
Code changes look good, and I tested following combinations against new code:
old code, partial chain, main network old code, partial chain, test network old code, fresh sync, test network