Merge ElementsProject/elements#1604: [master] sigcache: harden range proof cache keys and add -norangeproofcache option

### src/init.cpp
@@ -636,6 +636,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-mocktime=<n>", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_VALIDATION_CACHE_BYTES >> 20), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
+    argsman.AddArg("-rangeproofcache", strprintf("Enable the range proof validation cache (default: %u). Use -norangeproofcache to disable.", 1), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-maxtipage=<n>",
                    strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)",
                              Ticks<std::chrono::seconds>(DEFAULT_MAX_TIP_AGE)),

### src/script/sigcache.cpp
@@ -5,7 +5,9 @@
 
 #include <script/sigcache.h>
 
+#include <common/args.h>
 #include <crypto/sha256.h>
+#include <hash.h>
 #include <logging.h>
 #include <pubkey.h>
 #include <random.h>
@@ -20,10 +22,8 @@
 SignatureCache::SignatureCache(const size_t max_size_bytes)
 {
     uint256 nonce = GetRandHash();
-    // We want the nonce to be 64 bytes long to force the hasher to process
-    // this chunk, which makes later hash computations more efficient. We
-    // just write our 32-byte entropy, and then pad with 'E' for ECDSA and
-    // 'S' for Schnorr (followed by 0 bytes).
+    // Use 64-byte, type-specific salted midstates so later hash computations
+    // can start after the first SHA256 chunk.
     static constexpr unsigned char PADDING_ECDSA[32] = {'E'};
     static constexpr unsigned char PADDING_SCHNORR[32] = {'S'};
     static constexpr unsigned char PADDING_RANGE_PROOF[32] = {'r'};
@@ -32,10 +32,8 @@ SignatureCache::SignatureCache(const size_t max_size_bytes)
     m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
     m_salted_hasher_schnorr.Write(nonce.begin(), 32);
     m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
-    m_salted_hasher_range_proof.Write(nonce.begin(), 32);
-    m_salted_hasher_range_proof.Write(PADDING_RANGE_PROOF, 32);
-    m_salted_hasher_surjection_proof.Write(nonce.begin(), 32);
-    m_salted_hasher_surjection_proof.Write(PADDING_SURJECTION_PROOF, 32);
+    m_salted_hasher_range_proof << nonce << PADDING_RANGE_PROOF;
+    m_salted_hasher_surjection_proof << nonce << PADDING_SURJECTION_PROOF;
 
     const auto [num_elems, approx_size_bytes] = setValid.setup_bytes(max_size_bytes);
     LogPrintf("Using %zu MiB out of %zu MiB requested for signature cache, able to store %zu elements\n",
@@ -55,13 +53,41 @@ void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, Sp
 }
 
 // ELEMENTS:
-void SignatureCache::ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const {
-    CSHA256 hasher = m_salted_hasher_range_proof;
-    hasher.Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Write(asset_commitment.data(), asset_commitment.size()).Write(scriptPubKey.data(), scriptPubKey.size()).Finalize(entry.begin());
+void SignatureCache::ComputeEntryRangeProof(uint256& entry,
+                                            const std::vector<unsigned char>& proof,
+                                            const std::vector<unsigned char>& commitment,
+                                            const std::vector<unsigned char>& asset_commitment,
+                                            const CScript& script_pub_key) const
+{
+    HashWriter hasher = m_salted_hasher_range_proof;
+    // We commit to both commitments and the scriptPubKey because these are
+    // committed to by the rangeproof itself; a change in any of them would
+    // invalidate the proof. Since these are exactly the arguments to
+    // CachingRangeProofChecker::VerifyRangeProof (below), there is no
+    // additional data that could affect the rangeproof's validity.
+    // Serialization length-prefixes every field, including the variable-length
+    // proof and script, so distinct argument tuples cannot share an encoding.
+    hasher << proof << commitment << asset_commitment << script_pub_key;
+    entry = hasher.GetSHA256();
 }
-void SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const {
-    CSHA256 hasher = m_salted_hasher_surjection_proof;
-    hasher.Write(hash.begin(), 32).Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Finalize(entry.begin());
+
+void SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<secp256k1_generator>& vTags) const
+{
+    HashWriter hasher = m_salted_hasher_surjection_proof;
+    // We hash all arguments passed to CachingSurjectionProofChecker::VerifySurjectionProof,
+    // to ensure that any change in the way that the verification function is called will
+    // trigger a cache miss and explicit verification. However, we note that the `wtxid`
+    // (hash) commits to all the other data such that we could technically hash only it.
+    // We retain the other data as a defense against future refactorings.
+    //
+    // Serialize vTags as a flat byte vector (each secp256k1_generator is 64 bytes).
+    std::vector<unsigned char> vTagsBytes;
+    vTagsBytes.reserve(vTags.size() * 64);
+    for (const auto& tag : vTags) {
+        vTagsBytes.insert(vTagsBytes.end(), std::begin(tag.data), std::end(tag.data));
+    }
+    hasher << hash << proof << commitment << vTagsBytes;
+    entry = hasher.GetSHA256();
 }
 
 bool SignatureCache::Get(const uint256& entry, const bool erase)
@@ -109,6 +135,10 @@ namespace {
 // To be called once in AppInit2/TestingSetup to initialize the rangeproof cache
 bool InitRangeproofCache(size_t max_size_bytes)
 {
+    if (!gArgs.GetBoolArg("-rangeproofcache", true)) {
+        LogPrintf("Range proof cache disabled via -norangeproofcache\n");
+        return true;
+    }
     auto setup_results = rangeProofCache.setup_bytes(max_size_bytes);
     if (!setup_results) return false;
     const auto [num_elems, approx_size_bytes] = *setup_results;
@@ -130,11 +160,18 @@ bool InitSurjectionproofCache(size_t max_size_bytes)
 
 bool CachingRangeProofChecker::VerifyRangeProof(const std::vector<unsigned char>& vchRangeProof, const std::vector<unsigned char>& vchValueCommitment, const std::vector<unsigned char>& vchAssetCommitment, const CScript& scriptPubKey, const secp256k1_context* secp256k1_ctx_verify_amounts) const
 {
+    // ELEMENTS: NOTE FOR FUTURE EDITORS: every argument to this function that
+    // carries data (i.e. everything except the secp256k1 context, which is
+    // stateless) MUST be included in ComputeEntryRangeProof. Omitting any
+    // argument risks returning a cached positive result for a proof that was
+    // verified with different inputs.
     uint256 entry;
-    rangeProofCache.ComputeEntryRangeProof(entry, vchRangeProof, vchValueCommitment, vchAssetCommitment, scriptPubKey);
-
-    if (rangeProofCache.Get(entry, !store)) {
-        return true;
+    const bool useCache = gArgs.GetBoolArg("-rangeproofcache", true);
+    if (useCache) {
+        rangeProofCache.ComputeEntryRangeProof(entry, vchRangeProof, vchValueCommitment, vchAssetCommitment, scriptPubKey);
+        if (rangeProofCache.Get(entry, !store)) {
+            return true;
+        }
     }
 
     if (vchRangeProof.size() == 0) {
@@ -163,7 +200,7 @@ bool CachingRangeProofChecker::VerifyRangeProof(const std::vector<unsigned char>
         return false;
     }
 
-    if (store) {
+    if (useCache && store) {
         rangeProofCache.Set(entry);
     }
 
@@ -182,7 +219,7 @@ bool CachingSurjectionProofChecker::VerifySurjectionProof(secp256k1_surjectionpr
     // wtxid commits to all data including surj targets
     // we need to specify the proof and output asset point to be unique
     uint256 entry;
-    surjectionProofCache.ComputeEntrySurjectionProof(entry, wtxid, vchproof, std::vector<unsigned char>(std::begin(gen.data), std::end(gen.data)));
+    surjectionProofCache.ComputeEntrySurjectionProof(entry, wtxid, vchproof, std::vector<unsigned char>(std::begin(gen.data), std::end(gen.data)), vTags);
 
     if (surjectionProofCache.Get(entry, !store)) {
         return true;
@@ -199,5 +236,24 @@ bool CachingSurjectionProofChecker::VerifySurjectionProof(secp256k1_surjectionpr
     return true;
 }
 
+// Test-only hooks (see sigcache.h). Forward to the anonymous-namespace caches.
+void TestComputeEntryRangeProof(uint256& entry,
+                                const std::vector<unsigned char>& proof,
+                                const std::vector<unsigned char>& commitment,
+                                const std::vector<unsigned char>& asset_commitment,
+                                const CScript& script_pub_key)
+{
+    rangeProofCache.ComputeEntryRangeProof(entry, proof, commitment, asset_commitment, script_pub_key);
+}
+
+void TestComputeEntrySurjectionProof(uint256& entry,
+                                     const uint256& hash,
+                                     const std::vector<unsigned char>& proof,
+                                     const std::vector<unsigned char>& commitment,
+                                     const std::vector<secp256k1_generator>& vTags)
+{
+    surjectionProofCache.ComputeEntrySurjectionProof(entry, hash, proof, commitment, vTags);
+}
+
 // END ELEMENTS
 //

### src/script/sigcache.h
@@ -9,6 +9,7 @@
 #include <consensus/amount.h>
 #include <crypto/sha256.h>
 #include <cuckoocache.h>
+#include <hash.h>
 #include <script/interpreter.h>
 #include <random.h>
 #include <span.h>
@@ -43,11 +44,11 @@ static_assert(DEFAULT_VALIDATION_CACHE_BYTES == DEFAULT_SIGNATURE_CACHE_BYTES +
 class SignatureCache
 {
 private:
-    //! Entries are SHA256(nonce || 'E' or 'S' || 31 zero bytes || signature hash || public key || signature):
+    //! Salted SHA256 midstates, domain-separated by signature or proof type.
     CSHA256 m_salted_hasher_ecdsa;
     CSHA256 m_salted_hasher_schnorr;
-    CSHA256 m_salted_hasher_range_proof;
-    CSHA256 m_salted_hasher_surjection_proof;
+    HashWriter m_salted_hasher_range_proof;
+    HashWriter m_salted_hasher_surjection_proof;
     typedef CuckooCache::cache<uint256, SignatureCacheHasher> map_type;
     map_type setValid;
     std::shared_mutex cs_sigcache;
@@ -56,10 +57,8 @@ class SignatureCache
     SignatureCache()
     {
         uint256 nonce = GetRandHash();
-        // We want the nonce to be 64 bytes long to force the hasher to process
-        // this chunk, which makes later hash computations more efficient. We
-        // just write our 32-byte entropy, and then pad with 'E' for ECDSA and
-        // 'S' for Schnorr (followed by 0 bytes).
+        // Use 64-byte, type-specific salted midstates so later hash computations
+        // can start after the first SHA256 chunk.
         static constexpr unsigned char PADDING_ECDSA[32] = {'E'};
         static constexpr unsigned char PADDING_SCHNORR[32] = {'S'};
         static constexpr unsigned char PADDING_RANGE_PROOF[32] = {'r'};
@@ -68,10 +67,8 @@ class SignatureCache
         m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32);
         m_salted_hasher_schnorr.Write(nonce.begin(), 32);
         m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32);
-        m_salted_hasher_range_proof.Write(nonce.begin(), 32);
-        m_salted_hasher_range_proof.Write(PADDING_RANGE_PROOF, 32);
-        m_salted_hasher_surjection_proof.Write(nonce.begin(), 32);
-        m_salted_hasher_surjection_proof.Write(PADDING_SURJECTION_PROOF, 32);
+        m_salted_hasher_range_proof << nonce << PADDING_RANGE_PROOF;
+        m_salted_hasher_surjection_proof << nonce << PADDING_SURJECTION_PROOF;
     }
 
     SignatureCache(size_t max_size_bytes);
@@ -84,9 +81,13 @@ class SignatureCache
     void ComputeEntrySchnorr(uint256& entry, const uint256 &hash, Span<const unsigned char> sig, const XOnlyPubKey& pubkey) const;
 
     // ELEMENTS:
-    void ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const;
+    void ComputeEntryRangeProof(uint256& entry,
+                                const std::vector<unsigned char>& proof,
+                                const std::vector<unsigned char>& commitment,
+                                const std::vector<unsigned char>& asset_commitment,
+                                const CScript& script_pub_key) const;
 
-    void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const;
+    void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<secp256k1_generator>& vTags) const;
 
     bool Get(const uint256& entry, const bool erase);
 
@@ -145,6 +146,20 @@ class CachingSurjectionProofChecker
 [[nodiscard]] bool InitRangeproofCache(size_t max_size_bytes);
 [[nodiscard]] bool InitSurjectionproofCache(size_t max_size_bytes);
 
+// Test-only hooks: expose the (anonymous-namespace) cache-entry computation so
+// unit tests can verify collision-resistance and domain separation. These are
+// NOT part of the consensus/validation API and are only used by unit tests.
+void TestComputeEntryRangeProof(uint256& entry,
+                                const std::vector<unsigned char>& proof,
+                                const std::vector<unsigned char>& commitment,
+                                const std::vector<unsigned char>& asset_commitment,
+                                const CScript& script_pub_key);
+void TestComputeEntrySurjectionProof(uint256& entry,
+                                     const uint256& hash,
+                                     const std::vector<unsigned char>& proof,
+                                     const std::vector<unsigned char>& commitment,
+                                     const std::vector<secp256k1_generator>& vTags);
+
 // END ELEMENTS
 //
 

### src/test/CMakeLists.txt
@@ -96,6 +96,7 @@ add_executable(test_elements
   serfloat_tests.cpp
   serialize_tests.cpp
   settings_tests.cpp
+  sigcache_tests.cpp
   sighash_tests.cpp
   sigopcount_tests.cpp
   skiplist_tests.cpp

### src/test/sigcache_tests.cpp
@@ -0,0 +1,164 @@
+// Copyright (c) 2026 The Elements developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+//
+// Tests for the Elements proof-cache entry computation (script/sigcache.cpp).
+//
+// These tests guard the collision-resistance and domain-separation properties
+// of the cache keys used for the range-proof and surjection-proof caches.
+// A cache entry is a *positive* verification result, so a key collision means
+// accepting a proof without ever verifying it. The keys are computed with
+// CHashWriter serialization, which length-prefixes every field, so two
+// distinct argument tuples must never produce the same cache entry, and the
+// two proof types must live in disjoint key spaces (domain separation).
+
+#include <script/sigcache.h>
+#include <test/util/setup_common.h>
+#include <uint256.h>
+
+#include <secp256k1_generator.h>
+
+#include <boost/test/unit_test.hpp>
+
+#include <vector>
+
+BOOST_FIXTURE_TEST_SUITE(sigcache_tests, BasicTestingSetup)
+
+// Two (proof, commitment) tuples whose concatenations would be byte-identical
+// under the OLD raw-CSHA256 scheme (proof=AB commitment=CD  vs  proof=ABC
+// commitment=D) must produce DIFFERENT cache entries under the new
+// length-prefixing scheme. This is the core collision-resistance property.
+BOOST_AUTO_TEST_CASE(rangeproof_entry_field_boundary)
+{
+    std::vector<unsigned char> proof_a   = {0xAA, 0xBB};
+    std::vector<unsigned char> commit_a  = {0xCC, 0xDD};
+    std::vector<unsigned char> asset_a   = {0x11};
+
+    std::vector<unsigned char> proof_b   = {0xAA, 0xBB, 0xCC};
+    std::vector<unsigned char> commit_b  = {0xDD};
+    std::vector<unsigned char> asset_b   = {0x11};
+
+    // Concatenations are identical: AA BB CC DD 11 == AA BB CC DD 11.
+    // Under the old raw-Write scheme these collided; they must not now.
+    CScript script;
+
+    uint256 entry_a, entry_b;
+    TestComputeEntryRangeProof(entry_a, proof_a, commit_a, asset_a, script);
+    TestComputeEntryRangeProof(entry_b, proof_b, commit_b, asset_b, script);
+
+    BOOST_CHECK(entry_a != entry_b);
+}
+
+// Same field-boundary property for the surjection-proof entry, varying the
+// proof vs commitment split with a fixed hash.
+BOOST_AUTO_TEST_CASE(surjectionproof_entry_field_boundary)
+{
+    uint256 hash = uint256S("0x1234");
+
+    std::vector<unsigned char> proof_a  = {0x01, 0x02};
+    std::vector<unsigned char> commit_a = {0x03, 0x04};
+
+    std::vector<unsigned char> proof_b  = {0x01, 0x02, 0x03};
+    std::vector<unsigned char> commit_b = {0x04};
+
+    std::vector<secp256k1_generator> vTags; // empty tags — same in both cases
+
+    uint256 entry_a, entry_b;
+    TestComputeEntrySurjectionProof(entry_a, hash, proof_a, commit_a, vTags);
+    TestComputeEntrySurjectionProof(entry_b, hash, proof_b, commit_b, vTags);
+
+    BOOST_CHECK(entry_a != entry_b);
+}
+
+// The scriptPubKey is variable-length and part of the range-proof key. Two
+// calls differing only in the script must produce different entries (an
+// attacker must not be able to replay a cached proof against a different
+// output script).
+BOOST_AUTO_TEST_CASE(rangeproof_entry_script_sensitivity)
+{
+    std::vector<unsigned char> proof  = {0x01, 0x02, 0x03};
+    std::vector<unsigned char> commit = {0x04, 0x05};
+    std::vector<unsigned char> asset  = {0x06};
+
+    CScript script_a;
+    script_a << OP_TRUE;
+    CScript script_b;
+    script_b << OP_FALSE;
+
+    uint256 entry_a, entry_b;
+    TestComputeEntryRangeProof(entry_a, proof, commit, asset, script_a);
+    TestComputeEntryRangeProof(entry_b, proof, commit, asset, script_b);
+
+    BOOST_CHECK(entry_a != entry_b);
+}
+
+// Domain separation: a range-proof tuple and a surjection-proof tuple must
+// never share a cache entry even when their byte content is arranged to look
+// similar. The two caches use distinct salted midstates ('r' vs 's'), so the
+// same logical content must hash differently across the two domains.
+BOOST_AUTO_TEST_CASE(proof_caches_domain_separation)
+{
+    uint256 hash = uint256S("0xabcd");
+    std::vector<unsigned char> proof  = {0x01, 0x02, 0x03};
+    std::vector<unsigned char> commit = {0x04, 0x05, 0x06};
+    std::vector<unsigned char> asset  = {0x07, 0x08};
+    CScript script;
+    std::vector<secp256k1_generator> vTags;
+
+    uint256 range_entry, surj_entry;
+    TestComputeEntryRangeProof(range_entry, proof, commit, asset, script);
+    TestComputeEntrySurjectionProof(surj_entry, hash, proof, commit, vTags);
+
+    // Different domains must not collide with each other's entries.
+    BOOST_CHECK(range_entry != surj_entry);
+}
+
+// Determinism: the same tuple must always produce the same entry within a
+// process (the salt is fixed per-process), which is what makes the cache
+// usable at all.
+BOOST_AUTO_TEST_CASE(entries_are_deterministic)
+{
+    std::vector<unsigned char> proof  = {0xde, 0xad};
+    std::vector<unsigned char> commit = {0xbe, 0xef};
+    std::vector<unsigned char> asset  = {0x00};
+    CScript script;
+    script << OP_RETURN;
+
+    uint256 e1, e2;
+    TestComputeEntryRangeProof(e1, proof, commit, asset, script);
+    TestComputeEntryRangeProof(e2, proof, commit, asset, script);
+    BOOST_CHECK(e1 == e2);
+
+    uint256 hash = uint256S("0x99");
+    std::vector<secp256k1_generator> vTags;
+    uint256 s1, s2;
+    TestComputeEntrySurjectionProof(s1, hash, proof, commit, vTags);
+    TestComputeEntrySurjectionProof(s2, hash, proof, commit, vTags);
+    BOOST_CHECK(s1 == s2);
+}
+
+// vTags sensitivity: two surjection-proof calls that differ only in vTags
+// must produce different cache entries (otherwise an attacker could cause
+// a cache hit for a proof verified with different input tags).
+BOOST_AUTO_TEST_CASE(surjectionproof_entry_vtags_sensitivity)
+{
+    uint256 hash = uint256S("0xdeadbeef");
+    std::vector<unsigned char> proof  = {0x01, 0x02};
+    std::vector<unsigned char> commit = {0x03, 0x04};
+
+    secp256k1_generator gen_a, gen_b;
+    // Fill with distinct byte patterns.
+    memset(gen_a.data, 0xAA, sizeof(gen_a.data));
+    memset(gen_b.data, 0xBB, sizeof(gen_b.data));
+
+    std::vector<secp256k1_generator> vTags_a = {gen_a};
+    std::vector<secp256k1_generator> vTags_b = {gen_b};
+
+    uint256 entry_a, entry_b;
+    TestComputeEntrySurjectionProof(entry_a, hash, proof, commit, vTags_a);
+    TestComputeEntrySurjectionProof(entry_b, hash, proof, commit, vTags_b);
+
+    BOOST_CHECK(entry_a != entry_b);
+}
+
+BOOST_AUTO_TEST_SUITE_END()