From 81961762439fb72fc2ef168164689ddc29d7ef94 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sun, 26 Jul 2020 23:25:27 -0400 Subject: [PATCH 1/2] Rewrite parent txid loop of requested transactions Previously, we would potentially add the same txid many times to the rolling bloom filter of recently announced transactions to a peer, if many outputs of the same txid appeared as inputs in a transaction. Eliminate this problem and avoid redundant lookups by asking the mempool for the unique parents of a requested transaction. --- src/net_processing.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c503a97be5d..b52427d84ba 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1734,16 +1734,27 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); mempool.RemoveUnbroadcastTx(tx->GetHash()); // As we're going to send tx, make sure its unconfirmed parents are made requestable. - for (const auto& txin : tx->vin) { - auto txinfo = mempool.info(txin.prevout.hash); - if (txinfo.tx && txinfo.m_time > now - UNCONDITIONAL_RELAY_DELAY) { - // Relaying a transaction with a recent but unconfirmed parent. - if (WITH_LOCK(pfrom.m_tx_relay->cs_tx_inventory, return !pfrom.m_tx_relay->filterInventoryKnown.contains(txin.prevout.hash))) { - LOCK(cs_main); - State(pfrom.GetId())->m_recently_announced_invs.insert(txin.prevout.hash); + std::vector parent_ids_to_add; + { + LOCK(mempool.cs); + auto txiter = mempool.GetIter(tx->GetHash()); + if (txiter) { + const CTxMemPool::setEntries& parents = mempool.GetMemPoolParents(*txiter); + parent_ids_to_add.reserve(parents.size()); + for (CTxMemPool::txiter parent_iter : parents) { + if (parent_iter->GetTime() > now - UNCONDITIONAL_RELAY_DELAY) { + parent_ids_to_add.push_back(parent_iter->GetTx().GetHash()); + } } } } + for (const uint256& parent_txid : parent_ids_to_add) { + // Relaying a transaction with a recent but unconfirmed parent. + if (WITH_LOCK(pfrom.m_tx_relay->cs_tx_inventory, return !pfrom.m_tx_relay->filterInventoryKnown.contains(parent_txid))) { + LOCK(cs_main); + State(pfrom.GetId())->m_recently_announced_invs.insert(parent_txid); + } + } } else { vNotFound.push_back(inv); } From 4c0731f9c50b0556f8a57b912c8f295c7a9ea89c Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sun, 26 Jul 2020 23:46:36 -0400 Subject: [PATCH 2/2] Deduplicate missing parents of orphan transactions In the logic for requesting missing parents of orphan transactions, parent transactions with multiple outputs being spent by the given orphan were being processed multiple times. Fix this by deduplicating the set of missing parent txids first. Co-authored-by: Anthony Towns --- src/net_processing.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b52427d84ba..2e724ad9481 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2990,8 +2990,19 @@ void ProcessMessage( else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { bool fRejectedParents = false; // It may be the case that the orphans parents have all been rejected + + // Deduplicate parent txids, so that we don't have to loop over + // the same parent txid more than once down below. + std::vector unique_parents; + unique_parents.reserve(tx.vin.size()); for (const CTxIn& txin : tx.vin) { - if (recentRejects->contains(txin.prevout.hash)) { + // We start with all parents, and then remove duplicates below. + unique_parents.push_back(txin.prevout.hash); + } + std::sort(unique_parents.begin(), unique_parents.end()); + unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end()); + for (const uint256& parent_txid : unique_parents) { + if (recentRejects->contains(parent_txid)) { fRejectedParents = true; break; } @@ -3000,14 +3011,14 @@ void ProcessMessage( uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime(); - for (const CTxIn& txin : tx.vin) { + for (const uint256& parent_txid : unique_parents) { // Here, we only have the txid (and not wtxid) of the // inputs, so we only request in txid mode, even for // wtxidrelay peers. // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. - CInv _inv(MSG_TX | nFetchFlags, txin.prevout.hash); - pfrom.AddKnownTx(txin.prevout.hash); + CInv _inv(MSG_TX | nFetchFlags, parent_txid); + pfrom.AddKnownTx(parent_txid); if (!AlreadyHave(_inv, mempool)) RequestTx(State(pfrom.GetId()), ToGenTxid(_inv), current_time); } AddOrphanTx(ptx, pfrom.GetId());