Details
- Reviewers
deadalnix nakihito - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project - Commits
- rABC2a8d9ce51932: Dedup net message writing code in the seeder
ninja ninja check-bitcoin-seeder
Edit chainparams.cpp to replace all seeds in TestNet with a single entry: vSeeds.emplace_back("localhost");
src/bitcoind -testnet -whitelist=127.0.0.1 -listen=1 -connect=0 -debug=net
Wait for the node to finish startng up
src/seeder/bitcoin-seeder -host=localhost -ns=blah.bitframe.org -port=8888 -mbox=info@bitframe.org -wipeban -wipeignore -testnet
From the seeder, the output should start as:
0/1 available (1 tried in 1s, 0 new, 0 active), 0 banned; 0 DNS requests, 0 db queries
And eventually become something like:
0/503 available (1 tried in 1s, 470 new, 32 active), 0 banned; 0 DNS requests, 0 db queries
Output from the node should be similar to the following:
Leaving InitialBlockDownload (latching to false) Added connection peer=0 connection from 127.0.0.1:58994 accepted received: version (111 bytes) peer=0 sending version (114 bytes) peer=0 send version message: version 70015, blocks=1383542, us=[::]:0, peer=0 Cleared nodestate for peer=0 sending verack (0 bytes) peer=0 receive version message: [127.0.0.1:58994] /bitcoin-cash-seeder:0.15/: version 70015, blocks=1378461, us=127.0.0.1:18333, peer=0 added time data, samples 2, offset +0 (+0 minutes) received: verack (0 bytes) peer=0 sending sendheaders (0 bytes) peer=0 sending sendcmpct (9 bytes) peer=0 sending ping (8 bytes) peer=0 sending feefilter (8 bytes) peer=0 received: getaddr (0 bytes) peer=0 sending addr (30003 bytes) peer=0 socket closed disconnecting peer=0 Cleared nodestate for peer=0
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- MessageWriter
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 11179 Build 20970: Default Diff Build & Tests Build 20969: Build without Wallet Build 20968: Build with clang-10 Build 20967: arc lint + arc unit
Event Timeline
There are still no explanation as to what the problem is with the previous version of this code. The only significant way this is changing things is by keeping more copies of the messages in memory instead of streaming them, which does not seem to be a good move.
| src/seeder/bitcoin.h | ||
|---|---|---|
| 34 | I'm fairly confident that this is the wrong move. | |
Updated summary to remove mention of previous seeder implementation being unable to send multiple messages. This was actually the result of a different bug that made it appear as if it was not possible.
- Rebase ontop of linker fixes: D8622
- Use netmagic from config instead of hardcoding the values in multiple places. (More cleanup can be done here, but out of scope of this patch)
- Use TransportSerializer.prepareForTransport to de-dup header serialization code.
More of a question that a change request, but please double check this.
| src/seeder/messagewriter.h | ||
|---|---|---|
| 34 ↗ | (On Diff #26255) | You want to move that vector to avoid a copy. But even then, is that really the way it is done in the rest of th codebase? That seems like it can create a lot of copies and be quite slow. |
This was a good question. There's multiple copies going on here and they aren't necessary.
While the handling of the outgoing data stream is not identical between the seeder and the
node software, the new WriteMessage function can easily write directly to the stream in
question instead of copying into an intermediate which then is copied into the outgoing stream.
I also cleaned up the tests a bit, since version was being passed around for no good reason.