From 62607d796cd93bfc5ce4ee5050c93cf0ca80ba91 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 5 Dec 2016 00:15:33 -0800 Subject: [PATCH] Convert COrphanTx to keep a CTransactionRef --- src/net_processing.cpp | 25 +++++++++++++------------ src/test/DoS_tests.cpp | 24 ++++++++++++------------ 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4c693eea0..10aaa77ff 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -51,7 +51,7 @@ struct IteratorComparator struct COrphanTx { // When modifying, adapt the copy of this definition in tests/DoS_tests. - CTransaction tx; + CTransactionRef tx; NodeId fromPeer; int64_t nTimeExpire; }; @@ -586,9 +586,9 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals) // mapOrphanTransactions // -bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - uint256 hash = tx.GetHash(); + const uint256& hash = tx->GetHash(); if (mapOrphanTransactions.count(hash)) return false; @@ -599,7 +599,7 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(c // have been mined or received. // 100 orphans, each of which is at most 99,999 bytes big is // at most 10 megabytes of orphans and somewhat more byprev index (in the worst case): - unsigned int sz = GetTransactionWeight(tx); + unsigned int sz = GetTransactionWeight(*tx); if (sz >= MAX_STANDARD_TX_WEIGHT) { LogPrint("mempool", "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString()); @@ -608,7 +608,7 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(c auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME}); assert(ret.second); - BOOST_FOREACH(const CTxIn& txin, tx.vin) { + BOOST_FOREACH(const CTxIn& txin, tx->vin) { mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); } @@ -622,7 +622,7 @@ int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) map::iterator it = mapOrphanTransactions.find(hash); if (it == mapOrphanTransactions.end()) return 0; - BOOST_FOREACH(const CTxIn& txin, it->second.tx.vin) + BOOST_FOREACH(const CTxIn& txin, it->second.tx->vin) { auto itPrev = mapOrphanTransactionsByPrev.find(txin.prevout); if (itPrev == mapOrphanTransactionsByPrev.end()) @@ -644,7 +644,7 @@ void EraseOrphansFor(NodeId peer) map::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid if (maybeErase->second.fromPeer == peer) { - nErased += EraseOrphanTx(maybeErase->second.tx.GetHash()); + nErased += EraseOrphanTx(maybeErase->second.tx->GetHash()); } } if (nErased > 0) LogPrint("mempool", "Erased %d orphan tx from peer %d\n", nErased, peer); @@ -665,7 +665,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRE { map::iterator maybeErase = iter++; if (maybeErase->second.nTimeExpire <= nNow) { - nErased += EraseOrphanTx(maybeErase->second.tx.GetHash()); + nErased += EraseOrphanTx(maybeErase->second.tx->GetHash()); } else { nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime); } @@ -736,7 +736,7 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout); if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { - const CTransaction& orphanTx = (*mi)->second.tx; + const CTransaction& orphanTx = *(*mi)->second.tx; const uint256& orphanHash = orphanTx.GetHash(); vOrphanErase.push_back(orphanHash); } @@ -1636,7 +1636,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, mi != itByPrev->second.end(); ++mi) { - const CTransaction& orphanTx = (*mi)->second.tx; + const CTransactionRef& porphanTx = (*mi)->second.tx; + const CTransaction& orphanTx = *porphanTx; const uint256& orphanHash = orphanTx.GetHash(); NodeId fromPeer = (*mi)->second.fromPeer; bool fMissingInputs2 = false; @@ -1648,7 +1649,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (setMisbehaving.count(fromPeer)) continue; - if (AcceptToMemoryPool(mempool, stateDummy, MakeTransactionRef(orphanTx), true, &fMissingInputs2)) { + if (AcceptToMemoryPool(mempool, stateDummy, porphanTx, true, &fMissingInputs2)) { LogPrint("mempool", " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanTx, connman); for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { @@ -1701,7 +1702,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, pfrom->AddInventoryKnown(_inv); if (!AlreadyHave(_inv)) pfrom->AskFor(_inv); } - AddOrphanTx(tx, pfrom->GetId()); + AddOrphanTx(ptx, pfrom->GetId()); // DoS prevention: do not allow mapOrphanTransactions to grow unbounded unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index 131d667e0..493a568b4 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -23,11 +23,11 @@ #include // Tests this internal-to-main.cpp method: -extern bool AddOrphanTx(const CTransaction& tx, NodeId peer); +extern bool AddOrphanTx(const CTransactionRef& tx, NodeId peer); extern void EraseOrphansFor(NodeId peer); extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans); struct COrphanTx { - CTransaction tx; + CTransactionRef tx; NodeId fromPeer; int64_t nTimeExpire; }; @@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) BOOST_CHECK(!connman->IsBanned(addr)); } -CTransaction RandomOrphan() +CTransactionRef RandomOrphan() { std::map::iterator it; it = mapOrphanTransactions.lower_bound(GetRandHash()); @@ -143,30 +143,30 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) tx.vout[0].nValue = 1*CENT; tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); - AddOrphanTx(tx, i); + AddOrphanTx(MakeTransactionRef(tx), i); } // ... and 50 that depend on other orphans: for (int i = 0; i < 50; i++) { - CTransaction txPrev = RandomOrphan(); + CTransactionRef txPrev = RandomOrphan(); CMutableTransaction tx; tx.vin.resize(1); tx.vin[0].prevout.n = 0; - tx.vin[0].prevout.hash = txPrev.GetHash(); + tx.vin[0].prevout.hash = txPrev->GetHash(); tx.vout.resize(1); tx.vout[0].nValue = 1*CENT; tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); - SignSignature(keystore, txPrev, tx, 0, SIGHASH_ALL); + SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL); - AddOrphanTx(tx, i); + AddOrphanTx(MakeTransactionRef(tx), i); } // This really-big orphan should be ignored: for (int i = 0; i < 10; i++) { - CTransaction txPrev = RandomOrphan(); + CTransactionRef txPrev = RandomOrphan(); CMutableTransaction tx; tx.vout.resize(1); @@ -176,15 +176,15 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) for (unsigned int j = 0; j < tx.vin.size(); j++) { tx.vin[j].prevout.n = j; - tx.vin[j].prevout.hash = txPrev.GetHash(); + tx.vin[j].prevout.hash = txPrev->GetHash(); } - SignSignature(keystore, txPrev, tx, 0, SIGHASH_ALL); + SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL); // Re-use same signature for other inputs // (they don't have to be valid for this test) for (unsigned int j = 1; j < tx.vin.size(); j++) tx.vin[j].scriptSig = tx.vin[0].scriptSig; - BOOST_CHECK(!AddOrphanTx(tx, i)); + BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i)); } // Test EraseOrphansFor: