Repository: bitcoin/bitcoin (language: cpp)

Issue:
Why does `submitpackage` require at least two transactions
### Please describe the feature you'd like to see added.

I want to call `submitpackage` and I don't want to have to check if I have more than one transaction to do it. I'm lazy but that restriction seems arbitrary.

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No response_


Discussion / hints:
It won't hurt to broadcast a single transaction if it passes consensus and policy checks via `submitpackage` RPC.

From the [package relay BIP](https://github.com/bitcoin/bips/blob/632f143fadaed8c91bba12d096e62feb2a308054/bip-0331.mediawiki#definitions) and the [policy documentation](https://github.com/bitcoin/bitcoin/blob/master/doc/policy/packages.md), I see that a package is defined as a child transaction with its unconfirmed ancestors, topologically sorted.

I think the intent of the check is to be strict and ensure that what's being broadcast is a package.
As such RPC behavior is correct, as an individual transaction doesn't qualify as a package based on this definition.
From a Bitcoin Core perspective, sure. From the users' perspective I have a bundle of transactions and I want to get them relayed. They may be a package, they may not, I want Bitcoin Core to figure that out, not me.
> I want to call `submitpackage` and I don't want to have to check if I have more than one transaction to do it. I'm lazy but that restriction seems arbitrary.

I had similar thoughts when reviewing `submitpackage` a while ago. Forcing the user to use different RPCs based on the number of transactions in a package feels arbitrary and not ergonomic to me. A package with size 1 should be a valid package and accepted as such in the relevant RPCs (slowly obsoleting their single-tx alternatives over time, but that's a different discussion).

> I see that a package is defined as a child transaction with its unconfirmed ancestors, topologically sorted.

A tx without unconfirmed ancestors would make for a valid package according to that definition, though.

Concept ACK.
I think it's reasonable to support. I'll take a look.
Opened #31096 

Fix patch:
diff --git a/doc/policy/packages.md b/doc/policy/packages.md
index b432008d7be25..cd705ca199521 100644
--- a/doc/policy/packages.md
+++ b/doc/policy/packages.md
@@ -76,7 +76,7 @@ The following rules are only enforced for packages to be submitted to the mempoo
 enforced for test accepts):
 
 * Packages must be child-with-unconfirmed-parents packages. This also means packages must contain at
-  least 2 transactions. (#22674)
+  least 1 transaction. (#31096)
 
    - *Rationale*: This allows for fee-bumping by CPFP. Allowing multiple parents makes it possible
      to fee-bump a batch of transactions. Restricting packages to a defined topology is easier to
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index b26d94b082580..0a67348461d19 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -926,7 +926,7 @@ static RPCHelpMan submitpackage()
         ,
         {
             {"package", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of raw transactions.\n"
-                "The package must solely consist of a child and its parents. None of the parents may depend on each other.\n"
+                "The package must solely consist of a child transaction and all of its unconfirmed parents, if any. None of the parents may depend on each other.\n"
                 "The package must be topologically sorted, with the child being the last element in the array.",
                 {
                     {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},
@@ -968,15 +968,15 @@ static RPCHelpMan submitpackage()
             },
         },
         RPCExamples{
-            HelpExampleRpc("submitpackage", R"(["rawtx1", "rawtx2"])") +
-            HelpExampleCli("submitpackage", R"('["rawtx1", "rawtx2"]')")
+            HelpExampleRpc("submitpackage", R"(["raw-parent-tx-1", "raw-parent-tx-2", "raw-child-tx"])") +
+            HelpExampleCli("submitpackage", R"('["raw-tx-without-unconfirmed-parents"]')")
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
             const UniValue raw_transactions = request.params[0].get_array();
-            if (raw_transactions.size() < 2 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
+            if (raw_transactions.empty() || raw_transactions.size() > MAX_PACKAGE_COUNT) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER,
-                                   "Array must contain between 2 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
+                                   "Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
             }
 
             // Fee check needs to be run with chainstate and package context
@@ -1007,7 +1007,8 @@ static RPCHelpMan submitpackage()
 
                 txns.emplace_back(MakeTransactionRef(std::move(mtx)));
             }
-            if (!IsChildWithParentsTree(txns)) {
+            CHECK_NONFATAL(!txns.empty());
+            if (txns.size() > 1 && !IsChildWithParentsTree(txns)) {
                 throw JSONRPCTransactionError(TransactionError::INVALID_PACKAGE, "package topology disallowed. not child-with-parents or parents depend on each other.");
             }
 
diff --git a/src/validation.cpp b/src/validation.cpp
index f9288fcc0d7c0..5727792cc02aa 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1672,10 +1672,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptSubPackage(const std::vector<CTr
 
 PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, ATMPArgs& args)
 {
+    Assert(!package.empty());
     AssertLockHeld(cs_main);
     // Used if returning a PackageMempoolAcceptResult directly from this function.
     PackageValidationState package_state_quit_early;
 
+    // There are two topologies we are able to handle through this function:
+    // (1) A single transaction
+    // (2) A child-with-unconfirmed-parents package.
     // Check that the package is well-formed. If it isn't, we won't try to validate any of the
     // transactions and thus won't return any MempoolAcceptResults, just a package-wide error.
 
@@ -1684,48 +1688,50 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
         return PackageMempoolAcceptResult(package_state_quit_early, {});
     }
 
-    // All transactions in the package must be a parent of the last transaction. This is just an
-    // opportunity for us to fail fast on a context-free check without taking the mempool lock.
-    if (!IsChildWithParents(package)) {
-        package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
-        return PackageMempoolAcceptResult(package_state_quit_early, {});
-    }
-
-    // IsChildWithParents() guarantees the package is > 1 transactions.
-    assert(package.size() > 1);
-    // The package must be 1 child with all of its unconfirmed parents. The package is expected to
-    // be sorted, so the last transaction is the child.
-    const auto& child = package.back();
-    std::unordered_set<uint256, SaltedTxidHasher> unconfirmed_parent_txids;
-    std::transform(package.cbegin(), package.cend() - 1,
-                   std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()),
-                   [](const auto& tx) { return tx->GetHash(); });
-
-    // All child inputs must refer to a preceding package transaction or a confirmed UTXO. The only
-    // way to verify this is to look up the child's inputs in our current coins view (not including
-    // mempool), and enforce that all parents not present in the package be available at chain tip.
-    // Since this check can bring new coins into the coins cache, keep track of these coins and
-    // uncache them if we don't end up submitting this package to the mempool.
-    const CCoinsViewCache& coins_tip_cache = m_active_chainstate.CoinsTip();
-    for (const auto& input : child->vin) {
-        if (!coins_tip_cache.HaveCoinInCache(input.prevout)) {
-            args.m_coins_to_uncache.push_back(input.prevout);
-        }
-    }
-    // Using the MemPoolAccept m_view cache allows us to look up these same coins faster later.
-    // This should be connecting directly to CoinsTip, not to m_viewmempool, because we specifically
-    // require inputs to be confirmed if they aren't in the package.
-    m_view.SetBackend(m_active_chainstate.CoinsTip());
-    const auto package_or_confirmed = [this, &unconfirmed_parent_txids](const auto& input) {
-         return unconfirmed_parent_txids.count(input.prevout.hash) > 0 || m_view.HaveCoin(input.prevout);
-    };
-    if (!std::all_of(child->vin.cbegin(), child->vin.cend(), package_or_confirmed)) {
-        package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-unconfirmed-parents");
-        return PackageMempoolAcceptResult(package_state_quit_early, {});
+    if (package.size() > 1) {
+        // All transactions in the package must be a parent of the last transaction. This is just an
+        // opportunity for us to fail fast on a context-free check without taking the mempool lock.
+        if (!IsChildWithParents(package)) {
+            package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
+            return PackageMempoolAcceptResult(package_state_quit_early, {});
+        }
+
+        // IsChildWithParents() guarantees the package is > 1 transactions.
+        assert(package.size() > 1);
+        // The package must be 1 child with all of its unconfirmed parents. The package is expected to
+        // be sorted, so the last transaction is the child.
+        const auto& child = package.back();
+        std::unordered_set<uint256, SaltedTxidHasher> unconfirmed_parent_txids;
+        std::transform(package.cbegin(), package.cend() - 1,
+                       std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()),
+                       [](const auto& tx) { return tx->GetHash(); });
+
+        // All child inputs must refer to a preceding package transaction or a confirmed UTXO. The only
+        // way to verify this is to look up the child's inputs in our current coins view (not including
+        // mempool), and enforce that all parents not present in the package be available at chain tip.
+        // Since this check can bring new coins into the coins cache, keep track of these coins and
+        // uncache them if we don't end up submitting this package to the mempool.
+        const CCoinsViewCache& coins_tip_cache = m_active_chainstate.CoinsTip();
+        for (const auto& input : child->vin) {
+            if (!coins_tip_cache.HaveCoinInCache(input.prevout)) {
+                args.m_coins_to_uncache.push_back(input.prevout);
+            }
+        }
+        // Using the MemPoolAccept m_view cache allows us to look up these same coins faster later.
+        // This should be connecting directly to CoinsTip, not to m_viewmempool, because we specifically
+        // require inputs to be confirmed if they aren't in the package.
+        m_view.SetBackend(m_active_chainstate.CoinsTip());
+        const auto package_or_confirmed = [this, &unconfirmed_parent_txids](const auto& input) {
+             return unconfirmed_parent_txids.count(input.prevout.hash) > 0 || m_view.HaveCoin(input.prevout);
+        };
+        if (!std::all_of(child->vin.cbegin(), child->vin.cend(), package_or_confirmed)) {
+            package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-unconfirmed-parents");
+            return PackageMempoolAcceptResult(package_state_quit_early, {});
+        }
+        // Protect against bugs where we pull more inputs from disk that miss being added to
+        // coins_to_uncache. The backend will be connected again when needed in PreChecks.
+        m_view.SetBackend(m_dummy);
     }
-    // Protect against bugs where we pull more inputs from disk that miss being added to
-    // coins_to_uncache. The backend will be connected again when needed in PreChecks.
-    m_view.SetBackend(m_dummy);
 
     LOCK(m_pool.cs);
     // Stores results from which we will create the returned PackageMempoolAcceptResult.
@@ -1735,6 +1741,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
     // this transaction. "Nonfinal" because if a transaction fails by itself but succeeds later
     // (i.e. when evaluated with a fee-bumping child), the result in this map may be discarded.
     std::map<uint256, MempoolAcceptResult> individual_results_nonfinal;
+    // Tracks whether we think package submission could result in successful entry to the mempool
     bool quit_early{false};
     std::vector<CTransactionRef> txns_package_eval;
     for (const auto& tx : package) {
@@ -1776,8 +1783,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
                 // in package validation, because its fees should only be "used" once.
                 assert(m_pool.exists(GenTxid::Wtxid(wtxid)));
                 results_final.emplace(wtxid, single_res);
-            } else if (single_res.m_state.GetResult() != TxValidationResult::TX_RECONSIDERABLE &&
-                       single_res.m_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) {
+            } else if (package.size() == 1 || // If there is only one transaction, no need to retry it "as a package"
+                       (single_res.m_state.GetResult() != TxValidationResult::TX_RECONSIDERABLE &&
+                       single_res.m_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS)) {
                 // Package validation policy only differs from individual policy in its evaluation
                 // of feerate. For example, if a transaction fails here due to violation of a
                 // consensus rule, the result will not change when it is submitted as part of a


Test patch:
diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
index ea211aedf3ac3..9891fed4e61b7 100644
--- a/src/test/txpackage_tests.cpp
+++ b/src/test/txpackage_tests.cpp
@@ -283,6 +283,8 @@ BOOST_AUTO_TEST_CASE(noncontextual_package_tests)
         BOOST_CHECK(GetPackageHash({tx_parent}) != GetPackageHash({tx_child}));
         BOOST_CHECK(GetPackageHash({tx_child, tx_child}) != GetPackageHash({tx_child}));
         BOOST_CHECK(GetPackageHash({tx_child, tx_parent}) != GetPackageHash({tx_child, tx_child}));
+        BOOST_CHECK(!IsChildWithParents({}));
+        BOOST_CHECK(!IsChildWithParentsTree({}));
     }
 
     // 24 Parents and 1 Child
@@ -492,6 +494,97 @@ BOOST_AUTO_TEST_CASE(package_submission_tests)
     }
 }
 
+// Tests for packages containing a single transaction
+BOOST_AUTO_TEST_CASE(package_single_tx)
+{
+    // Mine blocks to mature coinbases.
+    mineBlocks(3);
+    LOCK(cs_main);
+    auto expected_pool_size{m_node.mempool->size()};
+
+    const CAmount high_fee{1000};
+
+    // No unconfirmed parents
+    CKey single_key = GenerateRandomKey();
+    CScript single_locking_script = GetScriptForDestination(PKHash(single_key.GetPubKey()));
+    auto mtx_single = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
+                                                    /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
+                                                    /*output_destination=*/single_locking_script,
+                                                    /*output_amount=*/CAmount(49 * COIN), /*submit=*/false);
+    CTransactionRef tx_single = MakeTransactionRef(mtx_single);
+    Package package_tx_single{tx_single};
+    const auto result_single_tx = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                    package_tx_single, /*test_accept=*/false, /*client_maxfeerate=*/{});
+    expected_pool_size += 1;
+    BOOST_CHECK_MESSAGE(result_single_tx.m_state.IsValid(),
+                        "Package validation unexpectedly failed: " << result_single_tx.m_state.ToString());
+    BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+
+    // Parent and Child. Both submitted by themselves through the ProcessNewPackage interface.
+    CKey parent_key = GenerateRandomKey();
+    CScript parent_locking_script = GetScriptForDestination(WitnessV0KeyHash(parent_key.GetPubKey()));
+    auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[1], /*input_vout=*/0,
+                                                    /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
+                                                    /*output_destination=*/parent_locking_script,
+                                                    /*output_amount=*/CAmount(50 * COIN) - high_fee, /*submit=*/false);
+    CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
+    Package package_just_parent{tx_parent};
+    const auto result_just_parent = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_just_parent, /*test_accept=*/false, /*client_maxfeerate=*/{});
+    if (auto err_parent_child{CheckPackageMempoolAcceptResult(package_just_parent, result_just_parent, /*expect_valid=*/true, nullptr)}) {
+        BOOST_ERROR(err_parent_child.value());
+    } else {
+        auto it_parent = result_just_parent.m_tx_results.find(tx_parent->GetWitnessHash());
+        BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(), it_parent->second.m_state.ToString());
+        BOOST_CHECK(it_parent->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_parent)) == high_fee);
+        BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().size(), 1);
+        BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().front(), tx_parent->GetWitnessHash());
+    }
+    expected_pool_size += 1;
+    BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+
+    CKey child_key = GenerateRandomKey();
+    CScript child_locking_script = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey()));
+    auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0,
+                                                   /*input_height=*/101, /*input_signing_key=*/parent_key,
+                                                   /*output_destination=*/child_locking_script,
+                                                   /*output_amount=*/CAmount(50 * COIN) - 2 * high_fee, /*submit=*/false);
+    CTransactionRef tx_child = MakeTransactionRef(mtx_child);
+    Package package_just_child{tx_child};
+    const auto result_just_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_just_child, /*test_accept=*/false, /*client_maxfeerate=*/{});
+    if (auto err_parent_child{CheckPackageMempoolAcceptResult(package_just_child, result_just_child, /*expect_valid=*/true, nullptr)}) {
+        BOOST_ERROR(err_parent_child.value());
+    } else {
+        auto it_child = result_just_child.m_tx_results.find(tx_child->GetWitnessHash());
+        BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(), it_child->second.m_state.ToString());
+        BOOST_CHECK(it_child->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_child)) == high_fee);
+        BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().size(), 1);
+        BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().front(), tx_child->GetWitnessHash());
+    }
+    expected_pool_size += 1;
+    BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+
+    // Too-low fee to RBF tx_single
+    auto mtx_single_low_fee = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
+                                                    /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
+                                                    /*output_destination=*/single_locking_script,
+                                                    /*output_amount=*/CAmount(49 * COIN - 1), /*submit=*/false);
+    CTransactionRef tx_single_low_fee = MakeTransactionRef(mtx_single_low_fee);
+    Package package_tx_single_low_fee{tx_single_low_fee};
+    const auto result_single_tx_low_fee = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                    package_tx_single_low_fee, /*test_accept=*/false, /*client_maxfeerate=*/{});
+
+    BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+
+    BOOST_CHECK(!result_single_tx_low_fee.m_state.IsValid());
+    BOOST_CHECK_EQUAL(result_single_tx_low_fee.m_state.GetResult(), PackageValidationResult::PCKG_TX);
+    auto it_low_fee = result_single_tx_low_fee.m_tx_results.find(tx_single_low_fee->GetWitnessHash());
+    BOOST_CHECK_EQUAL(it_low_fee->second.m_state.GetResult(), TxValidationResult::TX_RECONSIDERABLE);
+    if (auto err_single{CheckPackageMempoolAcceptResult(package_tx_single_low_fee, result_single_tx_low_fee, /*expect_valid=*/false, m_node.mempool.get())}) {
+        BOOST_ERROR(err_single.value());
+    }
+    BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+}
+
 // Tests for packages containing transactions that have same-txid-different-witness equivalents in
 // the mempool.
 BOOST_AUTO_TEST_CASE(package_witness_swap_tests)
diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
index 539ff155cffc2..f5d42d0c34320 100755
--- a/test/functional/rpc_packages.py
+++ b/test/functional/rpc_packages.py
@@ -377,8 +377,8 @@ def test_submitpackage(self):
         assert txid_list[0] not in node.getrawmempool()
         assert txid_list[1] not in node.getrawmempool()
 
-        self.log.info("Submitpackage valid packages with 1 child and some number of parents")
-        for num_parents in [1, 2, 24]:
+        self.log.info("Submitpackage valid packages with 1 child and some number of parents (or none)")
+        for num_parents in [0, 1, 2, 24]:
             self.test_submit_child_with_parents(num_parents, False)
             self.test_submit_child_with_parents(num_parents, True)
 
@@ -389,10 +389,9 @@ def test_submitpackage(self):
         assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, chain_hex)
         assert_equal(legacy_pool, node.getrawmempool())
 
-        assert_raises_rpc_error(-8, f"Array must contain between 2 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, [])
-        assert_raises_rpc_error(-8, f"Array must contain between 2 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, [chain_hex[0]] * 1)
+        assert_raises_rpc_error(-8, f"Array must contain between 1 and {MAX_PACKAGE_COUNT} transactions.", node.submitpackage, [])
         assert_raises_rpc_error(
-            -8, f"Array must contain between 2 and {MAX_PACKAGE_COUNT} transactions.",
+            -8, f"Array must contain between 1 and {MAX_PACKAGE_COUNT} transactions.",
             node.submitpackage, [chain_hex[0]] * (MAX_PACKAGE_COUNT + 1)
         )