Differential D12806

[consensus + avalanche] Move miner fund from consensus to avalanche policy
AbandonedPublicDraft
Actions

Authored by sdulfari on Dec 8 2022, 04:08.

Details

Reviewers

None

Group Reviewers

Restricted Project

Summary

Moving the miner fund from consensus to policy makes it possible to do the
following without a flag day upgrade:

Change fund address
Change fund amount
Add staking rewards

Test Plan

ninja check check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Branch

minerfund-policy

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21745
Build 43125: Build Diff	build-without-wallet · build-debug · lint-circular-dependencies · build-diff · build-clang · build-clang-tidy
Build 43124: arc lint + arc unit

Event Timeline

sdulfari created this revision.Dec 8 2022, 04:08

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 8 2022, 04:08

Harbormaster completed remote builds in B21443: Diff 37089.Dec 8 2022, 04:08

Move policy fee checking to ConnectBlock
Add policy whitelist to mining RPC + getblocktemplate
More tests

Harbormaster completed remote builds in B21729: Diff 37511.Jan 10 2023, 21:50

Moved block policy logic into its own function to be called before attempting
to connect the block since short circuiting ConnectTip() is fragile.

Harbormaster completed remote builds in B21745: Diff 37535.Jan 13 2023, 21:48

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

Failed tests logs:

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

