TestNode() already takes in the majority of CServiceResult and only has one call site.
In addition, having it located in seeder/bitcoin.* when the call site is in seeder/main.cpp
make banning logic more complex and difficult to follow.
Details
Details
- Reviewers
jasonbcox - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project - Commits
- rSTAGING0a7fb85228f5: Remove TestNode()
rABC0a7fb85228f5: Remove TestNode()
Edit chainparams.cpp such that the only seed for TestNet is localhost.
./bitcoind -whitelist=127.0.0.1 --datadir=testdir -listen=1 -connect=0 -debug=net -datadir=testdir ./bitcoin-seeder -host=localhost -ns=blah.bitframe.org -port=8888 -mbox=info@bitframe.org --testnet --wipeban --wipeignore
Confirm seeder successfully connects with the local node and then connects to the peers obtained by the local node.
Output should start as:
0/2 available (0 tried in 0s, 0 new, 2 active), 0 banned; 0 DNS requests, 0 db queries
Then eventually become something like:
1/581 available (3 tried in 4s, 546 new, 32 active), 0 banned; 0 DNS requests, 0 db queries
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- RemoveTestNodeReturn
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 10338 Build 18497: Default Diff Build & Tests Build 18496: arc lint + arc unit
Event Timeline
Comment Actions
It looks like the old code was trying to keep a separation between the DB results and the seeder implementation. Your patch now entangles the two. Neither approach appears to be correct, so that begs the question: Why does TestNode() exist outside of main.cpp? or why does it exist at all when it's only call site is ThreadCrawler?
Unless I'm missing something, you'll likely find that consolidating the code to the one call site will simplify this patch and make the code much easier to understand.