Page MenuHomePhabricator

Use GetDesireableServiceFlags in seeds, dnsseeds, fixing static seed adding

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


  • 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

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

  • 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

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.Oct 5 2018, 10:28
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 5 2018, 10:28
schancel requested changes to this revision.Oct 5 2018, 22:28
schancel added a subscriber: schancel.
schancel added inline comments.
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.
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.
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.
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.

This revision was automatically updated to reflect the committed changes.