Parallel compact block downloads, take 3
ClosedPublic
Actions

Authored by Fabien on Tue, Nov 5, 21:21.

Details

Reviewers

PiRK

Group Reviewers

Restricted Project

Commits

rABC21a89c2d5e73: Parallel compact block downloads, take 3

Summary

This PR attempts to mitigate a single case, where high bandwidth peers can bail us out of a flakey
peer not completing blocks for us. We allow up to 2 additional getblocktxns requests per unique block.
This would hopefully allow the chance for an honest high bandwidth peer to hand us the transactions
even if the first in flight peer stalls out.

Backport of core#27626 and core#27743 (bugfix).

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Tue, Nov 5, 21:21

Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Nov 5, 21:21

Fabien requested review of this revision.Tue, Nov 5, 21:21

teamcity edited the summary of this revision. (Show Details)Tue, Nov 5, 21:21

Harbormaster completed remote builds in B31022: Diff 50687.Tue, Nov 5, 21:37

PiRK accepted this revision.Tue, Nov 5, 23:00

PiRK added a subscriber: PiRK.

PiRK added inline comments.

src/net_processing.cpp
1408 ↗	(On Diff #50687)	comment layout
test/functional/p2p_compactblocks.py
1004 ↗	(On Diff #50687)	I'm not sure why core kept that second test when they switched the test to using only segwit nodes. `self.segwit_node` and `self.additional_segwit_node` are exactly the same as far as I can tell. Back when I removed it (in D14872) the second test was doing something different for Core ( whereas for us it was already just a duplicate of the previous line) self.log.info("Testing SENDCMPCT p2p message... ") self.test_sendcmpct(self.segwit_node, old_node=self.old_node) self.test_sendcmpct(self.additional_segwit_node) https://github.com/bitcoin/bitcoin/pull/15660/files#diff-3d0d1b84c608319965e0c2675eefadc6820103793bf5a9d671562bc24dcbc3eaR806 But i'm not blocking on this, worst case we just duplicate a test.

This revision is now accepted and ready to land.Tue, Nov 5, 23:00

Fabien added inline comments.Wed, Nov 6, 08:47

test/functional/p2p_compactblocks.py
1004 ↗	(On Diff #50687)	I added this back because it doesn't hurt, it takes nearly zero time to complete, but it makes backport easier and it makes the new test correct (the ordering is now the same as core so the check for high bandwidth flag over the peers is correct).