From 008537dfaf6467a6a7614a2515bc139f758b972b Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 20 Aug 2024 16:06:49 +0100 Subject: [PATCH] wip: protect orphans with 1 missing parent provided by outbound peer untested --- src/node/txdownload_impl.cpp | 82 +++++++++++++++++++++++++++++++++--- src/node/txdownload_impl.h | 37 +++++++++++++++- src/node/txdownloadman.h | 3 ++ 3 files changed, 114 insertions(+), 8 deletions(-) diff --git a/src/node/txdownload_impl.cpp b/src/node/txdownload_impl.cpp index ad4108841d287a..72aeb0ff584d2c 100644 --- a/src/node/txdownload_impl.cpp +++ b/src/node/txdownload_impl.cpp @@ -24,13 +24,6 @@ void TxDownloadImpl::ActiveTipChange() void TxDownloadImpl::BlockConnected(const std::shared_ptr& pblock) { - // Orphanage may include transactions conflicted by this block. There should not be any - // transactions in m_orphan_resolution_tracker that aren't in orphanage, so this should include - // all of the relevant orphans we were working on. - for (const auto& erased_orphan_wtxid : m_orphanage.EraseForBlock(*pblock)) { - m_orphan_resolution_tracker.ForgetTxHash(erased_orphan_wtxid); - } - for (const auto& ptx : pblock->vtx) { RecentConfirmedTransactionsFilter().insert(ptx->GetHash().ToUint256()); if (ptx->HasWitness()) { @@ -38,6 +31,27 @@ void TxDownloadImpl::BlockConnected(const std::shared_ptr& pblock) } m_txrequest.ForgetTxHash(ptx->GetHash()); m_txrequest.ForgetTxHash(ptx->GetWitnessHash()); + + // Refund tokens used to protect orphans that have confirmed in this block, because they + // were used to protect a useful orphan even though we didn't download the tx in time. + // Calling RefundOrphanProtectors before EraseForBlock is important for 2 reasons: + // - the orphanage can return the list of peers who protected the orphan + // - this loop only includes the transactions that confirmed in this block, and not the + // orphans conflicted by the block. + RefundOrphanProtectors(ptx->GetWitnessHash()); + } + + // Orphanage may include transactions conflicted by this block. There should not be any + // transactions in m_orphan_resolution_tracker that aren't in orphanage, so this should include + // all of the relevant orphans we were working on. + for (const auto& erased_orphan_wtxid : m_orphanage.EraseForBlock(*pblock)) { + m_orphan_resolution_tracker.ForgetTxHash(erased_orphan_wtxid); + } + + // Once per BlockConnected, restore peers' orphan protection tokens. + for (auto& peer_it : m_peer_info) { + // noop for peers that don't have the protection ability. + peer_it.second.RefundTokens(MAX_ORPHAN_PROTECTED_BYTES / 2); } } @@ -195,6 +209,7 @@ std::vector TxDownloadImpl::GetRequestsToSend(NodeId nodeid, std::chron // All txhashes in m_orphan_resolution_tracker are wtxids. Assume(orphan_gtxid.IsWtxid()); const auto wtxid = Wtxid::FromUint256(orphan_gtxid.GetHash()); + // Remove peer as announcer and protector m_orphanage.EraseOrphanOfPeer(wtxid, nodeid); Assume(!m_orphanage.HaveTxAndPeer(wtxid, nodeid)); } @@ -226,6 +241,9 @@ std::vector TxDownloadImpl::GetRequestsToSend(NodeId nodeid, std::chron if (requesting) m_orphan_resolution_tracker.RequestedTx(nodeid, orphan_gtxid.GetHash(), current_time + ORPHAN_ANCESTOR_GETDATA_INTERVAL); } else { LogPrint(BCLog::TXPACKAGES, "couldn't find parent txids to resolve orphan %s with peer=%d\n", orphan_gtxid.GetHash().ToString(), nodeid); + // Undo protection if it exists. Don't restore tokens, as this orphan was not + // successfully resolved. + MaybeUndoProtectOrphan(nodeid, Wtxid::FromUint256(orphan_gtxid.GetHash())); m_orphan_resolution_tracker.ForgetTxHash(orphan_gtxid.GetHash()); } } @@ -296,6 +314,7 @@ void TxDownloadImpl::MempoolAcceptedTx(const CTransactionRef& tx) m_txrequest.ForgetTxHash(tx->GetHash()); m_txrequest.ForgetTxHash(tx->GetWitnessHash()); + RefundOrphanProtectors(tx->GetWitnessHash()); m_orphanage.AddChildrenToWorkSet(*tx); // If it came from the orphanage, remove it. No-op if the tx is not in txorphanage. m_orphanage.EraseTx(tx->GetWitnessHash()); @@ -361,6 +380,16 @@ node::RejectedTxTodo TxDownloadImpl::MempoolRejectedTx(const CTransactionRef& pt const auto& info = m_peer_info.at(nodeid).m_connection_info; m_orphanage.AddTx(orphan_tx, nodeid, unique_parents); m_orphan_resolution_tracker.ReceivedInv(nodeid, GenTxid::Wtxid(wtxid), info.m_preferred, now + *delay); + + // If this could be a 1p1c package, attempt to protect this orphan from eviction + // (only peers preferred for download have the protection ability). We give + // orphans with exactly 1 parent in m_lazy_recent_rejects_reconsiderable special + // treatment because orphan resolution is potentially the only way we will + // accept these transactions (the parent will not be accepted by itself). + if (rejected_parent_reconsiderable.has_value()) { + MaybeProtectOrphan(nodeid, wtxid); + } + LogDebug(BCLog::TXPACKAGES, "added peer=%d as a candidate for resolving orphan %s\n", nodeid, wtxid.ToString()); } }; @@ -453,6 +482,7 @@ node::RejectedTxTodo TxDownloadImpl::MempoolRejectedTx(const CTransactionRef& pt // If the tx failed in ProcessOrphanTx, it should be removed from the orphanage unless the // tx was still missing inputs. If the tx was not in the orphanage, EraseTx does nothing and returns 0. if (state.GetResult() != TxValidationResult::TX_MISSING_INPUTS && m_orphanage.EraseTx(ptx->GetWitnessHash()) > 0) { + // fixme: undoprotect, don't return tokens m_orphan_resolution_tracker.ForgetTxHash(ptx->GetWitnessHash()); LogDebug(BCLog::TXPACKAGES, " removed orphan tx %s (wtxid=%s)\n", ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()); } @@ -544,4 +574,42 @@ void TxDownloadImpl::CheckIsEmpty() assert(m_txrequest.Size() == 0); Assume(m_orphan_resolution_tracker.Size() == 0); } + +void TxDownloadImpl::MaybeProtectOrphan(NodeId nodeid, const Wtxid& wtxid) +{ + auto peer_info_it = m_peer_info.find(nodeid); + if (peer_info_it == m_peer_info.end()) return; + + // Cannot protect orphans or is already protecting this orphan + if (!peer_info_it->second.CanProtectOrphans()) return; + + const auto max_orphan_size{peer_info_it->second.AvailableProtectionTokens()}; + if (auto orphan_size{m_orphanage.ProtectOrphan(wtxid, nodeid, max_orphan_size)}) { + peer_info_it->second.m_protection_tokens -= orphan_size.value(); + } +} + +void TxDownloadImpl::MaybeUndoProtectOrphan(NodeId nodeid, const Wtxid& wtxid) +{ + auto peer_info_it = m_peer_info.find(nodeid); + if (peer_info_it == m_peer_info.end()) return; + + // Cannot protect orphans or is not protecting this orphan + if (!peer_info_it->second.CanProtectOrphans()) return; + + m_orphanage.UndoProtectOrphan(wtxid, nodeid); + + // Peer's orphan protection tokens are not refunded. +} + +void TxDownloadImpl::RefundOrphanProtectors(const Wtxid& wtxid) +{ + if (auto protection_info{m_orphanage.GetProtectors(wtxid)}) { + for (const auto& protector_peer : protection_info->second) { + auto peer_info_it = m_peer_info.find(protector_peer); + if (!Assume(peer_info_it != m_peer_info.end())) return; + peer_info_it->second.RefundTokens(protection_info->first); + } + } +} } // namespace node diff --git a/src/node/txdownload_impl.h b/src/node/txdownload_impl.h index 6dbe33de15305a..1e15bb13810a06 100644 --- a/src/node/txdownload_impl.h +++ b/src/node/txdownload_impl.h @@ -140,11 +140,36 @@ class TxDownloadImpl { /** Information relevant to scheduling tx requests. */ const TxDownloadConnectionInfo m_connection_info; + /** Current amount of orphan transactions that this peer can protect. 1 token = 1 byte of + * orphan data (data usage, not Wu or vB). */ + unsigned int m_protection_tokens; + unsigned int MaxOrphanBytes() const { return m_connection_info.m_preferred ? MAX_ORPHAN_BYTES_PREFERRED : MAX_ORPHAN_BYTES_NONPREFERRED; } - PeerInfo(const TxDownloadConnectionInfo& info) : m_connection_info{info} {} + bool CanProtectOrphans() const { + return m_connection_info.m_preferred; + } + + unsigned int MaxProtectionTokens() const { + return CanProtectOrphans() ? MAX_ORPHAN_PROTECTED_BYTES : 0; + } + + unsigned int AvailableProtectionTokens() const { + return MaxProtectionTokens() - m_protection_tokens; + } + + void RefundTokens(unsigned int new_tokens) { + // Token refund should never result in having more than the maximum number of allowed tokens. + // This can happen if a refund happens right after a scheduled top up. + m_protection_tokens = std::min(MaxProtectionTokens(), m_protection_tokens + new_tokens); + } + + PeerInfo(const TxDownloadConnectionInfo& info) : + m_connection_info{info}, + m_protection_tokens{info.m_preferred ? MAX_ORPHAN_PROTECTED_BYTES : 0} + {} }; /** Information for all of the peers we may download transactions from. This is not necessarily @@ -203,6 +228,16 @@ class TxDownloadImpl { * */ std::optional OrphanResolutionCandidate(NodeId nodeid, const Wtxid& orphan_wtxid); + /** Try to protect an orphan with this wtxid */ + void MaybeProtectOrphan(NodeId nodeid, const Wtxid& wtxid); + + /** Get the peers who have protected this transaction in the orphanage and refund their + * protection tokens. */ + void RefundOrphanProtectors(const Wtxid& wtxid); + + /** Try to remove protection by this nodeid for an orphan with this wtxid. The node's protection + * tokens are not refunded. */ + void MaybeUndoProtectOrphan(NodeId nodeid, const Wtxid& wtxid); }; } // namespace node #endif // BITCOIN_NODE_TXDOWNLOAD_IMPL_H diff --git a/src/node/txdownloadman.h b/src/node/txdownloadman.h index 11d987d5fe1fb3..41325883edde04 100644 --- a/src/node/txdownloadman.h +++ b/src/node/txdownloadman.h @@ -42,6 +42,9 @@ static constexpr unsigned int MAX_ORPHAN_BYTES_PREFERRED{4'000'000}; * balance orphan resolutions. Equivalent to 1 maximum size orphan, so all non-preferred peers are * always allowed at least 1 orphan. */ static constexpr unsigned int MAX_ORPHAN_BYTES_NONPREFERRED{400'000}; +/** Maximum number of orphan bytes that can be protected for a peer. Only given to peers who are + * preferred for download. */ +static constexpr unsigned int MAX_ORPHAN_PROTECTED_BYTES{400'000}; /** How long to delay requesting transactions via txids, if we have wtxid-relaying peers */ static constexpr auto TXID_RELAY_DELAY{2s}; /** How long to delay requesting transactions from non-preferred peers */