[validation] Fix peer punishment for bad blocks
ClosedPublic
Actions

Authored by PiRK on Oct 29 2020, 17:34.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rABC8a980e61c9f8: [validation] Fix peer punishment for bad blocks

Summary

Because the call to MaybePunishNode() in
PeerLogicValidation::BlockChecked() only previously happened if the
REJECT code was > 0 and < REJECT_INTERNAL, then there are cases were
MaybePunishNode() can get called where it wasn't previously:

when AcceptBlockHeader() fails with CACHED_INVALID.

when AcceptBlockHeader() fails with BLOCK_MISSING_PREV.

Note that BlockChecked() cannot fail with an 'internal' reject code. The
only internal reject code was REJECT_HIGHFEE, which was only set in
ATMP.

This change restores the behaviour pre-commit
https://github.com/bitcoin/bitcoin/commit/5d08c9c579ba8cc7b684105c6a08263992b08d52
which did punish nodes that
sent us CACHED_INVALID and BLOCK_MISSING_PREV blocks.

This is a backport of Core PR17004 [1/5]
Commit https://github.com/bitcoin/bitcoin/pull/17004/commits/a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 13750
Build 27519: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 27518: arc lint + arc unit

Event Timeline

PiRK created this revision.Oct 29 2020, 17:34

Owners added a reviewer: Restricted Owners Package.Oct 29 2020, 17:34

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 29 2020, 17:34

PiRK requested review of this revision.Oct 29 2020, 17:34

teamcity edited the summary of this revision. (Show Details)Oct 29 2020, 17:35

PiRK added a child revision: D8182: [logging] Don't log REJECT code when transaction is rejected.Oct 29 2020, 17:39

Harbormaster completed remote builds in B13750: Diff 25240.Oct 29 2020, 17:49

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: p2p_timeouts.py ======

------- Stdout: -------
2020-10-29T17:48:40.404000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201029_174808/p2p_timeouts_627
2020-10-29T17:48:45.716000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 209, in main
    self.run_test()
  File "/work/test/functional/p2p_timeouts.py", line 89, in run_test
    assert not no_verack_node.is_connected
AssertionError
2020-10-29T17:48:52.077000Z TestFramework (INFO): Stopping nodes
2020-10-29T17:48:52.379000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201029_174808/p2p_timeouts_627
2020-10-29T17:48:52.379000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201029_174808/p2p_timeouts_627/test_framework.log
2020-10-29T17:48:52.379000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201029_174808/p2p_timeouts_627' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: p2p_timeouts.py

jasonbcox accepted this revision.Oct 29 2020, 18:07

This revision is now accepted and ready to land.Oct 29 2020, 18:07

Closed by commit rABC8a980e61c9f8: [validation] Fix peer punishment for bad blocks (authored by John Newbery <john@johnnewbery.com>, committed by PiRK). · Explain WhyOct 29 2020, 21:39

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC8a980e61c9f8: [validation] Fix peer punishment for bad blocks.