test: Fix NODE_NETWORK_LIMITED_MIN_BLOCKS disconnection #21230

Fix several bugs. Also, fix #21227

utACK fa24247d0ff437a86b105692d342beb5f9f7a015 - thanks for fixing.

Code changes look good, but I don't understand what the problem is or how the fix works. I'm seeing the feature_blockfilterindex_prune.py failure during the sync_all() call https://cirrus-ci.com/task/5429627528151040?command=ci#L3364 and a "[ProcessGetBlockData] Ignore block request below NODE_NETWORK_LIMITED threshold from peer=0" message in the logs.

But I don't understand what the log message means means or why replacing one generate/sync sequence with two would fix the problem. I also don't understand why this error seems to only happen on one platform, and I can't tell from the bug report whether this is happens reliably is some kind of race condition.

Code review ACK fa24247d0ff437a86b105692d342beb5f9f7a015 with caveat above that I don't really understand the problem or fix. But the cleanups look good and the fix does seem perfectly safe. More description would be welcome!

But I don't understand what the log message means means or why replacing one generate/sync sequence with two would fix the problem. I also don't understand why this error seems to only happen on one platform, and I can't tell from the bug report whether this is happens reliably is some kind of race condition.

If you look at the next line after the logprint, you will see that this is the reason for disconnection:

        LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold from peer=%d\n", pfrom.GetId());

        //disconnect node and prevent it from stalling (would otherwise wait for the missing block)
        pfrom.fDisconnect = true;

If you then look up NODE_NETWORK_LIMITED_MIN_BLOCKS you will see that it is 288 blocks. Thus, the condition shouldn't be hit when less than 288 blocks (from the tip) are synced. (250 is less than 288)

This is a race condition because a node that "immediately" requests the block that has been announced to it won't request a block that is 288 blocks "old". Thus it won't hit the disconnect.

Very helpful explanation! Thanks! I didn't know there was a disconnection, and didn't realize ignoring request from a peer might also involve disconnecting the peer.