Page MenuHomePhabricator
Differential D13482

Log the reason for a block policy violation
ClosedPublic
Actions

Authored by sdulfari on Mar 27 2023, 19:21.

Details

Reviewers
PiRK
Fabien
Group Reviewers
Restricted Project
Commits
rABC3c6224afcb2c: Log the reason for a block policy violation
Summary

Introduce BlockPolicyValidationState so we can log reasons behind block policy
violations. This will also allow callers to make decisions based on the result
of the policy check rather than observing the resulting state (ie. block was
parked).

POLICY_VIOLATION is intentionally left vague. It is not clear in what other
ways policies might fail other than a simple rule violation but it is too early
to determine at this stage.

Test Plan
ninja check check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sdulfari created this revision.Mar 27 2023, 19:21
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 27 2023, 19:21
sdulfari requested review of this revision.Mar 27 2023, 19:21
Harbormaster completed remote builds in B22740: Diff 38946.Mar 27 2023, 19:27
teamcity added a comment.Mar 27 2023, 19:27
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Failed tests logs:

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

------- Stdout: -------
2023-03-27T19:25:00.332000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230327_192303/abc_feature_minerfund_96
2023-03-27T19:25:00.646000Z TestFramework (INFO): Create some history
2023-03-27T19:25:04.741000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 137, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 158, in run_test
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/usr/lib/python3.9/contextlib.py", line 124, in __exit__
    next(self.gen)
  File "/work/test/functional/test_framework/test_node.py", line 559, in assert_debug_log
    self._raise_assertion_error(
  File "/work/test/functional/test_framework/test_node.py", line 218, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['policy-bad-miner-fund']" does not partially match log:

 - 2023-03-27T19:25:02.731760Z (mocktime: 2036-07-18T13:30:00Z) [../../src/httpserver.cpp:249] [http_request_cb] Received a POST request for / from 127.0.0.1:58922
 - 2023-03-27T19:25:02.731816Z (mocktime: 2036-07-18T13:30:00Z) [../../src/rpc/request.cpp:182] [parse] ThreadRPCServer method=getchaintips user=__cookie__


2023-03-27T19:25:04.792000Z TestFramework (INFO): Stopping nodes
2023-03-27T19:25:04.844000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230327_192303/abc_feature_minerfund_96
2023-03-27T19:25:04.844000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230327_192303/abc_feature_minerfund_96/test_framework.log
2023-03-27T19:25:04.844000Z TestFramework (ERROR): 
2023-03-27T19:25:04.844000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230327_192303/abc_feature_minerfund_96' to consolidate all logs
2023-03-27T19:25:04.844000Z TestFramework (ERROR): 
2023-03-27T19:25:04.844000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-03-27T19:25:04.845000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-03-27T19:25:04.845000Z TestFramework (ERROR):

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

teamcity added a comment.Mar 27 2023, 19:34
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Failed tests logs:

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

------- Stdout: -------
2023-03-27T19:27:09.138000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192322/abc_feature_minerfund_96
2023-03-27T19:27:09.591000Z TestFramework (INFO): Create some history
2023-03-27T19:27:13.687000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 137, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 158, in run_test
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/usr/lib/python3.9/contextlib.py", line 124, in __exit__
    next(self.gen)
  File "/work/test/functional/test_framework/test_node.py", line 559, in assert_debug_log
    self._raise_assertion_error(
  File "/work/test/functional/test_framework/test_node.py", line 218, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['policy-bad-miner-fund']" does not partially match log:

 - 2023-03-27T19:27:11.676547Z (mocktime: 2036-07-18T13:30:00Z) [../../src/httpserver.cpp:249] [http_request_cb] Received a POST request for / from 127.0.0.1:60862
 - 2023-03-27T19:27:11.676611Z (mocktime: 2036-07-18T13:30:00Z) [../../src/rpc/request.cpp:182] [parse] ThreadRPCServer method=getchaintips user=__cookie__


2023-03-27T19:27:13.738000Z TestFramework (INFO): Stopping nodes
2023-03-27T19:27:13.840000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192322/abc_feature_minerfund_96
2023-03-27T19:27:13.840000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192322/abc_feature_minerfund_96/test_framework.log
2023-03-27T19:27:13.840000Z TestFramework (ERROR): 
2023-03-27T19:27:13.840000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192322/abc_feature_minerfund_96' to consolidate all logs
2023-03-27T19:27:13.840000Z TestFramework (ERROR): 
2023-03-27T19:27:13.840000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-03-27T19:27:13.840000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-03-27T19:27:13.840000Z TestFramework (ERROR):
====== Bitcoin ABC functional tests with the next upgrade activated: abc_feature_minerfund.py ======

------- Stdout: -------
2023-03-27T19:31:35.771000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192756/abc_feature_minerfund_96
2023-03-27T19:31:36.201000Z TestFramework (INFO): Create some history
2023-03-27T19:31:40.353000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 137, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 158, in run_test
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/usr/lib/python3.9/contextlib.py", line 124, in __exit__
    next(self.gen)
  File "/work/test/functional/test_framework/test_node.py", line 559, in assert_debug_log
    self._raise_assertion_error(
  File "/work/test/functional/test_framework/test_node.py", line 218, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['policy-bad-miner-fund']" does not partially match log:

 - 2023-03-27T19:31:38.342844Z (mocktime: 2036-07-18T13:30:00Z) [../../src/httpserver.cpp:249] [http_request_cb] Received a POST request for / from 127.0.0.1:37864
 - 2023-03-27T19:31:38.342905Z (mocktime: 2036-07-18T13:30:00Z) [../../src/rpc/request.cpp:182] [parse] ThreadRPCServer method=getchaintips user=__cookie__


2023-03-27T19:31:40.404000Z TestFramework (INFO): Stopping nodes
2023-03-27T19:31:40.506000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192756/abc_feature_minerfund_96
2023-03-27T19:31:40.506000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192756/abc_feature_minerfund_96/test_framework.log
2023-03-27T19:31:40.506000Z TestFramework (ERROR): 
2023-03-27T19:31:40.506000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230327_192756/abc_feature_minerfund_96' to consolidate all logs
2023-03-27T19:31:40.506000Z TestFramework (ERROR): 
2023-03-27T19:31:40.506000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-03-27T19:31:40.506000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-03-27T19:31:40.506000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_feature_minerfund.py
Bitcoin ABC functional tests with the next upgrade activated: abc_feature_minerfund.py

sdulfari planned changes to this revision.Mar 27 2023, 20:38
sdulfari updated this revision to Diff 38947.Mar 27 2023, 20:41

Fix assert_debug_log related flakiness

Harbormaster completed remote builds in B22741: Diff 38947.Mar 27 2023, 20:54
sdulfari added a child revision: D13483: Use block policy state instead of parked state to determine if a policy was violated.Mar 27 2023, 21:14
PiRK accepted this revision.Mar 28 2023, 09:27
This revision is now accepted and ready to land.Mar 28 2023, 09:27
Fabien requested changes to this revision.Mar 28 2023, 12:32
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/policy/block/minerfund.cpp
19 ↗(On Diff #38947)

You don't need the intermediate variable

24 ↗(On Diff #38947)

Add the block hash to the debug string

src/policy/block/parkingpolicy.h
8 ↗(On Diff #38947)

This kind of comment is never properly maintained, let's avoid it

src/test/policy_block_tests.cpp
78 ↗(On Diff #38947)

avoid the double negation:
BOOST_CHECK(state.IsValid());

87 ↗(On Diff #38947)

dito

src/validation.cpp
2730 ↗(On Diff #38947)

Does std::all stops on first false condition ?

2737 ↗(On Diff #38947)

You can avoid printing the hash if you previously added the hash to the validation state debug string

test/functional/abc_feature_minerfund.py
140 ↗(On Diff #38947)

Revert

154–158 ↗(On Diff #38947)
This revision now requires changes to proceed.Mar 28 2023, 12:32
Fabien mentioned this in D13483: Use block policy state instead of parked state to determine if a policy was violated.Mar 28 2023, 12:35
sdulfari updated this revision to Diff 39128.Apr 1 2023, 22:17
  • Feedback
  • Replace std::all_of() with std::find_if_not() to make early bail a guarantee
  • Apply new lint
Harbormaster completed remote builds in B22891: Diff 39128.Apr 1 2023, 22:32
Fabien accepted this revision.Apr 3 2023, 07:45
Fabien added inline comments.
src/test/policy_block_tests.cpp
51 ↗(On Diff #39128)

revert to size_t

This revision is now accepted and ready to land.Apr 3 2023, 07:45
Closed by commit rABC3c6224afcb2c: Log the reason for a block policy violation (authored by sdulfari). · Explain WhyApr 3 2023, 17:26
This revision was automatically updated to reflect the committed changes.
sdulfari added a commit: rABC3c6224afcb2c: Log the reason for a block policy violation.

Revision Contents
Changeset List

PathSize
src/
policy/
block/
2 lines
12 lines
17 lines
test/
15 lines
17 lines
test/
functional/
16 lines

Diff 39175

View Options

src/policy/block/minerfund.h

Loading...
View Options

src/policy/block/minerfund.cpp

Loading...
View Options

src/policy/block/parkingpolicy.h

Loading...
View Options

src/test/policy_block_tests.cpp

Loading...
View Options

src/validation.cpp

Loading...
View Options

test/functional/abc_feature_minerfund.py

Loading...