Don't disconnect masternode connections when we have less then the desired amount of outbound nodes #3255
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sired amount of outbound nodes
UdjinM6
reviewed
Dec 30, 2019
UdjinM6
pushed a commit
to UdjinM6/dash
that referenced
this pull request
Jan 10, 2020
…sired amount of outbound nodes (dashpay#3255)
barrystyle
pushed a commit
to PACGlobalOfficial/PAC
that referenced
this pull request
Jan 22, 2020
…sired amount of outbound nodes (dashpay#3255)
FornaxA
pushed a commit
to ioncoincore/ion
that referenced
this pull request
Jul 6, 2020
…sired amount of outbound nodes (dashpay#3255) Signed-off-by: cevap <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This avoids situations where we end up disconnecting (nearly) all outgoing peers when masternode connection cleanup is happening.
This also indirectly fixes test failures like this, where cleanup happened directly after a block was requested via
getdata.The real reason for this test failure is actually a different one, but I was not able to pin it down. What happens is that node4 gets a header announced from peer=1. Node4 then reacts with a
getdatato retrieve the block. Shortly after that, peer=1 is disconnected due to the masternode connection cleanup. Approximately at the same time node4 receives acmpctblockfrom peer=0, which is however ignored due to this code (FillBlockfails). It gets into this code becauseFinalizeNodewas not called at this point, so that the block is considered as in-flight.I would expect that even after peer=1 is disconnect,
FindNextBlocksToDownloadwould realize that the block in not in-flight anymore and needs to be re-requested from another node. But for some reason this is not happening. I was not able to reproduce this locally. As I'm unable to fix the actual bug, I decided to implement a fix that avoids the disconnect in this situation. This fix is also useful on its own as I believe.