Page MenuHomePhabricator

[avalanche] Manipulate peers via PeerId
ClosedPublic

Authored by deadalnix on Jun 30 2020, 16:12.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rABCc23e06f18a83: [avalanche] Manipulate peers via PeerId
Summary

This ensure we do not expose information from internal data structure and can propose a stable API.

Test Plan
ninja all check-avalanche

Diff Detail

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

Event Timeline

jasonbcox requested changes to this revision.Jun 30 2020, 22:23
jasonbcox added a subscriber: jasonbcox.

The nits aren't a big deal, but the comment in selectPeerImpl() needs to be addressed in some way.

src/avalanche/peermanager.cpp
10 ↗(On Diff #21899)

Nit: Add a TODO otherwise this comment is confusing

135 ↗(On Diff #21899)

I can't find a reference to this behavior in the rest of the code or tests. Assuming this is a mistake, it indicates there's a missing test case somewhere. More specifically, this could impact selectPeer() in weird ways.

src/avalanche/peermanager.h
12 ↗(On Diff #21899)

Nit: Introduction of PeerId could have been its own diff which would eliminate extra scrolling during review due to extra context in this diff.

This revision now requires changes to proceed.Jun 30 2020, 22:23

Check insertion and remove refactoring leftover

This revision is now accepted and ready to land.Jul 1 2020, 01:32
This revision was automatically updated to reflect the committed changes.