Merge bitcoin/bitcoin#34177: policy/refactor: remove constant parameter from IsWellFormedPackage

658d381 policy: remove constant parameter from `IsWellFormedPackage` (Lőrinc)

Pull request description:

  `IsWellFormedPackage()` already claims: "parents must appear before children." In practice the `require_sorted` argument was always passed as `true`, making the false-path dead code. It was introduced that way from the beginning in https://github.com/bitcoin/bitcoin/pull/28758/files#diff-f30090b30c9489972ee3f1181c302cf3a484bb890bade0fd7c9ca92ea8d347f6R79.

  Remove the unused parameter, updating callers/tests.

ACKs for top commit:
  billymcbip:
    tACK 658d381
  instagibbs:
    ACK 658d381

Tree-SHA512: 8b86dda7e2e1f0d48947ff258f0a3b6ec60676f54d4b506604d24e15c8b6465358ed2ccf174c7620125f5cad6bfc4df0bc482d920e5fc4cd0e1d72a9b16eafa5

Author: fanquake

src/policy/packages.cpp

@@ -76,7 +76,7 @@ bool IsConsistentPackage(const Package& txns)
     return true;
 }
 
-bool IsWellFormedPackage(const Package& txns, PackageValidationState& state, bool require_sorted)
+bool IsWellFormedPackage(const Package& txns, PackageValidationState& state)
 {
     const unsigned int package_count = txns.size();
 
@@ -105,7 +105,7 @@ bool IsWellFormedPackage(const Package& txns, PackageValidationState& state, boo
     // An unsorted package will fail anyway on missing-inputs, but it's better to quit earlier and
     // fail on something less ambiguous (missing-inputs could also be an orphan or trying to
     // spend nonexistent coins).
-    if (require_sorted && !IsTopoSortedPackage(txns, later_txids)) {
+    if (!IsTopoSortedPackage(txns, later_txids)) {
         return state.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-sorted");
     }
 

src/policy/packages.h

@@ -71,7 +71,7 @@ bool IsConsistentPackage(const Package& txns);
  * 3. If any dependencies exist between transactions, parents must appear before children.
  * 4. Transactions cannot conflict, i.e., spend the same inputs.
  */
-bool IsWellFormedPackage(const Package& txns, PackageValidationState& state, bool require_sorted);
+bool IsWellFormedPackage(const Package& txns, PackageValidationState& state);
 
 /** Context-free check that a package is exactly one child and its parents; not all parents need to
  * be present, but the package must not contain any transactions that are not the child's parents.

src/test/txpackage_tests.cpp

@@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(package_sanitization_tests)
         package_too_many.emplace_back(create_placeholder_tx(1, 1));
     }
     PackageValidationState state_too_many;
-    BOOST_CHECK(!IsWellFormedPackage(package_too_many, state_too_many, /*require_sorted=*/true));
+    BOOST_CHECK(!IsWellFormedPackage(package_too_many, state_too_many));
     BOOST_CHECK_EQUAL(state_too_many.GetResult(), PackageValidationResult::PCKG_POLICY);
     BOOST_CHECK_EQUAL(state_too_many.GetRejectReason(), "package-too-many-transactions");
 
@@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE(package_sanitization_tests)
     }
     BOOST_CHECK(package_too_large.size() <= MAX_PACKAGE_COUNT);
     PackageValidationState state_too_large;
-    BOOST_CHECK(!IsWellFormedPackage(package_too_large, state_too_large, /*require_sorted=*/true));
+    BOOST_CHECK(!IsWellFormedPackage(package_too_large, state_too_large));
     BOOST_CHECK_EQUAL(state_too_large.GetResult(), PackageValidationResult::PCKG_POLICY);
     BOOST_CHECK_EQUAL(state_too_large.GetRejectReason(), "package-too-large");
 
@@ -165,7 +165,7 @@ BOOST_AUTO_TEST_CASE(package_sanitization_tests)
         package_duplicate_txids_empty.emplace_back(MakeTransactionRef(empty_tx));
     }
     PackageValidationState state_duplicates;
-    BOOST_CHECK(!IsWellFormedPackage(package_duplicate_txids_empty, state_duplicates, /*require_sorted=*/true));
+    BOOST_CHECK(!IsWellFormedPackage(package_duplicate_txids_empty, state_duplicates));
     BOOST_CHECK_EQUAL(state_duplicates.GetResult(), PackageValidationResult::PCKG_POLICY);
     BOOST_CHECK_EQUAL(state_duplicates.GetRejectReason(), "package-contains-duplicates");
     BOOST_CHECK(!IsConsistentPackage(package_duplicate_txids_empty));
