Make check to distinguish between orphan txs and old txs more efficient.

[morcos]

Checking for the existence in the CCoinsViewCache of the outputs of a new tx
will result in a disk hit for every output since they will not be found. On the
other hand if those outputs exist already, then the inputs must also have been
missing, so we can move this check inside the input existence check so in the
common case of a new tx it doesn't need to run.

The purpose of the check is to avoid spamming the orphanMap with slightly old
txs which we have already seen in a block, but it is already only optimistic
(depending on the outputs not being spent), so make it even more efficient by
only checking the cache and not the entire pcoinsTip.

@sipa this is my biggest feedback to #10195

For a single valid tx with 500 outputs in regtest on a machine with an SSD, the time to ATMP dropped from 4 ms to 2.4 ms.

[sipa]

I believe this risks resulting in true for transactions that are in fact not yet confirmed and not in the mempool, but were just disconnected in a reorg. Is that something we care about? If not, perhaps add a comment to explain.

[sdaftuar]

Can we just change HaveCoinInCache() to have the same semantics as HaveCoin(), and only return true if the coin is unspent? Seems confusing that their semantics aren't the same.

For many of the existing usages of HaveCoinInCache(), we store the result to potentially Uncache() later -- but I don't think it's possible to ever have a spent coin in your cache that is able to be uncached (ie not marked DIRTY)?

This issue affects the existing usage of HaveCoinInCache() in AlreadyHave(), I think.

[morcos]

@sipa I actually think that's likely a better outcome, but it may be too confusing. If a tx was just disconnected in a reorg, and no longer has one of its inputs available, it means it or its parent was probably double spent and so it shouldn't be in the orphan map anyway.

[sipa]

@morcos Do you think it's worth the complexity? I like @sdaftuar's suggestion.

[morcos]

Yes I like his suggestion as well.
I think this PR is "correct" as is, and I will make that change in a separate PR as a fix to HaveCoinsInCache?

[sipa]

Ok.

[sipa]

utACK 301566d

[sipa]

Needs rebase.

[morcos]

Simple rebase due to #10503

[TheBlueMatt]

utACK 18bacec

[morcos]

Please tag for 0.15

[gmaxwell]

utACK. "Duh"-- it should have always done this.

[gmaxwell]

This will spam the orphan map somewhat after flushing, unfortunately-- but we can fix this by making flushing less hard in the future... and the orphan map is much less important than it used to be already.

[TheBlueMatt]

I still think we should take this for 15, but another thing to consider is to add a rolling bloom filter in net_processing ala TheBlueMatt/bitcoin@316516c (which is based on a big pile of changes, but could be pulled out separately), on top of this change.