diff --git a/src/net_processing.cpp b/src/net_processing.cpp index bdbf077ab5dce3..fbce965d63e2a1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -639,6 +639,9 @@ class PeerManagerImpl final : public PeerManager std::optional Find1P1CPackage(const CTransactionRef& ptx, NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); + std::optional Find1P1CPackageExtraTxn(const CTransactionRef& orphan_child, NodeId child_sender) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); + /** * Reconsider orphan transactions after a parent has been accepted to the mempool. * @@ -1893,6 +1896,24 @@ void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx) vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % m_opts.max_extra_txs; } +std::optional PeerManagerImpl::Find1P1CPackageExtraTxn(const CTransactionRef& orphan_child, + NodeId child_sender) +{ + // FIXME just look over orphanage too? + for (const auto& maybe_parent : vExtraTxnForCompact) { + // We stopped checking it earlier due to fee reasons, try again? + if (maybe_parent && m_recent_rejects_reconsiderable.contains(maybe_parent->GetWitnessHash().ToUint256())) { + Package maybe_cpfp_package{maybe_parent, orphan_child}; + if (IsChildWithParents(maybe_cpfp_package) && !m_recent_rejects_reconsiderable.contains(GetPackageHash(maybe_cpfp_package))) { + // FIXME store extra_txn sender to properly punish since we don't have PoW + return PeerManagerImpl::PackageToValidate{maybe_parent, orphan_child, /*parent_sender=*/-1, child_sender}; + + } + } + } + return std::nullopt; +} + void PeerManagerImpl::Misbehaving(Peer& peer, int howmuch, const std::string& message) { assert(howmuch > 0); @@ -4585,7 +4606,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { - bool fRejectedParents = false; // It may be the case that the orphans parents have all been rejected + bool fRejectedParents = false; // It may be the case that the an orphan parent has been rejected // Deduplicate parent txids, so that we don't have to loop over // the same parent txid more than once down below. @@ -4632,6 +4653,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) AddTxAnnouncement(pfrom, gtxid, current_time); } + // Maybe we have its parent in extra_txn? + if (auto package_to_validate{Find1P1CPackageExtraTxn(/*maybe_parent=*/ptx, pfrom.GetId())}) { + const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)}; + LogDebug(BCLog::TXPACKAGES, "package evaluation for %s from extra_txns: %s\n", package_to_validate->ToString(), + package_result.m_state.IsValid() ? "package accepted" : "package rejected"); + ProcessPackageResult(package_to_validate->m_txns, package_result, package_to_validate->m_senders); + + // Ended up succeeding + if (package_result.m_state.IsValid()) return; + } + if (m_orphanage.AddTx(ptx, pfrom.GetId())) { AddToCompactExtraTransactions(ptx); } diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py index e07acd548136ff..80f49f93d3e4e6 100755 --- a/test/functional/p2p_opportunistic_1p1c.py +++ b/test/functional/p2p_opportunistic_1p1c.py @@ -56,10 +56,17 @@ class PackageRelayTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - self.extra_args = [[ - "-datacarriersize=100000", - "-maxmempool=5", - ]] + if self.options.noextratxn: + self.extra_args = [[ + "-datacarriersize=100000", + "-maxmempool=5", + "-blockreconstructionextratxn=0", # require fetching parent even if cached + ]] + else: + self.extra_args = [[ + "-datacarriersize=100000", + "-maxmempool=5", + ]] self.supports_cli = False def create_tx_below_mempoolminfee(self, wallet): @@ -127,6 +134,13 @@ def test_basic_parent_then_child(self, wallet): peer_sender.wait_for_getdata([high_child_wtxid_int]) peer_sender.send_and_ping(msg_tx(high_fee_child["tx"])) + if not self.options.noextratxn: + # 3. It's in extra transactions. Done! + node_mempool = node.getrawmempool() + assert low_fee_parent["txid"] in node_mempool + assert high_fee_child["txid"] in node_mempool + return + # 3. Node requests the missing parent by txid. # It should do so even if it has previously rejected that parent for being too low feerate. parent_txid_int = int(low_fee_parent["txid"], 16) @@ -171,12 +185,14 @@ def test_low_and_high_child(self, wallet): peer_sender.wait_for_getdata([med_child_wtxid_int]) peer_sender.send_and_ping(msg_tx(med_fee_child["tx"])) - # 3. Node requests the orphan's missing parent. - parent_txid_int = int(low_fee_parent["txid"], 16) - peer_sender.wait_for_getdata([parent_txid_int]) + # Extra transactions will allow it to go ahead and validate the package + if self.options.noextratxn: + # 3. Node requests the orphan's missing parent. + parent_txid_int = int(low_fee_parent["txid"], 16) + peer_sender.wait_for_getdata([parent_txid_int]) - # 4. The low parent + low child are submitted as a package. They are not accepted due to low package feerate. - peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"])) + # 4. The low parent + low child are submitted as a package. They are not accepted due to low package feerate. + peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"])) assert low_fee_parent["txid"] not in node.getrawmempool() assert med_fee_child["txid"] not in node.getrawmempool() @@ -197,13 +213,15 @@ def test_low_and_high_child(self, wallet): peer_sender.wait_for_getdata([high_child_wtxid_int]) peer_sender.send_and_ping(msg_tx(high_fee_child["tx"])) - # 6. Node requests the orphan's parent, even though it has already been rejected, both by - # itself and with a child. This is necessary, otherwise high_fee_child can be censored. - parent_txid_int = int(low_fee_parent["txid"], 16) - peer_sender.wait_for_getdata([parent_txid_int]) + # Extra transactions will allow it to go ahead and validate the package + if self.options.noextratxn: + # 6. Node requests the orphan's parent, even though it has already been rejected, both by + # itself and with a child. This is necessary, otherwise high_fee_child can be censored. + parent_txid_int = int(low_fee_parent["txid"], 16) + peer_sender.wait_for_getdata([parent_txid_int]) - # 7. The low feerate parent + high feerate child are submitted as a package. - peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"])) + # 7. The low feerate parent + high feerate child are submitted as a package. + peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"])) # 8. Both transactions should now be in mempool node_mempool = node.getrawmempool() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index a2f767cc9898c0..7f6de5ac95d989 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -194,6 +194,8 @@ def parse_args(self): help="use BIP324 v2 connections between all nodes by default") parser.add_argument("--v1transport", dest="v1transport", default=False, action="store_true", help="Explicitly use v1 transport (can be used to overwrite global --v2transport option)") + parser.add_argument("--noextratxn", dest="noextratxn", default=False, action="store_true", + help="No extra transactions for compact blocks") self.add_options(parser) # Running TestShell in a Jupyter notebook causes an additional -f argument