Page MenuHomePhabricator

Merge #11531: Check that new headers are not a descendant of an invalid block (more effeciently)
AbandonedPublic

Authored by jasonbcox on Oct 26 2018, 23:51.

Details

Reviewers
deadalnix
schancel
Fabien
Group Reviewers
Restricted Project
Summary

f3d4adf Make p2p-acceptablock not an extended test (Matt Corallo)
00dcda6 [qa] test that invalid blocks on an invalid chain get a disconnect (Matt Corallo)
015a525 Reject headers building on invalid chains by tracking invalidity (Matt Corallo)
932f118 Accept unrequested blocks with work equal to our tip (Matt Corallo)
3d9c70c Stop always storing blocks from whitelisted peers (Matt Corallo)
3b4ac43 Rewrite p2p-acceptblock in preparation for slight behavior changes (Matt Corallo)

Pull request description:

@sdaftuar pointed out that the version in #11487 was somewhat DoS-able as someone could feed you a valid chain that forked off the the last checkpoint block and force you to do lots of work just walking backwards across blocks for each new block they gave you. We came up with a few proposals but settled on the one implemented here as likely the simplest without obvious DoS issues. It uses our existing on-load mapBlockIndex walk to make sure everything that descends from an invalid block is marked as such, and then simply caches blocks which we attempted to connect but which were found to be invalid. To avoid DoS issues during IBD, this will need to depend on #11458.

Includes tests from #11487.

Tree-SHA512: 46aff8332908e122dae72ceb5fe8cd241902c2281a87f58a5fb486bf69d46458d84a096fdcb5f3e8e07fbcf7466232b10c429f4d67855425f11b38ac0bf612e1

Backport of Core PR11531
Depends on D1976 D1982 D1987
Completes T456

Co-authored-by: Jason B. Cox <contact@jasonbcox.com>

Test Plan

ninja && test_runner.py p2p-acceptblock

Diff Detail

Repository
rABC Bitcoin ABC
Branch
pr11531-1
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3705
Build 5485: Bitcoin ABC Buildbot (legacy)
Build 5484: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Oct 27 2018, 19:27

This breaks parking.

src/validation.cpp
2792 ↗(On Diff #5559)

This whole block do not handle properly the difference between parking and invalid.

This revision now requires changes to proceed.Oct 27 2018, 19:27

Fix unwind logic for parked chains

Need to rebase on master with the fix in UnwindBlock.

Fixed bug where removing index candidates early disabled the fork alert warning.
Added comments to warn future backporters of this potential issue.

Abandoning in favor splitting up this diff:
D2149
D2246
D2254
D2645