Potential Fix for stuck nodes: Don't lose future blocks if multiple future blocks received at the same height

[jsolman]

I've been observing for some time now that nodes can seem to get stuck and stop syncing. After spending some time in the TaskManager it seemed that somehow a block hash must be getting into knownHashes but somehow getting lost and never making it into block_cache_unverified in Blockchain.cs. After a careful audit of how that could happen, I belive I've found a scenario that can cause it.

Update: The original scenario that this change started with turned out not to be an issue. The second change does fix the situation where a valid block could be removed from block_cache_unverified while still in knownHashes if an invalid future block were sent to the network above the current height.

[jsolman]

Added another commit also to fix receiving potential invalid future blocks being able to cause syncing to get stuck

[jsolman]

There is something else besides this needed to fix nodes getting stuck. The hunt continues.

It may have just been due to the bug I introduced. in b85e21f fixd by 9caf2e1. Testing again now.

[erikzhang]

So, the only scenario that this fix solves is that someone sent fake blocks on the network?

[jsolman]

Yeah at this point, it solves for someone sending fake blocks on the network, unless there is any way for CN nodes to ever send an invalid block out, which I think there isn't. Do we know for sure no one has sent fake blocks on the network?

[erikzhang]

If you have tested it well, I will merge it.

[jsolman]

Give me 12 hours to test more. I’m running it now and it seems fine, but I’m headed to bed now and would like will examine logs in the morning.

[erikzhang]

OK. If your test is complete, you can merge it by yourself.

[jsolman]

Thanks, I will do so when ready then.

[vncoelho]

@jsolman, yesterday we tested it and the results keep the same, it does not finally solve the issue.

However, @erikzhang, the following analysis may help us to keep tracking/hunting this syncing issue.

When syncing problems, a common state is:
99/22/79/39/37, namely:
{block.Index}/{Height}/{header_index.Count}/{block_cache.Count}/{block_cache_unverified.Count}

We receive a block with higher index and we do not have the header yet.
Some blocks are in the cache and some are unverified.

This keeps until on lucky we are able to process the process and reach the top again.