If I understand correctly, m_script_execution_cache_hasher is a pre-initialized hasher that we want to copy every time we use it, and we definitely don’t want to modify in-place. I think having it as a public struct member is not very robust to enforce that, perhaps better to use a getter here? E.g. something like:
0diff --git a/src/validation.cpp b/src/validation.cpp
1index 39ab7b56ab..600c427c93 100644
2--- a/src/validation.cpp
3+++ b/src/validation.cpp
4@@ -2095,17 +2095,22 @@ bool CScriptCheck::operator()() {
5 return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *m_signature_cache, *txdata), &error);
6 }
7
8-ValidationCache::ValidationCache(size_t script_execution_cache_bytes, size_t signature_cache_bytes)
9- : m_signature_cache{signature_cache_bytes / 2}
10+CSHA256 ValidationCache::CreateScriptExecutionCacheHasher()
11 {
12 // Setup the salted hasher
13 uint256 nonce = GetRandHash();
14 // We want the nonce to be 64 bytes long to force the hasher to process
15 // this chunk, which makes later hash computations more efficient. We
16 // just write our 32-byte entropy twice to fill the 64 bytes.
17- m_script_execution_cache_hasher.Write(nonce.begin(), 32);
18- m_script_execution_cache_hasher.Write(nonce.begin(), 32);
19+ CSHA256 hasher;
20+ hasher.Write(nonce.begin(), 32);
21+ hasher.Write(nonce.begin(), 32);
22+ return hasher;
23+}
24
25+ValidationCache::ValidationCache(size_t script_execution_cache_bytes, size_t signature_cache_bytes)
26+ : m_signature_cache{signature_cache_bytes / 2}
27+{
28 const auto [num_elems, approx_size_bytes] = m_script_execution_cache.setup_bytes(script_execution_cache_bytes);
29 LogPrintf("Using %zu MiB out of %zu MiB requested for script execution cache, able to store %zu elements\n",
30 approx_size_bytes >> 20, script_execution_cache_bytes >> 20, num_elems);
31@@ -2148,7 +2153,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
32 // properly commits to the scriptPubKey in the inputs view of that
33 // transaction).
34 uint256 hashCacheEntry;
35- CSHA256 hasher = validation_cache.m_script_execution_cache_hasher;
36+ auto hasher{validation_cache.ScriptExecutionCacheHasher()};
37 hasher.Write(UCharCast(tx.GetWitnessHash().begin()), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin());
38 AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks
39 if (validation_cache.m_script_execution_cache.contains(hashCacheEntry, !cacheFullScriptStore)) {
40diff --git a/src/validation.h b/src/validation.h
41index c4b49ec3e8..1e48f6b4a4 100644
42--- a/src/validation.h
43+++ b/src/validation.h
44@@ -369,11 +369,16 @@ static_assert(std::is_nothrow_destructible_v<CScriptCheck>);
45 * Convenience struct for initializing and passing the script execution cache
46 * and signature cache.
47 */
48-struct ValidationCache {
49+class ValidationCache {
50+public:
51 ScriptCache m_script_execution_cache;
52- CSHA256 m_script_execution_cache_hasher;
53+ CSHA256 ScriptExecutionCacheHasher() { return m_script_execution_cache_hasher; }
54 CSignatureCache m_signature_cache;
55 ValidationCache(size_t script_execution_cache_bytes, size_t signature_cache_bytes);
56+private:
57+ static CSHA256 CreateScriptExecutionCacheHasher();
58+ //! Pre-initialized hasher to avoid having to recreate it for every hash calculation
59+ const CSHA256 m_script_execution_cache_hasher{CreateScriptExecutionCacheHasher()};
60 };
61
62 /** Functions for validating blocks and updating the block tree */
It seems this is quite brittle on master too, so not a regression, but probably something to address here?
