diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -5,6 +5,7 @@ This release includes the following features and fixes: - Low level RPC error code changes (D500 / backport of PR9853) +- Reserve block space for high priority transactions (D485) Low-level RPC changes (D500) ---------------------------- @@ -31,3 +32,17 @@ - `fundrawtransaction` now returns RPC_WALLET_ERROR if bitcoind is unable to create the transaction. The error message provides further details. Previously returned RPC_INTERNAL_ERROR. + +Reserve block space for high priority transactions (D485) +--------------------------------------------------------- + +By default reserve 5% of the max generated block size parameter to hiprio transactions. +Hence a `bitcoind` instance running with an unmodified configuration will reserve 100K +for high priority transactions. The parameter name used for this configuration +`blockprioritypercentage`. While introducing this new parameter we deprecated +`blockprioritysize`(it was used to specify the amount of high prio reserved area in byte). + +A transaction is considered high priority if its priority is higher than this threshold: `COIN * 144 / 250`, +where COIN is the value of a one bitcoin UTXO expressed in satoshis. Thus a transaction +who as an input of 1 bitcoin and are 144 blocks old and whose size is 250 bytes is considered +the priority cut-off. diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -145,6 +145,7 @@ 'keypool.py', 'p2p-mempool.py', 'prioritise_transaction.py', + 'high_priority_transaction.py', 'invalidblockrequest.py', 'invalidtxrequest.py', 'p2p-versionbits-warning.py', @@ -289,10 +290,10 @@ time.time(), subprocess.Popen( (RPC_TESTS_DIR + t).split() + - self.flags + port_seed, - universal_newlines=True, - stdout=log_stdout, - stderr=log_stderr), + self.flags + port_seed, + universal_newlines=True, + stdout=log_stdout, + stderr=log_stderr), log_stdout, log_stderr)) if not self.jobs: diff --git a/qa/rpc-tests/bip68-sequence.py b/qa/rpc-tests/bip68-sequence.py --- a/qa/rpc-tests/bip68-sequence.py +++ b/qa/rpc-tests/bip68-sequence.py @@ -28,7 +28,8 @@ super().__init__() self.num_nodes = 2 self.setup_clean_chain = False - self.extra_args = [[], ["-acceptnonstdtxn=0"]] + self.extra_args = [["-blockprioritypercentage=0"], + ["-acceptnonstdtxn=0", "-blockprioritypercentage=0"]] def run_test(self): self.relayfee = self.nodes[0].getnetworkinfo()["relayfee"] diff --git a/qa/rpc-tests/high_priority_transaction.py b/qa/rpc-tests/high_priority_transaction.py new file mode 100755 --- /dev/null +++ b/qa/rpc-tests/high_priority_transaction.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# +# Test HighPriorityTransaction code +# + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import * +from test_framework.mininode import COIN +from test_framework.cdefs import LEGACY_MAX_BLOCK_SIZE, COINBASE_MATURITY + + +class HighPriorityTransactionTest(BitcoinTestFramework): + + def __init__(self): + super().__init__() + self.setup_clean_chain = True + self.is_network_split = False + self.num_nodes = 2 + + def setup_nodes(self): + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, extra_args=[ + ["-blockprioritypercentage=0", "-limitfreerelay=2"], + ["-limitfreerelay=2"] + ]) + + def create_small_transactions(self, node, utxos, num, fee): + addr = node.getnewaddress() + txids = [] + for _ in range(num): + t = utxos.pop() + inputs = [{"txid": t["txid"], "vout": t["vout"]}] + outputs = {} + change = t['amount'] - fee + outputs[addr] = satoshi_round(change) + rawtx = node.createrawtransaction(inputs, outputs) + signresult = node.signrawtransaction( + rawtx, None, None, "NONE|FORKID") + txid = node.sendrawtransaction(signresult["hex"], True) + txids.append(txid) + return txids + + def generate_high_priotransactions(self, node, count): + # generate a bunch of spendable utxos + self.txouts = gen_return_txouts() + # create 150 simple one input one output hi prio txns + hiprio_utxo_count = 150 + age = 250 + # be sure to make this utxo aged enough + hiprio_utxos = create_confirmed_utxos( + self.relayfee, node, hiprio_utxo_count, age) + txids = [] + + # Create hiprio_utxo_count number of txns with 0 fee + range_size = [0, hiprio_utxo_count] + start_range = range_size[0] + end_range = range_size[1] + txids = self.create_small_transactions( + node, hiprio_utxos[start_range:end_range], end_range - start_range, 0) + return txids + + def run_test(self): + # this is the priority cut off as defined in AllowFreeThreshold() (see: src/txmempool.h) + # anything above that value is considered an high priority transaction + hiprio_threshold = COIN * 144 / 250 + self.relayfee = self.nodes[0].getnetworkinfo()['relayfee'] + + # first test step: 0 reserved prio space in block + txids = self.generate_high_priotransactions(self.nodes[0], 150) + mempool_size_pre = self.nodes[0].getmempoolinfo()['bytes'] + mempool = self.nodes[0].getrawmempool(True) + # assert that all the txns are in the mempool and that all of them are hi prio + for i in txids: + assert(i in mempool) + assert(mempool[i]['currentpriority'] > hiprio_threshold) + + # mine one block + self.nodes[0].generate(1) + + self.log.info( + "Assert that all high prio transactions haven't been mined") + assert_equal(self.nodes[0].getmempoolinfo()['bytes'], mempool_size_pre) + + # second test step: default reserved prio space in block (100K). + # the mempool size is about 25K this means that all txns will be + # included in the soon to be mined block + txids = self.generate_high_priotransactions(self.nodes[1], 150) + mempool_size_pre = self.nodes[1].getmempoolinfo()['bytes'] + mempool = self.nodes[1].getrawmempool(True) + # assert that all the txns are in the mempool and that all of them are hiprio + for i in txids: + assert(i in mempool) + assert(mempool[i]['currentpriority'] > hiprio_threshold) + + # mine one block + self.nodes[1].generate(1) + + self.log.info("Assert that all high prio transactions have been mined") + assert(self.nodes[1].getmempoolinfo()['bytes'] == 0) + + +if __name__ == '__main__': + HighPriorityTransactionTest().main() diff --git a/qa/rpc-tests/smartfees.py b/qa/rpc-tests/smartfees.py --- a/qa/rpc-tests/smartfees.py +++ b/qa/rpc-tests/smartfees.py @@ -214,14 +214,14 @@ # 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", + ["-blockprioritypercentage=9", "-blockmaxsize=17000", "-maxorphantx=1000"], stderr_checker=OutputChecker())) 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", + node2args = ["-blockprioritypercentage=0", "-blockmaxsize=8000", "-maxorphantx=1000"] self.nodes.append( @@ -293,5 +293,6 @@ self.log.info("Final estimates after emptying mempools") check_estimates(self.nodes[1], self.fees_per_kb, 2) + if __name__ == '__main__': EstimateFeeTest().main() diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -47,6 +47,7 @@ # Must be initialized with a unique integer for each process n = None + # Set Mocktime default to OFF. # MOCKTIME is only needed for scripts that use the # cached version of the blockchain. If the cached @@ -194,6 +195,7 @@ timeout -= wait raise AssertionError("Mempool sync failed") + bitcoind_processes = {} @@ -322,8 +324,8 @@ from_dir = os.path.join(cachedir, "node" + str(i)) to_dir = os.path.join(test_dir, "node" + str(i)) shutil.copytree(from_dir, to_dir) + # Overwrite port/rpcport in bitcoin.conf initialize_datadir(test_dir, i) - # Overwrite port/rpcport in bitcoin.conf def initialize_chain_clean(test_dir, num_nodes): @@ -627,7 +629,7 @@ def assert_equal(thing1, thing2, *args): if thing1 != thing2 or any(thing1 != arg for arg in args): raise AssertionError("not(%s)" % " == ".join(str(arg) - for arg in (thing1, thing2) + args)) + for arg in (thing1, thing2) + args)) def assert_greater_than(thing1, thing2): @@ -749,8 +751,8 @@ # Pass in a fee that is sufficient for relay and mining new transactions. -def create_confirmed_utxos(fee, node, count): - node.generate(int(0.5 * count) + 101) +def create_confirmed_utxos(fee, node, count, age=101): + node.generate(int(0.5 * count) + age) utxos = node.listunspent() iterations = count - len(utxos) addr1 = node.getnewaddress() diff --git a/src/config.h b/src/config.h --- a/src/config.h +++ b/src/config.h @@ -15,6 +15,9 @@ public: virtual bool SetMaxBlockSize(uint64_t maxBlockSize) = 0; virtual uint64_t GetMaxBlockSize() const = 0; + virtual bool + SetBlockPriorityPercentage(int64_t blockPriorityPercentage) = 0; + virtual uint8_t GetBlockPriorityPercentage() const = 0; virtual const CChainParams &GetChainParams() const = 0; }; @@ -22,6 +25,8 @@ public: bool SetMaxBlockSize(uint64_t maxBlockSize); uint64_t GetMaxBlockSize() const; + bool SetBlockPriorityPercentage(int64_t blockPriorityPercentage); + uint8_t GetBlockPriorityPercentage() const; const CChainParams &GetChainParams() const; }; diff --git a/src/config.cpp b/src/config.cpp --- a/src/config.cpp +++ b/src/config.cpp @@ -22,6 +22,19 @@ return nMaxBlockSize; } +bool GlobalConfig::SetBlockPriorityPercentage(int64_t blockPriorityPercentage) { + // blockPriorityPercentage has to belong to [0..100] + if ((blockPriorityPercentage < 0) || (blockPriorityPercentage > 100)) { + return false; + } + nBlockPriorityPercentage = blockPriorityPercentage; + return true; +} + +uint8_t GlobalConfig::GetBlockPriorityPercentage() const { + return nBlockPriorityPercentage; +} + const CChainParams &GlobalConfig::GetChainParams() const { return Params(); } diff --git a/src/globals.h b/src/globals.h --- a/src/globals.h +++ b/src/globals.h @@ -9,5 +9,6 @@ /** The largest block size this node will accept. */ extern uint64_t nMaxBlockSize; +extern uint64_t nBlockPriorityPercentage; #endif // BITCOIN_GLOBALS_H diff --git a/src/globals.cpp b/src/globals.cpp --- a/src/globals.cpp +++ b/src/globals.cpp @@ -5,5 +5,7 @@ #include "globals.h" #include "consensus/consensus.h" +#include "policy/policy.h" uint64_t nMaxBlockSize = DEFAULT_MAX_BLOCK_SIZE; +uint64_t nBlockPriorityPercentage = DEFAULT_BLOCK_PRIORITY_PERCENTAGE; diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -784,11 +784,11 @@ "-blockmaxsize=", strprintf(_("Set maximum block size in bytes (default: %d)"), DEFAULT_MAX_GENERATED_BLOCK_SIZE)); - strUsage += - HelpMessageOpt("-blockprioritysize=", - strprintf(_("Set maximum size of high-priority/low-fee " - "transactions in bytes (default: %d)"), - DEFAULT_BLOCK_PRIORITY_SIZE)); + strUsage += HelpMessageOpt( + "-blockprioritypercentage=", + strprintf(_("Set maximum percentage of a block reserved to " + "high-priority/low-fee transactions (default: %d)"), + DEFAULT_BLOCK_PRIORITY_PERCENTAGE)); strUsage += HelpMessageOpt( "-blockmintxfee=", strprintf(_("Set lowest fee rate (in %s/kB) for transactions to be " @@ -1274,6 +1274,15 @@ return InitError(_("Prune mode is incompatible with -txindex.")); } + // if space reserved for high priority transactions is misconfigured + // stop program execution and warn the user with a proper error message + const int64_t blkprio = + GetArg("-blockprioritypercentage", DEFAULT_BLOCK_PRIORITY_PERCENTAGE); + if (!config.SetBlockPriorityPercentage(blkprio)) { + return InitError(_("Block priority percentage has to belong to the " + "[0..100] interval.")); + } + // Make sure enough file descriptors are available int nBind = std::max( (mapMultiArgs.count("-bind") ? mapMultiArgs.at("-bind").size() : 0) + diff --git a/src/miner.cpp b/src/miner.cpp --- a/src/miner.cpp +++ b/src/miner.cpp @@ -561,13 +561,13 @@ void BlockAssembler::addPriorityTxs() { // How much of the block should be dedicated to high-priority transactions, // included regardless of the fees they pay. - uint64_t nBlockPrioritySize = - GetArg("-blockprioritysize", DEFAULT_BLOCK_PRIORITY_SIZE); - nBlockPrioritySize = std::min(nMaxGeneratedBlockSize, nBlockPrioritySize); - if (nBlockPrioritySize == 0) { + if (config->GetBlockPriorityPercentage() == 0) { return; } + uint64_t nBlockPrioritySize = + nMaxGeneratedBlockSize * config->GetBlockPriorityPercentage() / 100; + // This vector will be sorted into a priority queue: std::vector vecPriority; TxCoinAgePriorityCompare pricomparer; @@ -590,7 +590,8 @@ CTxMemPool::txiter iter; - // Add a tx from priority queue to fill the blockprioritysize. + // Add a tx from priority queue to fill the part of block reserved to + // priority transactions. while (!vecPriority.empty() && !blockFinished) { iter = vecPriority.front().second; actualPriority = vecPriority.front().first; diff --git a/src/policy/policy.h b/src/policy/policy.h --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -17,9 +17,9 @@ /** Default for -blockmaxsize, which controls the maximum size of block the * mining code will create **/ static const uint64_t DEFAULT_MAX_GENERATED_BLOCK_SIZE = 2 * ONE_MEGABYTE; -/** Default for -blockprioritysize, maximum space for zero/low-fee transactions - * **/ -static const uint64_t DEFAULT_BLOCK_PRIORITY_SIZE = 0; +/** Default for -blockprioritypercentage, define the amount of block space + * reserved to high priority transactions **/ +static const uint64_t DEFAULT_BLOCK_PRIORITY_PERCENTAGE = 5; /** Default for -blockmintxfee, which sets the minimum feerate for a transaction * in blocks created by mining code **/ static const unsigned int DEFAULT_BLOCK_MIN_TX_FEE = 1000; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -15,6 +15,7 @@ #include "init.h" #include "miner.h" #include "net.h" +#include "policy/policy.h" #include "pow.h" #include "rpc/server.h" #include "txmempool.h" @@ -288,6 +289,9 @@ obj.push_back(Pair("currentblocksize", uint64_t(nLastBlockSize))); obj.push_back(Pair("currentblocktx", uint64_t(nLastBlockTx))); obj.push_back(Pair("difficulty", double(GetDifficulty()))); + obj.push_back(Pair("blockprioritypercentage", + uint8_t(GetArg("-blockprioritypercentage", + DEFAULT_BLOCK_PRIORITY_PERCENTAGE)))); obj.push_back(Pair("errors", GetWarnings("statusbar"))); obj.push_back(Pair("networkhashps", getnetworkhashps(config, request))); obj.push_back(Pair("pooledtx", uint64_t(mempool.size()))); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -78,7 +78,7 @@ // 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. +// Note that this test assumes blockprioritypercentage is 0. void TestPackageSelection(const CChainParams &chainparams, CScript scriptPubKey, std::vector &txFirst) { // Test the ancestor feerate transaction selection. @@ -86,6 +86,9 @@ GlobalConfig config; + // these 3 tests assume blockprioritypercentage is 0. + config.SetBlockPriorityPercentage(0); + // Test that a medium fee transaction will be selected after a higher fee // rate package with a low fee rate parent. CMutableTransaction tx;