Skip to content

Commit

Permalink
Convert COrphanTx to keep a CTransactionRef
Browse files Browse the repository at this point in the history
  • Loading branch information
@sipa
sipa committed Dec 22, 2016
1 parent c44e4c4 commit 62607d7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 24 deletions.
25 changes: 13 additions & 12 deletions src/net_processing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -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;

Expand All @@ -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());
Expand All @@ -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);
}

Expand All @@ -622,7 +622,7 @@ int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
map<uint256, COrphanTx>::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())
Expand All @@ -644,7 +644,7 @@ void EraseOrphansFor(NodeId peer)
map<uint256, COrphanTx>::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);
Expand All @@ -665,7 +665,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRE
{
map<uint256, COrphanTx>::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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand All @@ -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++) {
Expand Down Expand Up @@ -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));
Expand Down
24 changes: 12 additions & 12 deletions src/test/DoS_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
#include <boost/test/unit_test.hpp>

// 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;
};
Expand Down Expand Up @@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
BOOST_CHECK(!connman->IsBanned(addr));
}

CTransaction RandomOrphan()
CTransactionRef RandomOrphan()
{
std::map<uint256, COrphanTx>::iterator it;
it = mapOrphanTransactions.lower_bound(GetRandHash());
Expand Down Expand Up @@ -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);
Expand All @@ -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:
Expand Down

0 comments on commit 62607d7

Please sign in to comment.