Use GetDesireableServiceFlags in seeds, dnsseeds, fixing static seed adding
ClosedPublic
Actions

Authored by deadalnix on Oct 5 2018, 10:28.

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGINGda8d757ee9ac: Use GetDesireableServiceFlags in seeds, dnsseeds, fixing static seed adding
rABCda8d757ee9ac: Use GetDesireableServiceFlags in seeds, dnsseeds, fixing static seed adding

Summary

Use GetDesireableServiceFlags in static seeds, document this.

44407100f broke inserting entries into addrman from static seeds
(as well as dnsseeds which did not support service bits). Static
seeds were already being filtered by UA for 0.13.1+ (ie
NODE_WITNESS), so simply changing the default service bits to
include NODE_WITNESS (and updating docs appropriately) is
sufficient.

For DNS Seeds, we will later fix by falling back to oneshot if a
seed does not support filtering.

Fall back to oneshot for DNS Seeds which don't support filtering.

This allows us to not have to update the chainparams whenever a
DNS Seed changes its filtering support, as well fixes a bug
introduced in 44407100f where returned nodes will never be
attempted.

Update chainparams comment for more info on service bits per dnsseed

This is a backport of Core PR11512

Depends on D1863

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

deadalnix created this revision.Oct 5 2018, 10:28

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 5 2018, 10:28

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B3516: Diff 5283!Oct 5 2018, 10:46

deadalnix added a child revision: D1883: Limit the number of IPs we use from each DNS seeder.Oct 5 2018, 12:07

schancel requested changes to this revision.Oct 5 2018, 22:28

schancel added a subscriber: schancel.

schancel added inline comments.

src/net.cpp
1727 ↗	(On Diff #5283)	I don't understand how this is related to filtering service bits. Where is that checked?

This revision now requires changes to proceed.Oct 5 2018, 22:28

deadalnix requested review of this revision.Oct 7 2018, 10:48

deadalnix marked an inline comment as done.

deadalnix added inline comments.

src/net.cpp
1727 ↗	(On Diff #5283)	There is a comment to explain why it is done that way.

schancel accepted this revision.Oct 9 2018, 00:37

schancel added inline comments.

src/net.cpp
1710 ↗	(On Diff #5283)	This needs a command. It's totally unclear that servicebits are flagged as part of the hostname, rather than in the seed protocol. Where can I read about what's going on here?
1727 ↗	(On Diff #5283)	Where is this comment? If it's the one that I already read, which is immediately below, it doesn't answer my question.m This else case is not based on service bits at all. So how do we know seed does not support the filtering service bit?

This revision is now accepted and ready to land.Oct 9 2018, 00:37

deadalnix marked 2 inline comments as done.Oct 9 2018, 00:42

deadalnix added inline comments.

src/net.cpp
1710 ↗	(On Diff #5283)	Not sure it is documented anywhere.
1727 ↗	(On Diff #5283)	We do not not know. seeds are provided by DNS. We connect to them, check the bits, untill we have enough peers with the serivce bits we are looking for. We still want to add nodes that do not have the proper bits, so that we can reach transitively these who do.