[avalanche] Send a getavaaddr message to our avalanche outbound peers
ClosedPublic
Actions

Authored by Fabien on Feb 8 2022, 15:25.

Details

Reviewers

tyler-smith

Group Reviewers

Restricted Project

Maniphest Tasks

Restricted Maniphest Task

Commits

rABC42fa8032b212: [avalanche] Send a getavaaddr message to our avalanche outbound peers

Summary

This is a step toward active peer discovery: request addresses of good avalanche peers to our avalanche outbounds. Another mechanism will send more getavaaddr message later in case we are still missing nodes, but is out of scope for this diff.

Ref T1696.

Depends on D11010.

Test Plan

ninja check-extended

Diff Detail

Repository

rABC Bitcoin ABC

Branch

avalanche_getavaaddr_outbound

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 18204
Build 36216: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-clang · build-debug · build-clang-tidy
Build 36215: arc lint + arc unit

Event Timeline

Fabien created this revision.Feb 8 2022, 15:25

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 8 2022, 15:25

Fabien requested review of this revision.Feb 8 2022, 15:25

Harbormaster completed remote builds in B18204: Diff 32281.Feb 8 2022, 15:44

Fabien added a child revision: D11035: [avalanche] Request for more avalanche nodes when needed.Feb 10 2022, 15:14

This change looks correct and well tested. There is one small code-order change I suggest.

src/net_processing.cpp
3720	I think it makes more sense to increase `peer->m_addr_token_bucket` before sending the request. I can't imagine it would be an issue in practice but it feels like an unnecessary race regardless.

This revision is now accepted and ready to land.Feb 15 2022, 20:42

Fabien added inline comments.Feb 16 2022, 08:25

src/net_processing.cpp
3720	I tend to agree on principle, but I'll leave it for now for 2 reasons: This is consistent with the other places the token is incremented There is in fact no race because it written/read sequentially in the same thread, even in the same method actually so it's safe I agree this would make it easier to reason about though, so this can be a move only change in a follow up diff.