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

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

Branch

avalanche_outbound

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 18192
Build 36192: Build Diff	build-without-wallet · build-debug · lint-circular-dependencies · build-clang-tidy · build-clang · build-diff
Build 36191: arc lint + arc unit

Event Timeline

Fabien created this revision.Feb 8 2022, 11:51

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 8 2022, 11:52

Fabien requested review of this revision.Feb 8 2022, 11:52

Harbormaster completed remote builds in B18192: Diff 32262.Feb 8 2022, 12:07

Fabien added a child revision: D11020: [avalanche] Send a getavaaddr message to our avalanche outbound peers.Feb 8 2022, 15:25

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	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	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.)