From ddf58c75739efc7509d529002975ea447a269b3a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 20 Dec 2016 23:20:31 +0100 Subject: [PATCH 01/13] wallet: Remove sendfree This removes the option from the wallet to not pay a fee on "small" transactions which spend "old" inputs. This code is no longer worth keeping around, as almost all miners prefer not to include transactions which pay no fee at all. --- contrib/debian/examples/bitcoin.conf | 3 --- contrib/devtools/check-doc.py | 2 +- src/qt/coincontroldialog.cpp | 20 ----------------- src/wallet/wallet.cpp | 33 ++-------------------------- src/wallet/wallet.h | 5 ----- 5 files changed, 3 insertions(+), 60 deletions(-) diff --git a/contrib/debian/examples/bitcoin.conf b/contrib/debian/examples/bitcoin.conf index afbc7882e..0c6eaa0a3 100644 --- a/contrib/debian/examples/bitcoin.conf +++ b/contrib/debian/examples/bitcoin.conf @@ -118,9 +118,6 @@ # Transaction Fee Changes in 0.10.0 -# Send transactions as zero-fee transactions if possible (default: 0) -#sendfreetransactions=0 - # Create transactions that have enough fees (or priority) so they are likely to begin confirmation within n blocks (default: 1). # This setting is over-ridden by the -paytxfee option. #txconfirmtarget=n diff --git a/contrib/devtools/check-doc.py b/contrib/devtools/check-doc.py index 249214e93..445175ec2 100755 --- a/contrib/devtools/check-doc.py +++ b/contrib/devtools/check-doc.py @@ -21,7 +21,7 @@ CMD_GREP_DOCS = r"egrep -r -I 'HelpMessageOpt\(\"\-[^\"=]+?(=|\")' %s" % (CMD_RO REGEX_ARG = re.compile(r'(?:map(?:Multi)?Args(?:\.count\(|\[)|Get(?:Bool)?Arg\()\"(\-[^\"]+?)\"') REGEX_DOC = re.compile(r'HelpMessageOpt\(\"(\-[^\"=]+?)(?:=|\")') # list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-rpcssl', '-benchmark', '-h', '-help', '-socks', '-tor', '-debugnet', '-whitelistalwaysrelay', '-prematurewitness', '-walletprematurewitness', '-promiscuousmempoolflags', '-blockminsize']) +SET_DOC_OPTIONAL = set(['-rpcssl', '-benchmark', '-h', '-help', '-socks', '-tor', '-debugnet', '-whitelistalwaysrelay', '-prematurewitness', '-walletprematurewitness', '-promiscuousmempoolflags', '-blockminsize', '-sendfreetransactions']) def main(): used = check_output(CMD_GREP_ARGS, shell=True) diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index d4fd8bd37..1d19c6575 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -444,11 +444,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) CAmount nChange = 0; unsigned int nBytes = 0; unsigned int nBytesInputs = 0; - double dPriority = 0; - double dPriorityInputs = 0; unsigned int nQuantity = 0; - int nQuantityUncompressed = 0; - bool fAllowFree = false; bool fWitness = false; std::vector vCoinControl; @@ -473,9 +469,6 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) // Amount nAmount += out.tx->tx->vout[out.i].nValue; - // Priority - dPriorityInputs += (double)out.tx->tx->vout[out.i].nValue * (out.nDepth+1); - // Bytes CTxDestination address; int witnessversion = 0; @@ -492,8 +485,6 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) if (keyid && model->getPubKey(*keyid, pubkey)) { nBytesInputs += (pubkey.IsCompressed() ? 148 : 180); - if (!pubkey.IsCompressed()) - nQuantityUncompressed++; } else nBytesInputs += 148; // in all error cases, simply assume 148 here @@ -525,17 +516,6 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) if (nPayFee > 0 && coinControl->nMinimumTotalFee > nPayFee) nPayFee = coinControl->nMinimumTotalFee; - - // Allow free? (require at least hard-coded threshold and default to that if no estimate) - double mempoolEstimatePriority = mempool.estimateSmartPriority(nTxConfirmTarget); - dPriority = dPriorityInputs / (nBytes - nBytesInputs + (nQuantityUncompressed * 29)); // 29 = 180 - 151 (uncompressed public keys are over the limit. max 151 bytes of the input are ignored for priority) - double dPriorityNeeded = std::max(mempoolEstimatePriority, AllowFreeThreshold()); - fAllowFree = (dPriority >= dPriorityNeeded); - - if (fSendFreeTransactions) - if (fAllowFree && nBytes <= MAX_FREE_TRANSACTION_CREATE_SIZE) - nPayFee = 0; - if (nPayAmount > 0) { nChange = nAmount - nPayAmount; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 63501b04b..cf3c6ee12 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -39,7 +39,6 @@ CWallet* pwalletMain = NULL; CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE); unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET; bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE; -bool fSendFreeTransactions = DEFAULT_SEND_FREE_TRANSACTIONS; bool fWalletRbf = DEFAULT_WALLET_RBF; const char * DEFAULT_WALLET_DAT = "wallet.dat"; @@ -2449,7 +2448,6 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt CAmount nValueToSelect = nValue; if (nSubtractFeeFromAmount == 0) nValueToSelect += nFeeRet; - double dPriority = 0; // vouts to the payees for (const auto& recipient : vecSend) { @@ -2490,19 +2488,6 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt strFailReason = _("Insufficient funds"); return false; } - for (const auto& pcoin : setCoins) - { - CAmount nCredit = pcoin.first->tx->vout[pcoin.second].nValue; - //The coin age after the next block (depth+1) is used instead of the current, - //reflecting an assumption the user would accept a bit more delay for - //a chance at a free transaction. - //But mempool inputs might still be in the mempool, so their age stays 0 - int age = pcoin.first->GetDepthInMainChain(); - assert(age >= 0); - if (age != 0) - age += 1; - dPriority += (double)nCredit * age; - } const CAmount nChange = nValueIn - nValueToSelect; if (nChange > 0) @@ -2614,7 +2599,6 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt unsigned int nBytes = GetVirtualTransactionSize(txNew); CTransaction txNewConst(txNew); - dPriority = txNewConst.ComputePriority(dPriority, nBytes); // Remove scriptSigs to eliminate the fee calculation dummy signatures for (auto& vin : txNew.vin) { @@ -2627,16 +2611,6 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt if (coinControl && coinControl->nConfirmTarget > 0) currentConfirmationTarget = coinControl->nConfirmTarget; - // Can we complete this as a free transaction? - if (fSendFreeTransactions && nBytes <= MAX_FREE_TRANSACTION_CREATE_SIZE) - { - // Not enough fee: enough priority? - double dPriorityNeeded = mempool.estimateSmartPriority(currentConfirmationTarget); - // Require at least hard-coded AllowFree. - if (dPriority >= dPriorityNeeded && AllowFree(dPriority)) - break; - } - CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, mempool); if (coinControl && nFeeNeeded > 0 && coinControl->nMinimumTotalFee > nFeeNeeded) { nFeeNeeded = coinControl->nMinimumTotalFee; @@ -3548,8 +3522,6 @@ std::string CWallet::GetWalletHelpString(bool showDebug) CURRENCY_UNIT, FormatMoney(payTxFee.GetFeePerK()))); strUsage += HelpMessageOpt("-rescan", _("Rescan the block chain for missing wallet transactions on startup")); strUsage += HelpMessageOpt("-salvagewallet", _("Attempt to recover private keys from a corrupt wallet on startup")); - if (showDebug) - strUsage += HelpMessageOpt("-sendfreetransactions", strprintf(_("Send transactions as zero-fee transactions if possible (default: %u)"), DEFAULT_SEND_FREE_TRANSACTIONS)); strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE)); strUsage += HelpMessageOpt("-txconfirmtarget=", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET)); strUsage += HelpMessageOpt("-usehd", _("Use hierarchical deterministic key generation (HD) after BIP32. Only has effect during wallet creation/first start") + " " + strprintf(_("(default: %u)"), DEFAULT_USE_HD_WALLET)); @@ -3868,11 +3840,10 @@ bool CWallet::ParameterInteraction() } nTxConfirmTarget = GetArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET); bSpendZeroConfChange = GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); - fSendFreeTransactions = GetBoolArg("-sendfreetransactions", DEFAULT_SEND_FREE_TRANSACTIONS); fWalletRbf = GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); - if (fSendFreeTransactions && GetArg("-limitfreerelay", DEFAULT_LIMITFREERELAY) <= 0) - return InitError("Creation of free transactions with their relay disabled is not supported."); + if (GetBoolArg("-sendfreetransactions", false)) + InitWarning("The argument -sendfreetransactions is no longer supported."); return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 98e4fb87b..29af32375 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -39,7 +39,6 @@ extern CWallet* pwalletMain; extern CFeeRate payTxFee; extern unsigned int nTxConfirmTarget; extern bool bSpendZeroConfChange; -extern bool fSendFreeTransactions; extern bool fWalletRbf; static const unsigned int DEFAULT_KEYPOOL_SIZE = 100; @@ -57,16 +56,12 @@ static const CAmount MIN_CHANGE = CENT; static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; -//! Default for -sendfreetransactions -static const bool DEFAULT_SEND_FREE_TRANSACTIONS = false; //! Default for -walletrejectlongchains static const bool DEFAULT_WALLET_REJECT_LONG_CHAINS = false; //! -txconfirmtarget default static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6; //! -walletrbf default static const bool DEFAULT_WALLET_RBF = false; -//! Largest (in bytes) free transaction we're willing to create -static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; //! if set, all keys will be derived by using BIP32 From 12839cdd56ab5ebb06c59bbf9a064f4bd23b7398 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 13:20:08 -0500 Subject: [PATCH 02/13] [rpc] Remove estimatepriority and estimatesmartpriority. The RPC calls were already deprecated. --- src/rpc/client.cpp | 2 -- src/rpc/mining.cpp | 65 ---------------------------------------------- 2 files changed, 67 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 29bdb3768..2d16868d4 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -107,9 +107,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "keypoolrefill", 0, "newsize" }, { "getrawmempool", 0, "verbose" }, { "estimatefee", 0, "nblocks" }, - { "estimatepriority", 0, "nblocks" }, { "estimatesmartfee", 0, "nblocks" }, - { "estimatesmartpriority", 0, "nblocks" }, { "prioritisetransaction", 1, "priority_delta" }, { "prioritisetransaction", 2, "fee_delta" }, { "setban", 2, "bantime" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 7708771d0..d0d9e0599 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -811,33 +811,6 @@ UniValue estimatefee(const JSONRPCRequest& request) return ValueFromAmount(feeRate.GetFeePerK()); } -UniValue estimatepriority(const JSONRPCRequest& request) -{ - if (request.fHelp || request.params.size() != 1) - throw runtime_error( - "estimatepriority nblocks\n" - "\nDEPRECATED. Estimates the approximate priority a zero-fee transaction needs to begin\n" - "confirmation within nblocks blocks.\n" - "\nArguments:\n" - "1. nblocks (numeric, required)\n" - "\nResult:\n" - "n (numeric) estimated priority\n" - "\n" - "A negative value is returned if not enough transactions and blocks\n" - "have been observed to make an estimate.\n" - "\nExample:\n" - + HelpExampleCli("estimatepriority", "6") - ); - - RPCTypeCheck(request.params, boost::assign::list_of(UniValue::VNUM)); - - int nBlocks = request.params[0].get_int(); - if (nBlocks < 1) - nBlocks = 1; - - return mempool.estimatePriority(nBlocks); -} - UniValue estimatesmartfee(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 1) @@ -875,42 +848,6 @@ UniValue estimatesmartfee(const JSONRPCRequest& request) return result; } -UniValue estimatesmartpriority(const JSONRPCRequest& request) -{ - if (request.fHelp || request.params.size() != 1) - throw runtime_error( - "estimatesmartpriority nblocks\n" - "\nDEPRECATED. WARNING: This interface is unstable and may disappear or change!\n" - "\nEstimates the approximate priority a zero-fee transaction needs to begin\n" - "confirmation within nblocks blocks if possible and return the number of blocks\n" - "for which the estimate is valid.\n" - "\nArguments:\n" - "1. nblocks (numeric, required)\n" - "\nResult:\n" - "{\n" - " \"priority\" : x.x, (numeric) estimated priority\n" - " \"blocks\" : n (numeric) block number where estimate was found\n" - "}\n" - "\n" - "A negative value is returned if not enough transactions and blocks\n" - "have been observed to make an estimate for any number of blocks.\n" - "However if the mempool reject fee is set it will return 1e9 * MAX_MONEY.\n" - "\nExample:\n" - + HelpExampleCli("estimatesmartpriority", "6") - ); - - RPCTypeCheck(request.params, boost::assign::list_of(UniValue::VNUM)); - - int nBlocks = request.params[0].get_int(); - - UniValue result(UniValue::VOBJ); - int answerFound; - double priority = mempool.estimateSmartPriority(nBlocks, &answerFound); - result.push_back(Pair("priority", priority)); - result.push_back(Pair("blocks", answerFound)); - return result; -} - static const CRPCCommand commands[] = { // category name actor (function) okSafeMode // --------------------- ------------------------ ----------------------- ---------- @@ -924,9 +861,7 @@ static const CRPCCommand commands[] = { "generating", "generatetoaddress", &generatetoaddress, true, {"nblocks","address","maxtries"} }, { "util", "estimatefee", &estimatefee, true, {"nblocks"} }, - { "util", "estimatepriority", &estimatepriority, true, {"nblocks"} }, { "util", "estimatesmartfee", &estimatesmartfee, true, {"nblocks"} }, - { "util", "estimatesmartpriority", &estimatesmartpriority, true, {"nblocks"} }, }; void RegisterMiningRPCCommands(CRPCTable &t) From 272b25a6a99057fdcd5db5bce70b49625e973080 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 13:58:42 -0500 Subject: [PATCH 03/13] [mining] Remove -blockprioritysize. Remove ability of mining code to fill part of a block with transactions sorted by coin age. --- qa/rpc-tests/bip68-sequence.py | 4 +- qa/rpc-tests/smartfees.py | 4 +- src/init.cpp | 1 - src/miner.cpp | 153 +-------------------------------- src/miner.h | 12 --- src/policy/policy.h | 2 - src/test/miner_tests.cpp | 1 - src/txmempool.h | 13 --- 8 files changed, 6 insertions(+), 184 deletions(-) diff --git a/qa/rpc-tests/bip68-sequence.py b/qa/rpc-tests/bip68-sequence.py index 74ac393fe..f3a025afa 100755 --- a/qa/rpc-tests/bip68-sequence.py +++ b/qa/rpc-tests/bip68-sequence.py @@ -24,8 +24,8 @@ class BIP68Test(BitcoinTestFramework): def setup_network(self): self.nodes = [] - self.nodes.append(start_node(0, self.options.tmpdir, ["-debug", "-blockprioritysize=0"])) - self.nodes.append(start_node(1, self.options.tmpdir, ["-debug", "-blockprioritysize=0", "-acceptnonstdtxn=0"])) + self.nodes.append(start_node(0, self.options.tmpdir, ["-debug"])) + self.nodes.append(start_node(1, self.options.tmpdir, ["-debug", "-acceptnonstdtxn=0"])) self.is_network_split = False self.relayfee = self.nodes[0].getnetworkinfo()["relayfee"] connect_nodes(self.nodes[0], 1) diff --git a/qa/rpc-tests/smartfees.py b/qa/rpc-tests/smartfees.py index bde454968..5a2b90767 100755 --- a/qa/rpc-tests/smartfees.py +++ b/qa/rpc-tests/smartfees.py @@ -193,13 +193,13 @@ class EstimateFeeTest(BitcoinTestFramework): # NOTE: the CreateNewBlock code starts counting block size at 1,000 bytes, # (17k is room enough for 110 or so transactions) self.nodes.append(start_node(1, self.options.tmpdir, - ["-blockprioritysize=1500", "-blockmaxsize=17000", + ["-blockmaxsize=17000", "-maxorphantx=1000", "-debug=estimatefee"])) connect_nodes(self.nodes[1], 0) # Node2 is a stingy miner, that # produces too small blocks (room for only 55 or so transactions) - node2args = ["-blockprioritysize=0", "-blockmaxsize=8000", "-maxorphantx=1000"] + node2args = ["-blockmaxsize=8000", "-maxorphantx=1000"] self.nodes.append(start_node(2, self.options.tmpdir, node2args)) connect_nodes(self.nodes[0], 2) diff --git a/src/init.cpp b/src/init.cpp index 196b840cb..e66472186 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -476,7 +476,6 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageGroup(_("Block creation options:")); strUsage += HelpMessageOpt("-blockmaxweight=", strprintf(_("Set maximum BIP141 block weight (default: %d)"), DEFAULT_BLOCK_MAX_WEIGHT)); strUsage += HelpMessageOpt("-blockmaxsize=", strprintf(_("Set maximum block size in bytes (default: %d)"), DEFAULT_BLOCK_MAX_SIZE)); - strUsage += HelpMessageOpt("-blockprioritysize=", strprintf(_("Set maximum size of high-priority/low-fee transactions in bytes (default: %d)"), DEFAULT_BLOCK_PRIORITY_SIZE)); strUsage += HelpMessageOpt("-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))); if (showDebug) strUsage += HelpMessageOpt("-blockversion=", "Override block version to test forking scenarios"); diff --git a/src/miner.cpp b/src/miner.cpp index d01edd93b..1198dbb09 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -39,8 +39,8 @@ // // Unconfirmed transactions in the memory pool often depend on other // transactions in the memory pool. When we select transactions from the -// pool, we select by highest priority or fee rate, so we might consider -// transactions that depend on transactions that aren't yet in the block. +// pool, we select by highest fee rate of a transaction combined with all +// its ancestors. uint64_t nLastBlockTx = 0; uint64_t nLastBlockSize = 0; @@ -122,9 +122,6 @@ 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) @@ -167,7 +164,6 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc // transaction (which in most cases can be a no-op). fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()); - addPriorityTxs(); addPackageTxs(); nLastBlockTx = nBlockTx; @@ -204,17 +200,6 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc return std::move(pblocktemplate); } -bool BlockAssembler::isStillDependent(CTxMemPool::txiter iter) -{ - BOOST_FOREACH(CTxMemPool::txiter parent, mempool.GetMemPoolParents(iter)) - { - if (!inBlock.count(parent)) { - return true; - } - } - return false; -} - void BlockAssembler::onlyUnconfirmed(CTxMemPool::setEntries& testSet) { for (CTxMemPool::setEntries::iterator iit = testSet.begin(); iit != testSet.end(); ) { @@ -262,58 +247,6 @@ bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& packa return true; } -bool BlockAssembler::TestForBlock(CTxMemPool::txiter iter) -{ - if (nBlockWeight + iter->GetTxWeight() >= nBlockMaxWeight) { - // 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 || 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; - } - - if (fNeedSizeAccounting) { - if (nBlockSize + ::GetSerializeSize(iter->GetTx(), SER_NETWORK, PROTOCOL_VERSION) >= nBlockMaxSize) { - if (nBlockSize > nBlockMaxSize - 100 || lastFewTxs > 50) { - blockFinished = true; - return false; - } - if (nBlockSize > nBlockMaxSize - 1000) { - lastFewTxs++; - } - return false; - } - } - - if (nBlockSigOpsCost + iter->GetSigOpCost() >= MAX_BLOCK_SIGOPS_COST) { - // If the block has room for no more sig ops then - // flag that the block is finished - if (nBlockSigOpsCost > MAX_BLOCK_SIGOPS_COST - 8) { - blockFinished = true; - return false; - } - // Otherwise attempt to find another tx with fewer sigops - // to put in the block. - return false; - } - - // Must check that lock times are still valid - // This can be removed once MTP is always enforced - // as long as reorgs keep the mempool consistent. - if (!IsFinalTx(iter->GetTx(), nHeight, nLockTimeCutoff)) - return false; - - return true; -} - void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) { pblock->vtx.emplace_back(iter->GetSharedTx()); @@ -512,88 +445,6 @@ void BlockAssembler::addPackageTxs() } } -void BlockAssembler::addPriorityTxs() -{ - // How much of the block should be dedicated to high-priority transactions, - // included regardless of the fees they pay - unsigned int nBlockPrioritySize = 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); - CAmount dummy; - mempool.ApplyDeltas(mi->GetTx().GetHash(), dPriority, dummy); - 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); - - // If now that this txs is added we've surpassed our desired priority size - // or have dropped below the AllowFreeThreshold, then we're done adding priority txs - if (nBlockSize >= nBlockPrioritySize || !AllowFree(actualPriority)) { - break; - } - - // This tx was successfully added, so - // add transactions that depend on this one to the priority queue to try again - BOOST_FOREACH(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; -} - void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce) { // Update nExtraNonce diff --git a/src/miner.h b/src/miner.h index 3ba92b16b..625ffe97f 100644 --- a/src/miner.h +++ b/src/miner.h @@ -158,10 +158,6 @@ private: int64_t nLockTimeCutoff; const CChainParams& chainparams; - // Variables used for addPriorityTxs - int lastFewTxs; - bool blockFinished; - public: BlockAssembler(const CChainParams& chainparams); /** Construct a new block template with coinbase to scriptPubKeyIn */ @@ -175,17 +171,9 @@ private: void AddToBlock(CTxMemPool::txiter iter); // Methods for how to add transactions to a block. - /** Add transactions based on tx "priority" */ - void addPriorityTxs(); /** Add transactions based on feerate including unconfirmed ancestors */ void addPackageTxs(); - // 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); - // helper functions for addPackageTxs() /** Remove confirmed (inBlock) entries from given set */ void onlyUnconfirmed(CTxMemPool::setEntries& testSet); diff --git a/src/policy/policy.h b/src/policy/policy.h index 9b1323ac2..6df541bc0 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -16,8 +16,6 @@ class CCoinsViewCache; /** Default for -blockmaxsize, which controls the maximum size of block the mining code will create **/ static const unsigned int DEFAULT_BLOCK_MAX_SIZE = 750000; -/** Default for -blockprioritysize, maximum space for zero/low-fee transactions **/ -static const unsigned int DEFAULT_BLOCK_PRIORITY_SIZE = 0; /** Default for -blockmaxweight, which controls the range of block weights the mining code will create **/ static const unsigned int DEFAULT_BLOCK_MAX_WEIGHT = 3000000; /** Default for -blockmintxfee, which sets the minimum feerate for a transaction in blocks created by mining code **/ diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index f856d8a91..1d49848df 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -79,7 +79,6 @@ bool TestSequenceLocks(const CTransaction &tx, int flags) // Test suite for ancestor feerate transaction selection. // Implemented as an additional function, rather than a separate test case, // to allow reusing the blockchain created in CreateNewBlock_validity. -// Note that this test assumes blockprioritysize is 0. void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, std::vector& txFirst) { // Test the ancestor feerate transaction selection. diff --git a/src/txmempool.h b/src/txmempool.h index a7ecb6439..4cba02430 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -720,17 +720,4 @@ public: bool HaveCoins(const uint256 &txid) const; }; -// 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; - } -}; - #endif // BITCOIN_TXMEMPOOL_H From 400b15147cf1c4757927935222611e8d3481e739 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 14:59:28 -0500 Subject: [PATCH 04/13] [debug] Change -printpriority option -printpriority output is now changed to only show the fee rate and hash of transactions included in a block by the mining code. --- src/init.cpp | 2 +- src/miner.cpp | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index e66472186..c6026646d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -456,7 +456,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-printtoconsole", _("Send trace/debug info to console instead of debug.log file")); if (showDebug) { - strUsage += HelpMessageOpt("-printpriority", strprintf("Log transaction priority and fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY)); + strUsage += HelpMessageOpt("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY)); } strUsage += HelpMessageOpt("-shrinkdebugfile", _("Shrink debug.log file on client startup (default: 1 when no -debug)")); diff --git a/src/miner.cpp b/src/miner.cpp index 1198dbb09..75c9d82a2 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -263,11 +263,7 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) bool fPrintPriority = GetBoolArg("-printpriority", DEFAULT_PRINTPRIORITY); if (fPrintPriority) { - double dPriority = iter->GetPriority(nHeight); - CAmount dummy; - mempool.ApplyDeltas(iter->GetTx().GetHash(), dPriority, dummy); - LogPrintf("priority %.1f fee %s txid %s\n", - dPriority, + LogPrintf("fee %s txid %s\n", CFeeRate(iter->GetModifiedFee(), iter->GetTxSize()).ToString(), iter->GetTx().GetHash().ToString()); } From fe282acd7604b5265762b24e531bdf1ebb1f009b Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 15:17:33 -0500 Subject: [PATCH 05/13] [cleanup] Remove estimatePriority and estimateSmartPriority Unused everywhere now except one test. --- src/policy/fees.cpp | 18 ------------------ src/policy/fees.h | 15 --------------- src/test/policyestimator_tests.cpp | 2 -- src/txmempool.cpp | 10 ---------- src/txmempool.h | 9 --------- 5 files changed, 54 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 8f6a1e60f..89220f26b 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -452,24 +452,6 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun return CFeeRate(median); } -double CBlockPolicyEstimator::estimatePriority(int confTarget) -{ - return -1; -} - -double CBlockPolicyEstimator::estimateSmartPriority(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool) -{ - if (answerFoundAtTarget) - *answerFoundAtTarget = confTarget; - - // If mempool is limiting txs, no priority txs are allowed - CAmount minPoolFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); - if (minPoolFee > 0) - return INF_PRIORITY; - - return -1; -} - void CBlockPolicyEstimator::Write(CAutoFile& fileout) { fileout << nBestSeenHeight; diff --git a/src/policy/fees.h b/src/policy/fees.h index 064466afe..f2f430861 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -182,7 +182,6 @@ static const double SUFFICIENT_FEETXS = 1; static constexpr double MIN_FEERATE = 10; static const double MAX_FEERATE = 1e7; static const double INF_FEERATE = MAX_MONEY; -static const double INF_PRIORITY = 1e9 * MAX_MONEY; // We have to lump transactions into buckets based on feerate, but we want to be able // to give accurate estimates over a large range of potential feerates @@ -223,20 +222,6 @@ public: */ CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool); - /** Return a priority estimate. - * DEPRECATED - * Returns -1 - */ - double estimatePriority(int confTarget); - - /** Estimate priority needed to get be included in a block within - * confTarget blocks. - * DEPRECATED - * Returns -1 unless mempool is currently limited then returns INF_PRIORITY - * answerFoundAtTarget is set to confTarget - */ - double estimateSmartPriority(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool); - /** Write estimation data to a file */ void Write(CAutoFile& fileout); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 0c060801b..a4f85c598 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -185,7 +185,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) } // Test that if the mempool is limited, estimateSmartFee won't return a value below the mempool min fee - // and that estimateSmartPriority returns essentially an infinite value mpool.addUnchecked(tx.GetHash(), entry.Fee(feeV[5]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); // evict that transaction which should set a mempool min fee of minRelayTxFee + feeV[5] mpool.TrimToSize(1); @@ -193,7 +192,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int i = 1; i < 10; i++) { BOOST_CHECK(mpool.estimateSmartFee(i).GetFeePerK() >= mpool.estimateFee(i).GetFeePerK()); BOOST_CHECK(mpool.estimateSmartFee(i).GetFeePerK() >= mpool.GetMinFee(1).GetFeePerK()); - BOOST_CHECK(mpool.estimateSmartPriority(i) == INF_PRIORITY); } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 942a6fcce..ef4d4f3c6 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -875,16 +875,6 @@ CFeeRate CTxMemPool::estimateSmartFee(int nBlocks, int *answerFoundAtBlocks) con LOCK(cs); return minerPolicyEstimator->estimateSmartFee(nBlocks, answerFoundAtBlocks, *this); } -double CTxMemPool::estimatePriority(int nBlocks) const -{ - LOCK(cs); - return minerPolicyEstimator->estimatePriority(nBlocks); -} -double CTxMemPool::estimateSmartPriority(int nBlocks, int *answerFoundAtBlocks) const -{ - LOCK(cs); - return minerPolicyEstimator->estimateSmartPriority(nBlocks, answerFoundAtBlocks, *this); -} bool CTxMemPool::WriteFeeEstimates(CAutoFile& fileout) const diff --git a/src/txmempool.h b/src/txmempool.h index 4cba02430..4af85195c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -648,15 +648,6 @@ public: /** Estimate fee rate needed to get into the next nBlocks */ CFeeRate estimateFee(int nBlocks) const; - /** Estimate priority needed to get into the next nBlocks - * If no answer can be given at nBlocks, return an estimate - * at the lowest number of blocks where one can be given - */ - double estimateSmartPriority(int nBlocks, int *answerFoundAtBlocks = NULL) const; - - /** Estimate priority needed to get into the next nBlocks */ - double estimatePriority(int nBlocks) const; - /** Write/Read estimates to disk */ bool WriteFeeEstimates(CAutoFile& fileout) const; bool ReadFeeEstimates(CAutoFile& filein); From ad727f4eaf1cb3ed7a0e340f690bd804299e698c Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 20 Jan 2017 08:39:25 -0500 Subject: [PATCH 06/13] [rpc] sendrawtransaction no longer bypasses minRelayTxFee The prioritisetransaction API can always be used if a transaction needs to be submitted that bypasses minRelayTxFee. --- qa/rpc-tests/nulldummy.py | 4 ++-- src/rpc/rawtransaction.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/nulldummy.py b/qa/rpc-tests/nulldummy.py index 66c4e90f2..b44a98bfd 100755 --- a/qa/rpc-tests/nulldummy.py +++ b/qa/rpc-tests/nulldummy.py @@ -74,7 +74,7 @@ class NULLDUMMYTest(BitcoinTestFramework): self.block_submit(self.nodes[0], test1txs, False, True) print ("Test 2: Non-NULLDUMMY base multisig transaction should not be accepted to mempool before activation") - test2tx = self.create_transaction(self.nodes[0], txid2, self.ms_address, 48) + test2tx = self.create_transaction(self.nodes[0], txid2, self.ms_address, 47) trueDummy(test2tx) txid4 = self.tx_submit(self.nodes[0], test2tx, NULLDUMMY_ERROR) @@ -82,7 +82,7 @@ class NULLDUMMYTest(BitcoinTestFramework): self.block_submit(self.nodes[0], [test2tx], False, True) print ("Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation") - test4tx = self.create_transaction(self.nodes[0], txid4, self.address, 47) + test4tx = self.create_transaction(self.nodes[0], txid4, self.address, 46) test6txs=[CTransaction(test4tx)] trueDummy(test4tx) self.tx_submit(self.nodes[0], test4tx, NULLDUMMY_ERROR) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index bf16f2749..796db1de5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -891,7 +891,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) CTransactionRef tx(MakeTransactionRef(std::move(mtx))); const uint256& hashTx = tx->GetHash(); - bool fLimitFree = false; + bool fLimitFree = true; CAmount nMaxRawTxFee = maxTxFee; if (request.params.size() > 1 && request.params[1].get_bool()) nMaxRawTxFee = 0; From f8380054442d8aca4999277fe7617067edf231b9 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 22:07:56 -0500 Subject: [PATCH 07/13] No longer allow "free" transactions Remove -limitfreerelay and always enforce minRelayTxFee in the mempool (except from disconnected blocks) Remove -relaypriority, the option was only used for the ability to allow free transactions to be relayed regardless of their priority. Both notions no longer apply. --- qa/rpc-tests/abandonconflict.py | 1 - qa/rpc-tests/smartfees.py | 3 +-- src/init.cpp | 4 +--- src/net_processing.cpp | 7 +++---- src/rpc/misc.cpp | 2 +- src/rpc/net.cpp | 2 +- src/txmempool.h | 12 ------------ src/validation.cpp | 27 +++------------------------ src/validation.h | 2 -- src/wallet/wallet.cpp | 3 --- 10 files changed, 10 insertions(+), 53 deletions(-) diff --git a/qa/rpc-tests/abandonconflict.py b/qa/rpc-tests/abandonconflict.py index b32d4e2ce..5a860e439 100755 --- a/qa/rpc-tests/abandonconflict.py +++ b/qa/rpc-tests/abandonconflict.py @@ -80,7 +80,6 @@ class AbandonConflictTest(BitcoinTestFramework): # Restart the node with a higher min relay fee so the parent tx is no longer in mempool # TODO: redo with eviction - # Note had to make sure tx did not have AllowFree priority stop_node(self.nodes[0],0) self.nodes[0]=start_node(0, self.options.tmpdir, ["-debug","-logtimemicros","-minrelaytxfee=0.0001"]) diff --git a/qa/rpc-tests/smartfees.py b/qa/rpc-tests/smartfees.py index 5a2b90767..4289c7b0b 100755 --- a/qa/rpc-tests/smartfees.py +++ b/qa/rpc-tests/smartfees.py @@ -188,8 +188,7 @@ class EstimateFeeTest(BitcoinTestFramework): # Now we can connect the other nodes, didn't want to connect them earlier # so the estimates would not be affected by the splitting transactions - # Node1 mines small blocks but that are bigger than the expected transaction rate, - # and allows free transactions. + # Node1 mines small blocks but that are bigger than the expected transaction rate. # NOTE: the CreateNewBlock code starts counting block size at 1,000 bytes, # (17k is room enough for 110 or so transactions) self.nodes.append(start_node(1, self.options.tmpdir, diff --git a/src/init.cpp b/src/init.cpp index c6026646d..3276ec9f1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -444,8 +444,6 @@ std::string HelpMessage(HelpMessageMode mode) { strUsage += HelpMessageOpt("-logtimemicros", strprintf("Add microsecond precision to debug timestamps (default: %u)", DEFAULT_LOGTIMEMICROS)); strUsage += HelpMessageOpt("-mocktime=", "Replace actual time with seconds since epoch (default: 0)"); - strUsage += HelpMessageOpt("-limitfreerelay=", strprintf("Continuously rate-limit free transactions to *1000 bytes per minute (default: %u)", DEFAULT_LIMITFREERELAY)); - strUsage += HelpMessageOpt("-relaypriority", strprintf("Require high priority for relaying free or low-fee transactions (default: %u)", DEFAULT_RELAYPRIORITY)); strUsage += HelpMessageOpt("-maxsigcachesize=", strprintf("Limit size of signature cache to MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE)); strUsage += HelpMessageOpt("-maxtipage=", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE)); } @@ -975,7 +973,7 @@ bool AppInitParameterInteraction() if (nConnectTimeout <= 0) nConnectTimeout = DEFAULT_CONNECT_TIMEOUT; - // Fee-per-kilobyte amount considered the same as "free" + // Fee-per-kilobyte amount required for mempool acceptance and relay // If you are mining, be careful setting this: // if you set it to zero then // a transaction spammer can cheaply fill blocks using diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3ec1a1c27..71a0a1de2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1853,7 +1853,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint("mempool", " invalid orphan tx %s\n", orphanHash.ToString()); } // Has inputs but not accepted to mempool - // Probably non-standard or insufficient fee/priority + // Probably non-standard or insufficient fee LogPrint("mempool", " removed orphan tx %s\n", orphanHash.ToString()); vEraseQueue.push_back(orphanHash); if (!orphanTx.HasWitness() && !stateDummy.CorruptionPossible()) { @@ -3249,9 +3249,8 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic& interr static CFeeRate default_feerate(DEFAULT_MIN_RELAY_TX_FEE); static FeeFilterRounder filterRounder(default_feerate); CAmount filterToSend = filterRounder.round(currentFilter); - // If we don't allow free transactions, then we always have a fee filter of at least minRelayTxFee - if (GetArg("-limitfreerelay", DEFAULT_LIMITFREERELAY) <= 0) - filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK()); + // We always have a fee filter of at least minRelayTxFee + filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK()); if (filterToSend != pto->lastSentFeeFilter) { connman.PushMessage(pto, msgMaker.Make(NetMsgType::FEEFILTER, filterToSend)); pto->lastSentFeeFilter = filterToSend; diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 140cb4840..3045fa2b5 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -61,7 +61,7 @@ UniValue getinfo(const JSONRPCRequest& request) " \"keypoolsize\": xxxx, (numeric) how many new keys are pre-generated\n" " \"unlocked_until\": ttt, (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked\n" " \"paytxfee\": x.xxxx, (numeric) the transaction fee set in " + CURRENCY_UNIT + "/kB\n" - " \"relayfee\": x.xxxx, (numeric) minimum relay fee for non-free transactions in " + CURRENCY_UNIT + "/kB\n" + " \"relayfee\": x.xxxx, (numeric) minimum relay fee for transactions in " + CURRENCY_UNIT + "/kB\n" " \"errors\": \"...\" (string) any error messages\n" "}\n" "\nExamples:\n" diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index f590db5ef..a7169749c 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -417,7 +417,7 @@ UniValue getnetworkinfo(const JSONRPCRequest& request) " }\n" " ,...\n" " ],\n" - " \"relayfee\": x.xxxxxxxx, (numeric) minimum relay fee for non-free transactions in " + CURRENCY_UNIT + "/kB\n" + " \"relayfee\": x.xxxxxxxx, (numeric) minimum relay fee for transactions in " + CURRENCY_UNIT + "/kB\n" " \"incrementalfee\": x.xxxxxxxx, (numeric) minimum fee increment for mempool limiting or BIP 125 replacement in " + CURRENCY_UNIT + "/kB\n" " \"localaddresses\": [ (array) list of local addresses\n" " {\n" diff --git a/src/txmempool.h b/src/txmempool.h index 4af85195c..9af65bd07 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -30,18 +30,6 @@ class CAutoFile; class CBlockIndex; -inline double AllowFreeThreshold() -{ - return COIN * 144 / 250; -} - -inline bool AllowFree(double dPriority) -{ - // Large (in bytes) low-priority (new, small-coin) transactions - // need a fee. - return dPriority > AllowFreeThreshold(); -} - /** Fake height value used in CCoins to signify they are only in the memory pool (since 0.8) */ static const unsigned int MEMPOOL_HEIGHT = 0x7FFFFFFF; diff --git a/src/validation.cpp b/src/validation.cpp index e84b1a728..a21dc25a5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -753,32 +753,11 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C CAmount mempoolRejectFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize); if (mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) { return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", false, strprintf("%d < %d", nFees, mempoolRejectFee)); - } else if (GetBoolArg("-relaypriority", DEFAULT_RELAYPRIORITY) && nModifiedFees < ::minRelayTxFee.GetFee(nSize) && !AllowFree(entry.GetPriority(chainActive.Height() + 1))) { - // Require that free transactions have sufficient priority to be mined in the next block. - return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "insufficient priority"); } - // Continuously rate-limit free (really, very-low-fee) transactions - // This mitigates 'penny-flooding' -- sending thousands of free transactions just to - // be annoying or make others' transactions take longer to confirm. - if (fLimitFree && nModifiedFees < ::minRelayTxFee.GetFee(nSize)) - { - static CCriticalSection csFreeLimiter; - static double dFreeCount; - static int64_t nLastTime; - int64_t nNow = GetTime(); - - LOCK(csFreeLimiter); - - // Use an exponentially decaying ~10-minute window: - dFreeCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime)); - nLastTime = nNow; - // -limitfreerelay unit is thousand-bytes-per-minute - // At default rate it would take over a month to fill 1GB - if (dFreeCount + nSize >= GetArg("-limitfreerelay", DEFAULT_LIMITFREERELAY) * 10 * 1000) - return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "rate limited free transaction"); - LogPrint("mempool", "Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount+nSize); - dFreeCount += nSize; + // No transactions are allowed below minRelayTxFee except from disconnected blocks + if (fLimitFree && nModifiedFees < ::minRelayTxFee.GetFee(nSize)) { + return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "min relay fee not met"); } if (nAbsurdFee && nFees > nAbsurdFee) diff --git a/src/validation.h b/src/validation.h index 9c606f241..43f0dbae3 100644 --- a/src/validation.h +++ b/src/validation.h @@ -122,8 +122,6 @@ static const int64_t BLOCK_DOWNLOAD_TIMEOUT_BASE = 1000000; /** Additional block download timeout per parallel downloading peer (i.e. 5 min) */ static const int64_t BLOCK_DOWNLOAD_TIMEOUT_PER_PEER = 500000; -static const unsigned int DEFAULT_LIMITFREERELAY = 0; -static const bool DEFAULT_RELAYPRIORITY = true; static const int64_t DEFAULT_MAX_TIP_AGE = 24 * 60 * 60; /** Maximum age of our tip in seconds for us to be considered current for fee estimation */ static const int64_t MAX_FEE_ESTIMATION_TIP_AGE = 3 * 60 * 60; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cf3c6ee12..ba16cdf26 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3842,9 +3842,6 @@ bool CWallet::ParameterInteraction() bSpendZeroConfChange = GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); fWalletRbf = GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); - if (GetBoolArg("-sendfreetransactions", false)) - InitWarning("The argument -sendfreetransactions is no longer supported."); - return true; } From 0315888d0db2f39c784afac19ff5db5086f30820 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 22:46:50 -0500 Subject: [PATCH 08/13] [test] Remove priority from tests Remove all coin age priority functionality from unit tests and RPC tests. --- qa/rpc-tests/prioritise_transaction.py | 30 ++++++------------ qa/rpc-tests/smartfees.py | 9 +++--- qa/rpc-tests/test_framework/util.py | 41 ------------------------- src/test/mempool_tests.cpp | 42 ++++++++++++-------------- src/test/miner_tests.cpp | 3 +- src/test/policyestimator_tests.cpp | 8 ++--- src/test/test_bitcoin.cpp | 13 +++----- src/test/test_bitcoin.h | 9 ++---- 8 files changed, 47 insertions(+), 108 deletions(-) diff --git a/qa/rpc-tests/prioritise_transaction.py b/qa/rpc-tests/prioritise_transaction.py index 9a63d0838..91d5d6be7 100755 --- a/qa/rpc-tests/prioritise_transaction.py +++ b/qa/rpc-tests/prioritise_transaction.py @@ -50,10 +50,8 @@ class PrioritiseTransactionTest(BitcoinTestFramework): assert(sizes[i] > MAX_BLOCK_BASE_SIZE) # Fail => raise utxo_count # add a fee delta to something in the cheapest bucket and make sure it gets mined - # also check that a different entry in the cheapest bucket is NOT mined (lower - # the priority to ensure its not mined due to priority) + # also check that a different entry in the cheapest bucket is NOT mined self.nodes[0].prioritisetransaction(txids[0][0], 0, int(3*base_fee*COIN)) - self.nodes[0].prioritisetransaction(txids[0][1], -1e15, 0) self.nodes[0].generate(1) @@ -96,7 +94,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): if (x != high_fee_tx): assert(x not in mempool) - # Create a free, low priority transaction. Should be rejected. + # Create a free transaction. Should be rejected. utxo_list = self.nodes[0].listunspent() assert(len(utxo_list) > 0) utxo = utxo_list[0] @@ -104,37 +102,27 @@ class PrioritiseTransactionTest(BitcoinTestFramework): inputs = [] outputs = {} inputs.append({"txid" : utxo["txid"], "vout" : utxo["vout"]}) - outputs[self.nodes[0].getnewaddress()] = utxo["amount"] - self.relayfee + outputs[self.nodes[0].getnewaddress()] = utxo["amount"] raw_tx = self.nodes[0].createrawtransaction(inputs, outputs) tx_hex = self.nodes[0].signrawtransaction(raw_tx)["hex"] - txid = self.nodes[0].sendrawtransaction(tx_hex) - - # A tx that spends an in-mempool tx has 0 priority, so we can use it to - # test the effect of using prioritise transaction for mempool acceptance - inputs = [] - inputs.append({"txid": txid, "vout": 0}) - outputs = {} - outputs[self.nodes[0].getnewaddress()] = utxo["amount"] - self.relayfee - raw_tx2 = self.nodes[0].createrawtransaction(inputs, outputs) - tx2_hex = self.nodes[0].signrawtransaction(raw_tx2)["hex"] - tx2_id = self.nodes[0].decoderawtransaction(tx2_hex)["txid"] + tx_id = self.nodes[0].decoderawtransaction(tx_hex)["txid"] try: - self.nodes[0].sendrawtransaction(tx2_hex) + self.nodes[0].sendrawtransaction(tx_hex) except JSONRPCException as exp: assert_equal(exp.error['code'], -26) # insufficient fee - assert(tx2_id not in self.nodes[0].getrawmempool()) + assert(tx_id not in self.nodes[0].getrawmempool()) else: assert(False) # This is a less than 1000-byte transaction, so just set the fee # to be the minimum for a 1000 byte transaction and check that it is # accepted. - self.nodes[0].prioritisetransaction(tx2_id, 0, int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(tx_id, 0, int(self.relayfee*COIN)) print("Assert that prioritised free transaction is accepted to mempool") - assert_equal(self.nodes[0].sendrawtransaction(tx2_hex), tx2_id) - assert(tx2_id in self.nodes[0].getrawmempool()) + assert_equal(self.nodes[0].sendrawtransaction(tx_hex), tx_id) + assert(tx_id in self.nodes[0].getrawmempool()) if __name__ == '__main__': PrioritiseTransactionTest().main() diff --git a/qa/rpc-tests/smartfees.py b/qa/rpc-tests/smartfees.py index 4289c7b0b..9ce5b8e86 100755 --- a/qa/rpc-tests/smartfees.py +++ b/qa/rpc-tests/smartfees.py @@ -69,10 +69,11 @@ def small_txpuzzle_randfee(from_node, conflist, unconflist, amount, min_fee, fee def split_inputs(from_node, txins, txouts, initial_split = False): """ - We need to generate a lot of very small inputs so we can generate a ton of transactions - and they will have low priority. + We need to generate a lot of inputs so we can generate a ton of transactions. This function takes an input from txins, and creates and sends a transaction which splits the value into 2 outputs which are appended to txouts. + Previously this was designed to be small inputs so they wouldn't have + a high coin age when the notion of priority still existed. """ prevtxout = txins.pop() inputs = [] @@ -150,7 +151,7 @@ class EstimateFeeTest(BitcoinTestFramework): def setup_network(self): """ We'll setup the network to have 3 nodes that all mine with different parameters. - But first we need to use one node to create a lot of small low priority outputs + But first we need to use one node to create a lot of outputs which we will use to generate our transactions. """ self.nodes = [] @@ -159,7 +160,7 @@ class EstimateFeeTest(BitcoinTestFramework): "-whitelist=127.0.0.1"])) print("This test is time consuming, please be patient") - print("Splitting inputs to small size so we can generate low priority tx's") + print("Splitting inputs so we can generate tx's") self.txouts = [] self.txouts2 = [] # Split a coinbase into two transaction puzzle outputs diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index e838a4058..408c266aa 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -442,47 +442,6 @@ def make_change(from_node, amount_in, amount_out, fee): outputs[from_node.getnewaddress()] = change return outputs -def send_zeropri_transaction(from_node, to_node, amount, fee): - """ - Create&broadcast a zero-priority transaction. - Returns (txid, hex-encoded-txdata) - Ensures transaction is zero-priority by first creating a send-to-self, - then using its output - """ - - # Create a send-to-self with confirmed inputs: - self_address = from_node.getnewaddress() - (total_in, inputs) = gather_inputs(from_node, amount+fee*2) - outputs = make_change(from_node, total_in, amount+fee, fee) - outputs[self_address] = float(amount+fee) - - self_rawtx = from_node.createrawtransaction(inputs, outputs) - self_signresult = from_node.signrawtransaction(self_rawtx) - self_txid = from_node.sendrawtransaction(self_signresult["hex"], True) - - vout = find_output(from_node, self_txid, amount+fee) - # Now immediately spend the output to create a 1-input, 1-output - # zero-priority transaction: - inputs = [ { "txid" : self_txid, "vout" : vout } ] - outputs = { to_node.getnewaddress() : float(amount) } - - rawtx = from_node.createrawtransaction(inputs, outputs) - signresult = from_node.signrawtransaction(rawtx) - txid = from_node.sendrawtransaction(signresult["hex"], True) - - return (txid, signresult["hex"]) - -def random_zeropri_transaction(nodes, amount, min_fee, fee_increment, fee_variants): - """ - Create a random zero-priority transaction. - Returns (txid, hex-encoded-transaction-data, fee) - """ - from_node = random.choice(nodes) - to_node = random.choice(nodes) - fee = min_fee + fee_increment*random.randint(0,fee_variants) - (txid, txhex) = send_zeropri_transaction(from_node, to_node, amount, fee) - return (txid, txhex, fee) - def random_transaction(nodes, amount, min_fee, fee_increment, fee_variants): """ Create a random transaction. diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 91f549fe4..cdef1f5aa 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -126,28 +126,28 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).Priority(10.0).FromTx(tx1)); + pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1)); /* highest fee */ CMutableTransaction tx2 = CMutableTransaction(); tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx2.vout[0].nValue = 2 * COIN; - pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).Priority(9.0).FromTx(tx2)); + pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).FromTx(tx2)); /* lowest fee */ CMutableTransaction tx3 = CMutableTransaction(); tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx3.vout[0].nValue = 5 * COIN; - pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).Priority(100.0).FromTx(tx3)); + pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).FromTx(tx3)); /* 2nd highest fee */ CMutableTransaction tx4 = CMutableTransaction(); tx4.vout.resize(1); tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx4.vout[0].nValue = 6 * COIN; - pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).Priority(1.0).FromTx(tx4)); + pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).FromTx(tx4)); /* equal fee rate to tx1, but newer */ CMutableTransaction tx5 = CMutableTransaction(); @@ -155,7 +155,6 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx5.vout[0].nValue = 11 * COIN; entry.nTime = 1; - entry.dPriority = 10.0; pool.addUnchecked(tx5.GetHash(), entry.Fee(10000LL).FromTx(tx5)); BOOST_CHECK_EQUAL(pool.size(), 5); @@ -328,14 +327,14 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).Priority(10.0).FromTx(tx1)); + pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1)); /* highest fee */ CMutableTransaction tx2 = CMutableTransaction(); tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx2.vout[0].nValue = 2 * COIN; - pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).Priority(9.0).FromTx(tx2)); + pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).FromTx(tx2)); uint64_t tx2Size = GetVirtualTransactionSize(tx2); /* lowest fee */ @@ -343,14 +342,14 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx3.vout[0].nValue = 5 * COIN; - pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).Priority(100.0).FromTx(tx3)); + pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).FromTx(tx3)); /* 2nd highest fee */ CMutableTransaction tx4 = CMutableTransaction(); tx4.vout.resize(1); tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx4.vout[0].nValue = 6 * COIN; - pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).Priority(1.0).FromTx(tx4)); + pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).FromTx(tx4)); /* equal fee rate to tx1, but newer */ CMutableTransaction tx5 = CMutableTransaction(); @@ -434,7 +433,6 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { CTxMemPool pool(CFeeRate(1000)); TestMemPoolEntryHelper entry; - entry.dPriority = 10.0; CMutableTransaction tx1 = CMutableTransaction(); tx1.vin.resize(1); @@ -442,7 +440,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_1 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1, &pool)); + pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).FromTx(tx1)); CMutableTransaction tx2 = CMutableTransaction(); tx2.vin.resize(1); @@ -450,7 +448,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_2 << OP_EQUAL; tx2.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx2.GetHash(), entry.Fee(5000LL).FromTx(tx2, &pool)); + pool.addUnchecked(tx2.GetHash(), entry.Fee(5000LL).FromTx(tx2)); pool.TrimToSize(pool.DynamicMemoryUsage()); // should do nothing BOOST_CHECK(pool.exists(tx1.GetHash())); @@ -460,7 +458,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(pool.exists(tx1.GetHash())); BOOST_CHECK(!pool.exists(tx2.GetHash())); - pool.addUnchecked(tx2.GetHash(), entry.FromTx(tx2, &pool)); + pool.addUnchecked(tx2.GetHash(), entry.FromTx(tx2)); CMutableTransaction tx3 = CMutableTransaction(); tx3.vin.resize(1); tx3.vin[0].prevout = COutPoint(tx2.GetHash(), 0); @@ -468,7 +466,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_3 << OP_EQUAL; tx3.vout[0].nValue = 10 * COIN; - pool.addUnchecked(tx3.GetHash(), entry.Fee(20000LL).FromTx(tx3, &pool)); + pool.addUnchecked(tx3.GetHash(), entry.Fee(20000LL).FromTx(tx3)); pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // tx3 should pay for tx2 (CPFP) BOOST_CHECK(!pool.exists(tx1.GetHash())); @@ -531,10 +529,10 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx7.vout[1].scriptPubKey = CScript() << OP_7 << OP_EQUAL; tx7.vout[1].nValue = 10 * COIN; - pool.addUnchecked(tx4.GetHash(), entry.Fee(7000LL).FromTx(tx4, &pool)); - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5, &pool)); - pool.addUnchecked(tx6.GetHash(), entry.Fee(1100LL).FromTx(tx6, &pool)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); + pool.addUnchecked(tx4.GetHash(), entry.Fee(7000LL).FromTx(tx4)); + pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5)); + pool.addUnchecked(tx6.GetHash(), entry.Fee(1100LL).FromTx(tx6)); + pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); // we only require this remove, at max, 2 txn, because its not clear what we're really optimizing for aside from that pool.TrimToSize(pool.DynamicMemoryUsage() - 1); @@ -543,8 +541,8 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(!pool.exists(tx7.GetHash())); if (!pool.exists(tx5.GetHash())) - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5, &pool)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); + pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5)); + pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); pool.TrimToSize(pool.DynamicMemoryUsage() / 2); // should maximize mempool size by only removing 5/7 BOOST_CHECK(pool.exists(tx4.GetHash())); @@ -552,8 +550,8 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(pool.exists(tx6.GetHash())); BOOST_CHECK(!pool.exists(tx7.GetHash())); - pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5, &pool)); - pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); + pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5)); + pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); std::vector vtx; SetMockTime(42); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 1d49848df..195d1dd75 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -193,7 +193,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) uint256 hash; TestMemPoolEntryHelper entry; entry.nFee = 11; - entry.dPriority = 111.0; entry.nHeight = 11; LOCK(cs_main); @@ -298,7 +297,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) BOOST_CHECK_THROW(BlockAssembler(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error); mempool.clear(); - // child with higher priority than parent + // child with higher feerate than parent tx.vin[0].scriptSig = CScript() << OP_1; tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = BLOCKSUBSIDY-HIGHFEE; diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index a4f85c598..d01ef7c6e 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -56,7 +56,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 4; k++) { // add 4 fee txs tx.vin[0].prevout.n = 10000*blocknum+100*j+k; // make transaction unique uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); + mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); txHashes[j].push_back(hash); } } @@ -132,7 +132,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 4; k++) { // add 4 fee txs tx.vin[0].prevout.n = 10000*blocknum+100*j+k; uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); + mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); txHashes[j].push_back(hash); } } @@ -169,7 +169,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 4; k++) { // add 4 fee txs tx.vin[0].prevout.n = 10000*blocknum+100*j+k; uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); + mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Height(blocknum).FromTx(tx)); CTransactionRef ptx = mpool.get(hash); if (ptx) block.push_back(ptx); @@ -185,7 +185,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) } // Test that if the mempool is limited, estimateSmartFee won't return a value below the mempool min fee - mpool.addUnchecked(tx.GetHash(), entry.Fee(feeV[5]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); + mpool.addUnchecked(tx.GetHash(), entry.Fee(feeV[5]).Time(GetTime()).Height(blocknum).FromTx(tx)); // evict that transaction which should set a mempool min fee of minRelayTxFee + feeV[5] mpool.TrimToSize(1); BOOST_CHECK(mpool.GetMinFee(1).GetFeePerK() > feeV[5]); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 51fc6ae0b..e6d34de1a 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -141,17 +141,14 @@ TestChain100Setup::~TestChain100Setup() } -CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx, CTxMemPool *pool) { +CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx) { CTransaction txn(tx); - return FromTx(txn, pool); + return FromTx(txn); } -CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransaction &txn, CTxMemPool *pool) { - // Hack to assume either it's completely dependent on other mempool txs or not at all - CAmount inChainValue = pool && pool->HasNoInputsOf(txn) ? txn.GetValueOut() : 0; - - return CTxMemPoolEntry(MakeTransactionRef(txn), nFee, nTime, dPriority, nHeight, - inChainValue, spendsCoinbase, sigOpCost, lp); +CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransaction &txn) { + return CTxMemPoolEntry(MakeTransactionRef(txn), nFee, nTime, 0.0, nHeight, + 0, spendsCoinbase, sigOpCost, lp); } void Shutdown(void* parg) diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h index 5ef6fa764..a593f136e 100644 --- a/src/test/test_bitcoin.h +++ b/src/test/test_bitcoin.h @@ -61,30 +61,27 @@ struct TestChain100Setup : public TestingSetup { }; class CTxMemPoolEntry; -class CTxMemPool; struct TestMemPoolEntryHelper { // Default values CAmount nFee; int64_t nTime; - double dPriority; unsigned int nHeight; bool spendsCoinbase; unsigned int sigOpCost; LockPoints lp; TestMemPoolEntryHelper() : - nFee(0), nTime(0), dPriority(0.0), nHeight(1), + nFee(0), nTime(0), nHeight(1), spendsCoinbase(false), sigOpCost(4) { } - CTxMemPoolEntry FromTx(const CMutableTransaction &tx, CTxMemPool *pool = NULL); - CTxMemPoolEntry FromTx(const CTransaction &tx, CTxMemPool *pool = NULL); + CTxMemPoolEntry FromTx(const CMutableTransaction &tx); + CTxMemPoolEntry FromTx(const CTransaction &tx); // Change the default value TestMemPoolEntryHelper &Fee(CAmount _fee) { nFee = _fee; return *this; } TestMemPoolEntryHelper &Time(int64_t _time) { nTime = _time; return *this; } - TestMemPoolEntryHelper &Priority(double _priority) { dPriority = _priority; return *this; } TestMemPoolEntryHelper &Height(unsigned int _height) { nHeight = _height; return *this; } TestMemPoolEntryHelper &SpendsCoinbase(bool _flag) { spendsCoinbase = _flag; return *this; } TestMemPoolEntryHelper &SigOpsCost(unsigned int _sigopsCost) { sigOpCost = _sigopsCost; return *this; } From 49be7e1bef23afec617ed211a804f375f1620a22 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 15:24:29 -0500 Subject: [PATCH 09/13] [rpc] Remove priority information from mempool RPC calls "startingpriority" and "currentpriority" are no longer returned in the JSON information about a mempool entry. This affects getmempoolancestors, getmempooldescendants, getmempooolentry, and getrawmempool. --- src/rpc/blockchain.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6826ce4a7..1919ca392 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -341,8 +341,6 @@ std::string EntryDescriptionString() " \"modifiedfee\" : n, (numeric) transaction fee with fee deltas used for mining priority\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) DEPRECATED. Priority when transaction entered pool\n" - " \"currentpriority\" : n, (numeric) DEPRECATED. 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)\n" @@ -363,8 +361,6 @@ void entryToJSON(UniValue &info, const CTxMemPoolEntry &e) info.push_back(Pair("modifiedfee", ValueFromAmount(e.GetModifiedFee()))); info.push_back(Pair("time", e.GetTime())); info.push_back(Pair("height", (int)e.GetHeight())); - info.push_back(Pair("startingpriority", e.GetPriority(e.GetHeight()))); - info.push_back(Pair("currentpriority", e.GetPriority(chainActive.Height()))); info.push_back(Pair("descendantcount", e.GetCountWithDescendants())); info.push_back(Pair("descendantsize", e.GetSizeWithDescendants())); info.push_back(Pair("descendantfees", e.GetModFeesWithDescendants())); From f9b9371c6027905f73a2558d6bcaca8a355c28a6 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 23:37:15 -0500 Subject: [PATCH 10/13] [rpc] Remove priorityDelta from prioritisetransaction This a breaking API change to the prioritisetransaction RPC call which previously required exactly three arguments and now requires exactly two (hash and feeDelta). The function prioritiseTransaction is also updated. --- qa/rpc-tests/bip68-sequence.py | 4 ++-- qa/rpc-tests/mempool_packages.py | 4 ++-- qa/rpc-tests/prioritise_transaction.py | 6 +++--- qa/rpc-tests/replace-by-fee.py | 4 ++-- src/rpc/client.cpp | 3 +-- src/rpc/mining.cpp | 19 ++++++++--------- src/txmempool.cpp | 28 ++++++++++++-------------- src/txmempool.h | 6 +++--- src/validation.cpp | 10 ++++----- 9 files changed, 38 insertions(+), 46 deletions(-) diff --git a/qa/rpc-tests/bip68-sequence.py b/qa/rpc-tests/bip68-sequence.py index f3a025afa..fbf31b25f 100755 --- a/qa/rpc-tests/bip68-sequence.py +++ b/qa/rpc-tests/bip68-sequence.py @@ -256,7 +256,7 @@ class BIP68Test(BitcoinTestFramework): # Now mine some blocks, but make sure tx2 doesn't get mined. # Use prioritisetransaction to lower the effective feerate to 0 - self.nodes[0].prioritisetransaction(tx2.hash, -1e15, int(-self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(tx2.hash, int(-self.relayfee*COIN)) cur_time = int(time.time()) for i in range(10): self.nodes[0].setmocktime(cur_time + 600) @@ -269,7 +269,7 @@ class BIP68Test(BitcoinTestFramework): test_nonzero_locks(tx2, self.nodes[0], self.relayfee, use_height_lock=False) # Mine tx2, and then try again - self.nodes[0].prioritisetransaction(tx2.hash, 1e15, int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(tx2.hash, int(self.relayfee*COIN)) # Advance the time on the node so that we can test timelocks self.nodes[0].setmocktime(cur_time+600) diff --git a/qa/rpc-tests/mempool_packages.py b/qa/rpc-tests/mempool_packages.py index 388889d07..3437c89ec 100755 --- a/qa/rpc-tests/mempool_packages.py +++ b/qa/rpc-tests/mempool_packages.py @@ -103,7 +103,7 @@ class MempoolPackagesTest(BitcoinTestFramework): # Check that descendant modified fees includes fee deltas from # prioritisetransaction - self.nodes[0].prioritisetransaction(chain[-1], 0, 1000) + self.nodes[0].prioritisetransaction(chain[-1], 1000) mempool = self.nodes[0].getrawmempool(True) descendant_fees = 0 @@ -124,7 +124,7 @@ class MempoolPackagesTest(BitcoinTestFramework): assert_equal(len(self.nodes[0].getrawmempool()), 0) # Prioritise a transaction that has been mined, then add it back to the # mempool by using invalidateblock. - self.nodes[0].prioritisetransaction(chain[-1], 0, 2000) + self.nodes[0].prioritisetransaction(chain[-1], 2000) self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) # Keep node1's tip synced with node0 self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash()) diff --git a/qa/rpc-tests/prioritise_transaction.py b/qa/rpc-tests/prioritise_transaction.py index 91d5d6be7..13be6eeea 100755 --- a/qa/rpc-tests/prioritise_transaction.py +++ b/qa/rpc-tests/prioritise_transaction.py @@ -51,7 +51,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # add a fee delta to something in the cheapest bucket and make sure it gets mined # also check that a different entry in the cheapest bucket is NOT mined - self.nodes[0].prioritisetransaction(txids[0][0], 0, int(3*base_fee*COIN)) + self.nodes[0].prioritisetransaction(txids[0][0], int(3*base_fee*COIN)) self.nodes[0].generate(1) @@ -70,7 +70,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # Add a prioritisation before a tx is in the mempool (de-prioritising a # high-fee transaction so that it's now low fee). - self.nodes[0].prioritisetransaction(high_fee_tx, -1e15, -int(2*base_fee*COIN)) + self.nodes[0].prioritisetransaction(high_fee_tx, -int(2*base_fee*COIN)) # Add everything back to mempool self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) @@ -118,7 +118,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # This is a less than 1000-byte transaction, so just set the fee # to be the minimum for a 1000 byte transaction and check that it is # accepted. - self.nodes[0].prioritisetransaction(tx_id, 0, int(self.relayfee*COIN)) + self.nodes[0].prioritisetransaction(tx_id, int(self.relayfee*COIN)) print("Assert that prioritised free transaction is accepted to mempool") assert_equal(self.nodes[0].sendrawtransaction(tx_hex), tx_id) diff --git a/qa/rpc-tests/replace-by-fee.py b/qa/rpc-tests/replace-by-fee.py index a3c1deddf..51cbb4dc4 100755 --- a/qa/rpc-tests/replace-by-fee.py +++ b/qa/rpc-tests/replace-by-fee.py @@ -543,7 +543,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert(False) # Use prioritisetransaction to set tx1a's fee to 0. - self.nodes[0].prioritisetransaction(tx1a_txid, 0, int(-0.1*COIN)) + self.nodes[0].prioritisetransaction(tx1a_txid, int(-0.1*COIN)) # Now tx1b should be able to replace tx1a tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, True) @@ -575,7 +575,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert(False) # Now prioritise tx2b to have a higher modified fee - self.nodes[0].prioritisetransaction(tx2b.hash, 0, int(0.1*COIN)) + self.nodes[0].prioritisetransaction(tx2b.hash, int(0.1*COIN)) # tx2b should now be accepted tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, True) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 2d16868d4..a8c5c21ef 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -108,8 +108,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getrawmempool", 0, "verbose" }, { "estimatefee", 0, "nblocks" }, { "estimatesmartfee", 0, "nblocks" }, - { "prioritisetransaction", 1, "priority_delta" }, - { "prioritisetransaction", 2, "fee_delta" }, + { "prioritisetransaction", 1, "fee_delta" }, { "setban", 2, "bantime" }, { "setban", 3, "absolute" }, { "setnetworkactive", 0, "state" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d0d9e0599..4ff8f39ed 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -258,31 +258,28 @@ UniValue getmininginfo(const JSONRPCRequest& request) // NOTE: Unlike wallet RPC (which use BTC values), mining RPCs follow GBT (BIP 22) in using satoshi amounts UniValue prioritisetransaction(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 3) + if (request.fHelp || request.params.size() != 2) throw runtime_error( - "prioritisetransaction \n" + "prioritisetransaction \n" "Accepts the transaction into mined blocks at a higher (or lower) priority\n" "\nArguments:\n" "1. \"txid\" (string, required) The transaction id.\n" - "2. priority_delta (numeric, required) 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" - "3. fee_delta (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).\n" + "2. fee_delta (numeric, required) The fee value (in satoshis) to add (or subtract, if negative).\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.\n" "\nResult:\n" "true (boolean) Returns true\n" "\nExamples:\n" - + HelpExampleCli("prioritisetransaction", "\"txid\" 0.0 10000") - + HelpExampleRpc("prioritisetransaction", "\"txid\", 0.0, 10000") + + HelpExampleCli("prioritisetransaction", "\"txid\" 10000") + + HelpExampleRpc("prioritisetransaction", "\"txid\", 10000") ); LOCK(cs_main); uint256 hash = ParseHashStr(request.params[0].get_str(), "txid"); - CAmount nAmount = request.params[2].get_int64(); + CAmount nAmount = request.params[1].get_int64(); - mempool.PrioritiseTransaction(hash, request.params[1].get_real(), nAmount); + mempool.PrioritiseTransaction(hash, nAmount); return true; } @@ -853,7 +850,7 @@ static const CRPCCommand commands[] = // --------------------- ------------------------ ----------------------- ---------- { "mining", "getnetworkhashps", &getnetworkhashps, true, {"nblocks","height"} }, { "mining", "getmininginfo", &getmininginfo, true, {} }, - { "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","priority_delta","fee_delta"} }, + { "mining", "prioritisetransaction", &prioritisetransaction, true, {"txid","fee_delta"} }, { "mining", "getblocktemplate", &getblocktemplate, true, {"template_request"} }, { "mining", "submitblock", &submitblock, true, {"hexdata","parameters"} }, diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ef4d4f3c6..58a71ad95 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -404,11 +404,11 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, // Update transaction for any feeDelta created by PrioritiseTransaction // TODO: refactor so that the fee delta is calculated before inserting // into mapTx. - std::map >::const_iterator pos = mapDeltas.find(hash); + std::map::const_iterator pos = mapDeltas.find(hash); if (pos != mapDeltas.end()) { - const std::pair &deltas = pos->second; - if (deltas.second) { - mapTx.modify(newit, update_fee_delta(deltas.second)); + const CAmount &delta = pos->second; + if (delta) { + mapTx.modify(newit, update_fee_delta(delta)); } } @@ -910,16 +910,15 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein) return true; } -void CTxMemPool::PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta) +void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) { { LOCK(cs); - std::pair &deltas = mapDeltas[hash]; - deltas.first += dPriorityDelta; - deltas.second += nFeeDelta; + CAmount &delta = mapDeltas[hash]; + delta += nFeeDelta; txiter it = mapTx.find(hash); if (it != mapTx.end()) { - mapTx.modify(it, update_fee_delta(deltas.second)); + mapTx.modify(it, update_fee_delta(delta)); // Now update all ancestors' modified fees with descendants setEntries setAncestors; uint64_t nNoLimit = std::numeric_limits::max(); @@ -930,18 +929,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, double dPriorityDelt } } } - LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", hash.ToString(), dPriorityDelta, FormatMoney(nFeeDelta)); + LogPrintf("PrioritiseTransaction: %s feerate += %s\n", hash.ToString(), FormatMoney(nFeeDelta)); } -void CTxMemPool::ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const +void CTxMemPool::ApplyDelta(const uint256 hash, 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 std::pair &deltas = pos->second; - dPriorityDelta += deltas.first; - nFeeDelta += deltas.second; + const CAmount &delta = pos->second; + nFeeDelta += delta; } void CTxMemPool::ClearPrioritisation(const uint256 hash) diff --git a/src/txmempool.h b/src/txmempool.h index 9af65bd07..6b2e68030 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -501,7 +501,7 @@ private: public: indirectmap mapNextTx; - std::map > mapDeltas; + std::map mapDeltas; /** Create a new CTxMemPool. */ @@ -543,8 +543,8 @@ public: bool HasNoInputsOf(const CTransaction& tx) const; /** Affect CreateNewBlock prioritisation of transactions */ - void PrioritiseTransaction(const uint256& hash, double dPriorityDelta, const CAmount& nFeeDelta); - void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const; + void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); + void ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const; void ClearPrioritisation(const uint256 hash); public: diff --git a/src/validation.cpp b/src/validation.cpp index a21dc25a5..1f57468bc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -720,8 +720,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C CAmount nFees = nValueIn-nValueOut; // nModifiedFees includes any fee deltas from PrioritiseTransaction CAmount nModifiedFees = nFees; - double nPriorityDummy = 0; - pool.ApplyDeltas(hash, nPriorityDummy, nModifiedFees); + pool.ApplyDelta(hash, nModifiedFees); CAmount inChainInputValue; double dPriority = view.GetPriority(tx, chainActive.Height(), inChainInputValue); @@ -4184,7 +4183,6 @@ bool LoadMempool(void) } uint64_t num; file >> num; - double prioritydummy = 0; while (num--) { CTransactionRef tx; int64_t nTime; @@ -4195,7 +4193,7 @@ bool LoadMempool(void) CAmount amountdelta = nFeeDelta; if (amountdelta) { - mempool.PrioritiseTransaction(tx->GetHash(), prioritydummy, amountdelta); + mempool.PrioritiseTransaction(tx->GetHash(), amountdelta); } CValidationState state; if (nTime + nExpiryTimeout > nNow) { @@ -4216,7 +4214,7 @@ bool LoadMempool(void) file >> mapDeltas; for (const auto& i : mapDeltas) { - mempool.PrioritiseTransaction(i.first, prioritydummy, i.second); + mempool.PrioritiseTransaction(i.first, i.second); } } catch (const std::exception& e) { LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); @@ -4237,7 +4235,7 @@ void DumpMempool(void) { LOCK(mempool.cs); for (const auto &i : mempool.mapDeltas) { - mapDeltas[i.first] = i.second.second; + mapDeltas[i.first] = i.second; } vinfo = mempool.infoAll(); } From 359e8a03d1667dca3e8375695131b8b5e6c54f0a Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 20 Jan 2017 09:24:35 -0500 Subject: [PATCH 11/13] [cleanup] Remove coin age priority completely. Remove GetPriority and ComputePriority. Remove internal machinery for tracking priority in CTxMemPoolEntry. --- src/bench/mempool_eviction.cpp | 5 ++--- src/coins.cpp | 19 ------------------- src/coins.h | 7 ------- src/primitives/transaction.cpp | 26 -------------------------- src/primitives/transaction.h | 6 ------ src/test/mempool_tests.cpp | 1 - src/test/test_bitcoin.cpp | 4 ++-- src/txmempool.cpp | 19 ++----------------- src/txmempool.h | 12 ++---------- src/validation.cpp | 7 ++----- src/wallet/wallet.cpp | 2 +- 11 files changed, 11 insertions(+), 97 deletions(-) diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index 5790d51a8..31a392ae7 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -12,14 +12,13 @@ static void AddTx(const CTransaction& tx, const CAmount& nFee, CTxMemPool& pool) { int64_t nTime = 0; - double dPriority = 10.0; unsigned int nHeight = 1; bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; pool.addUnchecked(tx.GetHash(), CTxMemPoolEntry( - MakeTransactionRef(tx), nFee, nTime, dPriority, nHeight, - tx.GetValueOut(), spendsCoinbase, sigOpCost, lp)); + MakeTransactionRef(tx), nFee, nTime, nHeight, + spendsCoinbase, sigOpCost, lp)); } // Right now this is only testing eviction performance in an extremely small diff --git a/src/coins.cpp b/src/coins.cpp index 4d0e4bc0a..b2e33abf3 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -295,25 +295,6 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const return true; } -double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight, CAmount &inChainInputValue) const -{ - inChainInputValue = 0; - if (tx.IsCoinBase()) - return 0.0; - double dResult = 0.0; - BOOST_FOREACH(const CTxIn& txin, tx.vin) - { - const CCoins* coins = AccessCoins(txin.prevout.hash); - assert(coins); - if (!coins->IsAvailable(txin.prevout.n)) continue; - if (coins->nHeight <= nHeight) { - dResult += (double)(coins->vout[txin.prevout.n].nValue) * (nHeight-coins->nHeight); - inChainInputValue += coins->vout[txin.prevout.n].nValue; - } - } - return tx.ComputePriority(dResult); -} - CCoinsModifier::CCoinsModifier(CCoinsViewCache& cache_, CCoinsMap::iterator it_, size_t usage) : cache(cache_), it(it_), cachedCoinUsage(usage) { assert(!cache.hasModifier); cache.hasModifier = true; diff --git a/src/coins.h b/src/coins.h index 902cb57f6..d921f5c2a 100644 --- a/src/coins.h +++ b/src/coins.h @@ -460,13 +460,6 @@ public: //! Check whether all prevouts of the transaction are present in the UTXO set represented by this view bool HaveInputs(const CTransaction& tx) const; - /** - * 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, int nHeight, CAmount &inChainInputValue) const; - const CTxOut &GetOutputFor(const CTxIn& input) const; friend class CCoinsModifier; diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 790bc71d1..364a70adc 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -89,32 +89,6 @@ CAmount CTransaction::GetValueOut() const return nValueOut; } -double CTransaction::ComputePriority(double dPriorityInputs, unsigned int nTxSize) const -{ - nTxSize = CalculateModifiedSize(nTxSize); - if (nTxSize == 0) return 0.0; - - return dPriorityInputs / nTxSize; -} - -unsigned int CTransaction::CalculateModifiedSize(unsigned int nTxSize) const -{ - // 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(*this) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; - for (std::vector::const_iterator it(vin.begin()); it != vin.end(); ++it) - { - unsigned int offset = 41U + std::min(110U, (unsigned int)it->scriptSig.size()); - if (nTxSize > offset) - nTxSize -= offset; - } - return nTxSize; -} - unsigned int CTransaction::GetTotalSize() const { return ::GetSerializeSize(*this, SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index af2986a41..d413e8b08 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -361,12 +361,6 @@ public: // GetValueIn() is a method on CCoinsViewCache, because // inputs must be known to compute value in. - // Compute priority, given priority of inputs and (optionally) tx size - double ComputePriority(double dPriorityInputs, unsigned int nTxSize=0) const; - - // Compute modified tx size for priority calculation (optionally given tx size) - unsigned int CalculateModifiedSize(unsigned int nTxSize=0) const; - /** * Get the total transaction size in bytes, including witness data. * "Total Size" defined in BIP141 and BIP144. diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index cdef1f5aa..a3f706d9a 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -407,7 +407,6 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* set the fee to just below tx2's feerate when including ancestor */ CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1; - //CTxMemPoolEntry entry7(tx7, fee, 2, 10.0, 1, true); pool.addUnchecked(tx7.GetHash(), entry.Fee(fee).FromTx(tx7)); BOOST_CHECK_EQUAL(pool.size(), 7); sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString()); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index e6d34de1a..fc3747436 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -147,8 +147,8 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx) { } CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransaction &txn) { - return CTxMemPoolEntry(MakeTransactionRef(txn), nFee, nTime, 0.0, nHeight, - 0, spendsCoinbase, sigOpCost, lp); + return CTxMemPoolEntry(MakeTransactionRef(txn), nFee, nTime, nHeight, + spendsCoinbase, sigOpCost, lp); } void Shutdown(void* parg) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 58a71ad95..79dd0fb5f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -19,22 +19,17 @@ #include "version.h" CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, double _entryPriority, unsigned int _entryHeight, - CAmount _inChainInputValue, + int64_t _nTime, unsigned int _entryHeight, bool _spendsCoinbase, int64_t _sigOpsCost, LockPoints lp): - tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), - inChainInputValue(_inChainInputValue), + tx(_tx), nFee(_nFee), nTime(_nTime), entryHeight(_entryHeight), spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp) { nTxWeight = GetTransactionWeight(*tx); - nModSize = tx->CalculateModifiedSize(GetTxSize()); nUsageSize = RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx); nCountWithDescendants = 1; nSizeWithDescendants = GetTxSize(); nModFeesWithDescendants = nFee; - CAmount nValueIn = tx->GetValueOut()+nFee; - assert(inChainInputValue <= nValueIn); feeDelta = 0; @@ -49,16 +44,6 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) *this = other; } -double -CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const -{ - double deltaPriority = ((double)(currentHeight-entryHeight)*inChainInputValue)/nModSize; - double dResult = entryPriority + deltaPriority; - if (dResult < 0) // This should only happen if it was called with a height below entry height - dResult = 0; - return dResult; -} - void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) { nModFeesWithDescendants += newFeeDelta - feeDelta; diff --git a/src/txmempool.h b/src/txmempool.h index 6b2e68030..5d82e3016 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -73,12 +73,9 @@ private: CTransactionRef tx; CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups size_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) - size_t nModSize; //!< ... and modified size for priority size_t nUsageSize; //!< ... and total memory usage int64_t nTime; //!< Local time when entering the mempool - double entryPriority; //!< Priority when entering the mempool unsigned int entryHeight; //!< Chain height when entering the mempool - CAmount inChainInputValue; //!< Sum of all txin values that are already in blockchain bool spendsCoinbase; //!< keep track of transactions that spend a coinbase int64_t sigOpCost; //!< Total sigop cost int64_t feeDelta; //!< Used for determining the priority of the transaction for mining in a block @@ -101,19 +98,14 @@ private: public: CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, - int64_t _nTime, double _entryPriority, unsigned int _entryHeight, - CAmount _inChainInputValue, bool spendsCoinbase, + int64_t _nTime, unsigned int _entryHeight, + bool spendsCoinbase, int64_t nSigOpsCost, LockPoints lp); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() const { return *this->tx; } CTransactionRef GetSharedTx() const { return this->tx; } - /** - * Fast calculation of lower bound of current priority as update - * from entry priority. Only inputs that were originally in-chain will age. - */ - double GetPriority(unsigned int currentHeight) const; const CAmount& GetFee() const { return nFee; } size_t GetTxSize() const; size_t GetTxWeight() const { return nTxWeight; } diff --git a/src/validation.cpp b/src/validation.cpp index 1f57468bc..4a67dead3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -722,9 +722,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C CAmount nModifiedFees = nFees; pool.ApplyDelta(hash, nModifiedFees); - CAmount inChainInputValue; - double dPriority = view.GetPriority(tx, chainActive.Height(), 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; @@ -736,8 +733,8 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C } } - CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, dPriority, chainActive.Height(), - inChainInputValue, fSpendsCoinbase, nSigOpsCost, lp); + CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, chainActive.Height(), + fSpendsCoinbase, nSigOpsCost, lp); unsigned int nSize = entry.GetTxSize(); // Check that the transaction doesn't have an excessive number of diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ba16cdf26..12117abd1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2699,7 +2699,7 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt if (GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits LockPoints lp; - CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, false, 0, lp); CTxMemPool::setEntries setAncestors; size_t nLimitAncestors = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); size_t nLimitAncestorSize = GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; From 7d4e9509ade0c258728011d8f6544ec3e75d63dc Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 19 Jan 2017 21:48:10 -0500 Subject: [PATCH 12/13] Allow setting minrelaytxfee to 0 Setting minrelaytxfee to 0 will allow all transactions regardless of fee to enter your mempool until it reaches its size limit. However now that mempool limiting is governed by a separate incrementalrelay fee, it is an unnecessary restriction to prevent a minrelaytxfee of 0. --- src/init.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 3276ec9f1..47d18dad6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -977,13 +977,14 @@ bool AppInitParameterInteraction() // If you are mining, be careful setting this: // if you set it to zero then // a transaction spammer can cheaply fill blocks using - // 1-satoshi-fee transactions. It should be set above the real + // 0-fee transactions. It should be set above the real // cost to you of processing a transaction. if (IsArgSet("-minrelaytxfee")) { CAmount n = 0; - if (!ParseMoney(GetArg("-minrelaytxfee", ""), n) || 0 == n) + if (!ParseMoney(GetArg("-minrelaytxfee", ""), n)) { return InitError(AmountErrMsg("minrelaytxfee", GetArg("-minrelaytxfee", ""))); + } // High fee check is done afterward in CWallet::ParameterInteraction() ::minRelayTxFee = CFeeRate(n); } else if (incrementalRelayFee > ::minRelayTxFee) { From b421e6ddcf00f220732f43742393452bb8bf4cdd Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 3 Mar 2017 10:40:42 -0500 Subject: [PATCH 13/13] Update example bitcoin.conf --- contrib/debian/examples/bitcoin.conf | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contrib/debian/examples/bitcoin.conf b/contrib/debian/examples/bitcoin.conf index 0c6eaa0a3..923ab7531 100644 --- a/contrib/debian/examples/bitcoin.conf +++ b/contrib/debian/examples/bitcoin.conf @@ -116,9 +116,7 @@ # running on another host using this option: #rpcconnect=127.0.0.1 -# Transaction Fee Changes in 0.10.0 - -# Create transactions that have enough fees (or priority) so they are likely to begin confirmation within n blocks (default: 1). +# Create transactions that have enough fees so they are likely to begin confirmation within n blocks (default: 6). # This setting is over-ridden by the -paytxfee option. #txconfirmtarget=n