Page MenuHomePhabricator
Files F13115417

D13482.id39175.diff
No OneTemporary
Actions

D13482.id39175.diff
View Options

diff --git a/src/policy/block/minerfund.h b/src/policy/block/minerfund.h
--- a/src/policy/block/minerfund.h
+++ b/src/policy/block/minerfund.h
@@ -29,7 +29,7 @@
: m_block(block), m_blockReward(blockReward),
m_consensusParams(consensusParams), m_blockIndex(blockIndex) {}
- bool operator()() override;
+ bool operator()(BlockPolicyValidationState &state) override;
};
#endif // BITCOIN_POLICY_BLOCK_MINERFUND_H
diff --git a/src/policy/block/minerfund.cpp b/src/policy/block/minerfund.cpp
--- a/src/policy/block/minerfund.cpp
+++ b/src/policy/block/minerfund.cpp
@@ -8,7 +8,7 @@
#include <consensus/activation.h>
#include <minerfund.h>
-bool MinerFundPolicy::operator()() {
+bool MinerFundPolicy::operator()(BlockPolicyValidationState &state) {
if (!m_blockIndex.pprev ||
!IsWellingtonEnabled(m_consensusParams, m_blockIndex.pprev)) {
// Do not apply the miner fund policy before Wellington activates.
@@ -16,6 +16,12 @@
}
assert(m_block.vtx.size());
- return CheckMinerFund(m_consensusParams, m_blockIndex.pprev,
- m_block.vtx[0]->vout, m_blockReward);
+ if (!CheckMinerFund(m_consensusParams, m_blockIndex.pprev,
+ m_block.vtx[0]->vout, m_blockReward)) {
+ return state.Invalid(BlockPolicyValidationResult::POLICY_VIOLATION,
+ "policy-bad-miner-fund",
+ strprintf("Block %s violates miner fund policy",
+ m_blockIndex.GetBlockHash().ToString()));
+ }
+ return true;
}
diff --git a/src/policy/block/parkingpolicy.h b/src/policy/block/parkingpolicy.h
--- a/src/policy/block/parkingpolicy.h
+++ b/src/policy/block/parkingpolicy.h
@@ -5,11 +5,26 @@
#ifndef BITCOIN_POLICY_BLOCK_PARKINGPOLICY_H
#define BITCOIN_POLICY_BLOCK_PARKINGPOLICY_H
+#include <consensus/validation.h>
+
+/**
+ * A "reason" why a block did not pass block policy checks.
+ */
+enum class BlockPolicyValidationResult {
+ //! Initial value. Policy rule has not yet been violated.
+ POLICY_RESULT_UNSET = 0,
+ //! A block policy rule was violated. This block should be parked.
+ POLICY_VIOLATION,
+};
+
+class BlockPolicyValidationState
+ : public ValidationState<BlockPolicyValidationResult> {};
+
struct ParkingPolicy {
virtual ~ParkingPolicy() {}
// Return true if a policy succeeds. False will park the block.
- virtual bool operator()() = 0;
+ virtual bool operator()(BlockPolicyValidationState &state) = 0;
};
#endif // BITCOIN_POLICY_BLOCK_PARKINGPOLICY_H
diff --git a/src/test/policy_block_tests.cpp b/src/test/policy_block_tests.cpp
--- a/src/test/policy_block_tests.cpp
+++ b/src/test/policy_block_tests.cpp
@@ -46,8 +46,11 @@
const CChainParams &chainparams = Params();
const Consensus::Params &consensusParams = chainparams.GetConsensus();
+ std::array<BlockHash, 12> blockhashes;
std::array<CBlockIndex, 12> blocks;
for (size_t i = 1; i < blocks.size(); ++i) {
+ blockhashes[i] = BlockHash(uint256(i));
+ blocks[i].phashBlock = &blockhashes[i];
blocks[i].pprev = &blocks[i - 1];
blocks[i].nHeight = consensusParams.gluonHeight + i;
}
@@ -72,14 +75,22 @@
CBlock block = BlockWithoutMinerFund(blockReward);
MinerFundPolicy check(consensusParams, blockIndex, block,
blockReward);
- BOOST_CHECK(check());
+
+ BlockPolicyValidationState state;
+ BOOST_CHECK(check(state));
+ BOOST_CHECK(state.IsValid());
}
};
auto checkMinerFundPolicy = [&](CBlock block, bool expected) {
+ BlockPolicyValidationState state;
BOOST_CHECK_EQUAL(MinerFundPolicy(consensusParams, lastBlockIndexRef,
- block, blockReward)(),
+ block, blockReward)(state),
expected);
+ BOOST_CHECK_EQUAL(state.IsValid(), expected);
+ if (!expected) {
+ BOOST_CHECK_EQUAL(state.GetRejectReason(), "policy-bad-miner-fund");
+ }
};
const std::vector<Amount> minerFundsTooSmall = {1 * SATOSHI, minerFund / 2,
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2727,10 +2727,19 @@
parkingPolicies.emplace_back(std::make_unique<MinerFundPolicy>(
consensusParams, *pindexNew, blockConnecting, blockReward));
- if (!std::all_of(parkingPolicies.begin(), parkingPolicies.end(),
- [&](const auto &policy) { return (*policy)(); })) {
- LogPrintf("Park block %s because it violated a block policy\n",
- blockhash.ToString());
+ // If any block policy is violated, bail on the first one found
+ if (std::find_if_not(
+ parkingPolicies.begin(), parkingPolicies.end(),
+ [&](const auto &policy) {
+ BlockPolicyValidationState blockPolicyState;
+ bool ret = (*policy)(blockPolicyState);
+ if (!ret) {
+ LogPrintf("Park block because it "
+ "violated a block policy: %s\n",
+ blockPolicyState.ToString());
+ }
+ return ret;
+ }) != parkingPolicies.end()) {
pindexNew->nStatus = pindexNew->nStatus.withParked();
m_blockman.m_dirty_blockindex.insert(pindexNew);
return false;
diff --git a/test/functional/abc_feature_minerfund.py b/test/functional/abc_feature_minerfund.py
--- a/test/functional/abc_feature_minerfund.py
+++ b/test/functional/abc_feature_minerfund.py
@@ -25,9 +25,9 @@
self.num_nodes = 2
self.extra_args = [[
'-enableminerfund',
- '-wellingtonactivationtime={}'.format(WELLINGTON_ACTIVATION_TIME),
+ f'-wellingtonactivationtime={WELLINGTON_ACTIVATION_TIME}',
], [
- '-wellingtonactivationtime={}'.format(WELLINGTON_ACTIVATION_TIME),
+ f'-wellingtonactivationtime={WELLINGTON_ACTIVATION_TIME}',
]]
def run_test(self):
@@ -138,11 +138,13 @@
n.invalidateblock(first_block_has_miner_fund)
# node1 mines a block without a coinbase output to the miner fund.
- first_block_no_miner_fund = self.generatetoaddress(
- self.nodes[1],
- nblocks=1,
- address=address,
- sync_fun=self.no_op)[0]
+ with node.assert_debug_log(expected_msgs=['policy-bad-miner-fund']):
+ first_block_no_miner_fund = self.generatetoaddress(
+ self.nodes[1],
+ nblocks=1,
+ address=address,
+ sync_fun=self.no_op)[0]
+
coinbase = get_best_coinbase(self.nodes[1])
assert_equal(len(coinbase['vout']), 1)

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 1, 11:04 (11 h, 37 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5187476
Default Alt Text
D13482.id39175.diff (6 KB)

Event Timeline