Skip to content

Commit

Permalink
[validation] better handle errors in SubmitPackage
Browse files Browse the repository at this point in the history
Behavior change: don't quit right after LimitMempoolSize() when a
package is partially submitted. We should still send
TransactionAddedToMempool notifications for
transactions that were submitted.

Not behavior change: add a new package validation result for mempool logic errors.
  • Loading branch information
glozow committed Jan 17, 2022
1 parent 9d88853 commit de075a9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/policy/packages.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ enum class PackageValidationResult {
PCKG_RESULT_UNSET = 0, //!< Initial value. The package has not yet been rejected.
PCKG_POLICY, //!< The package itself is invalid (e.g. too many transactions).
PCKG_TX, //!< At least one tx is invalid.
PCKG_MEMPOOL_ERROR, //!< Mempool logic error.
};

/** A package is an ordered list of transactions. The transactions cannot conflict with (spend the
Expand Down
17 changes: 12 additions & 5 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,10 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
if (!ConsensusScriptChecks(args, ws)) {
results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
// Since PolicyScriptChecks() passed, this should never fail.
all_submitted = Assume(false);
all_submitted = false;
package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR,
strprintf("BUG! PolicyScriptChecks succeeded but ConsensusScriptChecks failed: %s",
ws.m_ptx->GetHash().ToString()));
}

// Re-calculate mempool ancestors to call addUnchecked(). They may have changed since the
Expand All @@ -1069,7 +1072,10 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
m_limit_descendant_size, unused_err_string)) {
results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
// Since PreChecks() and PackageMempoolChecks() both enforce limits, this should never fail.
all_submitted = Assume(false);
all_submitted = false;
package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR,
strprintf("BUG! Mempool ancestors or descendants were underestimated: %s",
ws.m_ptx->GetHash().ToString()));
}
// If we call LimitMempoolSize() for each individual Finalize(), the mempool will not take
// the transaction's descendant feerate into account because it hasn't seen them yet. Also,
Expand All @@ -1079,7 +1085,9 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
if (!Finalize(args, ws)) {
results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
// Since LimitMempoolSize() won't be called, this should never fail.
all_submitted = Assume(false);
all_submitted = false;
package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR,
strprintf("BUG! Adding to mempool failed: %s", ws.m_ptx->GetHash().ToString()));
}
}

Expand All @@ -1088,7 +1096,6 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip(),
gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000,
std::chrono::hours{gArgs.GetIntArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)});
if (!all_submitted) return false;

// Find the wtxids of the transactions that made it into the mempool. Allow partial submission,
// but don't report success unless they all made it into the mempool.
Expand Down Expand Up @@ -1194,7 +1201,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
if (args.m_test_accept) return PackageMempoolAcceptResult(package_state, std::move(results));

if (!SubmitPackage(args, workspaces, package_state, results)) {
package_state.Invalid(PackageValidationResult::PCKG_TX, "submission failed");
// PackageValidationState filled in by SubmitPackage().
return PackageMempoolAcceptResult(package_state, std::move(results));
}

Expand Down

0 comments on commit de075a9

Please sign in to comment.