Skip to content

Commit

Permalink
Stricter handling of orphan transactions
Browse files Browse the repository at this point in the history 
Prevent denial-of-service attacks by banning
peers that send us invalid orphan transactions
and only storing orphan transactions given to
us by a peer while the peer is connected.

Rebased-From: c74332c

Conflicts:
	src/main.cpp
  • Loading branch information
@gavinandresen
gavinandresen authored and Ross Nicoll committed Oct 4, 2014
1 parent 76ebacb commit e67aaca
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 18 deletions.
63 changes: 50 additions & 13 deletions src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@ struct COrphanBlock {
map<uint256, COrphanBlock*> mapOrphanBlocks;
multimap<uint256, COrphanBlock*> mapOrphanBlocksByPrev;

map<uint256, CTransaction> mapOrphanTransactions;
struct COrphanTx {
CTransaction tx;
NodeId fromPeer;
};
map<uint256, COrphanTx> mapOrphanTransactions;
map<uint256, set<uint256> > mapOrphanTransactionsByPrev;
void EraseOrphansFor(NodeId peer);

// Constant stuff for coinbase transactions we create:
CScript COINBASE_FLAGS;
Expand Down Expand Up @@ -257,6 +262,7 @@ void FinalizeNode(NodeId nodeid) {
mapBlocksInFlight.erase(entry.hash);
BOOST_FOREACH(const uint256& hash, state->vBlocksToDownload)
mapBlocksToDownload.erase(hash);
EraseOrphansFor(nodeid);

mapNodeState.erase(nodeid);
}
Expand Down Expand Up @@ -412,7 +418,7 @@ CBlockTreeDB *pblocktree = NULL;
// mapOrphanTransactions
//

bool AddOrphanTx(const CTransaction& tx)
bool AddOrphanTx(const CTransaction& tx, NodeId peer)
{
uint256 hash = tx.GetHash();
if (mapOrphanTransactions.count(hash))
Expand All @@ -432,21 +438,22 @@ bool AddOrphanTx(const CTransaction& tx)
return false;
}

mapOrphanTransactions[hash] = tx;
mapOrphanTransactions[hash].tx = tx;
mapOrphanTransactions[hash].fromPeer = peer;
BOOST_FOREACH(const CTxIn& txin, tx.vin)
mapOrphanTransactionsByPrev[txin.prevout.hash].insert(hash);

LogPrint("mempool", "stored orphan tx %s (mapsz %" PRIszu")\n", hash.ToString(),
mapOrphanTransactions.size());
LogPrint("mempool", "stored orphan tx %s (mapsz %u prevsz %u)\n", hash.ToString(),
mapOrphanTransactions.size(), mapOrphanTransactionsByPrev.size());
return true;
}

