AlreadyHave function can fail for txs already in blocks

[AM5800]

I am observing lots of orphan transactions processed by my node:

2019-04-16 21:52:08 [     mempool] mapOrphan overflow, removed 1 tx
:stored orphan tx 18111cc415b22869db9d160df25f05f6cd2693654c75276e154de89c6354a173 (mapsz 101 outsz 2247)
2019-04-16 21:52:08 [     mempool] mapOrphan overflow, removed 1 tx
:stored orphan tx 74e8d36605948c9f754711e0b5024b254a748c5652460165581d2b3622896b49 (mapsz 101 outsz 2247)
2019-04-16 21:52:08 [     mempool] mapOrphan overflow, removed 1 tx
:stored orphan tx aa89c2859bc2a036818094e0a3908fdf937c0e00c0d288159c4834b5449c1d4c (mapsz 101 outsz 2247)
2019-04-16 21:52:08 [     mempool] mapOrphan overflow, removed 1 tx
:stored orphan tx 0062a63558c7fc100f0ec21fc9a9f5ae43be1658f0aa0c4b52f66108cd335fd4 (mapsz 101 outsz 2218)
2019-04-16 21:52:08 [     mempool] mapOrphan overflow, removed 1 tx
:stored orphan tx 24883465b96aa25408612bcec316e8c93503c1892333d302c2042a2d81053226 (mapsz 101 outsz 2189)
2019-04-16 21:52:08 [     mempool] mapOrphan overflow, removed 1 tx
:stored orphan tx 1dd14bcebc9abf4f1fafb444dde3eb0a00760fbe6d6c50e0a821de91d346ee26 (mapsz 101 outsz 2189)
2019-04-16 21:52:08 [     mempool] mapOrphan overflow, removed 1 tx
:stored orphan tx c89e5e7bc0abc042b94e0fdc2a89d322334c781a4a456b08a042e31be4444b24 (mapsz 101 outsz 2189)

[AM5800]

There are many orphans, so I believe there are many different reasons for them. I will try to list here what I find:

1. Locktime issue If a node lags behind in block processing - it might reject transactions that have a locktime. So all children of such transactions will be orphans

[AM5800]

I think I found the main source of orphans:

Orphans I was observing were not truly orphans. They were already included in a block. And this block was already processed by my node. No forking occurred. What went wrong is the AlreadyHave function.

This function looks for tx hash in mempool and also:

pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1
pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 1));

Setup: We have a transaction a in block A. In the following block B we spend the only output of a.

And now, If some node lags a bit - it might send you a while you already have B. AlreadyHave will treat this as a new transaction!(because this tx is not in mempool anymore and it's outputs are spent). a ends up in orphan pool because it's inputs are spent by... a.

I tried making a workaround fix by storing all recently connected txs in ConnectBlock and checking for them in AlreadhyHave and I have 0 orphans. So this confirms my theory.

This all leads to lots of inefficiencies in tx transfer - such transactions are relayed several extra times and debug log is also misleading. Not to mention orphan pool pollution.

I suggest moving this issue into next milestone. Because mentioned inefficiencies are not critical.

[thothd]

Ok thanks for looking further, can you also change the issue title to reflect the more precise problem?