From c73554042886fb63fb48edf29cf827951edde341 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 9 Jan 2017 14:11:15 -0500 Subject: [PATCH 01/10] Move ORPHAN constants from validation.h to net_processing.h --- src/net_processing.h | 7 +++++++ src/validation.h | 6 ------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/net_processing.h b/src/net_processing.h index 230d805bd..92ea21df7 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -9,6 +9,13 @@ #include "net.h" #include "validationinterface.h" +/** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ +static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; +/** Expiration time for orphan transactions in seconds */ +static const int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; +/** Minimum time between orphan transactions expire time checks in seconds */ +static const int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; + /** Register with a network node to receive its signals */ void RegisterNodeSignals(CNodeSignals& nodeSignals); /** Unregister a network node */ diff --git a/src/validation.h b/src/validation.h index 631602a70..7848c787a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -58,12 +58,6 @@ static const CAmount DEFAULT_TRANSACTION_MAXFEE = 0.1 * COIN; static const CAmount HIGH_TX_FEE_PER_KB = 0.01 * COIN; //! -maxtxfee will warn if called with a higher fee than this amount (in satoshis) static const CAmount HIGH_MAX_TX_FEE = 100 * HIGH_TX_FEE_PER_KB; -/** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ -static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; -/** Expiration time for orphan transactions in seconds */ -static const int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; -/** Minimum time between orphan transactions expire time checks in seconds */ -static const int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; /** Default for -limitancestorcount, max number of in-mempool ancestors */ static const unsigned int DEFAULT_ANCESTOR_LIMIT = 25; /** Default for -limitancestorsize, maximum kilobytes of tx + all in-mempool ancestors */ From edded808fc4eee94c178e1779b90d1c87a08c23a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 4 Dec 2016 18:53:26 -0800 Subject: [PATCH 02/10] Make ATMP optionally return the CTransactionRefs it replaced --- src/rpc/rawtransaction.cpp | 2 +- src/test/txvalidationcache_tests.cpp | 2 +- src/validation.cpp | 18 +++++++++++------- src/validation.h | 9 ++++++--- src/wallet/wallet.cpp | 2 +- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 276ebfda2..e01879adb 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -899,7 +899,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) // push to local node and sync with wallets CValidationState state; bool fMissingInputs; - if (!AcceptToMemoryPool(mempool, state, std::move(tx), fLimitFree, &fMissingInputs, false, nMaxRawTxFee)) { + if (!AcceptToMemoryPool(mempool, state, std::move(tx), fLimitFree, &fMissingInputs, NULL, false, nMaxRawTxFee)) { if (state.IsInvalid()) { throw JSONRPCError(RPC_TRANSACTION_REJECTED, strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())); } else { diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index acccaee98..c5367208b 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -23,7 +23,7 @@ ToMemPool(CMutableTransaction& tx) LOCK(cs_main); CValidationState state; - return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), false, NULL, true, 0); + return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), false, NULL, NULL, true, 0); } BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) diff --git a/src/validation.cpp b/src/validation.cpp index 37a4186e0..ff1d9b0dc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -538,8 +538,8 @@ static bool IsCurrentForFeeEstimation() } bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree, - bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, - std::vector& vHashTxnToUncache) + bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, + bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector& vHashTxnToUncache) { const CTransaction& tx = *ptx; const uint256 hash = tx.GetHash(); @@ -950,6 +950,8 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C hash.ToString(), FormatMoney(nModifiedFees - nConflictingFees), (int)nSize - (int)nConflictingSize); + if (plTxnReplaced) + plTxnReplaced->push_back(it->GetSharedTx()); } pool.RemoveStaged(allConflicting, false); @@ -975,10 +977,11 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C } bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree, - bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit, const CAmount nAbsurdFee) + bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, + bool fOverrideMempoolLimit, const CAmount nAbsurdFee) { std::vector vHashTxToUncache; - bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, fOverrideMempoolLimit, nAbsurdFee, vHashTxToUncache); + bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, plTxnReplaced, fOverrideMempoolLimit, nAbsurdFee, vHashTxToUncache); if (!res) { BOOST_FOREACH(const uint256& hashTx, vHashTxToUncache) pcoinsTip->Uncache(hashTx); @@ -990,9 +993,10 @@ bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const } bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree, - bool* pfMissingInputs, bool fOverrideMempoolLimit, const CAmount nAbsurdFee) + bool* pfMissingInputs, std::list* plTxnReplaced, + bool fOverrideMempoolLimit, const CAmount nAbsurdFee) { - return AcceptToMemoryPoolWithTime(pool, state, tx, fLimitFree, pfMissingInputs, GetTime(), fOverrideMempoolLimit, nAbsurdFee); + return AcceptToMemoryPoolWithTime(pool, state, tx, fLimitFree, pfMissingInputs, GetTime(), plTxnReplaced, fOverrideMempoolLimit, nAbsurdFee); } /** Return transaction in txOut, and if it was found inside a block, its hash is placed in hashBlock */ @@ -2138,7 +2142,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara const CTransaction& tx = *it; // ignore validation errors in resurrected transactions CValidationState stateDummy; - if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, it, false, NULL, true)) { + if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, it, false, NULL, NULL, true)) { mempool.removeRecursive(tx); } else if (mempool.exists(tx.GetHash())) { vHashUpdate.push_back(tx.GetHash()); diff --git a/src/validation.h b/src/validation.h index 7848c787a..0c79ed30b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -304,13 +304,16 @@ void FlushStateToDisk(); /** Prune block files and flush state to disk. */ void PruneAndFlush(); -/** (try to) add transaction to memory pool **/ +/** (try to) add transaction to memory pool + * plTxnReplaced will be appended to with all transactions replaced from mempool **/ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree, - bool* pfMissingInputs, bool fOverrideMempoolLimit=false, const CAmount nAbsurdFee=0); + bool* pfMissingInputs, std::list* plTxnReplaced = NULL, + bool fOverrideMempoolLimit=false, const CAmount nAbsurdFee=0); /** (try to) add transaction to memory pool with a specified acceptance time **/ bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree, - bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit=false, const CAmount nAbsurdFee=0); + bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced = NULL, + bool fOverrideMempoolLimit=false, const CAmount nAbsurdFee=0); /** Convert CValidationState to a human-readable message for logging */ std::string FormatStateMessage(const CValidationState &state); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2775f4def..a199591ad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3847,5 +3847,5 @@ int CMerkleTx::GetBlocksToMaturity() const bool CMerkleTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state) { - return ::AcceptToMemoryPool(mempool, state, tx, true, NULL, false, nAbsurdFee); + return ::AcceptToMemoryPool(mempool, state, tx, true, NULL, NULL, false, nAbsurdFee); } From 1531652e02d856c1b8c3ba6f6e51e1983ac0540c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 9 Jan 2017 14:38:06 -0500 Subject: [PATCH 03/10] Keep shared_ptrs to recently-replaced txn for compact blocks --- src/init.cpp | 1 + src/net_processing.cpp | 24 ++++++++++++++++++++++-- src/net_processing.h | 2 ++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 992ce8ebd..7a89fb887 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -346,6 +346,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-maxorphantx=", strprintf(_("Keep at most unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS)); strUsage += HelpMessageOpt("-maxmempool=", strprintf(_("Keep the transaction memory pool below megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE)); strUsage += HelpMessageOpt("-mempoolexpiry=", strprintf(_("Do not keep transactions in the mempool longer than hours (default: %u)"), DEFAULT_MEMPOOL_EXPIRY)); + strUsage += HelpMessageOpt("-blockreconstructionextratxn=", strprintf(_("Extra transactions to keep in memory for compact block reconstructions (default: %u)"), DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN)); strUsage += HelpMessageOpt("-par=", strprintf(_("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)"), -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS)); #ifndef WIN32 diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3a956e89e..c549b9eb9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -59,6 +59,9 @@ map mapOrphanTransactions GUARDED_BY(cs_main); map::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(cs_main); void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +static size_t vExtraTxnForCompactIt = 0; +static std::vector> vExtraTxnForCompact GUARDED_BY(cs_main); + static const uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL; // SHA256("main address relay")[0:8] // Internal stuff @@ -586,6 +589,17 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals) // mapOrphanTransactions // +void AddToCompactExtraTransactions(const CTransactionRef& tx) +{ + size_t max_extra_txn = GetArg("-blockreconstructionextratxn", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN); + if (max_extra_txn <= 0) + return; + if (!vExtraTxnForCompact.size()) + vExtraTxnForCompact.resize(max_extra_txn); + vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetWitnessHash(), tx); + vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % max_extra_txn; +} + bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { const uint256& hash = tx->GetHash(); @@ -1615,7 +1629,9 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, pfrom->setAskFor.erase(inv.hash); mapAlreadyAskedFor.erase(inv.hash); - if (!AlreadyHave(inv) && AcceptToMemoryPool(mempool, state, ptx, true, &fMissingInputs)) { + std::list lRemovedTxn; + + if (!AlreadyHave(inv) && AcceptToMemoryPool(mempool, state, ptx, true, &fMissingInputs, &lRemovedTxn)) { mempool.check(pcoinsTip); RelayTransaction(tx, connman); for (unsigned int i = 0; i < tx.vout.size(); i++) { @@ -1653,7 +1669,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (setMisbehaving.count(fromPeer)) continue; - if (AcceptToMemoryPool(mempool, stateDummy, porphanTx, true, &fMissingInputs2)) { + if (AcceptToMemoryPool(mempool, stateDummy, porphanTx, true, &fMissingInputs2, &lRemovedTxn)) { LogPrint("mempool", " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanTx, connman); for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { @@ -1743,6 +1759,10 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } } } + + for (const CTransactionRef& tx : lRemovedTxn) + AddToCompactExtraTransactions(tx); + int nDoS = 0; if (state.IsInvalid(nDoS)) { diff --git a/src/net_processing.h b/src/net_processing.h index 92ea21df7..a338e14a9 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -15,6 +15,8 @@ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; static const int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static const int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; +/** Default number of orphan+recently-replaced txn to keep around for block reconstruction */ +static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; /** Register with a network node to receive its signals */ void RegisterNodeSignals(CNodeSignals& nodeSignals); From 93380c5247526e2248248a7d539233ec48d11bdd Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 4 Dec 2016 20:44:37 -0800 Subject: [PATCH 04/10] Use replaced transactions in compact block reconstruction --- src/blockencodings.cpp | 32 ++++++++++++++++++++++++++++++- src/blockencodings.h | 3 ++- src/net_processing.cpp | 4 ++-- src/test/blockencodings_tests.cpp | 10 ++++++---- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 72fe17bdc..43b4cb9b8 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -47,7 +47,7 @@ uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const uint256& txhash) const { -ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock) { +ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, std::vector>& extra_txn) { if (cmpctblock.header.IsNull() || (cmpctblock.shorttxids.empty() && cmpctblock.prefilledtxn.empty())) return READ_STATUS_INVALID; if (cmpctblock.shorttxids.size() + cmpctblock.prefilledtxn.size() > MAX_BLOCK_BASE_SIZE / MIN_TRANSACTION_BASE_SIZE) @@ -104,6 +104,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c return READ_STATUS_FAILED; // Short ID collision std::vector have_txn(txn_available.size()); + { LOCK(pool->cs); const std::vector >& vTxHashes = pool->vTxHashes; for (size_t i = 0; i < vTxHashes.size(); i++) { @@ -130,6 +131,35 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c if (mempool_count == shorttxids.size()) break; } + } + + for (size_t i = 0; i < extra_txn.size(); i++) { + uint64_t shortid = cmpctblock.GetShortID(extra_txn[i].first); + std::unordered_map::iterator idit = shorttxids.find(shortid); + if (idit != shorttxids.end()) { + if (!have_txn[idit->second]) { + txn_available[idit->second] = extra_txn[i].second; + have_txn[idit->second] = true; + mempool_count++; + } else { + // If we find two mempool txn that match the short id, just request it. + // This should be rare enough that the extra bandwidth doesn't matter, + // but eating a round-trip due to FillBlock failure would be annoying + // Note that we dont want duplication between extra_txn and mempool to + // trigger this case, so we compare witness hashes first + if (txn_available[idit->second] && + txn_available[idit->second]->GetWitnessHash() != extra_txn[i].second->GetWitnessHash()) { + txn_available[idit->second].reset(); + mempool_count--; + } + } + } + // Though ideally we'd continue scanning for the two-txn-match-shortid case, + // the performance win of an early exit here is too good to pass up and worth + // the extra risk. + if (mempool_count == shorttxids.size()) + break; + } LogPrint("cmpctblock", "Initialized PartiallyDownloadedBlock for block %s using a cmpctblock of size %lu\n", cmpctblock.header.GetHash().ToString(), GetSerializeSize(cmpctblock, SER_NETWORK, PROTOCOL_VERSION)); diff --git a/src/blockencodings.h b/src/blockencodings.h index 809ccbf93..7bbd9c661 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -200,7 +200,8 @@ public: CBlockHeader header; PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} - ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock); + // extra_txn is a list of extra transactions to look at, in form + ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, std::vector>& extra_txn); bool IsTxAvailable(size_t index) const; ReadStatus FillBlock(CBlock& block, const std::vector& vtx_missing); }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c549b9eb9..8e23e54e0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1879,7 +1879,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } PartiallyDownloadedBlock& partialBlock = *(*queuedBlockIt)->partialBlock; - ReadStatus status = partialBlock.InitData(cmpctblock); + ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case of whitelist Misbehaving(pfrom->GetId(), 100); @@ -1921,7 +1921,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Optimistically try to reconstruct anyway since we might be // able to without any round trips. PartiallyDownloadedBlock tempBlock(&mempool); - ReadStatus status = tempBlock.InitData(cmpctblock); + ReadStatus status = tempBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status != READ_STATUS_OK) { // TODO: don't ignore failures return true; diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index e3876e969..311ac024f 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -11,6 +11,8 @@ #include +std::vector> extra_txn; + struct RegtestingSetup : public TestingSetup { RegtestingSetup() : TestingSetup(CBaseChainParams::REGTEST) {} }; @@ -73,7 +75,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) stream >> shortIDs2; PartiallyDownloadedBlock partialBlock(&pool); - BOOST_CHECK(partialBlock.InitData(shortIDs2) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.InitData(shortIDs2, extra_txn) == READ_STATUS_OK); BOOST_CHECK( partialBlock.IsTxAvailable(0)); BOOST_CHECK(!partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); @@ -179,7 +181,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) stream >> shortIDs2; PartiallyDownloadedBlock partialBlock(&pool); - BOOST_CHECK(partialBlock.InitData(shortIDs2) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.InitData(shortIDs2, extra_txn) == READ_STATUS_OK); BOOST_CHECK(!partialBlock.IsTxAvailable(0)); BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); @@ -245,7 +247,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) stream >> shortIDs2; PartiallyDownloadedBlock partialBlock(&pool); - BOOST_CHECK(partialBlock.InitData(shortIDs2) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.InitData(shortIDs2, extra_txn) == READ_STATUS_OK); BOOST_CHECK( partialBlock.IsTxAvailable(0)); BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); @@ -300,7 +302,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) stream >> shortIDs2; PartiallyDownloadedBlock partialBlock(&pool); - BOOST_CHECK(partialBlock.InitData(shortIDs2) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.InitData(shortIDs2, extra_txn) == READ_STATUS_OK); BOOST_CHECK(partialBlock.IsTxAvailable(0)); CBlock block2; From 7f8c8cab1e537809848088d3bfaa13ecca7ac8cc Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 9 Jan 2017 14:38:16 -0500 Subject: [PATCH 05/10] Consider all orphan txn for compact-block-extra-txn cache --- src/net_processing.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8e23e54e0..bdcfacc06 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -626,6 +626,8 @@ bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRE mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); } + AddToCompactExtraTransactions(tx); + LogPrint("mempool", "stored orphan tx %s (mapsz %u outsz %u)\n", hash.ToString(), mapOrphanTransactions.size(), mapOrphanTransactionsByPrev.size()); return true; From 863edb45b9841c3058e883015c7576e6f69e3cc6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 9 Jan 2017 14:30:43 -0500 Subject: [PATCH 06/10] Consider all (<100k memusage) txn for compact-block-extra-txn cache --- src/net_processing.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index bdcfacc06..6025615e4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1741,7 +1741,10 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // See https://github.com/bitcoin/bitcoin/issues/8279 for details. assert(recentRejects); recentRejects->insert(tx.GetHash()); - } + if (RecursiveDynamicUsage(*ptx) < 100000) + AddToCompactExtraTransactions(ptx); + } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000) + AddToCompactExtraTransactions(ptx); if (pfrom->fWhitelisted && GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) { // Always relay transactions received from whitelisted peers, even From b55b4163462756cbb2341294b839a881333d8f1e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 12 Jan 2017 12:19:14 -0800 Subject: [PATCH 07/10] Add extra_count lower bound to compact reconstruction debug print --- src/blockencodings.cpp | 4 +++- src/blockencodings.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 43b4cb9b8..c27cde216 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -141,6 +141,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c txn_available[idit->second] = extra_txn[i].second; have_txn[idit->second] = true; mempool_count++; + extra_count++; } else { // If we find two mempool txn that match the short id, just request it. // This should be rare enough that the extra bandwidth doesn't matter, @@ -151,6 +152,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c txn_available[idit->second]->GetWitnessHash() != extra_txn[i].second->GetWitnessHash()) { txn_available[idit->second].reset(); mempool_count--; + extra_count--; } } } @@ -206,7 +208,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< return READ_STATUS_CHECKBLOCK_FAILED; } - LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool and %lu txn requested\n", hash.ToString(), prefilled_count, mempool_count, vtx_missing.size()); + LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool (incl at least %lu from extra pool) and %lu txn requested\n", hash.ToString(), prefilled_count, mempool_count, extra_count, vtx_missing.size()); if (vtx_missing.size() < 5) { for (const auto& tx : vtx_missing) LogPrint("cmpctblock", "Reconstructed block %s required tx %s\n", hash.ToString(), tx->GetHash().ToString()); diff --git a/src/blockencodings.h b/src/blockencodings.h index 7bbd9c661..971476cbb 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -194,7 +194,7 @@ public: class PartiallyDownloadedBlock { protected: std::vector txn_available; - size_t prefilled_count = 0, mempool_count = 0; + size_t prefilled_count = 0, mempool_count = 0, extra_count = 0; CTxMemPool* pool; public: CBlockHeader header; From fac4c78028d132ee7becc1dc54a0c881ae1a0287 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 12 Jan 2017 12:20:11 -0800 Subject: [PATCH 08/10] Make PartiallyDownloadedBlock::InitData's second param const --- src/blockencodings.cpp | 2 +- src/blockencodings.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index c27cde216..48431cfb3 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -47,7 +47,7 @@ uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const uint256& txhash) const { -ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, std::vector>& extra_txn) { +ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector>& extra_txn) { if (cmpctblock.header.IsNull() || (cmpctblock.shorttxids.empty() && cmpctblock.prefilledtxn.empty())) return READ_STATUS_INVALID; if (cmpctblock.shorttxids.size() + cmpctblock.prefilledtxn.size() > MAX_BLOCK_BASE_SIZE / MIN_TRANSACTION_BASE_SIZE) diff --git a/src/blockencodings.h b/src/blockencodings.h index 971476cbb..281db9fe0 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -201,7 +201,7 @@ public: PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} // extra_txn is a list of extra transactions to look at, in form - ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, std::vector>& extra_txn); + ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector>& extra_txn); bool IsTxAvailable(size_t index) const; ReadStatus FillBlock(CBlock& block, const std::vector& vtx_missing); }; From 1ccfe9b1c93460e3d345948c3740cfeca45e2ee3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 16 Jan 2017 22:58:06 -0500 Subject: [PATCH 09/10] Clarify comment about mempool/extra conflicts --- src/blockencodings.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 48431cfb3..4a311cbba 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -143,7 +143,8 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c mempool_count++; extra_count++; } else { - // If we find two mempool txn that match the short id, just request it. + // If we find two mempool/extra txn that match the short id, just + // request it. // This should be rare enough that the extra bandwidth doesn't matter, // but eating a round-trip due to FillBlock failure would be annoying // Note that we dont want duplication between extra_txn and mempool to From c5945804ca7e5edd9fce8c6de9b1f0ef775f9e79 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 16 Jan 2017 23:00:58 -0500 Subject: [PATCH 10/10] Add braces around AddToCompactExtraTransactions --- src/net_processing.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6025615e4..b1e606aba 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1741,10 +1741,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // See https://github.com/bitcoin/bitcoin/issues/8279 for details. assert(recentRejects); recentRejects->insert(tx.GetHash()); - if (RecursiveDynamicUsage(*ptx) < 100000) + if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); - } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000) + } + } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); + } if (pfrom->fWhitelisted && GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) { // Always relay transactions received from whitelisted peers, even