[validation] when quitting early in AcceptPackage, set package_state and tx result

Bug: not setting package_state means package_state.IsValid() == true and
the caller does not know that this failed.

We won't be validating this transaction again, so it makes sense to return this
failure to the caller.

Rename package_state to package_state_quit_early to make it more clear
what this variable is used for and what its scope is.

Co-authored-by: Greg Sanders <[email protected]>

Author: glozow

src/validation.cpp

@@ -1273,19 +1273,20 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
 PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, ATMPArgs& args)
 {
     AssertLockHeld(cs_main);
-    PackageValidationState package_state;
+    // Used if returning a PackageMempoolAcceptResult directly from this function.
+    PackageValidationState package_state_quit_early;
 
     // 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.
 
     // Context-free package checks.
-    if (!CheckPackage(package, package_state)) return PackageMempoolAcceptResult(package_state, {});
+    if (!CheckPackage(package, package_state_quit_early)) 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.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
-        return PackageMempoolAcceptResult(package_state, {});
+        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.
@@ -1317,8 +1318,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
          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.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-unconfirmed-parents");
-        return PackageMempoolAcceptResult(package_state, {});
+        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.
@@ -1381,6 +1382,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
                 // future.  Continue individually validating the rest of the transactions, because
                 // some of them may still be valid.
                 quit_early = true;
+                package_state_quit_early.Invalid(PackageValidationResult::PCKG_TX, "transaction failed");
+                results.emplace(wtxid, single_res);
             } else {
                 txns_new.push_back(tx);
             }
@@ -1390,7 +1393,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
     // Nothing to do if the entire package has already been submitted.
     if (quit_early || txns_new.empty()) {
         // No package feerate when no package validation was done.
-        return PackageMempoolAcceptResult(package_state, std::move(results));
+        return PackageMempoolAcceptResult(package_state_quit_early, std::move(results));
     }
     // Validate the (deduplicated) transactions as a package.
     auto submission_result = AcceptMultipleTransactions(txns_new, args);