Page MenuHomePhabricator

Introduce constant default variables to initialize seeder options
ClosedPublic

Authored by nakihito on Oct 14 2019, 20:05.

Details

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

Owners added a reviewer: Restricted Owners Package.Oct 14 2019, 20:05
jasonbcox requested changes to this revision.Oct 14 2019, 21:06

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

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

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

jasonbcox requested changes to this revision.Oct 14 2019, 23:42

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)
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.

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

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

Changed default variable names and pattern from *_DEFAULT to DEFAULT_*.

This revision is now accepted and ready to land.Oct 16 2019, 16:21