Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts #32473

pull sipa wants to merge 4 commits into bitcoin:master from sipa:202504_sighash_cache changing 17 files +221 −68
  1. sipa commented at 5:49 pm on May 12, 2025: member

    This introduces a per-txin cache for sighash midstate computation to the script interpreter for legacy (bare), P2SH, P2WSH, and (as collateral effect, but not actually useful) P2WPKH. This reduces the impact of certain types of quadratic hashing attacks that use standard transactions. It is not known to improve the situation for attacks involving non-standard transaction attacks.

    The cache works by remembering for each of the 6 sighash modes a (scriptCode, midstate) tuple, which gives a midstate CSHA256 object right before the appending of the sighash type itself (to permit all 256, rather than just the 6 ones that match the modes). The midstate is only reused if the scriptCode matches. This works because - within a single input - only the sighash type and the scriptCode affect the actual sighash used.

    The PR implements two different approaches:

    • The initial commits introduce the caching effect always, for both consensus and relay relation validation. Despite being primarily intended for improving the situation for standard transactions only, I chose this approach as the code paths are already largely common between the two, and this approach I believe involves fewer code changes than a more targetted approach, and furthermore, it should not hurt (it may even help common multisig cases slightly).
    • The final commit changes the behavior to only using the cache for non-consensus script validation. I’m open to feedback about whether adding this commit is worth it.

    Functional tests are included that construct contrived cases with many sighash types (standard and non-standard ones) and OP_CODESEPARATORs in all script types (including P2TR, which isn’t modified by this PR).

  2. DrahtBot commented at 5:49 pm on May 12, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32473.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK darosior

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #33050 (net, validation: don’t punish peers for consensus-invalid txs by ajtowns)
    • #33012 (script: return verification flag responsible for error upon validation failure by darosior)
    • #32998 (Bump SCRIPT_VERIFY flags to 64 bit by ajtowns)
    • #32876 (refactor: use options struct for signing and PSBT operations by Sjors)
    • #32857 (wallet: allow skipping script paths by Sjors)
    • #32575 (refactor: Split multithreaded case out of CheckInputScripts by fjahr)
    • #32317 (kernel: Separate UTXO set access from validation functions by TheCharlatan)
    • #32301 (test: cover invalid codesep positions for signature in taproot by instagibbs)
    • #32247 (BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only) by jamesob)
    • #31576 (test: Move script_assets_tests into its own suite by hebasto)
    • #29491 ([EXPERIMENTAL] Schnorr batch verification for blocks by fjahr)
    • #21283 (Implement BIP 370 PSBTv2 by achow101)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. in test/functional/feature_taproot.py:225 in 26fced465b outdated
    221@@ -218,6 +222,20 @@ def default_controlblock(ctx):
    222     """Default expression for "controlblock": combine leafversion, negflag, pubkey_internal, merklebranch."""
    223     return bytes([get(ctx, "leafversion") + get(ctx, "negflag")]) + get(ctx, "pubkey_internal") + get(ctx, "merklebranch")
    224 
    225+def default_scriptcode_suffix(ctx):
    


    darosior commented at 6:00 pm on May 12, 2025:
    Should it be named default_scriptcode_wsh to be explicit that it implements the Segwit v0 (and not legacy) logic with regard to OP_CODESEPARATOR serialization?

    sipa commented at 6:29 pm on May 12, 2025:
    Hmm, this didn’t change between legacy and witv0, I think?

    darosior commented at 7:03 pm on May 12, 2025:
    It did, legacy skips OP_CODESEPARATORs in the suffix (i.e. “upcoming CODESEPs”) whereas Segwit v0 doesn’t.

    sipa commented at 7:15 pm on May 12, 2025:
    Ah, yes, but this all happens before the call to LegacySignatureMsg(), which filters out the OP_CODESEPARATORs that remain. In either case, this function default_scriptcode_suffix is exercised for both p2wsh and p2sh, so the fact that the test works should mean something…
  4. bitcoin deleted a comment on May 12, 2025
  5. laanwj added the label Validation on May 13, 2025
  6. darosior commented at 2:20 pm on May 13, 2025: member

    Neat. Concept ACK.

    I’m open to discussing alternatives, however.

    I think enabling it by consensus is unnecessarily risky. I would rather take some slightly more complicate logic, for instance by having CScriptCheck own the sighash cache:

      0diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
      1index ffac2acb948..fe24dae6c54 100644
      2--- a/src/script/interpreter.cpp
      3+++ b/src/script/interpreter.cpp
      4@@ -1700,7 +1700,7 @@ bool GenericTransactionSignatureChecker<T>::CheckECDSASignature(const std::vecto
      5     // Witness sighashes need the amount.
      6     if (sigversion == SigVersion::WITNESS_V0 && amount < 0) return HandleMissingData(m_mdb);
      7 
      8-    uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->txdata, &m_sighash_cache);
      9+    uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->txdata, m_sighash_cache);
     10 
     11     if (!VerifyECDSASignature(vchSig, pubkey, sighash))
     12         return false;
     13diff --git a/src/script/interpreter.h b/src/script/interpreter.h
     14index 2108e3992fc..6c84c6dd74c 100644
     15--- a/src/script/interpreter.h
     16+++ b/src/script/interpreter.h
     17@@ -308,15 +308,15 @@ private:
     18     unsigned int nIn;
     19     const CAmount amount;
     20     const PrecomputedTransactionData* txdata;
     21-    mutable SigHashCache m_sighash_cache;
     22+    mutable SigHashCache* m_sighash_cache;
     23 
     24 protected:
     25     virtual bool VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const;
     26     virtual bool VerifySchnorrSignature(std::span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const;
     27 
     28 public:
     29-    GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, MissingDataBehavior mdb) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
     30-    GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn, MissingDataBehavior mdb) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
     31+    GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, MissingDataBehavior mdb, SigHashCache* sighash_cache = nullptr) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(nullptr), m_sighash_cache(sighash_cache) {}
     32+    GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn, MissingDataBehavior mdb, SigHashCache* sighash_cache = nullptr) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(&txdataIn), m_sighash_cache(sighash_cache) {}
     33     bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override;
     34     bool CheckSchnorrSignature(std::span<const unsigned char> sig, std::span<const unsigned char> pubkey, SigVersion sigversion, ScriptExecutionData& execdata, ScriptError* serror = nullptr) const override;
     35     bool CheckLockTime(const CScriptNum& nLockTime) const override;
     36diff --git a/src/script/sigcache.h b/src/script/sigcache.h
     37index ca4b44910e6..89aa5e85ad8 100644
     38--- a/src/script/sigcache.h
     39+++ b/src/script/sigcache.h
     40@@ -67,7 +67,7 @@ private:
     41     SignatureCache& m_signature_cache;
     42 
     43 public:
     44-    CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, SignatureCache& signature_cache, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL), store(storeIn), m_signature_cache(signature_cache)  {}
     45+    CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, SignatureCache& signature_cache, PrecomputedTransactionData& txdataIn, SigHashCache* sighash_cache = nullptr) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL, sighash_cache), store(storeIn), m_signature_cache(signature_cache) {}
     46 
     47     bool VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const override;
     48     bool VerifySchnorrSignature(std::span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override;
     49diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
     50index af36a956931..d2f97f47461 100644
     51--- a/src/test/txvalidationcache_tests.cpp
     52+++ b/src/test/txvalidationcache_tests.cpp
     53@@ -24,7 +24,8 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
     54                        const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
     55                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
     56                        ValidationCache& validation_cache,
     57-                       std::vector<CScriptCheck>* pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     58+                       std::vector<CScriptCheck>* pvChecks,
     59+                       bool cache_sighash = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     60 
     61 BOOST_AUTO_TEST_SUITE(txvalidationcache_tests)
     62 
     63diff --git a/src/validation.cpp b/src/validation.cpp
     64index 5ad2ebdcd7e..58bdc8baaf3 100644
     65--- a/src/validation.cpp
     66+++ b/src/validation.cpp
     67@@ -142,7 +142,8 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
     68                        const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
     69                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
     70                        ValidationCache& validation_cache,
     71-                       std::vector<CScriptCheck>* pvChecks = nullptr)
     72+                       std::vector<CScriptCheck>* pvChecks = nullptr,
     73+                       bool cache_sighash = false)
     74                        EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     75 
     76 bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx)
     77@@ -1237,13 +1238,13 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws)
     78 
     79     // Check input scripts and signatures.
     80     // This is done last to help prevent CPU exhaustion denial-of-service attacks.
     81-    if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
     82+    if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata, GetValidationCache(), /* pvChecks= */ nullptr, /* cache_sighash= */ true)) {
     83         // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we
     84         // need to turn both off, and compare against just turning off CLEANSTACK
     85         // to see if the failure is specifically due to witness validation.
     86         TxValidationState state_dummy; // Want reported failures to be from first CheckInputScripts
     87-        if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata, GetValidationCache()) &&
     88-                !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata, GetValidationCache())) {
     89+        if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata, GetValidationCache(), /* pvChecks= */ nullptr, /* cache_sighash= */ true) &&
     90+                !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata, GetValidationCache(), /* pvChecks= */ nullptr, /* cache_sighash= */ true)) {
     91             // Only the witness is missing, so the transaction itself may be fine.
     92             state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED,
     93                     state.GetRejectReason(), state.GetDebugMessage());
     94@@ -2119,7 +2120,7 @@ std::optional<std::pair<ScriptError, std::string>> CScriptCheck::operator()() {
     95     const CScript &scriptSig = ptxTo->vin[nIn].scriptSig;
     96     const CScriptWitness *witness = &ptxTo->vin[nIn].scriptWitness;
     97     ScriptError error{SCRIPT_ERR_UNKNOWN_ERROR};
     98-    if (VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *m_signature_cache, *txdata), &error)) {
     99+    if (VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *m_signature_cache, *txdata, m_sighash_cache.get()), &error)) {
    100         return std::nullopt;
    101     } else {
    102         auto debug_str = strprintf("input %i of %s (wtxid %s), spending %s:%i", nIn, ptxTo->GetHash().ToString(), ptxTo->GetWitnessHash().ToString(), ptxTo->vin[nIn].prevout.hash.ToString(), ptxTo->vin[nIn].prevout.n);
    103@@ -2166,7 +2167,8 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
    104                        const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
    105                        bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
    106                        ValidationCache& validation_cache,
    107-                       std::vector<CScriptCheck>* pvChecks)
    108+                       std::vector<CScriptCheck>* pvChecks,
    109+                       bool cache_sighash)
    110 {
    111     if (tx.IsCoinBase()) return true;
    112 
    113diff --git a/src/validation.h b/src/validation.h
    114index 7411ab400c2..842e3f0d28a 100644
    115--- a/src/validation.h
    116+++ b/src/validation.h
    117@@ -338,10 +338,11 @@ private:
    118     bool cacheStore;
    119     PrecomputedTransactionData *txdata;
    120     SignatureCache* m_signature_cache;
    121+    std::unique_ptr<SigHashCache> m_sighash_cache;
    122 
    123 public:
    124-    CScriptCheck(const CTxOut& outIn, const CTransaction& txToIn, SignatureCache& signature_cache, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
    125-        m_tx_out(outIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), txdata(txdataIn), m_signature_cache(&signature_cache) { }
    126+    CScriptCheck(const CTxOut& outIn, const CTransaction& txToIn, SignatureCache& signature_cache, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn, bool cache_sighash = false) :
    127+        m_tx_out(outIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), txdata(txdataIn), m_signature_cache(&signature_cache), m_sighash_cache(cache_sighash ? std::make_unique<SigHashCache>() : nullptr) { }
    128 
    129     CScriptCheck(const CScriptCheck&) = delete;
    130     CScriptCheck& operator=(const CScriptCheck&) = delete;
    
  7. in src/script/interpreter.cpp:1609 in cbf10305d2 outdated
    1577+                return uint256::ONE;
    1578+            }
    1579+        }
    1580+    }
    1581+
    1582+    HashWriter ss{};
    


    theuni commented at 5:58 pm on May 13, 2025:
    HashWriter ss{} in the if() branch below now shadows this one. Rename to avoid confusion (and warnings) ?

    sipa commented at 6:29 pm on May 13, 2025:
    Done.
  8. instagibbs commented at 6:14 pm on May 13, 2025: member
    @darosior just noting the tests are specifically written to not worry about standardness, so at least these would have to be augmented to cover the cases this PR is ostensibly supporting
  9. sipa commented at 6:21 pm on May 13, 2025: member
    @instagibbs Yes and no, depending on the meaning of standardness I guess. The worrisome transactions are non-standard (in the sense that they are not relayed/accepted), but are standard in the IsStandard() sense, and thus not immediately rejected before script execution even begins.
  10. sipa force-pushed on May 13, 2025
  11. in test/functional/feature_taproot.py:1244 in 902fdf31e7 outdated
    1233@@ -1207,6 +1234,70 @@ def predict_sigops_ratio(n, dummy_size):
    1234                 standard = hashtype in VALID_SIGHASHES_ECDSA and (p2sh or witv0)
    1235                 add_spender(spenders, "compat/nocsa", hashtype=hashtype, p2sh=p2sh, witv0=witv0, standard=standard, script=CScript([OP_IF, OP_11, pubkey1, OP_CHECKSIGADD, OP_12, OP_EQUAL, OP_ELSE, pubkey1, OP_CHECKSIG, OP_ENDIF]), key=eckey1, sigops_weight=4-3*witv0, inputs=[getter("sign"), b''], failure={"inputs": [getter("sign"), b'\x01']}, **ERR_BAD_OPCODE)
    1236 
    1237+    # == sighash caching tests ==
    1238+
    1239+    # Sighash caching in legacy.
    1240+    for p2sh in [False, True]:
    


    instagibbs commented at 6:44 pm on May 13, 2025:
    scriptsig-size failures are being hit for non-witness cases, which I think would stop caching?

    sipa commented at 8:57 pm on July 9, 2025:
    Fixed.

    instagibbs commented at 6:16 pm on July 11, 2025:
    looks right, 20 maximally sized sigs should fit under limits
  12. theuni commented at 8:16 pm on May 13, 2025: member

    Hmm, not relevant to what’s being addressed here, but couldn’t the per-tx cache->hashPrevouts and cache->hashSequence be replaced with a similar midstate cache trick?

    Edit: Given their size I guess that’s not nearly as interesting.

  13. tests: add sighash caching tests to feature_taproot 97165111ab
  14. script: (refactor) prepare for introducing sighash midstate cache 352a0ee832
  15. script: (optimization) introduce sighash midstate caching 2d5615367b
  16. sipa force-pushed on Jul 9, 2025
  17. sipa commented at 9:02 pm on July 9, 2025: member
    @darosior I did it slightly differently, but added a commit that makes the use of the sighash cache optional. I avoided using default values as they can result in unexpected/missed call sites.
  18. sipa force-pushed on Jul 9, 2025
  19. DrahtBot added the label CI failed on Jul 9, 2025
  20. DrahtBot commented at 9:32 pm on July 9, 2025: contributor

    🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/runs/45674313491 LLM reason (✨ experimental): Clang-tidy reported argument comment mismatches caused by warnings treated as errors, leading to build failure.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  21. sipa force-pushed on Jul 10, 2025
  22. sipa force-pushed on Jul 11, 2025
  23. sipa force-pushed on Jul 11, 2025
  24. script: (belt-and-suspenders) make sighash cache optional fb7e9decf3
  25. sipa force-pushed on Jul 11, 2025
  26. DrahtBot removed the label CI failed on Jul 11, 2025
  27. in test/functional/feature_taproot.py:181 in fb7e9decf3
    178     """Return a callable that evaluates name in its passed context."""
    179-    return lambda ctx: get(ctx, name)
    180+    if len(kwargs) == 0:
    181+        return lambda ctx: get(ctx, name)
    182+    else:
    183+        return lambda ctx: get({**ctx, **kwargs}, name)
    


    darosior commented at 3:06 pm on July 21, 2025:

    nit: no need to special-case when kwargs is empty since ctx == {**ctx, **{}}.

    0    return lambda ctx: get({**ctx, **kwargs}, name)
    
  28. in test/functional/feature_taproot.py:328 in fb7e9decf3
    324+    mode = get(ctx, "mode")
    325+    hashtype_actual = get(ctx, "hashtype_actual")
    326+    if mode != "taproot" or hashtype_actual != 0:
    327+        return bytes([hashtype_actual])
    328+    else:
    329+        return bytes()
    


    darosior commented at 7:04 pm on July 21, 2025:
    nit: why is that needed? Looks like it may have been necessary for an earlier version of the test that had a 0 sighash type in the ops list for the legacy signature caching test, but it is not necessary anymore.
  29. in src/script/interpreter.cpp:1573 in fb7e9decf3
    1568+{
    1569+    // Note that we do not distinguish between BASE and WITNESS_V0 to determine the cache index,
    1570+    // because no input can simultaneously use both.
    1571+    return 3 * !!(hash_type & SIGHASH_ANYONECANPAY) +
    1572+           2 * ((hash_type & 0x1f) == SIGHASH_SINGLE) +
    1573+           1 * ((hash_type & 0x1f) == SIGHASH_NONE);
    


    darosior commented at 7:56 pm on July 21, 2025:

    Various modifications of this code does not make any test fail. For instance:

     0diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
     1index 528d8851d5f..81fbc4ab151 100644
     2--- a/src/script/interpreter.cpp
     3+++ b/src/script/interpreter.cpp
     4@@ -1569,8 +1569,7 @@ int SigHashCache::CacheIndex(int32_t hash_type) const noexcept
     5     // Note that we do not distinguish between BASE and WITNESS_V0 to determine the cache index,
     6     // because no input can simultaneously use both.
     7     return 3 * !!(hash_type & SIGHASH_ANYONECANPAY) +
     8-           2 * ((hash_type & 0x1f) == SIGHASH_SINGLE) +
     9-           1 * ((hash_type & 0x1f) == SIGHASH_NONE);
    10+           2 * ((hash_type & 0x1f) == SIGHASH_SINGLE);
    11 }
    12 
    13 bool SigHashCache::Load(int32_t hash_type, const CScript& script_code, HashWriter& writer) const noexcept
    
  30. darosior commented at 4:03 pm on July 22, 2025: member

    I’ve been playing with this code for the past couple of days and this is looking good. I wrote a unit test which sanity checks the caching and notably exercises false positive cache hits, which appears to not currently be covered (see #32473 (review)): https://github.com/darosior/bitcoin/commit/f303c6d68b5b9b3b23af34c8ef9efa7822fbe38b. I ran IBD on mainnet and signet with caching for blocks enabled and it did not result in any historical block being rejected on either of these networks. I’ll do testnet3 and testnet4 next.

    I’m now looking into adding some basic fuzzing coverage that computing the sighash with or without a cache is equivalent. I’ll also be running two mainnet nodes on the same machine, one with caching and one without. Since about 5% of all inputs are legacy or p2wsh multisigs, i’m curious to see if it makes any difference.

  31. darosior commented at 5:16 pm on July 23, 2025: member

    Here is a fuzz test which asserts the cached and not cached versions of the sighash function always return the same result: https://github.com/bitcoin/bitcoin/commit/3c056d766d6311e4120d43705b53059e3b1b0bb1. Running it for a while here did not reveal anything.

    I ran IBD on mainnet and signet with caching for blocks enabled and it did not result in any historical block being rejected on either of these networks. I’ll do testnet3 and testnet4 next.

    Done that and it didn’t cause any issue either. (Stopped at December 2024 for testnet3.)

  32. DrahtBot added the label Needs rebase on Jul 25, 2025
  33. DrahtBot commented at 9:55 pm on July 25, 2025: contributor
    🐙 This pull request conflicts with the target branch and needs rebase.

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-26 12:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me