Page MenuHomePhabricator

[seeder] Request headers from new connections
ClosedPublic

Authored by jasonbcox on May 28 2020, 19:30.

Details

Reviewers
deadalnix
Fabien
nakihito
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC8099bc1028b2: [seeder] Request headers from new connections
Summary

The next step is to use the response to verify the connected node is (probably) on
the right chain using the latest checkpoint.

Depends on D6357

Test Plan
ninja check-seeder

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

Owners added a reviewer: Restricted Owners Package.May 28 2020, 19:30
nakihito planned changes to this revision.
Fabien requested changes to this revision.Jun 1 2020, 07:13
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/seeder/bitcoin.cpp
182 ↗(On Diff #20679)

As far as I can tell this is the only line that differentiate this block from the above one. Please wrap this into a function to avoid copy and paste.

This revision now requires changes to proceed.Jun 1 2020, 07:13
deadalnix requested changes to this revision.Jun 1 2020, 23:03

The trigger for getheaders call doesn't not make a lot of sense.

src/seeder/bitcoin.cpp
104 ↗(On Diff #20762)

This only uses vSend from the object, so that a good tell it doesn't really belong as a member function.

177 ↗(On Diff #20762)

It does not make sense to respond a an addr request by a getheaders request.

This revision now requires changes to proceed.Jun 1 2020, 23:03

Rebased off D6357. Adjusted trigger for GETHEADERS message to occur with GETADDR.

Make use of CBlockLocator class.

nakihito requested review of this revision.Jun 6 2020, 00:41
Fabien requested changes to this revision.Jun 15 2020, 08:50

Clearing my queue for now, this is subject to changes depending on D6357

src/seeder/bitcoin.cpp
9 ↗(On Diff #20975)

You also need to #include <primitives/blockhash.h> and chainparams.h

This revision now requires changes to proceed.Jun 15 2020, 08:50

Added includes for primitives/blockhash.h and chainparams.h.

jasonbcox edited reviewers, added: nakihito; removed: jasonbcox.

Rebase and add some basic test coverage

jasonbcox retitled this revision from Seeder sends GETHEADERS message to [seeder] Request headers from new connections.Dec 10 2020, 23:06
jasonbcox edited the summary of this revision. (Show Details)
jasonbcox edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Dec 11 2020, 22:27