Page MenuHomePhabricator

[avalanche] Manipulate peers via PeerId

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


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

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

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Tue, Jun 30, 16:12
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Jun 30, 16:12
deadalnix requested review of this revision.Tue, Jun 30, 16:12
jasonbcox requested changes to this revision.Tue, Jun 30, 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.

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.

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.Tue, Jun 30, 22:23
deadalnix updated this revision to Diff 21915.Tue, Jun 30, 23:27

Check insertion and remove refactoring leftover

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