nit: I'd prefer avoiding having this dual state. Here are 2 variations which do that:
Preserving behavior:
diff --git a/src/validation.cpp b/src/validation.cpp
index 7e82c3bbad..008bfe4d71 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2423,8 +2423,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
return true;
}
- auto fScriptChecks{true};
- std::string script_check_reason;
+ const char* script_check_reason{nullptr};
if (!m_chainman.AssumedValidBlock().IsNull()) {
constexpr int64_t TWO_WEEKS_IN_SECONDS{60 * 60 * 24 * 7 * 2};
// We've been configured with the hash of a block which has been externally verified to have a valid history.
@@ -2458,7 +2457,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
// artificially set the default assumed verified block further back.
// The test against the minimum chain work prevents the skipping when denied access to any chain at
// least as good as the expected chain.
- fScriptChecks = false;
}
} else {
script_check_reason = "assumevalid=0 (always verify)";
@@ -2573,16 +2571,15 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
Ticks<SecondsDouble>(m_chainman.time_forks),
Ticks<MillisecondsDouble>(m_chainman.time_forks) / m_chainman.num_blocks_total);
- if (fScriptChecks != m_prev_script_checks_logged && GetRole() == ChainstateRole::NORMAL) {
- if (fScriptChecks) {
- Assume(!script_check_reason.empty());
+ if ((script_check_reason != nullptr) != m_prev_script_checks_logged && GetRole() == ChainstateRole::NORMAL) {
+ if (script_check_reason) {
LogInfo("Enabling script verification at block #%d (%s): %s.",
pindex->nHeight, block_hash.ToString(), script_check_reason);
} else {
LogInfo("Disabling script verification at block #%d (%s).",
pindex->nHeight, block_hash.ToString());
}
- m_prev_script_checks_logged = fScriptChecks;
+ m_prev_script_checks_logged = script_check_reason != nullptr;
}
CBlockUndo blockundo;
@@ -2593,7 +2590,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
// doesn't invalidate pointers into the vector, and keep txsdata in scope
// for as long as `control`.
std::optional<CCheckQueueControl<CScriptCheck>> control;
- if (auto& queue = m_chainman.GetCheckQueue(); queue.HasThreads() && fScriptChecks) control.emplace(queue);
+ if (auto& queue = m_chainman.GetCheckQueue(); queue.HasThreads() && script_check_reason != nullptr) control.emplace(queue);
std::vector<PrecomputedTransactionData> txsdata(block.vtx.size());
@@ -2652,7 +2649,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
break;
}
- if (!tx.IsCoinBase() && fScriptChecks)
+ if (!tx.IsCoinBase() && script_check_reason != nullptr)
{
bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
bool tx_ok;
<details><summary>The below version will output more often, when switching reasons, not sure which behavior is more desirable (there is no risk of dangling pointers from using `const char*`, but it requires us to be vigilant going forward):</summary>
diff --git a/src/validation.cpp b/src/validation.cpp
index 7e82c3bbad..79ad71b64c 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2423,8 +2423,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
return true;
}
- auto fScriptChecks{true};
- std::string script_check_reason;
+ const char* script_check_reason{nullptr};
if (!m_chainman.AssumedValidBlock().IsNull()) {
constexpr int64_t TWO_WEEKS_IN_SECONDS{60 * 60 * 24 * 7 * 2};
// We've been configured with the hash of a block which has been externally verified to have a valid history.
@@ -2458,7 +2457,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
// artificially set the default assumed verified block further back.
// The test against the minimum chain work prevents the skipping when denied access to any chain at
// least as good as the expected chain.
- fScriptChecks = false;
}
} else {
script_check_reason = "assumevalid=0 (always verify)";
@@ -2573,16 +2571,15 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
Ticks<SecondsDouble>(m_chainman.time_forks),
Ticks<MillisecondsDouble>(m_chainman.time_forks) / m_chainman.num_blocks_total);
- if (fScriptChecks != m_prev_script_checks_logged && GetRole() == ChainstateRole::NORMAL) {
- if (fScriptChecks) {
- Assume(!script_check_reason.empty());
+ if (script_check_reason != m_prev_script_check_reason && GetRole() == ChainstateRole::NORMAL) {
+ if (script_check_reason) {
LogInfo("Enabling script verification at block #%d (%s): %s.",
pindex->nHeight, block_hash.ToString(), script_check_reason);
} else {
LogInfo("Disabling script verification at block #%d (%s).",
pindex->nHeight, block_hash.ToString());
}
- m_prev_script_checks_logged = fScriptChecks;
+ m_prev_script_check_reason = script_check_reason;
}
CBlockUndo blockundo;
@@ -2593,7 +2590,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
// doesn't invalidate pointers into the vector, and keep txsdata in scope
// for as long as `control`.
std::optional<CCheckQueueControl<CScriptCheck>> control;
- if (auto& queue = m_chainman.GetCheckQueue(); queue.HasThreads() && fScriptChecks) control.emplace(queue);
+ if (auto& queue = m_chainman.GetCheckQueue(); queue.HasThreads() && script_check_reason) control.emplace(queue);
std::vector<PrecomputedTransactionData> txsdata(block.vtx.size());
@@ -2652,7 +2649,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
break;
}
- if (!tx.IsCoinBase() && fScriptChecks)
+ if (!tx.IsCoinBase() && script_check_reason)
{
bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */
bool tx_ok;
diff --git a/src/validation.h b/src/validation.h
index fea11c5515..6e426a4660 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -560,7 +560,7 @@ protected:
//! Cached result of LookupBlockIndex(*m_from_snapshot_blockhash)
mutable const CBlockIndex* m_cached_snapshot_base GUARDED_BY(::cs_main){nullptr};
- std::optional<bool> m_prev_script_checks_logged GUARDED_BY(::cs_main){};
+ std::optional<const char*> m_prev_script_check_reason GUARDED_BY(::cs_main){};
public:
//! Reference to a BlockManager instance which itself is shared across all
</details>
