Page MenuHomePhabricator

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

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
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.
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.

This revision was automatically updated to reflect the committed changes.