------- Stdout: -------
2023-01-13T21:46:59.193000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230113_214449/abc_feature_minerfund_160
2023-01-13T21:46:59.506000Z TestFramework (INFO): Create some history
2023-01-13T21:47:01.551000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 135, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in run_test
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/test_framework/test_framework.py", line 703, in wait_until
    return wait_until_helper(test_function, timeout=timeout,
  File "/work/test/functional/test_framework/util.py", line 273, in wait_until_helper
    if predicate():
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in <lambda>
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/abc_feature_minerfund.py", line 153, in parked_block
    assert tip["status"] != "active"
AssertionError
2023-01-13T21:47:01.602000Z TestFramework (INFO): Stopping nodes
2023-01-13T21:47:01.703000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230113_214449/abc_feature_minerfund_160
2023-01-13T21:47:01.704000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230113_214449/abc_feature_minerfund_160/test_framework.log
2023-01-13T21:47:01.704000Z TestFramework (ERROR): 
2023-01-13T21:47:01.704000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20230113_214449/abc_feature_minerfund_160' to consolidate all logs
2023-01-13T21:47:01.704000Z TestFramework (ERROR): 
2023-01-13T21:47:01.704000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-01-13T21:47:01.704000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-01-13T21:47:01.704000Z TestFramework (ERROR):

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

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

Failed tests logs:

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

------- Stdout: -------
2023-01-13T21:49:16.513000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20230113_214309/abc_feature_minerfund_160
2023-01-13T21:49:17.125000Z TestFramework (INFO): Create some history
2023-01-13T21:49:19.205000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 135, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in run_test
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/test_framework/test_framework.py", line 703, in wait_until
    return wait_until_helper(test_function, timeout=timeout,
  File "/work/test/functional/test_framework/util.py", line 273, in wait_until_helper
    if predicate():
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in <lambda>
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/abc_feature_minerfund.py", line 153, in parked_block
    assert tip["status"] != "active"
AssertionError
2023-01-13T21:49:19.256000Z TestFramework (INFO): Stopping nodes
2023-01-13T21:49:19.358000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20230113_214309/abc_feature_minerfund_160
2023-01-13T21:49:19.358000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20230113_214309/abc_feature_minerfund_160/test_framework.log
2023-01-13T21:49:19.359000Z TestFramework (ERROR): 
2023-01-13T21:49:19.359000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20230113_214309/abc_feature_minerfund_160' to consolidate all logs
2023-01-13T21:49:19.359000Z TestFramework (ERROR): 
2023-01-13T21:49:19.359000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-01-13T21:49:19.359000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-01-13T21:49:19.359000Z TestFramework (ERROR):

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

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

Failed tests logs:

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

------- Stdout: -------
2023-01-13T21:50:24.002000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_214627/abc_feature_minerfund_160
2023-01-13T21:50:24.399000Z TestFramework (INFO): Create some history
2023-01-13T21:50:26.477000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 135, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in run_test
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/test_framework/test_framework.py", line 703, in wait_until
    return wait_until_helper(test_function, timeout=timeout,
  File "/work/test/functional/test_framework/util.py", line 273, in wait_until_helper
    if predicate():
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in <lambda>
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/abc_feature_minerfund.py", line 153, in parked_block
    assert tip["status"] != "active"
AssertionError
2023-01-13T21:50:26.528000Z TestFramework (INFO): Stopping nodes
2023-01-13T21:50:26.630000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_214627/abc_feature_minerfund_160
2023-01-13T21:50:26.630000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_214627/abc_feature_minerfund_160/test_framework.log
2023-01-13T21:50:26.630000Z TestFramework (ERROR): 
2023-01-13T21:50:26.630000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_214627/abc_feature_minerfund_160' to consolidate all logs
2023-01-13T21:50:26.630000Z TestFramework (ERROR): 
2023-01-13T21:50:26.630000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-01-13T21:50:26.630000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-01-13T21:50:26.630000Z TestFramework (ERROR):
====== Bitcoin ABC functional tests with the next upgrade activated: abc_feature_minerfund.py ======

------- Stdout: -------
2023-01-13T21:54:41.019000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_215036/abc_feature_minerfund_160
2023-01-13T21:54:41.430000Z TestFramework (INFO): Create some history
2023-01-13T21:54:43.510000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 135, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in run_test
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/test_framework/test_framework.py", line 703, in wait_until
    return wait_until_helper(test_function, timeout=timeout,
  File "/work/test/functional/test_framework/util.py", line 273, in wait_until_helper
    if predicate():
  File "/work/test/functional/abc_feature_minerfund.py", line 156, in <lambda>
    self.wait_until(lambda: parked_block(first_block_no_miner_fund))
  File "/work/test/functional/abc_feature_minerfund.py", line 153, in parked_block
    assert tip["status"] != "active"
AssertionError
2023-01-13T21:54:43.561000Z TestFramework (INFO): Stopping nodes
2023-01-13T21:54:43.663000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_215036/abc_feature_minerfund_160
2023-01-13T21:54:43.663000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_215036/abc_feature_minerfund_160/test_framework.log
2023-01-13T21:54:43.663000Z TestFramework (ERROR): 
2023-01-13T21:54:43.663000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20230113_215036/abc_feature_minerfund_160' to consolidate all logs
2023-01-13T21:54:43.663000Z TestFramework (ERROR): 
2023-01-13T21:54:43.663000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-01-13T21:54:43.663000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2023-01-13T21:54:43.663000Z 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

Fabien added a subscriber: Fabien.Jan 17 2023, 17:25

Fabien added inline comments.

src/minerfund.cpp
81	No need for the pre-gluon address
src/rpc/mining.cpp
1044
src/validation.cpp
3043	This will be duplicated for each policy we want to add. It might be better to start thinking at scale here as it's fairly simple to do. Example (don't copy the names, there might be better ones) : Create a BlockPolicy object that holds a callback function and its own cache data Create a BlockPolicyContainer that we can add several BlockPolicy into, can run them all and aggregate the result This will make it easier to test a new policy, and also let the container run each policy in parallel.
3059	That's worth a comment about why it's needed. It's also a shame it can't be turned into a single loop, but I don't know if there is a solution here
3067	Supposedly the consensus check passed already, you can avoid the duplication here
3078	This is why I don't like the function name. Here you return false because the check failed BUT it's not supposed to park because the block is invalid. In definitive this will cause your logic to connect the invalid block ! Other failures you will return true to indicate it needs to be parked, which is backwards imo: a check fails then you return true ?
3080	Should exclude the coinbase transaction
3106	That reads weird, see above comment
3176
3176	pthisBlock is not a good name. blockCandidate ? tipCandidate ?
3182
3189	Is that necessary ?
3205	I think you should add the policy failures to the BlockValidationState instead
src/validation.h
1001	Not a very good name, it implies that you know the outcome for the checks. Just CheckBlockPolicies ? We also discussed using another name instead of 'policies'