@@ -184,7 +184,7 @@ BOOST_AUTO_TEST_CASE(package_sanitization_tests)
     // Transactions are considered sorted when they have no dependencies.
     BOOST_CHECK(IsTopoSortedPackage(package_conflicts));
     PackageValidationState state_conflicts;
-    BOOST_CHECK(!IsWellFormedPackage(package_conflicts, state_conflicts, /*require_sorted=*/true));
+    BOOST_CHECK(!IsWellFormedPackage(package_conflicts, state_conflicts));
     BOOST_CHECK_EQUAL(state_conflicts.GetResult(), PackageValidationResult::PCKG_POLICY);
     BOOST_CHECK_EQUAL(state_conflicts.GetRejectReason(), "conflict-in-package");
 
@@ -274,8 +274,8 @@ BOOST_AUTO_TEST_CASE(noncontextual_package_tests)
         CTransactionRef tx_child = MakeTransactionRef(mtx_child);
 
         PackageValidationState state;
-        BOOST_CHECK(IsWellFormedPackage({tx_parent, tx_child}, state, /*require_sorted=*/true));
-        BOOST_CHECK(!IsWellFormedPackage({tx_child, tx_parent}, state, /*require_sorted=*/true));
+        BOOST_CHECK(IsWellFormedPackage({tx_parent, tx_child}, state));
+        BOOST_CHECK(!IsWellFormedPackage({tx_child, tx_parent}, state));
         BOOST_CHECK_EQUAL(state.GetResult(), PackageValidationResult::PCKG_POLICY);
         BOOST_CHECK_EQUAL(state.GetRejectReason(), "package-not-sorted");
         BOOST_CHECK(IsChildWithParents({tx_parent, tx_child}));
@@ -308,7 +308,7 @@ BOOST_AUTO_TEST_CASE(noncontextual_package_tests)
         package.push_back(MakeTransactionRef(child));
 
         PackageValidationState state;
-        BOOST_CHECK(IsWellFormedPackage(package, state, /*require_sorted=*/true));
+        BOOST_CHECK(IsWellFormedPackage(package, state));
         BOOST_CHECK(IsChildWithParents(package));
         BOOST_CHECK(IsChildWithParentsTree(package));
 
@@ -346,8 +346,8 @@ BOOST_AUTO_TEST_CASE(noncontextual_package_tests)
         BOOST_CHECK(!IsChildWithParentsTree({tx_parent, tx_parent_also_child, tx_child}));
         // IsChildWithParents does not detect unsorted parents.
         BOOST_CHECK(IsChildWithParents({tx_parent_also_child, tx_parent, tx_child}));
-        BOOST_CHECK(IsWellFormedPackage({tx_parent, tx_parent_also_child, tx_child}, state, /*require_sorted=*/true));
-        BOOST_CHECK(!IsWellFormedPackage({tx_parent_also_child, tx_parent, tx_child}, state, /*require_sorted=*/true));
+        BOOST_CHECK(IsWellFormedPackage({tx_parent, tx_parent_also_child, tx_child}, state));
+        BOOST_CHECK(!IsWellFormedPackage({tx_parent_also_child, tx_parent, tx_child}, state));
         BOOST_CHECK_EQUAL(state.GetResult(), PackageValidationResult::PCKG_POLICY);
         BOOST_CHECK_EQUAL(state.GetRejectReason(), "package-not-sorted");
     }

src/validation.cpp

@@ -1440,7 +1440,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(con
 
     // These context-free package limits can be done before taking the mempool lock.
     PackageValidationState package_state;
-    if (!IsWellFormedPackage(txns, package_state, /*require_sorted=*/true)) return PackageMempoolAcceptResult(package_state, {});
+    if (!IsWellFormedPackage(txns, package_state)) return PackageMempoolAcceptResult(package_state, {});
 
     std::vector<Workspace> workspaces{};
     workspaces.reserve(txns.size());
@@ -1637,7 +1637,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
     // transactions and thus won't return any MempoolAcceptResults, just a package-wide error.
 
     // Context-free package checks.
-    if (!IsWellFormedPackage(package, package_state_quit_early, /*require_sorted=*/true)) {
+    if (!IsWellFormedPackage(package, package_state_quit_early)) {
         return PackageMempoolAcceptResult(package_state_quit_early, {});
     }