p2p: Allow 1P1C to work with compact block extra_txn

src/net_processing.cpp

@@ -639,6 +639,9 @@ class PeerManagerImpl final : public PeerManager
     std::optional<PackageToValidate> Find1P1CPackage(const CTransactionRef& ptx, NodeId nodeid)
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main);
 
+    std::optional<PackageToValidate> 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::PackageToValidate> 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);
                 }

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()

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

test/functional/test_runner.py

@@ -185,6 +185,7 @@
     'rpc_misc.py',
     'p2p_1p1c_network.py',
     'p2p_opportunistic_1p1c.py',
+    'p2p_opportunistic_1p1c.py --noextratxn',
     'interface_rest.py',
     'mempool_spend_coinbase.py',
     'wallet_avoid_mixing_output_types.py --descriptors',