diff options
author | Ava Chow <github@achow101.com> | 2024-04-30 18:32:11 -0400 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-04-30 18:40:53 -0400 |
commit | d813ba1bc4b4da3ad1f3812b61ff125d1d664625 (patch) | |
tree | a770b5f817d50357e67755a75048b6d09484066b /src/test/fuzz/txorphan.cpp | |
parent | 2d3056751bb7d742a802a30503f07dbeb07310ee (diff) | |
parent | e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f (diff) | |
download | bitcoin-d813ba1bc4b4da3ad1f3812b61ff125d1d664625.tar.xz |
Merge bitcoin/bitcoin#28970: p2p: opportunistically accept 1-parent-1-child packages
e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f [functional test] opportunistic 1p1c package submission (glozow)
87c5c524d63c833cf490c7f2f73d72695ad480df [p2p] opportunistically accept 1-parent-1-child packages (glozow)
6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 [p2p] add separate rejections cache for reconsiderable txns (glozow)
410ebd6efaf20fe4715c9b825103b74db69f35ac [fuzz] break out parent functions and add GetChildrenFrom* coverage (glozow)
d095316c1c23e9460dfbd9fdbaf292063adcd080 [unit test] TxOrphanage::GetChildrenFrom* (glozow)
2f51cd680fb4323f1c792dae37d4c4e0e0e35804 [txorphanage] add method to get all orphans spending a tx (glozow)
092c978a42e8f4a02291b994713505ba8aac8b28 [txpackages] add canonical way to get hash of package (glozow)
c3c1e15831c463df7968b028a77e787da7e6256d [doc] restore comment about why we check if ptx HasWitness before caching rejected txid (glozow)
6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf guard against MempoolAcceptResult::m_replaced_transactions (glozow)
Pull request description:
This enables 1p1c packages to propagate in the "happy case" (i.e. not reliable if there are adversaries) and contains a lot of package relay-related code. See https://github.com/bitcoin/bitcoin/issues/27463 for overall package relay tracking.
Rationale: This is "non-robust 1-parent-1-child package relay" which is immediately useful.
- Relaying 1-parent-1-child CPFP when mempool min feerate is high would be a subset of all package relay use cases, but a pretty significant improvement over what we have today, where such transactions don't propagate at all. [1]
- Today, a miner can run this with a normal/small maxmempool to get revenue from 1p1c CPFP'd transactions without losing out on the ones with parents below mempool minimum feerate.
- The majority of this code is useful for building more featureful/robust package relay e.g. see the code in #27742.
The first 2 commits are followups from #29619:
- https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1523094034
- https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1519819257
Q: What makes this short of a more full package relay feature?
(1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463.
(2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer.
(3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031.
[1]: see this writeup and its links https://github.com/bitcoin/bips/blob/02ec218c7857ef60914e9a3d383b68caf987f70b/bip-0331.mediawiki#propagate-high-feerate-transactions
ACKs for top commit:
sr-gi:
tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
instagibbs:
reACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
theStack:
Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package:
dergoegge:
light Code review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
achow101:
ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
Tree-SHA512: 632579fbe7160cb763bbec6d82ca0dab484d5dbbc7aea90c187c0b9833b8d7c1e5d13b8587379edd3a3b4a02a5a1809020369e9cd09a4ebaf729921f65c15943
Diffstat (limited to 'src/test/fuzz/txorphan.cpp')
-rw-r--r-- | src/test/fuzz/txorphan.cpp | 32 |
1 files changed, 29 insertions, 3 deletions
diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index 5423ba8920..a44f47b00d 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -45,6 +45,8 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) // if true, allow duplicate input when constructing tx const bool duplicate_input = fuzzed_data_provider.ConsumeBool(); + CTransactionRef ptx_potential_parent = nullptr; + LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS) { // construct transaction @@ -78,6 +80,27 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) return new_tx; }(); + // Trigger orphanage functions that are called using parents. ptx_potential_parent is a tx we constructed in a + // previous loop and potentially the parent of this tx. + if (ptx_potential_parent) { + // Set up future GetTxToReconsider call. + orphanage.AddChildrenToWorkSet(*ptx_potential_parent); + + // Check that all txns returned from GetChildrenFrom* are indeed a direct child of this tx. + NodeId peer_id = fuzzed_data_provider.ConsumeIntegral<NodeId>(); + for (const auto& child : orphanage.GetChildrenFromSamePeer(ptx_potential_parent, peer_id)) { + assert(std::any_of(child->vin.cbegin(), child->vin.cend(), [&](const auto& input) { + return input.prevout.hash == ptx_potential_parent->GetHash(); + })); + } + for (const auto& [child, peer] : orphanage.GetChildrenFromDifferentPeer(ptx_potential_parent, peer_id)) { + assert(std::any_of(child->vin.cbegin(), child->vin.cend(), [&](const auto& input) { + return input.prevout.hash == ptx_potential_parent->GetHash(); + })); + assert(peer != peer_id); + } + } + // trigger orphanage functions LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS) { @@ -86,9 +109,6 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) CallOneOf( fuzzed_data_provider, [&] { - orphanage.AddChildrenToWorkSet(*tx); - }, - [&] { { CTransactionRef ref = orphanage.GetTxToReconsider(peer_id); if (ref) { @@ -136,6 +156,12 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) orphanage.LimitOrphans(limit, limit_orphans_rng); Assert(orphanage.Size() <= limit); }); + + } + // Set tx as potential parent to be used for future GetChildren() calls. + if (!ptx_potential_parent || fuzzed_data_provider.ConsumeBool()) { + ptx_potential_parent = tx; } + } } |