Convert COrphanTx to keep a CTransactionRef

This commit is contained in:
Pieter Wuille 2016-12-05 00:15:33 -08:00
parent c44e4c467c
commit 62607d796c
2 changed files with 25 additions and 24 deletions

View File

@ -51,7 +51,7 @@ struct IteratorComparator
struct COrphanTx { struct COrphanTx {
// When modifying, adapt the copy of this definition in tests/DoS_tests. // When modifying, adapt the copy of this definition in tests/DoS_tests.
CTransaction tx; CTransactionRef tx;
NodeId fromPeer; NodeId fromPeer;
int64_t nTimeExpire; int64_t nTimeExpire;
}; };
@ -586,9 +586,9 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals)
// mapOrphanTransactions // 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)) if (mapOrphanTransactions.count(hash))
return false; return false;
@ -599,7 +599,7 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(c
// have been mined or received. // have been mined or received.
// 100 orphans, each of which is at most 99,999 bytes big is // 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): // 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) if (sz >= MAX_STANDARD_TX_WEIGHT)
{ {
LogPrint("mempool", "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString()); 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}); auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME});
assert(ret.second); assert(ret.second);
BOOST_FOREACH(const CTxIn& txin, tx.vin) { BOOST_FOREACH(const CTxIn& txin, tx->vin) {
mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first);
} }
@ -622,7 +622,7 @@ int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(hash); map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(hash);
if (it == mapOrphanTransactions.end()) if (it == mapOrphanTransactions.end())
return 0; 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); auto itPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
if (itPrev == mapOrphanTransactionsByPrev.end()) if (itPrev == mapOrphanTransactionsByPrev.end())
@ -644,7 +644,7 @@ void EraseOrphansFor(NodeId peer)
map<uint256, COrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid map<uint256, COrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
if (maybeErase->second.fromPeer == peer) 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); 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<uint256, COrphanTx>::iterator maybeErase = iter++; map<uint256, COrphanTx>::iterator maybeErase = iter++;
if (maybeErase->second.nTimeExpire <= nNow) { if (maybeErase->second.nTimeExpire <= nNow) {
nErased += EraseOrphanTx(maybeErase->second.tx.GetHash()); nErased += EraseOrphanTx(maybeErase->second.tx->GetHash());
} else { } else {
nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime); 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); auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout);
if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; if (itByPrev == mapOrphanTransactionsByPrev.end()) continue;
for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { 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(); const uint256& orphanHash = orphanTx.GetHash();
vOrphanErase.push_back(orphanHash); vOrphanErase.push_back(orphanHash);
} }
@ -1636,7 +1636,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
mi != itByPrev->second.end(); mi != itByPrev->second.end();
++mi) ++mi)
{ {
const CTransaction& orphanTx = (*mi)->second.tx; const CTransactionRef& porphanTx = (*mi)->second.tx;
const CTransaction& orphanTx = *porphanTx;
const uint256& orphanHash = orphanTx.GetHash(); const uint256& orphanHash = orphanTx.GetHash();
NodeId fromPeer = (*mi)->second.fromPeer; NodeId fromPeer = (*mi)->second.fromPeer;
bool fMissingInputs2 = false; bool fMissingInputs2 = false;
@ -1648,7 +1649,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
if (setMisbehaving.count(fromPeer)) if (setMisbehaving.count(fromPeer))
continue; 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()); LogPrint("mempool", " accepted orphan tx %s\n", orphanHash.ToString());
RelayTransaction(orphanTx, connman); RelayTransaction(orphanTx, connman);
for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { 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); pfrom->AddInventoryKnown(_inv);
if (!AlreadyHave(_inv)) pfrom->AskFor(_inv); if (!AlreadyHave(_inv)) pfrom->AskFor(_inv);
} }
AddOrphanTx(tx, pfrom->GetId()); AddOrphanTx(ptx, pfrom->GetId());
// DoS prevention: do not allow mapOrphanTransactions to grow unbounded // 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)); unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS));

View File

@ -23,11 +23,11 @@
#include <boost/test/unit_test.hpp> #include <boost/test/unit_test.hpp>
// Tests this internal-to-main.cpp method: // 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 void EraseOrphansFor(NodeId peer);
extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans); extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
struct COrphanTx { struct COrphanTx {
CTransaction tx; CTransactionRef tx;
NodeId fromPeer; NodeId fromPeer;
int64_t nTimeExpire; int64_t nTimeExpire;
}; };
@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
BOOST_CHECK(!connman->IsBanned(addr)); BOOST_CHECK(!connman->IsBanned(addr));
} }
CTransaction RandomOrphan() CTransactionRef RandomOrphan()
{ {
std::map<uint256, COrphanTx>::iterator it; std::map<uint256, COrphanTx>::iterator it;
it = mapOrphanTransactions.lower_bound(GetRandHash()); it = mapOrphanTransactions.lower_bound(GetRandHash());
@ -143,30 +143,30 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
tx.vout[0].nValue = 1*CENT; tx.vout[0].nValue = 1*CENT;
tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID());
AddOrphanTx(tx, i); AddOrphanTx(MakeTransactionRef(tx), i);
} }
// ... and 50 that depend on other orphans: // ... and 50 that depend on other orphans:
for (int i = 0; i < 50; i++) for (int i = 0; i < 50; i++)
{ {
CTransaction txPrev = RandomOrphan(); CTransactionRef txPrev = RandomOrphan();
CMutableTransaction tx; CMutableTransaction tx;
tx.vin.resize(1); tx.vin.resize(1);
tx.vin[0].prevout.n = 0; 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.resize(1);
tx.vout[0].nValue = 1*CENT; tx.vout[0].nValue = 1*CENT;
tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); 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: // This really-big orphan should be ignored:
for (int i = 0; i < 10; i++) for (int i = 0; i < 10; i++)
{ {
CTransaction txPrev = RandomOrphan(); CTransactionRef txPrev = RandomOrphan();
CMutableTransaction tx; CMutableTransaction tx;
tx.vout.resize(1); tx.vout.resize(1);
@ -176,15 +176,15 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
for (unsigned int j = 0; j < tx.vin.size(); j++) for (unsigned int j = 0; j < tx.vin.size(); j++)
{ {
tx.vin[j].prevout.n = 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 // Re-use same signature for other inputs
// (they don't have to be valid for this test) // (they don't have to be valid for this test)
for (unsigned int j = 1; j < tx.vin.size(); j++) for (unsigned int j = 1; j < tx.vin.size(); j++)
tx.vin[j].scriptSig = tx.vin[0].scriptSig; tx.vin[j].scriptSig = tx.vin[0].scriptSig;
BOOST_CHECK(!AddOrphanTx(tx, i)); BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i));
} }
// Test EraseOrphansFor: // Test EraseOrphansFor: