From f15c2cde455174c7c899833fd5792460ed49a472 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 27 Jun 2016 10:58:58 -0400 Subject: CreateNewBlock: add support for size-accounting to addPackageTxs Includes a change to not continue to use size-accounting in addScoreTxs or addPackageTxs just because addPriorityTxs() is used. --- src/miner.cpp | 25 +++++++++++++++---------- src/miner.h | 2 +- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index cfc2dae56e..a3e29431d7 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -94,6 +94,7 @@ BlockAssembler::BlockAssembler(const CChainParams& _chainparams) nBlockMaxCost = nBlockMaxSize * WITNESS_SCALE_FACTOR; } } + // Limit cost to between 4K and MAX_BLOCK_COST-4K for sanity: nBlockMaxCost = std::max((unsigned int)4000, std::min((unsigned int)(MAX_BLOCK_COST-4000), nBlockMaxCost)); // Limit size to between 1K and MAX_BLOCK_SERIALIZED_SIZE-1K for sanity: @@ -167,13 +168,7 @@ CBlockTemplate* BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()); addPriorityTxs(); - if (fNeedSizeAccounting) { - // addPackageTxs (the CPFP-based algorithm) cannot deal with size based - // accounting, so fall back to the old algorithm. - addScoreTxs(); - } else { - addPackageTxs(); - } + addPackageTxs(); nLastBlockTx = nBlockTx; nLastBlockSize = nBlockSize; @@ -243,11 +238,19 @@ bool BlockAssembler::TestPackage(uint64_t packageSize, int64_t packageSigOpsCost // Block size and sigops have already been tested. Check that all transactions // are final. -bool BlockAssembler::TestPackageFinality(const CTxMemPool::setEntries& package) +bool BlockAssembler::TestPackageFinalityAndSerializedSize(const CTxMemPool::setEntries& package) { + uint64_t nPotentialBlockSize = nBlockSize; // only used with fNeedSizeAccounting BOOST_FOREACH (const CTxMemPool::txiter it, package) { if (!IsFinalTx(it->GetTx(), nHeight, nLockTimeCutoff)) return false; + if (fNeedSizeAccounting) { + uint64_t nTxSize = ::GetSerializeSize(it->GetTx(), SER_NETWORK, PROTOCOL_VERSION); + if (nPotentialBlockSize + nTxSize >= nBlockMaxSize) { + return false; + } + nPotentialBlockSize += nTxSize; + } } return true; } @@ -539,7 +542,7 @@ void BlockAssembler::addPackageTxs() ancestors.insert(iter); // Test if all tx's are Final - if (!TestPackageFinality(ancestors)) { + if (!TestPackageFinalityAndSerializedSize(ancestors)) { if (fUsingModified) { mapModifiedTx.get().erase(modit); failedTx.insert(iter); @@ -573,6 +576,7 @@ void BlockAssembler::addPriorityTxs() return; } + bool fSizeAccounting = fNeedSizeAccounting; fNeedSizeAccounting = true; // This vector will be sorted into a priority queue: @@ -624,7 +628,7 @@ void BlockAssembler::addPriorityTxs() // 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)) { - return; + break; } // This tx was successfully added, so @@ -640,6 +644,7 @@ void BlockAssembler::addPriorityTxs() } } } + fNeedSizeAccounting = fSizeAccounting; } void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce) diff --git a/src/miner.h b/src/miner.h index b303a8fa3c..bc4da63da0 100644 --- a/src/miner.h +++ b/src/miner.h @@ -193,7 +193,7 @@ private: /** Test if a new package would "fit" in the block */ bool TestPackage(uint64_t packageSize, int64_t packageSigOpsCost); /** Test if a set of transactions are all final */ - bool TestPackageFinality(const CTxMemPool::setEntries& package); + bool TestPackageFinalityAndSerializedSize(const CTxMemPool::setEntries& package); /** Return true if given transaction from mapTx has already been evaluated, * or if the transaction's cached data in mapTx is incorrect. */ bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx); -- cgit v1.2.3 From 6dd4bc289c71f622ac561f6f9651546b9ec4fa3e Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 30 Jun 2016 11:22:31 -0400 Subject: Exclude witness transactions in addPackageTxs() pre-segwit activation --- src/miner.cpp | 13 +++++++++---- src/miner.h | 7 +++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index a3e29431d7..f2ad1018b2 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -236,14 +236,19 @@ bool BlockAssembler::TestPackage(uint64_t packageSize, int64_t packageSigOpsCost return true; } -// Block size and sigops have already been tested. Check that all transactions -// are final. -bool BlockAssembler::TestPackageFinalityAndSerializedSize(const CTxMemPool::setEntries& package) +// Perform transaction-level checks before adding to block: +// - transaction finality (locktime) +// - premature witness (in case segwit transactions are added to mempool before +// segwit activation) +// - serialized size (in case -blockmaxsize is in use) +bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& package) { uint64_t nPotentialBlockSize = nBlockSize; // only used with fNeedSizeAccounting BOOST_FOREACH (const CTxMemPool::txiter it, package) { if (!IsFinalTx(it->GetTx(), nHeight, nLockTimeCutoff)) return false; + if (!fIncludeWitness && !it->GetTx().wit.IsNull()) + return false; if (fNeedSizeAccounting) { uint64_t nTxSize = ::GetSerializeSize(it->GetTx(), SER_NETWORK, PROTOCOL_VERSION); if (nPotentialBlockSize + nTxSize >= nBlockMaxSize) { @@ -542,7 +547,7 @@ void BlockAssembler::addPackageTxs() ancestors.insert(iter); // Test if all tx's are Final - if (!TestPackageFinalityAndSerializedSize(ancestors)) { + if (!TestPackageTransactions(ancestors)) { if (fUsingModified) { mapModifiedTx.get().erase(modit); failedTx.insert(iter); diff --git a/src/miner.h b/src/miner.h index bc4da63da0..9fab55611e 100644 --- a/src/miner.h +++ b/src/miner.h @@ -192,8 +192,11 @@ private: void onlyUnconfirmed(CTxMemPool::setEntries& testSet); /** Test if a new package would "fit" in the block */ bool TestPackage(uint64_t packageSize, int64_t packageSigOpsCost); - /** Test if a set of transactions are all final */ - bool TestPackageFinalityAndSerializedSize(const CTxMemPool::setEntries& package); + /** Perform checks on each transaction in a package: + * locktime, premature-witness, serialized size (if necessary) + * These checks should always succeed, and they're here + * only as an extra check in case of suboptimal node configuration */ + bool TestPackageTransactions(const CTxMemPool::setEntries& package); /** Return true if given transaction from mapTx has already been evaluated, * or if the transaction's cached data in mapTx is incorrect. */ bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx); -- cgit v1.2.3 From d2e46e1b5cf6c08829ec3bb2a923b4ba149ab3b7 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 30 Jun 2016 11:37:38 -0400 Subject: Remove addScoreTxs() --- src/miner.cpp | 60 ----------------------------------------------------------- src/miner.h | 6 ++---- 2 files changed, 2 insertions(+), 64 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index f2ad1018b2..eb71355e7a 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -338,66 +338,6 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) } } -void BlockAssembler::addScoreTxs() -{ - std::priority_queue, ScoreCompare> clearedTxs; - CTxMemPool::setEntries waitSet; - CTxMemPool::indexed_transaction_set::index::type::iterator mi = mempool.mapTx.get().begin(); - CTxMemPool::txiter iter; - while (!blockFinished && (mi != mempool.mapTx.get().end() || !clearedTxs.empty())) - { - // If no txs that were previously postponed are available to try - // again, then try the next highest score tx - if (clearedTxs.empty()) { - iter = mempool.mapTx.project<0>(mi); - mi++; - } - // If a previously postponed tx is available to try again, then it - // has higher score than all untried so far txs - else { - iter = clearedTxs.top(); - clearedTxs.pop(); - } - - // If tx already in block, skip (added by addPriorityTxs) - if (inBlock.count(iter)) { - continue; - } - - // cannot accept witness transactions into a non-witness block - if (!fIncludeWitness && !iter->GetTx().wit.IsNull()) - continue; - - // If tx is dependent on other mempool txs which haven't yet been included - // then put it in the waitSet - if (isStillDependent(iter)) { - waitSet.insert(iter); - continue; - } - - // If the fee rate is below the min fee rate for mining, then we're done - // adding txs based on score (fee rate) - if (iter->GetModifiedFee() < ::minRelayTxFee.GetFee(iter->GetTxSize()) && nBlockSize >= nBlockMinSize) { - return; - } - - // If this tx fits in the block add it, otherwise keep looping - if (TestForBlock(iter)) { - AddToBlock(iter); - - // 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)) - { - if (waitSet.count(child)) { - clearedTxs.push(child); - waitSet.erase(child); - } - } - } - } -} - void BlockAssembler::UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded, indexed_modified_transaction_set &mapModifiedTx) { diff --git a/src/miner.h b/src/miner.h index 9fab55611e..037639b2be 100644 --- a/src/miner.h +++ b/src/miner.h @@ -157,7 +157,7 @@ private: int64_t nLockTimeCutoff; const CChainParams& chainparams; - // Variables used for addScoreTxs and addPriorityTxs + // Variables used for addPriorityTxs int lastFewTxs; bool blockFinished; @@ -174,14 +174,12 @@ private: void AddToBlock(CTxMemPool::txiter iter); // Methods for how to add transactions to a block. - /** Add transactions based on modified feerate */ - void addScoreTxs(); /** Add transactions based on tx "priority" */ void addPriorityTxs(); /** Add transactions based on feerate including unconfirmed ancestors */ void addPackageTxs(); - // helper function for addScoreTxs and addPriorityTxs + // 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 */ -- cgit v1.2.3 From 27362dda4d583a43ebf687ae097d2f45ba1c4c32 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 30 Jun 2016 11:41:13 -0400 Subject: Remove -blockminsize option --- src/init.cpp | 1 - src/miner.cpp | 7 +------ src/miner.h | 2 +- src/policy/policy.h | 3 +-- 4 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 5d29f14eb8..fdf6301d2a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -453,7 +453,6 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageGroup(_("Block creation options:")); strUsage += HelpMessageOpt("-blockmaxcost=", strprintf(_("Set maximum block cost (default: %d)"), DEFAULT_BLOCK_MAX_COST)); - strUsage += HelpMessageOpt("-blockminsize=", strprintf(_("Set minimum block size in bytes (default: %u)"), DEFAULT_BLOCK_MIN_SIZE)); 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)); if (showDebug) diff --git a/src/miner.cpp b/src/miner.cpp index eb71355e7a..8153fb9f9e 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -100,13 +100,8 @@ BlockAssembler::BlockAssembler(const CChainParams& _chainparams) // Limit size to between 1K and MAX_BLOCK_SERIALIZED_SIZE-1K for sanity: nBlockMaxSize = std::max((unsigned int)1000, std::min((unsigned int)(MAX_BLOCK_SERIALIZED_SIZE-1000), nBlockMaxSize)); - // Minimum block size you want to create; block will be filled with free transactions - // until there are no more or the block reaches this size: - nBlockMinSize = GetArg("-blockminsize", DEFAULT_BLOCK_MIN_SIZE); - nBlockMinSize = std::min(nBlockMaxSize, nBlockMinSize); - // Whether we need to account for byte usage (in addition to cost usage) - fNeedSizeAccounting = (nBlockMaxSize < MAX_BLOCK_SERIALIZED_SIZE-1000) || (nBlockMinSize > 0); + fNeedSizeAccounting = (nBlockMaxSize < MAX_BLOCK_SERIALIZED_SIZE-1000); } void BlockAssembler::resetBlock() diff --git a/src/miner.h b/src/miner.h index 037639b2be..d16e37bb59 100644 --- a/src/miner.h +++ b/src/miner.h @@ -141,7 +141,7 @@ private: // Configuration parameters for the block size bool fIncludeWitness; - unsigned int nBlockMaxCost, nBlockMaxSize, nBlockMinSize; + unsigned int nBlockMaxCost, nBlockMaxSize; bool fNeedSizeAccounting; // Information on the current status of the block diff --git a/src/policy/policy.h b/src/policy/policy.h index fefb562ff9..29a8cc57c2 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -14,9 +14,8 @@ class CCoinsViewCache; -/** Default for -blockmaxsize and -blockminsize, which control the range of sizes the mining code will create **/ +/** Default for -blockmaxsize, which controls the maximum size of block the mining code will create **/ static const unsigned int DEFAULT_BLOCK_MAX_SIZE = 750000; -static const unsigned int DEFAULT_BLOCK_MIN_SIZE = 0; /** Default for -blockprioritysize, maximum space for zero/low-fee transactions **/ static const unsigned int DEFAULT_BLOCK_PRIORITY_SIZE = 0; /** Default for -blockmaxcost, which control the range of block costs the mining code will create **/ -- cgit v1.2.3 From c1d61fbd080bcc29589b8d467df98efb7e89d231 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 5 Jul 2016 15:50:48 -0400 Subject: Add warning if -blockminsize is used. --- contrib/devtools/check-doc.py | 2 +- src/init.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/devtools/check-doc.py b/contrib/devtools/check-doc.py index 06c9551ceb..9ea0131ac3 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']) +SET_DOC_OPTIONAL = set(['-rpcssl', '-benchmark', '-h', '-help', '-socks', '-tor', '-debugnet', '-whitelistalwaysrelay', '-prematurewitness', '-walletprematurewitness', '-promiscuousmempoolflags', '-blockminsize']) def main(): used = check_output(CMD_GREP_ARGS, shell=True) diff --git a/src/init.cpp b/src/init.cpp index fdf6301d2a..7f82893286 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -876,6 +876,9 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (GetBoolArg("-whitelistalwaysrelay", false)) InitWarning(_("Unsupported argument -whitelistalwaysrelay ignored, use -whitelistrelay and/or -whitelistforcerelay.")); + if (mapArgs.count("-blockminsize")) + InitWarning("Unsupported argument -blockminsize ignored."); + // Checkmempool and checkblockindex default to true in regtest mode int ratio = std::min(std::max(GetArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000); if (ratio != 0) { -- cgit v1.2.3