Fix/4732 #4759
Fix/4732 #4759
Conversation
…em; just don't talk to them). instead, track connections and connection attempts
…r, not that the stackerdb sync state machine is
…s, and don't use the PoX bitvec length to determine when to retry an inv sync
…depends on bootstrap nodes staying connected)
… latter happens fast enough on its own (it only needs to be done once per reward cycle)
…we waste a *ton* of CPU)
|
Okay, I have this running on my node, and have tested it in both IBD and steady-state modes of operation. No duplicate connections have been established. |
|
ping @kantai |
|
(note that this is blocking the next point release) |
There was a problem hiding this comment.
These changes look fine to me, but reading through this PR, it's not really clear to me exactly what behavior is being changed, and why, and how it is tested. It seems like there isn't sufficient testing here to prevent a regression: the only testing change is an assertion about the number of connections, which I can imagine being related to some of these changes, but not all of them.
|
@kantai I added some regression tests for the new functionality |
|
There are some build issues with the latest version. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This fixes #4732 by tracking pending connections in addition to completed connections, so that the state machine for StackerDB doesn't re-attempt a handshake if it already has one in-flight. It also disables disconnection from dead/broken StackerDB peers, since a StackerDB peer's inability to reply to a StackerDB request (itself a best-effort replication system) should not be grounds for disconnecting from and banning the node in its entirety.