Improve logic for advertising blocks #8958

Revisit #4846 (comments were to wait until headers first had been tested sufficiently first!)

OMG, do we really use the similar code somewhere else? Using ternary operator twice on one line and without parens is getting closer to the programmers hell!

@paveljanik It's similar to 2 * 3 + 1 which, as far as I know, most people don't use brackets for as it's understood what order the numbers are combined.

Rebased.

Personally, I'd highly appreciate a constant variable above for readability.

This won't compile, it's reintroducing a checkpoints dependency (nBlockEstimate) that was recently removed by 0278fb5 in PR #8865 (the whole block is not entered if we're in initial block download, and if we're not... then any block we'd consider would already be past the estimator that it gives us).

For future PR's it would be much more helpful to write what a change intends to accomplish (and, especially, WHY) in terms more specific than "improve". :) Hopefully every change is an improvement.

@gmaxwell Yes, I realise that #8865 broke it. Have now rebased correctly. Apologies for this.

I also agree that the word improve is entirely redundant if we assume that all PR are intended to improve :) (I tend to avoid assuming to the point of not even making common sense ones!)

@dcousens Have amended since your comment - it should be more readable now.

NACK. This is thoroughly busted -- first you need cs_main here to do what you're trying, second you can't skip announcing a block to a peer just because you think it's on a longer (not necessarily more work) chain than yours!

Needs rebase if still relevant.

@sdaftuar Not sure it's "thoroughly" busted but your comment has helped me to notice a mistake in the coding - it should be pindexLastCommonBlock not pindexBestKnownBlock that I am using. Making this change now.

@rebroad During IBD, nNewHeight is always larger than any pindexLastCommonBlock, so this removes the protection the original code was intended to give.