Page MenuHomePhabricator

Replace relevant services logic with a function suite.
ClosedPublic

Authored by deadalnix on Sep 10 2018, 23:22.

Details

Summary

Adds HasAllRelevantServices and GetRelevantServices, which check
for NETWORK|WITNESS.

This changes the following:

  • Removes nRelevantServices from CConnman, disconnecting it a bit more from protocol-level logic.
  • This has the added benefit of removing nServicesExpected from CNode - instead letting net_processing's VERSION message handling simply check HasAllRelevantServices.
  • In order to prevent this change from preventing connection to -connect= nodes which do not have the require flags, -connect nodes are now given the "addnode" flag. This also allows outbound connections to !NODE_NETWORK nodes for -connect nodes (which was already true of addnodes).
  • Has the (somewhat unintended) consequence of changing one of the eviction metrics from the same metric to requiring HasRelevantServices.

This should make NODE_NETWORK_LIMITED much simpler to implement.

  • Rename fAddnode to a more-descriptive "manual_connection"
  • Clarify docs for requirements/handling of addnode/connect nodes
  • Switch DNSSeed-needed metric to any-automatic-nodes, not services

This is a backport of core PR11456

Test Plan
make check

Diff Detail

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

Event Timeline

schancel added a subscriber: schancel.
schancel added inline comments.
src/protocol.h
323 ↗(On Diff #4870)

What's the parameter here for?

This revision is now accepted and ready to land.Sep 11 2018, 19:38
src/protocol.h
323 ↗(On Diff #4870)

For NODE_LIMITED support, which I'm not sure we'll ever support, but that's another story.

This revision was automatically updated to reflect the committed changes.