void static EraseOrphanTx(uint256 hash)
{
map<uint256, CTransaction>::iterator it = mapOrphanTransactions.find(hash);
map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(hash);
if (it == mapOrphanTransactions.end())
return;
BOOST_FOREACH(const CTxIn& txin, it->second.vin)
BOOST_FOREACH(const CTxIn& txin, it->second.tx.vin)
{
map<uint256, set<uint256> >::iterator itPrev = mapOrphanTransactionsByPrev.find(txin.prevout.hash);
if (itPrev == mapOrphanTransactionsByPrev.end())
Expand All @@ -458,14 +465,31 @@ void static EraseOrphanTx(uint256 hash)
mapOrphanTransactions.erase(it);
}

void EraseOrphansFor(NodeId peer)
{
int nErased = 0;
map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
while (iter != mapOrphanTransactions.end())
{
map<uint256, COrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
if (maybeErase->second.fromPeer == peer)
{
EraseOrphanTx(maybeErase->second.tx.GetHash());
++nErased;
}
}
if (nErased > 0) LogPrint("mempool", "Erased %d orphan tx from peer %d\n", nErased, peer);
}


unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
{
unsigned int nEvicted = 0;
while (mapOrphanTransactions.size() > nMaxOrphans)
{
// Evict a random orphan:
uint256 randomhash = GetRandHash();
map<uint256, CTransaction>::iterator it = mapOrphanTransactions.lower_bound(randomhash);
map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.lower_bound(randomhash);
if (it == mapOrphanTransactions.end())
it = mapOrphanTransactions.begin();
EraseOrphanTx(it->first);
Expand Down Expand Up @@ -4078,6 +4102,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
mempool.mapTx.size());

// Recursively process any orphan transactions that depended on this one
set<NodeId> setMisbehaving;
for (unsigned int i = 0; i < vWorkQueue.size(); i++)
{
map<uint256, set<uint256> >::iterator itByPrev = mapOrphanTransactionsByPrev.find(vWorkQueue[i]);
Expand All @@ -4088,25 +4113,36 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
++mi)
{
const uint256& orphanHash = *mi;
const CTransaction& orphanTx = mapOrphanTransactions[orphanHash];
const CTransaction& orphanTx = mapOrphanTransactions[orphanHash].tx;
NodeId fromPeer = mapOrphanTransactions[orphanHash].fromPeer;
bool fMissingInputs2 = false;
// Use a dummy CValidationState so someone can't setup nodes to counter-DoS based on orphan
// resolution (that is, feeding people an invalid transaction based on LegitTxX in order to get
// anyone relaying LegitTxX banned)
CValidationState stateDummy;

vEraseQueue.push_back(orphanHash);

if (setMisbehaving.count(fromPeer))
continue;
if (AcceptToMemoryPool(mempool, stateDummy, orphanTx, true, &fMissingInputs2))
{
LogPrint("mempool", " accepted orphan tx %s\n", orphanHash.ToString());
RelayTransaction(orphanTx, orphanHash);
mapAlreadyAskedFor.erase(CInv(MSG_TX, orphanHash));
vWorkQueue.push_back(orphanHash);
vEraseQueue.push_back(orphanHash);
}
else if (!fMissingInputs2)
{
// invalid or too-little-fee orphan
vEraseQueue.push_back(orphanHash);
int nDos = 0;
if (stateDummy.IsInvalid(nDos) && nDos > 0)
{
// Punish peer that gave us an invalid orphan tx
Misbehaving(fromPeer, nDos);
setMisbehaving.insert(fromPeer);
LogPrint("mempool", " invalid orphan tx %s\n", orphanHash.ToString());
}
// too-little-fee orphan
LogPrint("mempool", " removed orphan tx %s\n", orphanHash.ToString());
}
mempool.check(pcoinsTip);
Expand All @@ -4118,7 +4154,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
}
else if (fMissingInputs)
{
AddOrphanTx(tx);
AddOrphanTx(tx, 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 Expand Up @@ -4766,5 +4802,6 @@ class CMainCleanup

// orphan transactions
mapOrphanTransactions.clear();
mapOrphanTransactionsByPrev.clear();
}
} instance_of_cmaincleanup;
19 changes: 14 additions & 5 deletions src/test/DoS_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
#include <boost/test/unit_test.hpp>

// Tests this internal-to-main.cpp method:
extern bool AddOrphanTx(const CTransaction& tx);
extern bool AddOrphanTx(const CTransaction& tx, NodeId peer);
extern void EraseOrphansFor(NodeId peer);
extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
extern std::map<uint256, CTransaction> mapOrphanTransactions;
extern std::map<uint256, std::set<uint256> > mapOrphanTransactionsByPrev;
Expand Down Expand Up @@ -174,7 +175,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
tx.vout[0].nValue = 1*CENT;
tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID());

AddOrphanTx(tx);
AddOrphanTx(tx, i);
}

// ... and 50 that depend on other orphans:
Expand All @@ -191,7 +192,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID());
SignSignature(keystore, txPrev, tx, 0);

AddOrphanTx(tx);
AddOrphanTx(tx, i);
}

// This really-big orphan should be ignored:
Expand All @@ -215,7 +216,15 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
for (unsigned int j = 1; j < tx.vin.size(); j++)
tx.vin[j].scriptSig = tx.vin[0].scriptSig;

BOOST_CHECK(!AddOrphanTx(tx));
BOOST_CHECK(!AddOrphanTx(tx, i));
}

// Test EraseOrphansFor:
for (NodeId i = 0; i < 3; i++)
{
size_t sizeBefore = mapOrphanTransactions.size();
EraseOrphansFor(i);
BOOST_CHECK(mapOrphanTransactions.size() < sizeBefore);
}

// Test LimitOrphanTxSize() function:
Expand Down Expand Up @@ -252,7 +261,7 @@ BOOST_AUTO_TEST_CASE(DoS_checkSig)
tx.vout[0].nValue = 1*CENT;
tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID());

AddOrphanTx(tx);
AddOrphanTx(tx, 0);
}

// Create a transaction that depends on orphans:
Expand Down

0 comments on commit e67aaca

Please sign in to comment.