prefer outbound peers when syncing #3521
Conversation
antiochp
changed the title
|
Related bitcoin/bitcoin#5315
|
There was a problem hiding this comment.
👍 on the concept and initial review. Tested for a quick sync but not likely under the troublesome conditions. As mentioned elsewhere, we probably want to revisit this area after the HF to ensure we have a clear big-picture robustness with good edge case security wrt rules around incoming/outgoing peer connections.
| .connected() | ||
| .count(); | ||
| let peers_iter = || peers.iter().connected(); | ||
| let peers_count = peers_iter().count(); |
There was a problem hiding this comment.
The change to using empty closures here and in other files when iterating peers is just a rust convenience thing?
There was a problem hiding this comment.
We need to build a couple of instances of the same (or similar) iterator.
And the closure just makes this a lot easier to keep them consistent.
but use inbound peer for header and body sync if necessary sync state from inbound peer if no outbound peers to sync from
antiochp
force-pushed
the
most_work_peer_fix
branch
from
2309890 to
0b5a0f6
Compare
…imblewimble#3521) but use inbound peer for header and body sync if necessary sync state from inbound peer if no outbound peers to sync from
Resolves #3518
This PR fixes an edge case that we appear to be hitting on
testnetdue to low number of nodes and overall network health.There was an inconsistency during sync where we would see a
max_diffon an inbound peer that would trigger sync but then we would attempt to sync against a randomly chosen outbound peer. But it is possible that no outbound peers have sufficient difficulty to handle the sync request.When checking for known
max_diffwe need to ensure we check against a set of peers that is consistent with those that we will actually sync from.We want to prioritize syncing against outbound peers whenever possible.
But we we also want to handle the case where an inbound peer advertises more work than any other connected peers - in this case we will sync against that inbound peer, but only if no outbound peers are known to have the same total work.
Outbound peers are considered more reliable as they are more likely to be long lived (and likely with faster connections and more bandwidth).
Related discussion - bitcoin/bitcoin#5315 (comment)
Rules are as follows -
max_diffacross all connected peers (inbound and outbound)diff >= max_diffdiff >= max_diffThis means we prefer outbound peers when syncing. And we sync against inbound peers only when necessary.