I think this should go a little further. At this point, CheckInputScripts is essentially meant to be a mempool function and PrepareInputScriptChecks is for block validation, except for the weird edge-case when we’re single-threaded.
I think I’d rather just see this go all the way and make the split purposeful. Something like:
0diff --git a/src/validation.cpp b/src/validation.cpp
1index 1c81620889f..d9a447b460f 100644
2--- a/src/validation.cpp
3+++ b/src/validation.cpp
4@@ -2723,19 +2723,18 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
5 {
6 bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
7 TxValidationState tx_state;
8- // If parallel script checking is possible (worker threads are available) the checks are appended to a
9- // vector without running them. The vector is then added to control which runs the checks asynchronously.
10- // Otherwise, CheckInputScripts runs the checks on a single thread before returning.
11- if (control) {
12- std::vector<CScriptCheck> vChecks = PrepareInputScriptChecks(tx, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache);
13- control->Add(std::move(vChecks));
14- } else {
15- if (!CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache)) {
16- // Any transaction validation failure in ConnectBlock is a block consensus failure
17- state.Invalid(BlockValidationResult::BLOCK_CONSENSUS,
18- tx_state.GetRejectReason(), tx_state.GetDebugMessage());
19- break;
20+ // If parallel script checking is possible (worker threads are available), they are added to control which
21+ // which runs the checks asynchronously. Otherwise they are run here directly.
22+ std::vector<CScriptCheck> vChecks = PrepareInputScriptChecks(tx, view, flags, fCacheResults, fCacheResults, txsdata[i], m_chainman.m_validation_cache);
23+ if (control) control->Add(std::move(vChecks));
24+ else {
25+ for (auto& check : vChecks) {
26+ if (const auto& check_result = check()) {
27+ state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check_result->first)), check_result->second);
28+ break;
29+ }
30 }
31+ if (!state.IsValid()) break;
32 }
33 }
An alternative would be to make CCheckQueueControl work with 0 worker threads (maybe it already does?) and just always use it. That would completely unify the approach here.
