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 #include -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 + +/** + * 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 {}; + 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 blockhashes; std::array 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 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( 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)