From 678978f6f64a24972dec19f75f6b92b0e2643d44 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 11 Mar 2018 02:52:10 +0000 Subject: [PATCH] Policy: Restore support for mining based on coin-age priority This reverts commit 7036cf52aa080d2a63993c2298555252d507dd2f (and much more). --- src/Makefile.am | 2 + src/bench/mempool_eviction.cpp | 4 +- src/bench/mempool_stress.cpp | 3 +- src/bench/rpc_mempool.cpp | 2 +- src/init.cpp | 3 +- src/kernel/mempool_entry.h | 32 ++- src/node/interfaces.cpp | 2 +- src/node/miner.cpp | 15 +- src/node/miner.h | 13 + src/policy/coin_age_priority.cpp | 253 +++++++++++++++++++ src/policy/coin_age_priority.h | 26 ++ src/policy/policy.h | 4 + src/test/fuzz/partially_downloaded_block.cpp | 2 +- src/test/fuzz/policy_estimator.cpp | 2 + src/test/fuzz/rbf.cpp | 4 +- src/test/fuzz/util/mempool.cpp | 16 +- src/test/fuzz/util/mempool.h | 1 + src/test/util/setup_common.cpp | 6 +- src/test/util/txmempool.cpp | 4 +- src/txmempool.cpp | 9 + src/txmempool.h | 6 + src/validation.cpp | 11 +- 22 files changed, 404 insertions(+), 16 deletions(-) create mode 100644 src/policy/coin_age_priority.cpp create mode 100644 src/policy/coin_age_priority.h diff --git a/src/Makefile.am b/src/Makefile.am index 1ccb5332c4..2242e57e69 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -243,6 +243,7 @@ BITCOIN_CORE_H = \ node/warnings.h \ noui.h \ outputtype.h \ + policy/coin_age_priority.h \ policy/feerate.h \ policy/fees.h \ policy/fees_args.h \ @@ -445,6 +446,7 @@ libbitcoin_node_a_SOURCES = \ node/utxo_snapshot.cpp \ node/warnings.cpp \ noui.cpp \ + policy/coin_age_priority.cpp \ policy/fees.cpp \ policy/fees_args.cpp \ policy/packages.cpp \ diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index 1a9b013277..e118fca314 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -12,13 +12,15 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { int64_t nTime = 0; + double dPriority = 10.0; unsigned int nHeight = 1; uint64_t sequence = 0; bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; pool.addUnchecked(CTxMemPoolEntry( - tx, nFee, nTime, nHeight, sequence, + tx, nFee, nTime, dPriority, nHeight, sequence, + tx->GetValueOut(), spendsCoinbase, sigOpCost, lp)); } diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 3c82f55c19..88d7a255f3 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -16,12 +16,13 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { int64_t nTime = 0; + const double coinage_priority = 10.0; unsigned int nHeight = 1; uint64_t sequence = 0; bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); + pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, coinage_priority, nHeight, sequence, tx->GetValueOut(), spendsCoinbase, sigOpCost, lp)); } struct Available { diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index a55aa0c794..4e6a73ff7c 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -16,7 +16,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_priority=*/0, /*entry_height=*/0, /*entry_sequence=*/0, /*in_chain_input_value=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); } static void RpcMempool(benchmark::Bench& bench) diff --git a/src/init.cpp b/src/init.cpp index e8afc9e3a3..cf092fce9f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -631,7 +631,7 @@ void SetupServerArgs(ArgsManager& argsman) strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", Ticks(DEFAULT_MAX_TIP_AGE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-printpriority", strprintf("Log transaction fee rate in " + CURRENCY_UNIT + "/kvB when mining blocks (default: %u)", DEFAULT_PRINT_MODIFIED_FEE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-printpriority", strprintf("Log transaction priority and fee rate in " + CURRENCY_UNIT + "/kvB when mining blocks (default: %u)", DEFAULT_PRINT_MODIFIED_FEE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-uaappend=", "Append literal to the user agent string (should only be used for software embedding)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-uacomment=", "Append comment to the user agent string", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); @@ -660,6 +660,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-blockmaxsize=", strprintf("Set maximum block size in bytes (default: %d)", DEFAULT_BLOCK_MAX_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-blockmaxweight=", strprintf("Set maximum BIP141 block weight (default: %d)", DEFAULT_BLOCK_MAX_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-blockmintxfee=", strprintf("Set lowest fee rate (in %s/kvB) for transactions to be included in block creation. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_BLOCK_MIN_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); + argsman.AddArg("-blockprioritysize=", strprintf("Set maximum size of high-priority/low-fee transactions in bytes (default: %d)", DEFAULT_BLOCK_PRIORITY_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-blockversion=", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 5db9c2cac0..0d13d65476 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -8,12 +8,14 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -84,9 +86,14 @@ private: const size_t nUsageSize; //!< ... and total memory usage const int64_t nTime; //!< Local time when entering the mempool const uint64_t entry_sequence; //!< Sequence number used to determine whether this transaction is too recent for relay + const double entryPriority; //!< Priority when entering the mempool const unsigned int entryHeight; //!< Chain height when entering the mempool + double cachedPriority; //!< Last calculated priority + unsigned int cachedHeight; //!< Height at which priority was last calculated + CAmount inChainInputValue; //!< Sum of all txin values that are already in blockchain const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost + const size_t nModSize; //!< Cached modified size for priority CAmount m_modified_fee; //!< Used for determining the priority of the transaction for mining in a block mutable LockPoints lockPoints; //!< Track the height and time at which tx was final @@ -107,7 +114,8 @@ private: public: CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, - int64_t time, unsigned int entry_height, uint64_t entry_sequence, + int64_t time, double entry_priority, unsigned int entry_height, uint64_t entry_sequence, + CAmount in_chain_input_value, bool spends_coinbase, int64_t sigops_cost, LockPoints lp) : tx{tx}, @@ -116,16 +124,25 @@ public: nUsageSize{RecursiveDynamicUsage(tx)}, nTime{time}, entry_sequence{entry_sequence}, + entryPriority{entry_priority}, entryHeight{entry_height}, + cachedPriority{entry_priority}, + // Since entries arrive *after* the tip's height, their entry priority is for the height+1 + cachedHeight{entry_height + 1}, + inChainInputValue{in_chain_input_value}, spendsCoinbase{spends_coinbase}, sigOpCost{sigops_cost}, + nModSize{CalculateModifiedSize(*tx, GetTxSize())}, m_modified_fee{nFee}, lockPoints{lp}, nSizeWithDescendants{GetTxSize()}, nModFeesWithDescendants{nFee}, nSizeWithAncestors{GetTxSize()}, nModFeesWithAncestors{nFee}, - nSigOpCostWithAncestors{sigOpCost} {} + nSigOpCostWithAncestors{sigOpCost} { + CAmount nValueIn = tx->GetValueOut() + nFee; + assert(inChainInputValue <= nValueIn); + } CTxMemPoolEntry(ExplicitCopyTag, const CTxMemPoolEntry& entry) : CTxMemPoolEntry(entry) {} CTxMemPoolEntry& operator=(const CTxMemPoolEntry&) = delete; @@ -136,6 +153,17 @@ public: const CTransaction& GetTx() const { return *this->tx; } CTransactionRef GetSharedTx() const { return this->tx; } + double GetStartingPriority() const {return entryPriority; } + /** + * Fast calculation of priority as update from cached value, but only valid if + * currentHeight is greater than last height it was recalculated. + */ + double GetPriority(unsigned int currentHeight) const; + /** + * Recalculate the cached priority as of currentHeight and adjust inChainInputValue by + * valueInCurrentBlock which represents input that was just added to or removed from the blockchain. + */ + void UpdateCachedPriority(unsigned int currentHeight, CAmount valueInCurrentBlock); const CAmount& GetFee() const { return nFee; } int32_t GetTxSize() const { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 9cdc64b77c..5a7abfb9ff 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -726,7 +726,7 @@ public: { if (!m_node.mempool) return {}; LockPoints lp; - CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(tx, 0, 0, 0, 0, 0, 0, false, 0, lp); LOCK(m_node.mempool->cs); return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize()); } diff --git a/src/node/miner.cpp b/src/node/miner.cpp index ef57f31cfc..194cb2a228 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -124,6 +124,9 @@ void BlockAssembler::resetBlock() // These counters do not include coinbase tx nBlockTx = 0; nFees = 0; + + lastFewTxs = 0; + blockFinished = false; } std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) @@ -143,6 +146,9 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc pblock->vtx.emplace_back(); pblocktemplate->vTxFees.push_back(-1); // updated at end pblocktemplate->vTxSigOpsCost.push_back(-1); // updated at end + if (m_options.print_modified_fee) { + pblocktemplate->vTxPriorities.push_back(-1); // n/a + } LOCK(::cs_main); CBlockIndex* pindexPrev = m_chainstate.m_chain.Tip(); @@ -163,6 +169,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc int nDescendantsUpdated = 0; if (m_mempool) { LOCK(m_mempool->cs); + addPriorityTxs(*m_mempool, nPackagesSelected); addPackageTxs(*m_mempool, nPackagesSelected, nDescendantsUpdated); } @@ -273,9 +280,12 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) inBlock.insert(iter); if (m_options.print_modified_fee) { - LogPrintf("fee rate %s txid %s\n", + double dPriority = iter->GetPriority(nHeight); + LogPrintf("priority %.1f fee rate %s txid %s\n", + dPriority, CFeeRate(iter->GetModifiedFee(), iter->GetTxSize()).ToString(), iter->GetTx().GetHash().ToString()); + pblocktemplate->vTxPriorities.push_back(dPriority); } } @@ -340,6 +350,9 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // Keep track of entries that failed inclusion, to avoid duplicate work CTxMemPool::setEntries failedTx; + // Start by adding all descendants of previously added txs to mapModifiedTx + // and modifying them for their already included ancestors + nDescendantsUpdated += UpdatePackagesForAdded(mempool, inBlock, mapModifiedTx); CTxMemPool::indexed_transaction_set::index::type::iterator mi = mempool.mapTx.get().begin(); CTxMemPool::txiter iter; diff --git a/src/node/miner.h b/src/node/miner.h index 43a2c7337f..c36f4b258f 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -38,6 +38,7 @@ struct CBlockTemplate CBlock block; std::vector vTxFees; std::vector vTxSigOpsCost; + std::vector vTxPriorities; std::vector vchCoinbaseCommitment; }; @@ -160,6 +161,10 @@ private: const CTxMemPool* const m_mempool; Chainstate& m_chainstate; + // Variables used for addPriorityTxs + int lastFewTxs; + bool blockFinished; + public: struct Options : BlockCreateOptions { // Configuration parameters for the block size @@ -190,11 +195,19 @@ private: void AddToBlock(CTxMemPool::txiter iter); // Methods for how to add transactions to a block. + /** Add transactions based on tx "priority" */ + void addPriorityTxs(const CTxMemPool& mempool, int &nPackagesSelected) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs); /** Add transactions based on feerate including unconfirmed ancestors * Increments nPackagesSelected / nDescendantsUpdated with corresponding * statistics from the package selection (for logging statistics). */ void addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs); + // helper function for addPriorityTxs + /** Test if tx will still "fit" in the block */ + bool TestForBlock(CTxMemPool::txiter iter); + /** Test if tx still has unconfirmed parents not yet in block */ + bool isStillDependent(const CTxMemPool& mempool, CTxMemPool::txiter iter) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs); + // helper functions for addPackageTxs() /** Remove confirmed (inBlock) entries from given set */ void onlyUnconfirmed(CTxMemPool::setEntries& testSet); diff --git a/src/policy/coin_age_priority.cpp b/src/policy/coin_age_priority.cpp new file mode 100644 index 0000000000..d9bd430cfc --- /dev/null +++ b/src/policy/coin_age_priority.cpp @@ -0,0 +1,253 @@ +// Copyright (c) 2012-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +using node::BlockAssembler; + +unsigned int CalculateModifiedSize(const CTransaction& tx, unsigned int nTxSize) +{ + // In order to avoid disincentivizing cleaning up the UTXO set we don't count + // the constant overhead for each txin and up to 110 bytes of scriptSig (which + // is enough to cover a compressed pubkey p2sh redemption) for priority. + // Providing any more cleanup incentive than making additional inputs free would + // risk encouraging people to create junk outputs to redeem later. + if (nTxSize == 0) + nTxSize = (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; + for (std::vector::const_iterator it(tx.vin.begin()); it != tx.vin.end(); ++it) + { + unsigned int offset = 41U + std::min(110U, (unsigned int)it->scriptSig.size()); + if (nTxSize > offset) + nTxSize -= offset; + } + return nTxSize; +} + +double ComputePriority(const CTransaction& tx, double dPriorityInputs, unsigned int nTxSize) +{ + nTxSize = CalculateModifiedSize(tx, nTxSize); + if (nTxSize == 0) return 0.0; + + return dPriorityInputs / nTxSize; +} + +double GetPriority(const CTransaction &tx, const CCoinsViewCache& view, int nHeight, CAmount &inChainInputValue) +{ + inChainInputValue = 0; + if (tx.IsCoinBase()) + return 0.0; + double dResult = 0.0; + for (const CTxIn& txin : tx.vin) + { + const Coin& coin = view.AccessCoin(txin.prevout); + if (coin.IsSpent()) { + continue; + } + if (coin.nHeight <= nHeight) { + dResult += (double)(coin.out.nValue) * (nHeight - coin.nHeight); + inChainInputValue += coin.out.nValue; + } + } + return ComputePriority(tx, dResult); +} + +void CTxMemPoolEntry::UpdateCachedPriority(unsigned int currentHeight, CAmount valueInCurrentBlock) +{ + int heightDiff = int(currentHeight) - int(cachedHeight); + double deltaPriority = ((double)heightDiff*inChainInputValue)/nModSize; + cachedPriority += deltaPriority; + cachedHeight = currentHeight; + inChainInputValue += valueInCurrentBlock; + assert(MoneyRange(inChainInputValue)); +} + +struct update_priority +{ + update_priority(unsigned int _height, CAmount _value) : + height(_height), value(_value) + {} + + void operator() (CTxMemPoolEntry &e) + { e.UpdateCachedPriority(height, value); } + + private: + unsigned int height; + CAmount value; +}; + +void CTxMemPool::UpdateDependentPriorities(const CTransaction &tx, unsigned int nBlockHeight, bool addToChain) +{ + LOCK(cs); + for (unsigned int i = 0; i < tx.vout.size(); i++) { + auto it = mapNextTx.find(COutPoint(tx.GetHash(), i)); + if (it == mapNextTx.end()) + continue; + uint256 hash = it->second->GetHash(); + txiter iter = mapTx.find(hash); + mapTx.modify(iter, update_priority(nBlockHeight, addToChain ? tx.vout[i].nValue : -tx.vout[i].nValue)); + } +} + +double +CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const +{ + // This will only return accurate results when currentHeight >= the heights + // at which all the in-chain inputs of the tx were included in blocks. + // Typical usage of GetPriority with chainActive.Height() will ensure this. + int heightDiff = currentHeight - cachedHeight; + double deltaPriority = ((double)heightDiff*inChainInputValue)/nModSize; + double dResult = cachedPriority + deltaPriority; + if (dResult < 0) // This should only happen if it was called with an invalid height + dResult = 0; + return dResult; +} + +// We want to sort transactions by coin age priority +typedef std::pair TxCoinAgePriority; + +struct TxCoinAgePriorityCompare +{ + bool operator()(const TxCoinAgePriority& a, const TxCoinAgePriority& b) + { + if (a.first == b.first) + return CompareTxMemPoolEntryByScore()(*(b.second), *(a.second)); //Reverse order to make sort less than + return a.first < b.first; + } +}; + +bool BlockAssembler::isStillDependent(const CTxMemPool& mempool, CTxMemPool::txiter iter) +{ + assert(iter != mempool.mapTx.end()); + for (const auto& parent : iter->GetMemPoolParentsConst()) { + auto parent_it = mempool.mapTx.iterator_to(parent); + if (!inBlock.count(parent_it)) { + return true; + } + } + return false; +} + +bool BlockAssembler::TestForBlock(CTxMemPool::txiter iter) +{ + uint64_t packageSize = iter->GetSizeWithAncestors(); + int64_t packageSigOps = iter->GetSigOpCostWithAncestors(); + if (!TestPackage(packageSize, packageSigOps)) { + // If the block is so close to full that no more txs will fit + // or if we've tried more than 50 times to fill remaining space + // then flag that the block is finished + if (nBlockWeight > m_options.nBlockMaxWeight - 400 || nBlockSigOpsCost > MAX_BLOCK_SIGOPS_COST - 8 || lastFewTxs > 50) { + blockFinished = true; + return false; + } + // Once we're within 4000 weight of a full block, only look at 50 more txs + // to try to fill the remaining space. + if (nBlockWeight > m_options.nBlockMaxWeight - 4000) { + ++lastFewTxs; + } + return false; + } + + CTxMemPool::setEntries package; + package.insert(iter); + if (!TestPackageTransactions(package)) { + if (nBlockSize > m_options.nBlockMaxSize - 100 || lastFewTxs > 50) { + blockFinished = true; + return false; + } + if (nBlockSize > m_options.nBlockMaxSize - 1000) { + ++lastFewTxs; + } + return false; + } + + return true; +} + +void BlockAssembler::addPriorityTxs(const CTxMemPool& mempool, int &nPackagesSelected) +{ + AssertLockHeld(mempool.cs); + + // How much of the block should be dedicated to high-priority transactions, + // included regardless of the fees they pay + uint64_t nBlockPrioritySize = gArgs.GetIntArg("-blockprioritysize", DEFAULT_BLOCK_PRIORITY_SIZE); + nBlockPrioritySize = std::min(m_options.nBlockMaxSize, nBlockPrioritySize); + + if (nBlockPrioritySize == 0) { + return; + } + + bool fSizeAccounting = fNeedSizeAccounting; + fNeedSizeAccounting = true; + + // This vector will be sorted into a priority queue: + std::vector vecPriority; + TxCoinAgePriorityCompare pricomparer; + std::map waitPriMap; + typedef std::map::iterator waitPriIter; + double actualPriority = -1; + + vecPriority.reserve(mempool.mapTx.size()); + for (auto mi = mempool.mapTx.begin(); mi != mempool.mapTx.end(); ++mi) { + double dPriority = mi->GetPriority(nHeight); + vecPriority.emplace_back(dPriority, mi); + } + std::make_heap(vecPriority.begin(), vecPriority.end(), pricomparer); + + CTxMemPool::txiter iter; + while (!vecPriority.empty() && !blockFinished) { // add a tx from priority queue to fill the blockprioritysize + iter = vecPriority.front().second; + actualPriority = vecPriority.front().first; + std::pop_heap(vecPriority.begin(), vecPriority.end(), pricomparer); + vecPriority.pop_back(); + + // If tx already in block, skip + if (inBlock.count(iter)) { + assert(false); // shouldn't happen for priority txs + continue; + } + + // If tx is dependent on other mempool txs which haven't yet been included + // then put it in the waitSet + if (isStillDependent(mempool, iter)) { + waitPriMap.insert(std::make_pair(iter, actualPriority)); + continue; + } + + // If this tx fits in the block add it, otherwise keep looping + if (TestForBlock(iter)) { + AddToBlock(iter); + + ++nPackagesSelected; + + // If now that this txs is added we've surpassed our desired priority size + // or have dropped below the minimum priority threshold, then we're done adding priority txs + if (nBlockSize >= nBlockPrioritySize || actualPriority <= MINIMUM_TX_PRIORITY) { + break; + } + + // This tx was successfully added, so + // add transactions that depend on this one to the priority queue to try again + for (const auto& child : iter->GetMemPoolChildrenConst()) + { + auto child_it = mempool.mapTx.iterator_to(child); + waitPriIter wpiter = waitPriMap.find(child_it); + if (wpiter != waitPriMap.end()) { + vecPriority.emplace_back(wpiter->second, child_it); + std::push_heap(vecPriority.begin(), vecPriority.end(), pricomparer); + waitPriMap.erase(wpiter); + } + } + } + } + fNeedSizeAccounting = fSizeAccounting; +} diff --git a/src/policy/coin_age_priority.h b/src/policy/coin_age_priority.h new file mode 100644 index 0000000000..ab2914f519 --- /dev/null +++ b/src/policy/coin_age_priority.h @@ -0,0 +1,26 @@ +// Copyright (c) 2012-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_POLICY_COIN_AGE_PRIORITY_H +#define BITCOIN_POLICY_COIN_AGE_PRIORITY_H + +#include + +class CCoinsViewCache; +class CTransaction; + +// Compute modified tx size for priority calculation (optionally given tx size) +unsigned int CalculateModifiedSize(const CTransaction& tx, unsigned int nTxSize=0); + +// Compute priority, given sum coin-age of inputs and (optionally) tx size +double ComputePriority(const CTransaction& tx, double dPriorityInputs, unsigned int nTxSize=0); + +/** + * Return priority of tx at height nHeight. Also calculate the sum of the values of the inputs + * that are already in the chain. These are the inputs that will age and increase priority as + * new blocks are added to the chain. + */ +double GetPriority(const CTransaction &tx, const CCoinsViewCache& view, int nHeight, CAmount &inChainInputValue); + +#endif // BITCOIN_POLICY_COIN_AGE_PRIORITY_H diff --git a/src/policy/policy.h b/src/policy/policy.h index f6cb824050..0287049f1c 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -22,6 +22,10 @@ class CScript; /** Default for -blockmaxsize, which controls the maximum size of block the mining code will create **/ static const unsigned int DEFAULT_BLOCK_MAX_SIZE = MAX_BLOCK_SERIALIZED_SIZE; +/** Default for -blockprioritysize, maximum space for zero/low-fee transactions **/ +static const unsigned int DEFAULT_BLOCK_PRIORITY_SIZE = 0; +/** Minimum priority for transactions to be accepted into the priority area **/ +static const double MINIMUM_TX_PRIORITY = COIN * 144 / 250; /** Default for -blockmaxweight, which controls the range of block weights the mining code will create **/ static constexpr unsigned int DEFAULT_BLOCK_MAX_WEIGHT{MAX_BLOCK_WEIGHT - 4000}; /** Default for -blockmintxfee, which sets the minimum feerate for a transaction in blocks created by mining code **/ diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index 77952cab9e..c7e9915744 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -76,7 +76,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) available.insert(i); } - if (add_to_mempool && !pool.exists(GenTxid::Txid(tx->GetHash()))) { + if (add_to_mempool && (!pool.exists(GenTxid::Txid(tx->GetHash()))) && SanityCheckForConsumeTxMemPoolEntry(*tx)) { LOCK2(cs_main, pool.cs); pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx)); available.insert(i); diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 1a6c10af2b..2201e4a559 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -44,6 +44,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) return; } const CTransaction tx{*mtx}; + if (!SanityCheckForConsumeTxMemPoolEntry(tx)) return; const CTxMemPoolEntry& entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx); const auto tx_submitted_in_package = fuzzed_data_provider.ConsumeBool(); const auto tx_has_mempool_parents = fuzzed_data_provider.ConsumeBool(); @@ -68,6 +69,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) break; } const CTransaction tx{*mtx}; + if (!SanityCheckForConsumeTxMemPoolEntry(tx)) return; mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx)); } std::vector txs; diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index eb981352ec..64d2d1ca0c 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -57,6 +57,8 @@ FUZZ_TARGET(rbf, .init = initialize_rbf) if (!mtx) { return; } + const CTransaction tx{*mtx}; + if (!SanityCheckForConsumeTxMemPoolEntry(tx)) return; bilingual_str error; CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node), error}; @@ -69,13 +71,13 @@ FUZZ_TARGET(rbf, .init = initialize_rbf) break; } const CTransaction another_tx{*another_mtx}; + if (!SanityCheckForConsumeTxMemPoolEntry(another_tx)) break; if (fuzzed_data_provider.ConsumeBool() && !mtx->vin.empty()) { mtx->vin[0].prevout = COutPoint{another_tx.GetHash(), 0}; } LOCK2(cs_main, pool.cs); pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, another_tx)); } - const CTransaction tx{*mtx}; if (fuzzed_data_provider.ConsumeBool()) { LOCK2(cs_main, pool.cs); pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx)); diff --git a/src/test/fuzz/util/mempool.cpp b/src/test/fuzz/util/mempool.cpp index 8e7499a860..08c7bf90e3 100644 --- a/src/test/fuzz/util/mempool.cpp +++ b/src/test/fuzz/util/mempool.cpp @@ -14,6 +14,17 @@ #include #include +bool SanityCheckForConsumeTxMemPoolEntry(const CTransaction& tx) noexcept +{ + try { + (void)tx.GetValueOut(); + return true; + } catch (const std::runtime_error&) { + return false; + } +} + +// NOTE: Transaction must pass SanityCheckForConsumeTxMemPoolEntry first CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept { // Avoid: @@ -24,8 +35,9 @@ CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, assert(MoneyRange(fee)); const int64_t time = fuzzed_data_provider.ConsumeIntegral(); const uint64_t entry_sequence{fuzzed_data_provider.ConsumeIntegral()}; - const unsigned int entry_height = fuzzed_data_provider.ConsumeIntegral(); + const double coinage_priority = fuzzed_data_provider.ConsumeFloatingPoint(); + const unsigned int entry_height = fuzzed_data_provider.ConsumeIntegralInRange(0, std::numeric_limits::max() - 1); const bool spends_coinbase = fuzzed_data_provider.ConsumeBool(); const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_BLOCK_SIGOPS_COST); - return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}}; + return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, coinage_priority, entry_height, entry_sequence, tx.GetValueOut(), spends_coinbase, sig_op_cost, {}}; } diff --git a/src/test/fuzz/util/mempool.h b/src/test/fuzz/util/mempool.h index 31b578dc4b..740225a0ee 100644 --- a/src/test/fuzz/util/mempool.h +++ b/src/test/fuzz/util/mempool.h @@ -21,6 +21,7 @@ public: } }; +[[nodiscard]] bool SanityCheckForConsumeTxMemPoolEntry(const CTransaction& tx) noexcept; [[nodiscard]] CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept; #endif // BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 62ff61b227..788053649b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -558,7 +558,8 @@ std::vector TestChain100Setup::PopulateMempool(FastRandomContex LOCK2(cs_main, m_node.mempool->cs); LockPoints lp; m_node.mempool->addUnchecked(CTxMemPoolEntry(ptx, /*fee=*/(total_in - num_outputs * amount_per_output), - /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, + /*time=*/0, /*entry_priority=*/0, /*entry_height=*/1, /*entry_sequence=*/0, + /*in_chain_input_value=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); } --num_transactions; @@ -588,7 +589,8 @@ void TestChain100Setup::MockMempoolMinFee(const CFeeRate& target_feerate) const auto tx_fee = target_feerate.GetFee(GetVirtualTransactionSize(*tx)) - m_node.mempool->m_opts.incremental_relay_feerate.GetFee(GetVirtualTransactionSize(*tx)); m_node.mempool->addUnchecked(CTxMemPoolEntry(tx, /*fee=*/tx_fee, - /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, + /*time=*/0, /*entry_priority=*/0, /*entry_height=*/1, /*entry_sequence=*/0, + /*in_chain_input_value=*/0, /*spends_coinbase=*/true, /*sigops_cost=*/1, lp)); m_node.mempool->TrimToSize(0); assert(m_node.mempool->GetMinFee() == target_feerate); diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 9d6b4810d0..57ca6d2496 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -37,7 +37,9 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) co CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const { - return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp}; + const double dPriority = 0; + const CAmount inChainValue = 0; + return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch(time), dPriority, nHeight, m_sequence, inChainValue, spendsCoinbase, sigOpCost, lp}; } std::optional CheckPackageMempoolAcceptResult(const Package& txns, diff --git a/src/txmempool.cpp b/src/txmempool.cpp index b523c5fe09..87a343cb60 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -636,6 +637,7 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne txs_removed_for_block.reserve(vtx.size()); for (const auto& tx : vtx) { + UpdateDependentPriorities(*tx, nBlockHeight, true); txiter it = mapTx.find(tx->GetHash()); if (it != mapTx.end()) { setEntries stage; @@ -672,6 +674,13 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei for (const auto& it : GetSortedDepthAndScore()) { checkTotal += it->GetTxSize(); + CAmount dummyValue; + double freshPriority = GetPriority(it->GetTx(), active_coins_tip, spendheight, dummyValue); + double cachePriority = it->GetPriority(spendheight); + double priDiff = cachePriority > freshPriority ? cachePriority - freshPriority : freshPriority - cachePriority; + // Verify that the difference between the on the fly calculation and a fresh calculation + // is small enough to be a result of double imprecision. + assert(priDiff < .0001 * freshPriority + 1); check_total_fee += it->GetFee(); innerUsage += it->DynamicMemoryUsage(); const CTransaction& tx = it->GetTx(); diff --git a/src/txmempool.h b/src/txmempool.h index d0cb41a078..877798ba3c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -484,6 +484,12 @@ public: * the tx is not dependent on other mempool transactions to be included in a block. */ bool HasNoInputsOf(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** + * Update all transactions in the mempool which depend on tx to recalculate their priority + * and adjust the input value that will age to reflect that the inputs from this transaction have + * either just been added to the chain or just been removed. + */ + void UpdateDependentPriorities(const CTransaction &tx, unsigned int nBlockHeight, bool addToChain); /** Affect CreateNewBlock prioritisation of transactions */ void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); diff --git a/src/validation.cpp b/src/validation.cpp index 06dc448d9c..cf5ce9d916 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -934,6 +935,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) ws.m_modified_fees = ws.m_base_fees; m_pool.ApplyDelta(hash, ws.m_modified_fees); + CAmount inChainInputValue; + // Since entries arrive *after* the tip's height, their priority is for the height+1 + double dPriority = GetPriority(tx, m_view, m_active_chainstate.m_chain.Height() + 1, inChainInputValue); + // Keep track of transactions that spend a coinbase, which we re-scan // during reorgs to ensure COINBASE_MATURITY is still met. bool fSpendsCoinbase = false; @@ -948,7 +953,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Set entry_sequence to 0 when rejectmsg_zero_mempool_entry_seq is used; this allows txs from a block // reorg to be marked earlier than any child txs that were already in the mempool. const uint64_t entry_sequence = args.m_ignore_rejects.count(rejectmsg_zero_mempool_entry_seq) ? 0 : m_pool.GetSequence(); - entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), entry_sequence, + entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, dPriority, m_active_chainstate.m_chain.Height(), entry_sequence, + inChainInputValue, fSpendsCoinbase, nSigOpsCost, lock_points.value())); ws.m_vsize = entry->GetTxSize(); @@ -3108,6 +3114,9 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra } if (disconnectpool && m_mempool) { + for (auto it = block.vtx.rbegin(); it != block.vtx.rend(); ++it) { + m_mempool->UpdateDependentPriorities(*(*it), pindexDelete->nHeight, false); + } // Save transactions to re-add to mempool at end of reorg. If any entries are evicted for // exceeding memory limits, remove them and their descendants from the mempool. for (auto&& evicted_tx : disconnectpool->AddTransactionsFromBlock(block.vtx)) {