bitcoin/bitcoin
1
1
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-05-17 23:56:39 +00:00
Code Issues Releases Activity

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

Browse Source 
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 <gsanders87@gmail.com>
This commit is contained in:
glozow 2022-11-03 15:01:17 +00:00
parent 196a43eddb
commit be2e4d94e5
1 changed files with 10 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/validation.cpp
View File

@ -1273,19 +1273,20 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, ATMPArgs& args) PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, ATMPArgs& args)
{ {
AssertLockHeld(cs_main); 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 // 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. // transactions and thus won't return any MempoolAcceptResults, just a package-wide error.
// Context-free package checks. // 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 // 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. // opportunity for us to fail fast on a context-free check without taking the mempool lock.
if (!IsChildWithParents(package)) { if (!IsChildWithParents(package)) {
package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents"); package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
return PackageMempoolAcceptResult(package_state, {}); return PackageMempoolAcceptResult(package_state_quit_early, {});
} }
// IsChildWithParents() guarantees the package is > 1 transactions. // 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); 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)) { 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"); package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-unconfirmed-parents");
return PackageMempoolAcceptResult(package_state, {}); return PackageMempoolAcceptResult(package_state_quit_early, {});
} }
// Protect against bugs where we pull more inputs from disk that miss being added to // 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. // 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 // future. Continue individually validating the rest of the transactions, because
// some of them may still be valid. // some of them may still be valid.
quit_early = true; quit_early = true;
package_state_quit_early.Invalid(PackageValidationResult::PCKG_TX, "transaction failed");
results.emplace(wtxid, single_res);
} else { } else {
txns_new.push_back(tx); 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. // Nothing to do if the entire package has already been submitted.
if (quit_early || txns_new.empty()) { if (quit_early || txns_new.empty()) {
// No package feerate when no package validation was done. // 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. // Validate the (deduplicated) transactions as a package.
auto submission_result = AcceptMultipleTransactions(txns_new, args); auto submission_result = AcceptMultipleTransactions(txns_new, args);