net: don't send feefilter messages before the version handshake is complete #9117

Caused by changes in #8708. Same fix as #8862.

This is a quick fix, I'll PR a small cleanup of the disconnect logic so that these cases don't continue to crop up.

Edit: Forgot to mention, this currently asserts due to #8708, which was intended to point out issues like this one. So this should be considered a crash-fix.

utACK 46625538d674a5aaf1bcfa7c8be8a49e5a23fa9e

What does !pto->fDisconnect have to do with the version handshake?

@sipa in this case, the handshake started, but stopped halfway-through and set fDisconnected because the node didn't have the services expected. I suppose I should've ignored that and just called this "don't send feefilter messages if fDisconnect has been set".

For a more correct/complete (but untested) change, see here: https://github.com/theuni/bitcoin/commits/feefilter-assert2 .

utACK 46625538d674a5aaf1bcfa7c8be8a49e5a23fa9e

I still don't understand why we don't simply put the fDisconnect check along with the check for nVersion != 0 then all of these little issues would not exist. The only messages we need to send once fDisconnect is set are messages directly relating to the disconnection, e.g. the "bye" message @laanwj proposed a while back.

@rebroad #9128 contains the complete fix to this, similar to what you're suggesting.

I'll leave this open for now in case the quick fix is desired to avoid the crash while the bigger PR is in review.

@theuni oh! you got there before me! Well, #9129 contains just the fix we're talking about, so might need less review...

@rebroad That only stops processing if fDisconnect is already set when SendMessages is called. But with your changes, if it's set part-way through SendMessages (as it is a few times), messages further down will now be allowed out!