[docs] fix findFork comment

[r8921039]

The return value of findFork is an ancestor of the specified block only when specified block is not on the active chain. When it is on the active chain, the return value is the specified block itself, not an ancestor of it.

[gmaxwell]

Return height of the highest block on the chain that is or has an ancestor

"is or has an ancestor" doesn't make much sense to me. The function is returning the highest common point, if the block is on the chain, that's obviously itself. The comment in chain.h is correct and I think not confusing: Find the last common block between this chain and a block index entry.

[r8921039]

Similarly, the following comment is true only if the locator is not created by a block on the active chain.

    //! Return height of the latest block common to locator and chain, which
    //! is guaranteed to be an ancestor of the block used to create the
    //! locator.
    virtual Optional<int> findLocatorFork(const CBlockLocator& locator) = 0;

[ryanofsky]

@r8921039, I wrote these comments and I can see how they are confusing. I wrote "ancestor" here when I really meant "ancestor-or-self" because it seemed less awkward (similar to how people write ⊂ instead of ⊆ in math for convenience, even though it can be less clear if you don't know the context).

I think "highest block on the chain that is or has an ancestor of the specified block" is hard to parse, though. Maybe it would be better to change these comments to:

• findFork: Return height of the specified block if it is on the chain, otherwise return the height of the highest block on chain that's an ancestor of the specified block, or nullopt if there is no common ancestor.
• findLocatorFork: Return height of the highest block on chain in common with the locator (which will either be the original block used to create the locator, or one of its ancestors.)

[r8921039]

@ryanofsky Thank you very much for your response and suggestions. The PR is updated accordingly.

[ryanofsky]

utACK b837d66

[maflcko]

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

[r8921039]

@MarcoFalke Yes sir, done as instructed.

[promag]

utACK c968780, however comment could be shorter.

[ryanofsky]

utACK c968780. Only change since last review is squash

[r8921039]

Would someone be willing to give a bit more love to this PR... Thanks.

[ryanofsky]

This could just be merged I think. Documentation update with no change in meaning.

[r8921039]

@MarcoFalke Do we need more utACK to proceed? If you think so, I will try asking on IRC. Thanks.

[r8921039]

Thanks a lot guys! :)