Page MenuHomePhabricator

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

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 Difflint-circular-dependencies · build-without-wallet · build-diff · build-clang · build-debug · build-clang-tidy
Build 36215: arc lint + arc unit

Event Timeline

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

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

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