From aa597f0d1a1493bfc08cab548b2afcc2b217feb0 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 11 Mar 2018 02:52:10 +0000 Subject: [PATCH 1/6] Policy: Restore support for mining based on coin-age priority --- src/Makefile.am | 2 + src/bench/mempool_eviction.cpp | 5 +- src/init.cpp | 3 +- src/miner.cpp | 17 ++- src/miner.h | 13 ++ src/policy/coin_age_priority.cpp | 253 +++++++++++++++++++++++++++++++ src/policy/coin_age_priority.h | 26 ++++ src/policy/policy.h | 4 + src/test/test_bitcoin.cpp | 6 +- src/txmempool.cpp | 26 +++- src/txmempool.h | 26 +++- src/validation.cpp | 10 +- src/wallet/wallet.cpp | 2 +- 13 files changed, 378 insertions(+), 15 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 d491530ca10fb..4784d71db6f73 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -157,6 +157,7 @@ BITCOIN_CORE_H = \ noui.h \ optional.h \ outputtype.h \ + policy/coin_age_priority.h \ policy/feerate.h \ policy/fees.h \ policy/policy.h \ @@ -263,6 +264,7 @@ libbitcoin_server_a_SOURCES = \ node/transaction.cpp \ noui.cpp \ outputtype.cpp \ + policy/coin_age_priority.cpp \ policy/fees.cpp \ policy/policy.cpp \ policy/rbf.cpp \ diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index ac8a182358354..468185470bdff 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -12,13 +12,14 @@ 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; bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; pool.addUnchecked(CTxMemPoolEntry( - tx, nFee, nTime, nHeight, - spendsCoinbase, sigOpCost, lp)); + tx, nFee, nTime, dPriority, nHeight, + tx->GetValueOut(), spendsCoinbase, sigOpCost, lp)); } // Right now this is only testing eviction performance in an extremely small diff --git a/src/init.cpp b/src/init.cpp index 7b7b02ca6264b..f5e89f4ecd1af 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -502,7 +502,7 @@ void SetupServerArgs() gArgs.AddArg("-maxtipage=", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-maxtxfee=", strprintf("Maximum total fees (in %s) to use in a single wallet transaction or raw transaction; setting this too low may abort large transactions (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), false, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), true, OptionsCategory::DEBUG_TEST); + gArgs.AddArg("-printpriority", strprintf("Log transaction priority and fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -daemon. To disable logging to file, set -nodebuglogfile)", false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-shrinkdebugfile", "Shrink debug.log file on client startup (default: 1 when no -debug)", false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-uacomment=", "Append comment to the user agent string", false, OptionsCategory::DEBUG_TEST); @@ -525,6 +525,7 @@ void SetupServerArgs() gArgs.AddArg("-blockmaxsize=", strprintf("Set maximum block size in bytes (default: %d)", DEFAULT_BLOCK_MAX_SIZE), false, OptionsCategory::BLOCK_CREATION); gArgs.AddArg("-blockmaxweight=", strprintf("Set maximum BIP141 block weight (default: %d)", DEFAULT_BLOCK_MAX_WEIGHT), false, OptionsCategory::BLOCK_CREATION); gArgs.AddArg("-blockmintxfee=", strprintf("Set lowest fee rate (in %s/kB) for transactions to be included in block creation. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_BLOCK_MIN_TX_FEE)), false, OptionsCategory::BLOCK_CREATION); + gArgs.AddArg("-blockprioritysize=", strprintf("Set maximum size of high-priority/low-fee transactions in bytes (default: %d)", DEFAULT_BLOCK_PRIORITY_SIZE), false, OptionsCategory::BLOCK_CREATION); gArgs.AddArg("-blockversion=", "Override block version to test forking scenarios", true, OptionsCategory::BLOCK_CREATION); gArgs.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), false, OptionsCategory::RPC); diff --git a/src/miner.cpp b/src/miner.cpp index 4b2ea385665f5..a5bb018d0b1b2 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -106,6 +106,9 @@ void BlockAssembler::resetBlock() // These counters do not include coinbase tx nBlockTx = 0; nFees = 0; + + lastFewTxs = 0; + blockFinished = false; } Optional BlockAssembler::m_last_block_num_txs{nullopt}; @@ -128,6 +131,10 @@ 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 + bool fPrintPriority = gArgs.GetBoolArg("-printpriority", DEFAULT_PRINTPRIORITY); + if (fPrintPriority) { + pblocktemplate->vTxPriorities.push_back(-1); // n/a + } LOCK2(cs_main, mempool.cs); CBlockIndex* pindexPrev = chainActive.Tip(); @@ -160,6 +167,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc int nPackagesSelected = 0; int nDescendantsUpdated = 0; + addPriorityTxs(nPackagesSelected); addPackageTxs(nPackagesSelected, nDescendantsUpdated); int64_t nTime1 = GetTimeMicros(); @@ -268,9 +276,14 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) bool fPrintPriority = gArgs.GetBoolArg("-printpriority", DEFAULT_PRINTPRIORITY); if (fPrintPriority) { - LogPrintf("fee %s txid %s\n", + double dPriority = iter->GetPriority(nHeight); + CAmount dummy; + mempool.ApplyDelta(iter->GetTx().GetHash(), dummy); + LogPrintf("priority %.1f fee %s txid %s\n", + dPriority, CFeeRate(iter->GetModifiedFee(), iter->GetTxSize()).ToString(), iter->GetTx().GetHash().ToString()); + pblocktemplate->vTxPriorities.push_back(dPriority); } } @@ -347,7 +360,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda // Start by adding all descendants of previously added txs to mapModifiedTx // and modifying them for their already included ancestors - UpdatePackagesForAdded(inBlock, mapModifiedTx); + nDescendantsUpdated += UpdatePackagesForAdded(inBlock, mapModifiedTx); CTxMemPool::indexed_transaction_set::index::type::iterator mi = mempool.mapTx.get().begin(); CTxMemPool::txiter iter; diff --git a/src/miner.h b/src/miner.h index 75b5498ba487d..489b106727c30 100644 --- a/src/miner.h +++ b/src/miner.h @@ -30,6 +30,7 @@ struct CBlockTemplate CBlock block; std::vector vTxFees; std::vector vTxSigOpsCost; + std::vector vTxPriorities; std::vector vchCoinbaseCommitment; }; @@ -151,6 +152,10 @@ class BlockAssembler int64_t nLockTimeCutoff; const CChainParams& chainparams; + // Variables used for addPriorityTxs + int lastFewTxs; + bool blockFinished; + public: struct Options { Options(); @@ -177,11 +182,19 @@ class BlockAssembler void AddToBlock(CTxMemPool::txiter iter); // Methods for how to add transactions to a block. + /** Add transactions based on tx "priority" */ + void addPriorityTxs(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(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(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 0000000000000..51c420fb7974a --- /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 + +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(CTxMemPool::txiter iter) +{ + for (CTxMemPool::txiter parent : mempool.GetMemPoolParents(iter)) + { + if (!inBlock.count(parent)) { + 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 > 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 > nBlockMaxWeight - 4000) { + ++lastFewTxs; + } + return false; + } + + CTxMemPool::setEntries package; + package.insert(iter); + if (!TestPackageTransactions(package)) { + if (nBlockSize > nBlockMaxSize - 100 || lastFewTxs > 50) { + blockFinished = true; + return false; + } + if (nBlockSize > nBlockMaxSize - 1000) { + ++lastFewTxs; + } + return false; + } + + return true; +} + +void BlockAssembler::addPriorityTxs(int &nPackagesSelected) +{ + // How much of the block should be dedicated to high-priority transactions, + // included regardless of the fees they pay + uint64_t nBlockPrioritySize = gArgs.GetArg("-blockprioritysize", DEFAULT_BLOCK_PRIORITY_SIZE); + nBlockPrioritySize = std::min(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 (CTxMemPool::indexed_transaction_set::iterator mi = mempool.mapTx.begin(); + mi != mempool.mapTx.end(); ++mi) + { + double dPriority = mi->GetPriority(nHeight); + vecPriority.push_back(TxCoinAgePriority(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; + } + + // cannot accept witness transactions into a non-witness block + if (!fIncludeWitness && iter->GetTx().HasWitness()) + continue; + + // If tx is dependent on other mempool txs which haven't yet been included + // then put it in the waitSet + if (isStillDependent(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 (CTxMemPool::txiter child : mempool.GetMemPoolChildren(iter)) + { + waitPriIter wpiter = waitPriMap.find(child); + if (wpiter != waitPriMap.end()) { + vecPriority.push_back(TxCoinAgePriority(wpiter->second,child)); + 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 0000000000000..d7afec8c99b4b --- /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 c543223fa4594..b99e83548d340 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -18,6 +18,10 @@ class CTxOut; /** 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 const 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/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 0c3fb7c3982b1..79e1b4df53e2d 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -167,8 +167,10 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx) { CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) { - return CTxMemPoolEntry(tx, nFee, nTime, nHeight, - spendsCoinbase, sigOpCost, lp); + const double dPriority = 0; + const CAmount inChainValue = 0; + return CTxMemPoolEntry(tx, nFee, nTime, dPriority, nHeight, + inChainValue, spendsCoinbase, sigOpCost, lp); } /** diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 68f47d5cce39e..af0e0053b6a6c 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -19,17 +20,25 @@ #include CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, unsigned int _entryHeight, + int64_t _nTime, double _entryPriority, unsigned int _entryHeight, + CAmount _inChainInputValue, bool _spendsCoinbase, int64_t _sigOpsCost, LockPoints lp) - : tx(_tx), nFee(_nFee), nTxWeight(GetTransactionWeight(*tx)), nUsageSize(RecursiveDynamicUsage(tx)), nTime(_nTime), entryHeight(_entryHeight), - spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp) + : tx(_tx), nFee(_nFee), nTxWeight(GetTransactionWeight(*tx)), nUsageSize(RecursiveDynamicUsage(tx)), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), + inChainInputValue(_inChainInputValue), + spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), nModSize(CalculateModifiedSize(*tx, GetTxSize())), lockPoints(lp) { nCountWithDescendants = 1; nSizeWithDescendants = GetTxSize(); nModFeesWithDescendants = nFee; + CAmount nValueIn = tx->GetValueOut()+nFee; + assert(inChainInputValue <= nValueIn); feeDelta = 0; + // Since entries arrive *after* the tip's height, their entry priority is for the height+1 + cachedHeight = entryHeight + 1; + cachedPriority = entryPriority; + nCountWithAncestors = 1; nSizeWithAncestors = GetTxSize(); nModFeesWithAncestors = nFee; @@ -562,6 +571,7 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne if (minerPolicyEstimator) {minerPolicyEstimator->processBlock(nBlockHeight, entries);} for (const auto& tx : vtx) { + UpdateDependentPriorities(*tx, nBlockHeight, true); txiter it = mapTx.find(tx->GetHash()); if (it != mapTx.end()) { setEntries stage; @@ -620,10 +630,20 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const CCoinsViewCache mempoolDuplicate(const_cast(pcoins)); const int64_t spendheight = GetSpendHeight(mempoolDuplicate); + const unsigned int nBlockHeight = chainActive.Height(); + CCoinsViewMemPool viewMemPool(pcoinsTip.get(), *this); + CCoinsViewCache view(&viewMemPool); std::list waitingOnDependants; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { unsigned int i = 0; checkTotal += it->GetTxSize(); + CAmount dummyValue; + double freshPriority = GetPriority(it->GetTx(), view, nBlockHeight + 1, dummyValue); + double cachePriority = it->GetPriority(nBlockHeight + 1); + 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); innerUsage += it->DynamicMemoryUsage(); const CTransaction& tx = it->GetTx(); txlinksMap::const_iterator linksiter = mapLinks.find(it); diff --git a/src/txmempool.h b/src/txmempool.h index 842acf916fd48..c810894a585b9 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -71,9 +71,14 @@ class CTxMemPoolEntry const size_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) const size_t nUsageSize; //!< ... and total memory usage const int64_t nTime; //!< Local time when entering the mempool + 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 int64_t feeDelta; //!< Used for determining the priority of the transaction for mining in a block LockPoints lockPoints; //!< Track the height and time at which tx was final @@ -92,12 +97,23 @@ class CTxMemPoolEntry public: CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, unsigned int _entryHeight, - bool spendsCoinbase, + int64_t _nTime, double _entryPriority, unsigned int _entryHeight, + CAmount _inChainInputValue, bool spendsCoinbase, int64_t nSigOpsCost, LockPoints lp); 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; } size_t GetTxSize() const; size_t GetTxWeight() const { return nTxWeight; } @@ -597,6 +613,12 @@ class CTxMemPool * the tx is not dependent on other mempool transactions to be included in a block. */ bool HasNoInputsOf(const CTransaction& tx) const; + /** + * 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 0d03422d56c01..168fa6f7c94fa 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -709,6 +710,10 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool CAmount nModifiedFees = nFees; pool.ApplyDelta(hash, nModifiedFees); + CAmount inChainInputValue; + // Since entries arrive *after* the tip's height, their priority is for the height+1 + double dPriority = GetPriority(tx, view, chainActive.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; @@ -720,8 +725,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool } } - CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, chainActive.Height(), - fSpendsCoinbase, nSigOpsCost, lp); + CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, dPriority, chainActive.Height(), + inChainInputValue, fSpendsCoinbase, nSigOpsCost, lp); unsigned int nSize = entry.GetTxSize(); // Check that the transaction doesn't have an excessive number of @@ -2308,6 +2313,7 @@ bool CChainState::DisconnectTip(CValidationState& state, const CChainParams& cha if (disconnectpool) { // Save transactions to re-add to mempool at end of reorg for (auto it = block.vtx.rbegin(); it != block.vtx.rend(); ++it) { + mempool.UpdateDependentPriorities(*(*it), pindexDelete->nHeight, false); disconnectpool->addTransaction(*it); } while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 388422bec84d3..bd3ae3dbb5c77 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3131,7 +3131,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits LockPoints lp; - CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(tx, 0, 0, 0, 0, 0, false, 0, lp); CTxMemPool::setEntries setAncestors; size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; From 741583682fac1b0293ec99e9a230c1ba2148e3d0 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 11 Mar 2018 03:05:30 +0000 Subject: [PATCH 2/6] Enable prioritisetransaction with priority delta --- src/miner.cpp | 2 +- src/policy/coin_age_priority.cpp | 2 ++ src/rpc/client.cpp | 2 +- src/rpc/mining.cpp | 26 +++++++++++++++++--------- src/txmempool.cpp | 24 ++++++++++++++---------- src/txmempool.h | 7 ++++--- src/validation.cpp | 5 +++-- 7 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index a5bb018d0b1b2..0b74d8fdade9c 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -278,7 +278,7 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) if (fPrintPriority) { double dPriority = iter->GetPriority(nHeight); CAmount dummy; - mempool.ApplyDelta(iter->GetTx().GetHash(), dummy); + mempool.ApplyDeltas(iter->GetTx().GetHash(), dPriority, dummy); LogPrintf("priority %.1f fee %s txid %s\n", dPriority, CFeeRate(iter->GetModifiedFee(), iter->GetTxSize()).ToString(), diff --git a/src/policy/coin_age_priority.cpp b/src/policy/coin_age_priority.cpp index 51c420fb7974a..f28a58c69cb07 100644 --- a/src/policy/coin_age_priority.cpp +++ b/src/policy/coin_age_priority.cpp @@ -196,6 +196,8 @@ void BlockAssembler::addPriorityTxs(int &nPackagesSelected) mi != mempool.mapTx.end(); ++mi) { double dPriority = mi->GetPriority(nHeight); + CAmount dummy; + mempool.ApplyDeltas(mi->GetTx().GetHash(), dPriority, dummy); vecPriority.push_back(TxCoinAgePriority(dPriority, mi)); } std::make_heap(vecPriority.begin(), vecPriority.end(), pricomparer); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 82424ab5213fb..7b70083b064f5 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -136,7 +136,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "estimatesmartfee", 0, "conf_target" }, { "estimaterawfee", 0, "conf_target" }, { "estimaterawfee", 1, "threshold" }, - { "prioritisetransaction", 1, "dummy" }, + { "prioritisetransaction", 1, "priority_delta" }, { "prioritisetransaction", 2, "fee_delta" }, { "setban", 2, "bantime" }, { "setban", 3, "absolute" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f05187282e364..3c60d29030b26 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -231,15 +231,16 @@ static UniValue getmininginfo(const JSONRPCRequest& request) // NOTE: Unlike wallet RPC (which use BTC values), mining RPCs follow GBT (BIP 22) in using satoshi amounts static UniValue prioritisetransaction(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 3) + if (request.fHelp || request.params.size() < 2 || request.params.size() > 3) throw std::runtime_error( RPCHelpMan{"prioritisetransaction", "Accepts the transaction into mined blocks at a higher (or lower) priority\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id."}, - {"dummy", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "API-Compatibility for previous API. Must be zero or null.\n" - " DEPRECATED. For forward compatibility use named arguments and omit this parameter."}, - {"fee_delta", RPCArg::Type::NUM, RPCArg::Optional::NO, "The fee value (in satoshis) to add (or subtract, if negative).\n" + {"priority_delta", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "The priority to add or subtract.\n" + " The transaction selection algorithm considers the tx as it would have a higher priority.\n" + " (priority of a transaction is calculated: coinage * value_in_satoshis / txsize)\n"}, + {"fee_delta", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "The fee value (in satoshis) to add (or subtract, if negative).\n" " Note, that this value is not a fee rate. It is a value to modify absolute fee of the TX.\n" " The fee is not actually paid, only the algorithm for selecting transactions into a block\n" " considers the transaction as it would have paid a higher (or lower) fee."}, @@ -256,13 +257,17 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) LOCK(cs_main); uint256 hash(ParseHashV(request.params[0], "txid")); - CAmount nAmount = request.params[2].get_int64(); + double priority_delta = 0; + CAmount nAmount = 0; - if (!(request.params[1].isNull() || request.params[1].get_real() == 0)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Priority is no longer supported, dummy argument to prioritisetransaction must be 0."); + if (!request.params[1].isNull()) { + priority_delta = request.params[1].get_real(); + } + if (!request.params[2].isNull()) { + nAmount = request.params[2].get_int64(); } - mempool.PrioritiseTransaction(hash, nAmount); + mempool.PrioritiseTransaction(hash, priority_delta, nAmount); return true; } @@ -576,6 +581,9 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) } entry.pushKV("sigops", nTxSigOps); entry.pushKV("weight", GetTransactionWeight(tx)); + if (index_in_template && !pblocktemplate->vTxPriorities.empty()) { + entry.pushKV("priority", pblocktemplate->vTxPriorities[index_in_template]); + } transactions.push_back(entry); } @@ -980,7 +988,7 @@ static const CRPCCommand commands[] = // --------------------- ------------------------ ----------------------- ---------- { "mining", "getnetworkhashps", &getnetworkhashps, {"nblocks","height"} }, { "mining", "getmininginfo", &getmininginfo, {} }, - { "mining", "prioritisetransaction", &prioritisetransaction, {"txid","dummy","fee_delta"} }, + { "mining", "prioritisetransaction", &prioritisetransaction, {"txid","priority_delta","fee_delta"} }, { "mining", "getblocktemplate", &getblocktemplate, {"template_request"} }, { "mining", "submitblock", &submitblock, {"hexdata","dummy"} }, { "mining", "submitheader", &submitheader, {"hexdata"} }, diff --git a/src/txmempool.cpp b/src/txmempool.cpp index af0e0053b6a6c..a13e8d1bdbfbf 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -373,8 +373,10 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces // Update transaction for any feeDelta created by PrioritiseTransaction // TODO: refactor so that the fee delta is calculated before inserting // into mapTx. + double priority_delta{0.}; CAmount delta{0}; - ApplyDelta(entry.GetTx().GetHash(), delta); + ApplyDeltas(entry.GetTx().GetHash(), priority_delta, delta); + // NOTE: priority_delta is handled in addPriorityTxs if (delta) { mapTx.modify(newit, update_fee_delta(delta)); } @@ -833,15 +835,16 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const return GetInfo(i); } -void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) +void CTxMemPool::PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta) { { LOCK(cs); - CAmount &delta = mapDeltas[hash]; - delta += nFeeDelta; + std::pair &deltas = mapDeltas[hash]; + deltas.first += dPriorityDelta; + deltas.second += nFeeDelta; txiter it = mapTx.find(hash); if (it != mapTx.end()) { - mapTx.modify(it, update_fee_delta(delta)); + mapTx.modify(it, update_fee_delta(deltas.second)); // Now update all ancestors' modified fees with descendants setEntries setAncestors; uint64_t nNoLimit = std::numeric_limits::max(); @@ -860,17 +863,18 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD ++nTransactionsUpdated; } } - LogPrintf("PrioritiseTransaction: %s feerate += %s\n", hash.ToString(), FormatMoney(nFeeDelta)); + LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", hash.ToString(), dPriorityDelta, FormatMoney(nFeeDelta)); } -void CTxMemPool::ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const +void CTxMemPool::ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const { LOCK(cs); - std::map::const_iterator pos = mapDeltas.find(hash); + std::map >::const_iterator pos = mapDeltas.find(hash); if (pos == mapDeltas.end()) return; - const CAmount &delta = pos->second; - nFeeDelta += delta; + const std::pair &deltas = pos->second; + dPriorityDelta += deltas.first; + nFeeDelta += deltas.second; } void CTxMemPool::ClearPrioritisation(const uint256 hash) diff --git a/src/txmempool.h b/src/txmempool.h index c810894a585b9..e9af481d39cc8 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -571,7 +571,7 @@ class CTxMemPool public: indirectmap mapNextTx GUARDED_BY(cs); - std::map mapDeltas; + std::map > mapDeltas; /** Create a new CTxMemPool. */ @@ -621,8 +621,9 @@ class CTxMemPool void UpdateDependentPriorities(const CTransaction &tx, unsigned int nBlockHeight, bool addToChain); /** Affect CreateNewBlock prioritisation of transactions */ - void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); - void ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const; + void PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta); + void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) { PrioritiseTransaction(hash, 0., nFeeDelta); } + void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const; void ClearPrioritisation(const uint256 hash); /** Get the transaction in the pool that spends the same prevout */ diff --git a/src/validation.cpp b/src/validation.cpp index 168fa6f7c94fa..cae6bd43cb8f6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -708,7 +708,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // nModifiedFees includes any fee deltas from PrioritiseTransaction CAmount nModifiedFees = nFees; - pool.ApplyDelta(hash, nModifiedFees); + double nPriorityDummy = 0; + pool.ApplyDeltas(hash, nPriorityDummy, nModifiedFees); CAmount inChainInputValue; // Since entries arrive *after* the tip's height, their priority is for the height+1 @@ -4840,7 +4841,7 @@ bool DumpMempool() { LOCK(mempool.cs); for (const auto &i : mempool.mapDeltas) { - mapDeltas[i.first] = std::make_pair(0.0, i.second); + mapDeltas[i.first] = std::make_pair(0.0, i.second.second); } vinfo = mempool.infoAll(); } From 90d2bc5746e61861e235312dbc4db1dbfaae67e6 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 11 Mar 2018 03:08:02 +0000 Subject: [PATCH 3/6] RPC: Restore "startingpriority" and "currentpriority" in mempool entries --- src/rpc/blockchain.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7e8e5e07d08c4..866741c471273 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -383,6 +383,8 @@ static std::string EntryDescriptionString() " \"modifiedfee\" : n, (numeric) transaction fee with fee deltas used for mining priority (DEPRECATED)\n" " \"time\" : n, (numeric) local time transaction entered pool in seconds since 1 Jan 1970 GMT\n" " \"height\" : n, (numeric) block height when transaction entered pool\n" + " \"startingpriority\" : n, (numeric) Priority when transaction entered pool\n" + " \"currentpriority\" : n, (numeric) Transaction priority now\n" " \"descendantcount\" : n, (numeric) number of in-mempool descendant transactions (including this one)\n" " \"descendantsize\" : n, (numeric) virtual transaction size of in-mempool descendants (including this one)\n" " \"descendantfees\" : n, (numeric) modified fees (see above) of in-mempool descendants (including this one) (DEPRECATED)\n" @@ -421,6 +423,8 @@ static void entryToJSON(UniValue &info, const CTxMemPoolEntry &e) EXCLUSIVE_LOCK info.pushKV("modifiedfee", ValueFromAmount(e.GetModifiedFee())); info.pushKV("time", e.GetTime()); info.pushKV("height", (int)e.GetHeight()); + info.pushKV("startingpriority", e.GetStartingPriority()); + info.pushKV("currentpriority", e.GetPriority(chainActive.Height() + 1)); info.pushKV("descendantcount", e.GetCountWithDescendants()); info.pushKV("descendantsize", e.GetSizeWithDescendants()); info.pushKV("descendantfees", e.GetModFeesWithDescendants()); From 3f38f46809b23405ff292b8469caebd84d0d095b Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 11 Mar 2018 03:11:39 +0000 Subject: [PATCH 4/6] Tests: Update for coin-age priority --- test/functional/mining_prioritisetransaction.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index ca4b621a7800f..f3ced214e5959 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -23,7 +23,6 @@ def run_test(self): # Test `prioritisetransaction` required parameters assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction) assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '') - assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '', 0) # Test `prioritisetransaction` invalid extra parameters assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '', 0, 0, 0) @@ -32,10 +31,9 @@ def run_test(self): assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0) assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000')", self.nodes[0].prioritisetransaction, txid='Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000', fee_delta=0) - # Test `prioritisetransaction` invalid `dummy` + # Test `prioritisetransaction` invalid `priority_delta` txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000' assert_raises_rpc_error(-1, "JSON value is not a number as expected", self.nodes[0].prioritisetransaction, txid, 'foo', 0) - assert_raises_rpc_error(-8, "Priority is no longer supported, dummy argument to prioritisetransaction must be 0.", self.nodes[0].prioritisetransaction, txid, 1, 0) # Test `prioritisetransaction` invalid `fee_delta` assert_raises_rpc_error(-1, "JSON value is not an integer as expected", self.nodes[0].prioritisetransaction, txid=txid, fee_delta='foo') From 169e9d9138c21034cbb916c030b9c526dbfd0851 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 11 Mar 2018 03:14:46 +0000 Subject: [PATCH 5/6] Test: Add test for coin-age priority mining --- test/functional/mining_coin_age_priority.py | 207 +++++++++++++++++++ test/functional/test_framework/blocktools.py | 34 ++- test/functional/test_runner.py | 2 + 3 files changed, 237 insertions(+), 6 deletions(-) create mode 100755 test/functional/mining_coin_age_priority.py diff --git a/test/functional/mining_coin_age_priority.py b/test/functional/mining_coin_age_priority.py new file mode 100755 index 0000000000000..5d61bbdbd2a2a --- /dev/null +++ b/test/functional/mining_coin_age_priority.py @@ -0,0 +1,207 @@ +#!/usr/bin/env python3 +# Copyright (c) 2016 The Bitcoin Core developers +# Distributed under the MIT/X11 software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +# + +from test_framework.blocktools import create_block, create_coinbase +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + connect_nodes_bi, + sync_blocks, +) + +from binascii import a2b_hex, b2a_hex +from decimal import Decimal + +def find_unspent(node, txid, amount): + for utxo in node.listunspent(0): + if utxo['txid'] != txid: + continue + if utxo['amount'] != amount: + continue + return {'txid': utxo['txid'], 'vout': utxo['vout']} + +def solve_template_hex(tmpl, txlist): + block = create_block(tmpl=tmpl, txlist=txlist) + block.solve() + b = block.serialize() + x = b2a_hex(b).decode('ascii') + return x + +def get_modified_size(node, txdata): + decoded = node.decoderawtransaction(txdata) + size = decoded['vsize'] + for inp in decoded['vin']: + offset = 41 + min(len(inp['scriptSig']['hex']) // 2, 110) + if offset <= size: + size -= offset + return size + +def assert_approximate(a, b): + assert_equal(int(a), int(b)) + +BTC = Decimal('100000000') + +class PriorityTest(BitcoinTestFramework): + + def set_test_params(self): + self.num_nodes = 4 + self.testmsg_num = 0 + + def add_options(self, parser): + parser.add_argument("--gbt", dest="test_gbt", default=False, action="store_true", + help="Test priorities used by GBT") + + def setup_nodes(self): + ppopt = [] + if self.options.test_gbt: + ppopt.append('-printpriority') + + self.extra_args = [ + ['-blockmaxsize=0'], + ['-blockprioritysize=1000000', '-blockmaxsize=1000000'] + ppopt, + ['-blockmaxsize=0'], + ['-blockmaxsize=0'], + ] + + super().setup_nodes() + + def assert_prio(self, txid, starting, current): + node = self.nodes[1] + + if self.options.test_gbt: + tmpl = node.getblocktemplate({'rules':('segwit',)}) + tmplentry = None + for tx in tmpl['transactions']: + if tx['txid'] == txid: + tmplentry = tx + break + # GBT does not expose starting priority, so we don't check that + assert_approximate(tmplentry['priority'], current) + else: + mempoolentry = node.getrawmempool(True)[txid] + assert_approximate(mempoolentry['startingpriority'], starting) + assert_approximate(mempoolentry['currentpriority'], current) + + def testmsg(self, msg): + self.testmsg_num += 1 + self.log.info('Test %d: %s' % (self.testmsg_num, msg)) + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + node = self.nodes[0] + miner = self.nodes[1] + + node.generate(50) + self.sync_all() + miner.generate(101) + self.sync_all() + + fee = Decimal('0.01') + amt = Decimal('11') + + txid_a = node.sendtoaddress(node.getnewaddress(), amt) + txdata_b = node.createrawtransaction([find_unspent(node, txid_a, amt)], {node.getnewaddress(): amt - fee}) + txdata_b = node.signrawtransactionwithwallet(txdata_b)['hex'] + txmodsize_b = get_modified_size(node, txdata_b) + txid_b = node.sendrawtransaction(txdata_b) + self.sync_all() + + self.testmsg('priority starts at 0 with all unconfirmed inputs') + self.assert_prio(txid_b, 0, 0) + + self.testmsg('priority increases correctly when that input is mined') + + # Mine only the sendtoaddress transaction + tmpl = node.getblocktemplate({'rules':('segwit',)}) + rawblock = solve_template_hex(tmpl, [create_coinbase(tmpl['height']), a2b_hex(node.getrawtransaction(txid_a))]) + assert_equal(node.submitblock(rawblock), None) + self.sync_all() + + self.assert_prio(txid_b, 0, amt * BTC / txmodsize_b) + + self.testmsg('priority continues to increase the deeper the block confirming its inputs gets buried') + + node.generate(2) + self.sync_all() + + self.assert_prio(txid_b, 0, amt * BTC * 3 / txmodsize_b) + + self.testmsg('with a confirmed input, the initial priority is calculated correctly') + + miner.generate(4) + self.sync_all() + + amt_c = (amt - fee) / 2 + amt_c2 = amt_c - (fee * 2) # could be just amt_c-fee, but then it'd be undiscernable later on + txdata_c = node.createrawtransaction([find_unspent(node, txid_b, amt - fee)], {node.getnewaddress(): amt_c, node.getnewaddress(): amt_c2}) + txdata_c = node.signrawtransactionwithwallet(txdata_c)['hex'] + txmodsize_c = get_modified_size(node, txdata_c) + txid_c = node.sendrawtransaction(txdata_c) + self.sync_all() + + txid_c_starting_prio = (amt - fee) * BTC * 4 / txmodsize_c + self.assert_prio(txid_c, txid_c_starting_prio, txid_c_starting_prio) + + self.testmsg('with an input confirmed prior to the transaction, the priority gets incremented correctly as it gets buried deeper') + + node.generate(1) + self.sync_all() + + self.assert_prio(txid_c, txid_c_starting_prio, (amt - fee) * BTC * 5 / txmodsize_c) + + self.testmsg('with an input confirmed prior to the transaction, the priority gets incremented correctly as it gets buried deeper and deeper') + + node.generate(2) + self.sync_all() + + self.assert_prio(txid_c, txid_c_starting_prio, (amt - fee) * BTC * 7 / txmodsize_c) + + self.log.info('(preparing for reorg test)') + + miner.generate(1) + self.sync_all() + + self.split_network() + sync_groups = [self.nodes[:2], self.nodes[2:]] + node = self.nodes[0] + miner = self.nodes[1] + competing_miner = self.nodes[2] + + txdata_d = node.createrawtransaction([find_unspent(node, txid_c, amt_c)], {node.getnewaddress(): amt_c - fee}) + txdata_d = node.signrawtransactionwithwallet(txdata_d)['hex'] + get_modified_size(node, txdata_d) + txid_d = node.sendrawtransaction(txdata_d) + self.sync_all(sync_groups) + + miner.generate(1) + self.sync_all(sync_groups) + + txdata_e = node.createrawtransaction([find_unspent(node, txid_d, amt_c - fee), find_unspent(node, txid_c, amt_c2)], {node.getnewaddress(): (amt_c - fee) + amt_c2 - fee}) + txdata_e = node.signrawtransactionwithwallet(txdata_e)['hex'] + txmodsize_e = get_modified_size(node, txdata_e) + txid_e = node.sendrawtransaction(txdata_e) + self.sync_all(sync_groups) + + txid_e_starting_prio = (((amt_c - fee) * BTC) + (amt_c2 * BTC * 2)) / txmodsize_e + self.assert_prio(txid_e, txid_e_starting_prio, txid_e_starting_prio) # Sanity check 1 + + competing_miner.generate(5) + self.sync_all(sync_groups) + + self.assert_prio(txid_e, txid_e_starting_prio, txid_e_starting_prio) # Sanity check 2 + + self.testmsg('priority is updated correctly when input-confirming block is reorganised out') + + connect_nodes_bi(self.nodes, 1, 2) + sync_blocks(self.nodes) + + txid_e_reorg_prio = (amt_c2 * BTC * 6) / txmodsize_e + self.assert_prio(txid_e, txid_e_starting_prio, txid_e_reorg_prio) + +if __name__ == '__main__': + PriorityTest().main() diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 15f4502994da9..966254f3f9a08 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -4,6 +4,10 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Utilities for manipulating blocks and transactions.""" +from binascii import a2b_hex +import io +import struct + from .address import ( key_to_p2sh_p2wpkh, key_to_p2wpkh, @@ -50,18 +54,36 @@ WITNESS_COMMITMENT_HEADER = b"\xaa\x21\xa9\xed" -def create_block(hashprev, coinbase, ntime=None, *, version=1): +def create_block(hashprev=None, coinbase=None, ntime=None, *, version=1, tmpl=None, txlist=None): """Create a block (with regtest difficulty).""" block = CBlock() block.nVersion = version - if ntime is None: + if tmpl: + block.nVersion = tmpl.get('version', block.nVersion) + if ntime is not None: + block.nTime = ntime + elif tmpl and not tmpl.get('curtime') is None: + block.nTime = tmpl['curtime'] + else: import time block.nTime = int(time.time() + 600) + if hashprev is not None: + block.hashPrevBlock = hashprev else: - block.nTime = ntime - block.hashPrevBlock = hashprev - block.nBits = 0x207fffff # difficulty retargeting is disabled in REGTEST chainparams - block.vtx.append(coinbase) + block.hashPrevBlock = int(tmpl['previousblockhash'], 0x10) + if tmpl and not tmpl.get('bits') is None: + block.nBits = struct.unpack('>I', a2b_hex(tmpl['bits']))[0] + else: + block.nBits = 0x207fffff # difficulty retargeting is disabled in REGTEST chainparams + if coinbase is not None: + block.vtx.append(coinbase) + if txlist: + for tx in txlist: + if not hasattr(tx, 'calc_sha256'): + txo = CTransaction() + txo.deserialize(io.BytesIO(tx)) + tx = txo + block.vtx.append(tx) block.hashMerkleRoot = block.calc_merkle_root() block.calc_sha256() return block diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index bb9ac58e46c94..48c90ddc7fbb4 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -109,6 +109,8 @@ 'tool_wallet.py', 'wallet_txn_clone.py', 'wallet_txn_clone.py --segwit', + 'mining_coin_age_priority.py', + 'mining_coin_age_priority.py --gbt', 'rpc_getchaintips.py', 'rpc_misc.py', 'interface_rest.py', From 1bb17a075bb375a53e30a16d8efee220f41389a2 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 11 Mar 2018 03:40:59 +0000 Subject: [PATCH 6/6] Load and save priority deltas in mempool dumps --- src/txmempool.h | 1 - src/validation.cpp | 12 ++---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/txmempool.h b/src/txmempool.h index e9af481d39cc8..0435087f857c3 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -622,7 +622,6 @@ class CTxMemPool /** Affect CreateNewBlock prioritisation of transactions */ void PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta); - void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) { PrioritiseTransaction(hash, 0., nFeeDelta); } void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const; void ClearPrioritisation(const uint256 hash); diff --git a/src/validation.cpp b/src/validation.cpp index cae6bd43cb8f6..bd2d6f8937074 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4744,14 +4744,8 @@ bool LoadMempool() if (it != mapData.end()) { try { CDataStream ss(it->second, SER_DISK, CLIENT_VERSION); - std::map> mapDeltas; - ss >> mapDeltas; LOCK(mempool.cs); - for (const auto& it : mapDeltas) { - const uint256& txid = it.first; - const CAmount& amountdelta = it.second.second; - mempool.PrioritiseTransaction(txid, amountdelta); - } + ss >> mempool.mapDeltas; } catch (const std::exception& e) { LogPrintf("Failed to deserialize mempool %s from disk: %s. Continuing anyway.\n", "deltas", e.what()); } @@ -4840,9 +4834,7 @@ bool DumpMempool() { LOCK(mempool.cs); - for (const auto &i : mempool.mapDeltas) { - mapDeltas[i.first] = std::make_pair(0.0, i.second.second); - } + mapDeltas = mempool.mapDeltas; vinfo = mempool.infoAll(); }