Page MenuHomePhabricator

[avalanche] Extend our outbound connections with avalanche enabled peers
ClosedPublic

Authored by Fabien on Feb 8 2022, 11:51.

Details

Reviewers
tyler-smith
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABCcca949b401c2: [avalanche] Extend our outbound connections with avalanche enabled peers
Summary

This adds a new AVALANCHE_OUTBOUND connection type which is just a special case of a FULL_OUTBOUND_RELAY peer which also provides the avalanche service. We add an extra 16 outbound slots reserved for these peers, which will be the ones we will be sending the getavaaddr messages to. If avalanche is not enabled these slots are not created and there is no change in behavior. The eviction and replacement mechanism is kept identical to the usual full outbound peers.

Ref T1696.

Depends on D11009.

Test Plan
ninja check-extended

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Feb 8 2022, 11:52
tyler-smith added a subscriber: tyler-smith.

This all looks correctly implemented based on existing patterns. I do have a question about the pattern for outbound connection limiting and handling dead peers, but not something that should be addressed in this revision anyways.

src/net.cpp
2254 ↗(On Diff #32262)

I see none of these outbound peer counters are ever decremented. As connections die and we make new connections, we'll eventually get a point where we just won't connect to any new outbound peers; correct?

This revision is now accepted and ready to land.Feb 15 2022, 22:43
Fabien marked an inline comment as done.Feb 16 2022, 08:04
Fabien added inline comments.
src/net.cpp
2254 ↗(On Diff #32262)

These counters are local and computed at each loop based on the actually connected nodes, which is where the data is stored. We might evict an outbound peer for various reasons: due to inactivity (see CConnman::DisconnectNodes() and CConnman::InactivityCheck() in net.cpp) or if our tip is stale (see PeerManagerImpl::EvictExtraOutboundPeers() in net_processing.cpp). This is not echaustive, you can search for the fDisconnect flag to find more (e.g. a peer with a bad header chain will be evicted with some exceptions, etc.)

This revision looks correct and sufficiently tested. (I failed to submit draft earlier)