Unlikely to have happened but it is possible a fork with lower difficulty reaches a height comparable to our ActiveChain's height and that one (or more) of these blocks were spoon-fed to our node (as our node would not request them due to insufficient nChainWork). In this case, we could be fingerprinted. Closing this unlikely yet possible attack vector.
Another way to close this vector is to not accept blocks received that are are unrequested and have less nChainWork than our ActiveChain's Tip.
Unlikely to have happened but it is possible a fork with lower
difficulty reaches a height comparable to our ActiveChain's height and
these blocks were spoon-fed to our node (as our node would not request
them due to insufficient nChainWork). In this case, we could be
fingerprinted. Closing this unlikely yet possible attack vector.
Another way to close this vector is to not accept blocks received that
are are unrequested and have less nChainWork than our ActiveChain's Tip.
You can't instantly stop responding to requests for thing you offered just because you had a reorg. Doing so would be abusive to your peers. This protection from fingerprinting is much more aggressive than what we do for getblocks (the original code was also more aggressive which is why we didn't consider fingerprinting an issue). No additional fingerprinting protection is provided by being more restrictive than getblocks.
@gmaxwell I think you misunderstand the purpose of this PR, but I realised I have not really explained it.
The current functionality is that getblocktxns requests are ignored for old blocks (old defined by superceeded by 10 or more blocks) UNLESS they are blocks on an alt chain, in which case the request is accepted as long as their height on the alt chain is within 10 of the height on the active chain.
Depending on when the alt chain forked, the block heights could have become out of sync with the active chain and not be representative of "10 blocks ago" based on the height check. i.e. on the alt chain, they could be blocks 20 blocks ago from that alt chain's tip, or if the difficulty really went down on the alt chain, they could be blocks from thousands of blocks ago (assuming the block time warp attack vulnerability still exists). Anyway, I realise the time warp attack would be incredibly expensive, and therefore unlikely to happen, but still, the code doesn't need to be vulnerable to this, IMHO. (And yes, I know that if a time warp occurred there would be "bigger" things to worry about than fingerprinting).
This change therefore changes the check to go back only the equivalent of 10 blocks in terms of work, after-all it is the work which decides which is the winning fork (as of version 0.3.3 anyway), not the height. The deciding factor for the re-org should surely be used here, not an irrelevant factor (i.e. the height).
Thanks for clarifying what you're trying to address, that wasn't clear to me before. I'll re-review with that understanding.
@gmaxwell I realise that the maths isn't perfect if the block data requests is within 10 blocks of a difficulty change, the result might mean that it is effectively 9 or 11 blocks, but I felt this was close enough to 10 to not worry about. The way to make it exactly 10 would be to perhaps use -->pprev->pprev->pprev etc to get back to the block 10 ago (of course checking that nHeight is at least 11 also, or maybe the smallest height that compact blocks became active), but I decided against this approach as the code was more complex for little extra gain.
Renamed as some additional commits to be added shortly.
No new commits have been pushed, and no more comments made. Feel free to re-open when you've pushed more code.