net: don't send feefilter messages before the version handshake is complete #9117
Conversation
|
utACK 4662553 |
|
What does |
|
@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 4662553 |
|
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 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! |
…ndshake is complete 4662553 net: don't send feefilter messages before the version handshake is complete (Cory Fields)
…mplete Github-Pull: bitcoin#9117 Rebased-From: 4662553
…sion handshake is complete 4662553 net: don't send feefilter messages before the version handshake is complete (Cory Fields)
…mplete Github-Pull: bitcoin#9117 Rebased-From: 4662553
…sion handshake is complete 4662553 net: don't send feefilter messages before the version handshake is complete (Cory Fields)
…sion handshake is complete 4662553 net: don't send feefilter messages before the version handshake is complete (Cory Fields)
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.