Bug - Mempool is not ignoring incoming network transactions while in IBD.

[soma42]

Expected behavior:

In the process of IBD, a node does not accept incoming transactions nor request mempool transactions.
https://bitcoin.org/en/full-node#initial-block-downloadibd

Actual behavior:

The mempool appears to be fully active while in IBD.

Problem:

The mempool has no way to check incoming or cached data against the utxo set. This causes exceptions in IBD and also invalid transactions might be propagated on the network.

While at it:
#192

[dangershony]

The fix is probably to add this in the method ProcessTxPayloadAsync

 if (this.initialBlockDownloadState.IsInitialBlockDownload())
            {
                this.logger.LogTrace("(-)[IS_IBD]");
                return;
            }

What I am not sure is why is it not there already and was there a good reason for that

[soma42]

Yes! Or maybe even better at the top of the method MempoolBehavior.ProcessMessageAsync(), or MempoolBehavior.OnMessageReceivedAsync since from what I read the mempool should simply not communicate all all while in IBD.

I don't think the change would be risky. (I think IsInitialBlockDownload() should return true instead of throwing a NullreferenceException).

In addition to that, the mempool should also only start sending old tx from mempool.dat, after IDB has completed, since the tx from a reloaded disk mempool.dat are most likely outdated after a restart.

[MithrilMan]

@soma42 can you elaborate more on

This causes exceptions in IBD and also invalid transactions might be propagated on the network.

what's the generated exception and how can this have different problems compared to received transactions when the node is synched?

[soma42]

@MithrilMan when the fullnode is not synced, aka in IBD, it does not have an up-to-date utxo set. So the question is, should it receive tx in a state where only limited validation is possible.

I'll try to provoke the exceptions that happen in that state and post an example.

For consistency, when node loads mempool.dat, it should probably also only do that, or process what's there, when IBD is finished.

If that all boils down to see what Bitcoin Core does, I need some time to find it.

[MithrilMan]

even if your node is synched you may receive a transaction that's generated by another pending transaction and can't be validated, so it's legit.

E.g. if I create a transaction that generate 5 outputs, and then I create another 2 transaction spending 2 of the first transaction output, I'm doing something legit and valid.

So your node may receive the second transaction first, and even if updated it doesn't know the input it's spending, so what should it do?

These transactions are called "orphan" because you don't know the parent of such transaction and you have to handle that case and not reject it (you may reject those spending a spent utxo, but not those who's utxo is unknown to you because transaction ordering isn't preserved in nodes networking protocol)

The only caveat I can see here is that a node that accepts everything can be flooded by fake transaction spending unknown utxo, but you can limit that by reserving a max amount of memory for orphan transaction and maybe accepting a max number of orphan tx from a peer before banning it.

I'm confident this scenario is covered in bitcoin anyway we don't have to stick 100% to what bitcoin does.