Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGINGaefa60f5c9fd: Introduce constant default variables to initialize seeder options
rABCaefa60f5c9fd: Introduce constant default variables to initialize seeder options

Summary

Introduces constant default variables for the seeder's parameters.
This will minimize the number of changes necessary if the default
parameters are ever in need of being changed and also paves the way
to replace CDnsSeedOpts with gArgs in a future diff.

Test Plan

make
./src/bitcoin-seeder

This should output something like below:

Supporting whitelisted filters: 0x1,0x5,0x11,0x15
No nameserver set. Not starting DNS server.
Loading dnsseed.dat...done
Starting seeder...done
Starting 96 crawler threads...2019-10-14T22:11:22Z connect() to [2600:8806:8600:1d0:a5f0:9863:3144:6f16]:28333 failed: Cannot assign requested address (99)
...

This checks -w defaults to filtering 0x1,0x5,0x11,0x15, -t defaults to 96, --wipeban, --wipeignore, and --testnet default to false, and that -o, -i, -k, and -ns default to nullptr.

./src/bitcoin-seeder -n dev.bitframe.org

This should error out and print the following:

Supporting whitelisted filters: 0x1,0x5,0x11,0x15
No hostname set. Please use -h.

This checks that -h is defaulted to nullptr

./src/bitcoin-seeder -n dev.bitframe.org -h seeder.bitframe.org

This should error out and print the following:

Supporting whitelisted filters: 0x1,0x5,0x11,0x15
No e-mail address set. Please use -m.

This checks that -m is defaulted to nullptr.

./src/bitcoin-seeder -n dev.bitframe.org -h seeder.bitframe.org -m info@bitframe.org

This should output something like below:

Supporting whitelisted filters: 0x1,0x5,0x11,0x15
Loading dnsseed.dat...done
Starting 4 DNS threads for seeder.bitframe.org on dev.bitframe.org (port 53).......done
Starting seeder...done
Starting 96 crawler threads...2019-10-14T23:51:43Z connect() to [2600:8806:8600:1d0:a5f0:9863:3144:6f16]:28333 failed: Cannot assign requested address (99)
...

This checks -p is defaulted to 53 and -d is defaulted to 4.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

UpdateSeederHelp-2

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7763
Build 13565: Bitcoin ABC Buildbot (legacy)
Build 13564: arc lint + arc unit

Event Timeline

nakihito created this revision.Oct 14 2019, 20:05

Owners added a reviewer: Restricted Owners Package.Oct 14 2019, 20:05

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 14 2019, 20:05

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7763: Diff 13538.Oct 14 2019, 20:14

For testing the defaults, the testplan should *not* include various arguments.

This revision now requires changes to proceed.Oct 14 2019, 21:06

nakihito edited the test plan for this revision. (Show Details)Oct 14 2019, 22:06

nakihito edited the test plan for this revision. (Show Details)Oct 14 2019, 22:11

Reverted the std::string constants to const char*. The const char*s will be changed to std::string in another diff.

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito planned changes to this revision.Oct 14 2019, 22:16

nakihito added a child revision: D4251: Change CDnsSeedOpts to use std::string instead of c-strings.

nakihito requested review of this revision.Oct 14 2019, 22:24

Harbormaster completed remote builds in B7767: Diff 13544.Oct 14 2019, 22:29

Build Bitcoin-ABC / Diffs / Diff Testing passed.

testplan doesn't verify that all modified defaults are still set as expected

This revision now requires changes to proceed.Oct 14 2019, 23:42

nakihito edited the test plan for this revision. (Show Details)Oct 14 2019, 23:54

nakihito requested review of this revision.Oct 15 2019, 00:33

nakihito edited the test plan for this revision. (Show Details)

jasonbcox requested changes to this revision.Oct 15 2019, 15:37

jasonbcox added inline comments.

src/seeder/main.cpp
32 ↗	(On Diff #13544)	Constants should be upper snake case.

This revision now requires changes to proceed.Oct 15 2019, 15:37

Changed constants to upper snake case and made static.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Harbormaster completed remote builds in B7780: Diff 13565.Oct 15 2019, 18:34

Build Bitcoin-ABC / Diffs / Diff Testing passed.

jasonbcox requested changes to this revision.Oct 15 2019, 22:31

jasonbcox added inline comments.

src/seeder/main.cpp
21 ↗	(On Diff #13565)	this name isn't self explanatory. we know it's a number, but we don't know what it's for. something like `DEFAULT_NUM_THREADS` (side nit: `DEFAULT_` rather than `_DEFAULT` reads more naturally)
23 ↗	(On Diff #13565)	DEFAULT_NUM_DNS_THREADS
27 ↗	(On Diff #13565)	DEFAULT_MAILBOX or DEFAULT_EMAIL. there's no sense in perpetuating the original poor naming.
28 ↗	(On Diff #13565)	DEFAULT_NAMESERVER