diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -604,6 +604,7 @@ node/transaction.cpp node/ui_interface.cpp noui.cpp + parkingpresets.cpp policy/fees.cpp policy/packages.cpp policy/settings.cpp diff --git a/src/minerfund.h b/src/minerfund.h --- a/src/minerfund.h +++ b/src/minerfund.h @@ -19,14 +19,20 @@ Amount GetMinerFundAmount(const Amount &coinbaseValue); +enum MinerFundFlags : uint8_t { + CONSENSUS = (1 << 0), + POLICY = (1 << 1), +}; + std::vector GetMinerFundWhitelist(const Consensus::Params ¶ms, - const CBlockIndex *pindexPrev); + const CBlockIndex *pindexPrev, const uint8_t flags); /** * Returns false if there is an invalid miner fund. True otherwise. */ bool CheckMinerFund(const Consensus::Params ¶ms, const CBlockIndex *pindex, - const CBlock &block, const Amount &blockFees); + const CBlock &block, const Amount &blockFees, + const uint8_t flags = MinerFundFlags::CONSENSUS); #endif // BITCOIN_MINERFUND_H diff --git a/src/minerfund.cpp b/src/minerfund.cpp --- a/src/minerfund.cpp +++ b/src/minerfund.cpp @@ -50,7 +50,7 @@ std::vector GetMinerFundWhitelist(const Consensus::Params ¶ms, - const CBlockIndex *pindexPrev) { + const CBlockIndex *pindexPrev, const uint8_t flags) { if (!gArgs.GetBoolArg("-enableminerfund", params.enableMinerFund)) { return {}; } @@ -59,12 +59,26 @@ return {}; } - return {GetMinerFundDestination(!IsGluonEnabled(params, pindexPrev))}; + if ((flags & MinerFundFlags::CONSENSUS) && + !IsWellingtonEnabled(params, pindexPrev)) { + // Wellington has not activated yet, so enforce the miner fund in + // consensus checks. + return {GetMinerFundDestination(!IsGluonEnabled(params, pindexPrev))}; + } + + if ((flags & MinerFundFlags::POLICY) && + IsWellingtonEnabled(params, pindexPrev)) { + // Wellington has activated, so enforce the miner fund in policy checks. + return {GetMinerFundDestination(false)}; + } + + return {}; } bool CheckMinerFund(const Consensus::Params ¶ms, const CBlockIndex *pindex, - const CBlock &block, const Amount &blockFees) { - const auto whitelist = GetMinerFundWhitelist(params, pindex->pprev); + const CBlock &block, const Amount &blockFees, + const uint8_t flags) { + const auto whitelist = GetMinerFundWhitelist(params, pindex->pprev, flags); if (whitelist.empty()) { return true; } diff --git a/src/node/miner.cpp b/src/node/miner.cpp --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -193,8 +193,9 @@ nFees + GetBlockSubsidy(nHeight, consensusParams); coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0; - const std::vector whitelisted = - GetMinerFundWhitelist(consensusParams, pindexPrev); + const std::vector whitelisted = GetMinerFundWhitelist( + consensusParams, pindexPrev, + MinerFundFlags::CONSENSUS | MinerFundFlags::POLICY); if (!whitelisted.empty()) { const Amount fund = GetMinerFundAmount(coinbaseTx.vout[0].nValue); coinbaseTx.vout[0].nValue -= fund; diff --git a/src/parkingpresets.h b/src/parkingpresets.h new file mode 100644 --- /dev/null +++ b/src/parkingpresets.h @@ -0,0 +1,46 @@ +// Copyright (c) 2023 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_PARKINGPRESETS_H +#define BITCOIN_PARKINGPRESETS_H + +#include +#include + +#include +#include + +class CBlockIndex; + +namespace Consensus { +struct Params; +} + +struct ParkingPreset { + virtual ~ParkingPreset() {} + + virtual bool operator()() { return false; } +}; + +struct MinerFundPolicy : ParkingPreset { + const CBlock &m_block; + const Amount &m_blockFees; + const Consensus::Params &m_consensusParams; + const CBlockIndex *m_pindex; + + MinerFundPolicy(const Consensus::Params &consensusParams, + const CBlockIndex *pindex, const CBlock &block, + const Amount &blockFees) + : m_block(block), m_blockFees(blockFees), + m_consensusParams(consensusParams), m_pindex(pindex) {} + + bool operator()() override; +}; + +std::vector> +GetParkingPresetsForBlock(const Consensus::Params ¶ms, + const CBlockIndex *pindex, const CBlock &block, + const Amount &blockFees); + +#endif // BITCOIN_PARKINGPRESETS_H diff --git a/src/parkingpresets.cpp b/src/parkingpresets.cpp new file mode 100644 --- /dev/null +++ b/src/parkingpresets.cpp @@ -0,0 +1,21 @@ +// Copyright (c) 2023 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +std::vector> +GetParkingPresetsForBlock(const Consensus::Params ¶ms, + const CBlockIndex *pindex, const CBlock &block, + const Amount &blockFees) { + return { + std::make_shared(params, pindex, block, blockFees)}; +} + +bool MinerFundPolicy::operator()() { + return !CheckMinerFund(m_consensusParams, m_pindex, m_block, m_blockFees, + MinerFundFlags::POLICY); +} diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1035,8 +1035,9 @@ UniValue minerFundList(UniValue::VARR); const Consensus::Params &consensusParams = chainparams.GetConsensus(); - for (const auto &fundDestination : - GetMinerFundWhitelist(consensusParams, pindexPrev)) { + for (const auto &fundDestination : GetMinerFundWhitelist( + consensusParams, pindexPrev, + MinerFundFlags::CONSENSUS | MinerFundFlags::POLICY)) { minerFundList.push_back( EncodeDestination(fundDestination, config)); } diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -689,6 +690,15 @@ const CBlockIndex *m_avalancheFinalizedBlockIndex GUARDED_BY(cs_avalancheFinalizedBlockIndex) = nullptr; + /** + * Filter to prevent parking a block due to block policies more than once. + * After first application of block policies, Avalanche voting will + * determine the final acceptance state. Rare false positives will be + * reconciled by the network and should not have any negative impact. + */ + CRollingBloomFilter m_filterParkingPresetsApplied = + CRollingBloomFilter{1000, 0.000001}; + public: //! Reference to a BlockManager instance which itself is shared across all //! CChainState instances. @@ -852,7 +862,8 @@ EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool ConnectBlock(const CBlock &block, BlockValidationState &state, CBlockIndex *pindex, CCoinsViewCache &view, - BlockValidationOptions options, bool fJustCheck = false) + BlockValidationOptions options, + Amount *blockFees = nullptr, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. @@ -973,7 +984,8 @@ CBlockIndex *pindexNew, const std::shared_ptr &pblock, ConnectTrace &connectTrace, - DisconnectedBlockTransactions &disconnectpool) + DisconnectedBlockTransactions &disconnectpool, + Amount *blockFees) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); void InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state) diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -1771,7 +1772,7 @@ bool CChainState::ConnectBlock(const CBlock &block, BlockValidationState &state, CBlockIndex *pindex, CCoinsViewCache &view, BlockValidationOptions options, - bool fJustCheck) { + Amount *blockFees, bool fJustCheck) { AssertLockHeld(cs_main); assert(pindex); @@ -2149,6 +2150,10 @@ "bad-cb-amount"); } + if (blockFees) { + *blockFees = nFees; + } + if (!CheckMinerFund(consensusParams, pindex, block, nFees)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cb-minerfund"); @@ -2604,7 +2609,8 @@ CBlockIndex *pindexNew, const std::shared_ptr &pblock, ConnectTrace &connectTrace, - DisconnectedBlockTransactions &disconnectpool) { + DisconnectedBlockTransactions &disconnectpool, + Amount *blockFees) { AssertLockHeld(cs_main); if (m_mempool) { AssertLockHeld(m_mempool->cs); @@ -2637,7 +2643,7 @@ { CCoinsViewCache view(&CoinsTip()); bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, - BlockValidationOptions(config)); + BlockValidationOptions(config), blockFees); GetMainSignals().BlockChecked(blockConnecting, state); if (!rv) { if (state.IsInvalid()) { @@ -2964,11 +2970,30 @@ // Connect new blocks. for (CBlockIndex *pindexConnect : reverse_iterate(vpindexToConnect)) { - if (!ConnectTip(config, state, pindexConnect, - pindexConnect == pindexMostWork - ? pblock - : std::shared_ptr(), - connectTrace, disconnectpool)) { + std::shared_ptr pblockConnect = + pindexConnect == pindexMostWork + ? pblock + : std::shared_ptr(); + // Read block from disk. + if (!pblockConnect) { + std::shared_ptr pblockLoad = std::make_shared(); + if (!ReadBlockFromDisk(*pblockLoad, pindexConnect, + m_params.GetConsensus())) { + // A system error occurred (disk space, database error, ...) + // Make the mempool consistent with the current tip, just in + // case any observers try to use it before shutdown. + if (m_mempool) { + disconnectpool.updateMempoolForReorg(config, *this, + false, *m_mempool); + } + return AbortNode(state, "Failed to read block"); + } + pblockConnect = pblockLoad; + } + + Amount blockFees{Amount::zero()}; + if (!ConnectTip(config, state, pindexConnect, pblockConnect, + connectTrace, disconnectpool, &blockFees)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (state.GetResult() != @@ -2990,6 +3015,59 @@ } return false; } else { + // Only check block parking presets the first time the block is + // connected. Avalanche voting can override the parking + // decision made by these presets. + const BlockHash blockhash = pindexConnect->GetBlockHash(); + if (!IsInitialBlockDownload() && + !m_filterParkingPresetsApplied.contains(blockhash)) { + m_filterParkingPresetsApplied.insert(blockhash); + + // The block is valid by consensus rules so now we check if + // the block passes all block preset rules. If not, then + // park the block. + const CBlock &connectedBlock = *pblockConnect; + const auto parkingPresets = GetParkingPresetsForBlock( + m_params.GetConsensus(), pindexConnect, connectedBlock, + blockFees); + if (std::any_of( + parkingPresets.begin(), parkingPresets.end(), + [&](const auto preset) { return (*preset)(); })) { + LogPrintf("Park block %s because it violated a block " + "policy\n", + blockhash.ToString()); + pindexConnect->nStatus = + pindexConnect->nStatus.withParked(); + m_blockman.m_dirty_blockindex.insert(pindexConnect); + + // The parked block is still the chaintip so disconnect + // it. + if (!DisconnectTip(state, &disconnectpool)) { + // This is likely a fatal error, but keep the + // mempool consistent, just in case. Only remove + // from the mempool in this case. + if (m_mempool) { + disconnectpool.updateMempoolForReorg( + config, *this, false, *m_mempool); + } + + // If we're unable to disconnect a block during + // normal operation, then that is a failure of our + // local system -- we should abort rather than stay + // on a less work chain. + AbortNode(state, "Failed to disconnect block; see " + "debug.log for details"); + return false; + } + fBlocksDisconnected = true; + + // TODO fix this hackiness + fInvalidFound = true; + fContinue = false; + break; + } + } + PruneBlockIndexCandidates(); if (!pindexOldTip || m_chain.Tip()->nChainWork > pindexOldTip->nChainWork) { @@ -4412,7 +4490,7 @@ } if (!chainstate.ConnectBlock(block, state, &indexDummy, viewNew, - validationOptions, true)) { + validationOptions, nullptr, true)) { 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 @@ -13,6 +13,7 @@ from test_framework.txtools import pad_tx from test_framework.util import assert_equal, assert_greater_than_or_equal +WELLINGTON_ACTIVATION_TIME = 2100000600 MINER_FUND_RATIO = 8 MINER_FUND_ADDR = 'ecregtest:prfhcnyqnl5cgrnmlfmms675w93ld7mvvq9jcw0zsn' MINER_FUND_ADDR_AXION = 'ecregtest:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgz0wv9ltl' @@ -21,26 +22,29 @@ class MinerFundTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 1 + self.num_nodes = 2 self.extra_args = [[ '-enableminerfund', + '-wellingtonactivationtime={}'.format(WELLINGTON_ACTIVATION_TIME), + ], [ + '-wellingtonactivationtime={}'.format(WELLINGTON_ACTIVATION_TIME), ]] def run_test(self): node = self.nodes[0] self.log.info('Create some history') - self.generate(node, 50, sync_fun=self.no_op) + self.generate(node, 10) - def get_best_coinbase(): - return node.getblock(node.getbestblockhash(), 2)['tx'][0] + def get_best_coinbase(n): + return n.getblock(n.getbestblockhash(), 2)['tx'][0] - coinbase = get_best_coinbase() + coinbase = get_best_coinbase(node) assert_greater_than_or_equal(len(coinbase['vout']), 2) block_reward = sum([vout['value'] for vout in coinbase['vout']]) def check_miner_fund_output(expected_address): - coinbase = get_best_coinbase() + coinbase = get_best_coinbase(node) assert_equal(len(coinbase['vout']), 2) assert_equal( coinbase['vout'][1]['scriptPubKey']['addresses'][0], @@ -113,6 +117,44 @@ node.submitblock(ToHex(good_block)) assert_equal(node.getbestblockhash(), good_block.hash) + # Move MTP forward to wellington activation. Next block will enforce + # new rules. + address = node.get_deterministic_priv_key().address + for n in self.nodes: + n.setmocktime(WELLINGTON_ACTIVATION_TIME) + self.generatetoaddress(node, nblocks=6, address=address) + assert_equal( + node.getblockchaininfo()['mediantime'], + WELLINGTON_ACTIVATION_TIME) + + # First block that does not have miner fund as a consensus requirement. + # node0 still mines a block with a coinbase output to the miner fund. + first_block_has_miner_fund = self.generatetoaddress( + node, nblocks=1, address=address)[0] + check_miner_fund_output(MINER_FUND_ADDR) + + # Invalidate it + for n in self.nodes: + n.invalidateblock(first_block_has_miner_fund) + + # node1 does not mine a block with 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] + coinbase = get_best_coinbase(self.nodes[1]) + assert_equal(len(coinbase['vout']), 1) + + # node0 parks the block since the miner fund is enforced by policy. + def parked_block(blockhash): + for tip in node.getchaintips(): + if tip["hash"] == blockhash: + assert tip["status"] != "active" + return tip["status"] == "parked" + return False + self.wait_until(lambda: parked_block(first_block_no_miner_fund)) + if __name__ == '__main__': MinerFundTest().main()