Page MenuHomePhabricator

[validation] Fix peer punishment for bad blocks
ClosedPublic

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

Details

Reviewers
jasonbcox
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

Event Timeline

Owners added a reviewer: Restricted Owners Package.Oct 29 2020, 17:34
PiRK requested review of this revision.Oct 29 2020, 17:34

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

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