diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1753,44 +1753,6 @@ vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % max_extra_txn; } -bool AddOrphanTx(const CTransactionRef &tx, NodeId peer) - EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) { - const TxId &txid = tx->GetId(); - if (mapOrphanTransactions.count(txid)) { - return false; - } - - // Ignore big transactions, to avoid a send-big-orphans memory exhaustion - // attack. If a peer has a legitimate large transaction with a missing - // parent then we assume it will rebroadcast it later, after the parent - // transaction(s) have been mined or received. - // 100 orphans, each of which is at most 100,000 bytes big is at most 10 - // megabytes of orphans and somewhat more byprev index (in the worst case): - unsigned int sz = tx->GetTotalSize(); - if (sz > MAX_STANDARD_TX_SIZE) { - LogPrint(BCLog::MEMPOOL, - "ignoring large orphan tx (size: %u, hash: %s)\n", sz, - txid.ToString()); - return false; - } - - auto ret = mapOrphanTransactions.emplace( - txid, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, - g_orphan_list.size()}); - assert(ret.second); - g_orphan_list.push_back(ret.first); - for (const CTxIn &txin : tx->vin) { - mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); - } - - AddToCompactExtraTransactions(tx); - - LogPrint(BCLog::MEMPOOL, "stored orphan tx %s (mapsz %u outsz %u)\n", - txid.ToString(), mapOrphanTransactions.size(), - mapOrphanTransactionsByPrev.size()); - return true; -} - void PeerManagerImpl::Misbehaving(const NodeId pnode, const int howmuch, const std::string &message) { assert(howmuch > 0); @@ -4349,7 +4311,10 @@ AddTxAnnouncement(pfrom, parent_txid, current_time); } } - AddOrphanTx(ptx, pfrom.GetId()); + + if (OrphanageAddTx(ptx, pfrom.GetId())) { + AddToCompactExtraTransactions(ptx); + } // Once added to the orphan pool, a tx is considered // AlreadyHave, and we shouldn't request it anymore. diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -43,9 +43,6 @@ }; } // namespace -// Tests these internal-to-net_processing.cpp methods: -extern bool AddOrphanTx(const CTransactionRef &tx, NodeId peer); - static CService ip(uint32_t i) { struct in_addr s; s.s_addr = i; @@ -337,9 +334,8 @@ peerLogic->FinalizeNode(config, dummyNode, dummy); } -static CTransactionRef RandomOrphan() { +static CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) { std::map::iterator it; - LOCK2(cs_main, g_cs_orphans); it = mapOrphanTransactions.lower_bound(TxId{InsecureRand256()}); if (it == mapOrphanTransactions.end()) { it = mapOrphanTransactions.begin(); @@ -353,6 +349,8 @@ FillableSigningProvider keystore; BOOST_CHECK(keystore.AddKey(key)); + LOCK(g_cs_orphans); + // 50 orphan transactions: for (int i = 0; i < 50; i++) { CMutableTransaction tx; @@ -364,7 +362,7 @@ tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey())); - AddOrphanTx(MakeTransactionRef(tx), i); + OrphanageAddTx(MakeTransactionRef(tx), i); } // ... and 50 that depend on other orphans: @@ -381,7 +379,7 @@ BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SigHashType().withForkId())); - AddOrphanTx(MakeTransactionRef(tx), i); + OrphanageAddTx(MakeTransactionRef(tx), i); } // This really-big orphan should be ignored: @@ -405,10 +403,9 @@ tx.vin[j].scriptSig = tx.vin[0].scriptSig; } - BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i)); + BOOST_CHECK(!OrphanageAddTx(MakeTransactionRef(tx), i)); } - LOCK2(cs_main, g_cs_orphans); // Test EraseOrphansFor: for (NodeId i = 0; i < 3; i++) { size_t sizeBefore = mapOrphanTransactions.size(); diff --git a/src/txorphanage.h b/src/txorphanage.h --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -9,9 +9,6 @@ #include #include -/** Expiration time for orphan transactions in seconds */ -static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; - /** Guards orphan transactions and extra txs for compact blocks */ extern RecursiveMutex g_cs_orphans; @@ -33,6 +30,8 @@ bool HaveOrphanTx(const TxId &txid) LOCKS_EXCLUDED(g_cs_orphans); std::pair GetOrphanTx(const TxId &txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); +bool OrphanageAddTx(const CTransactionRef &tx, NodeId peer) + EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); /** * Map from txid to orphan transaction record. Limited by diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -10,6 +10,8 @@ #include +/** Expiration time for orphan transactions in seconds */ +static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; @@ -24,6 +26,43 @@ std::vector::iterator> g_orphan_list GUARDED_BY(g_cs_orphans); +bool OrphanageAddTx(const CTransactionRef &tx, NodeId peer) { + AssertLockHeld(g_cs_orphans); + + const TxId &txid = tx->GetId(); + if (mapOrphanTransactions.count(txid)) { + return false; + } + + // Ignore big transactions, to avoid a send-big-orphans memory exhaustion + // attack. If a peer has a legitimate large transaction with a missing + // parent then we assume it will rebroadcast it later, after the parent + // transaction(s) have been mined or received. + // 100 orphans, each of which is at most 100,000 bytes big is at most 10 + // megabytes of orphans and somewhat more byprev index (in the worst case): + unsigned int sz = tx->GetTotalSize(); + if (sz > MAX_STANDARD_TX_SIZE) { + LogPrint(BCLog::MEMPOOL, + "ignoring large orphan tx (size: %u, hash: %s)\n", sz, + txid.ToString()); + return false; + } + + auto ret = mapOrphanTransactions.emplace( + txid, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, + g_orphan_list.size()}); + assert(ret.second); + g_orphan_list.push_back(ret.first); + for (const CTxIn &txin : tx->vin) { + mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); + } + + LogPrint(BCLog::MEMPOOL, "stored orphan tx %s (mapsz %u outsz %u)\n", + txid.ToString(), mapOrphanTransactions.size(), + mapOrphanTransactionsByPrev.size()); + return true; +} + int EraseOrphanTx(const TxId &txid) { std::map::iterator it = mapOrphanTransactions.find(txid); if (it == mapOrphanTransactions.end()) {