Merge #6826: refactor: introduce SignHash type for improved type safety in LLMQ signing

fd6ad86 refactor: Use uniform initialization syntax for SignHash (pasta)
b34fd1b chore: resolve clang-format issue (pasta)
ac9e5b7 refactor: standardize SignHash usage across LLMQ components (pasta)
f446223 refactor: leverage SignHash type throughout LLMQ signing code (pasta)
0465463 refactor: extract BuildSignHash to SignHash type for better static typing (pasta)

Pull request description:

  ## Summary

  This PR introduces a dedicated `SignHash` type to replace the generic `uint256` usage for LLMQ sign hashes, improving type safety and code clarity throughout the signing subsystem.

  ## Motivation

  Previously, `BuildSignHash` was a standalone function returning `uint256`, which:
  - Required forward declarations to avoid circular dependencies
  - Lacked type safety (any uint256 could be passed where a sign hash was expected)
  - Made code intent less clear

  ## Changes

  ### New SignHash Type
  - Created `SignHash` class extending `BaseHash<uint256>` (following existing patterns like `AssumeutxoHash`)
  - Encapsulates the hash calculation logic in the constructor
  - Provides serialization support for database compatibility
  - Includes `std::hash` specialization for use as unordered_map keys

  ### Refactored APIs
  - `CSigBase::buildSignHash()` now returns `SignHash` instead of `uint256`
  - Sign hash calculation moved from standalone function to `SignHash` constructor
  - Removed forward declarations from `assetlocktx.cpp` and `quorums.cpp`

  ### Type Safety Improvements
  - SignHash is now a distinct type throughout LLMQ code
  - Conversion to `uint256` via `.Get()` only at API boundaries (BLS verification, database operations)
  - Maintains clean abstraction - BLS library continues using `uint256` as it shouldn't depend on higher-level LLMQ types

  ## Benefits

  - **Elimination of circular dependencies** - No more forward declarations needed
  - **Better type safety** - Compiler catches type mismatches
  - **Clearer code intent** - Explicit when working with sign hashes vs other hashes
  - **Future flexibility** - Can add SignHash-specific methods if needed
  - **Zero runtime overhead** - All changes are compile-time type safety improvements

  ## Test plan

  - [x] Unit tests pass (`test_dash --run_test=evo_islock_tests`)
  - [x] Code compiles without warnings
  - [x] No circular dependencies introduced

  🤖 Generated with [Claude Code](https://claude.ai/code)

ACKs for top commit:
  UdjinM6:
    utACK fd6ad86

Tree-SHA512: dc4c54ce4154cebceaeca48e79fa8cf0fdeee2cd3025fa0c05f84b130dffe993af03bca0f80f9c3dc950bf0b2cd3986e706f5bb4824e9cf03058c97b98aaa1d5

Author: PastaPastaPasta

src/Makefile.am

@@ -255,6 +255,7 @@ BITCOIN_CORE_H = \
   llmq/options.h \
   llmq/params.h \
   llmq/quorums.h \
+  llmq/signhash.h \
   llmq/signing.h \
   llmq/signing_shares.h \
   llmq/snapshot.h \
@@ -517,6 +518,7 @@ libbitcoin_node_a_SOURCES = \
   llmq/ehf_signals.cpp \
   llmq/options.cpp \
   llmq/quorums.cpp \
+  llmq/signhash.cpp \
   llmq/signing.cpp \
   llmq/signing_shares.cpp \
   llmq/snapshot.cpp \

src/evo/assetlocktx.cpp

@@ -7,6 +7,7 @@
 
 #include <llmq/commitment.h>
 #include <llmq/quorums.h>
+#include <llmq/signhash.h>
 
 #include <chainparams.h>
 #include <consensus/params.h>
@@ -21,10 +22,6 @@
 
 using node::BlockManager;
 
-namespace llmq {
-// forward declaration to avoid circular dependency
-uint256 BuildSignHash(Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, const uint256& msgHash);
-} // namespace llmq
 
 /**
  *  Common code for Asset Lock and Asset Unlock
@@ -149,8 +146,8 @@ bool CAssetUnlockPayload::VerifySig(const llmq::CQuorumManager& qman, const uint
 
     const uint256 requestId = ::SerializeHash(std::make_pair(ASSETUNLOCK_REQUESTID_PREFIX, index));
 
-    if (const uint256 signHash = llmq::BuildSignHash(llmqType, quorum->qc->quorumHash, requestId, msgHash);
-            quorumSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash)) {
+    if (const llmq::SignHash signHash(llmqType, quorum->qc->quorumHash, requestId, msgHash);
+        quorumSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get())) {
         return true;
     }
 

src/evo/mnhftx.cpp

@@ -9,6 +9,7 @@
 #include <evo/specialtx.h>
 #include <llmq/commitment.h>
 #include <llmq/quorums.h>
+#include <llmq/signhash.h>
 #include <llmq/signing.h>
 #include <node/blockstorage.h>
 
@@ -102,8 +103,8 @@ bool MNHFTx::Verify(const llmq::CQuorumManager& qman, const uint256& quorumHash,
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-missing-quorum");
     }
 
-    const uint256 signHash = llmq::BuildSignHash(llmqType, quorum->qc->quorumHash, requestId, msgHash);
-    if (!sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash)) {
+    const llmq::SignHash signHash{llmqType, quorum->qc->quorumHash, requestId, msgHash};
+    if (!sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get())) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-invalid");
     }
 

src/instantsend/instantsend.cpp

@@ -17,6 +17,7 @@
 #include <instantsend/signing.h>
 #include <llmq/commitment.h>
 #include <llmq/quorums.h>
+#include <llmq/signhash.h>
 #include <masternode/sync.h>
 #include <spork.h>
 #include <stats/client.h>
@@ -282,7 +283,7 @@ std::unordered_set<uint256, StaticSaltedHasher> CInstantSendManager::ProcessPend
             // should not happen, but if one fails to select, all others will also fail to select
             return {};
         }
-        uint256 signHash = BuildSignHash(llmq_params.type, quorum->qc->quorumHash, id, islock->txid);
+        uint256 signHash = llmq::SignHash{llmq_params.type, quorum->qc->quorumHash, id, islock->txid}.Get();
         batchVerifier.PushMessage(nodeId, hash, signHash, islock->sig.Get(), quorum->qc->quorumPublicKey);
         verifyCount++;
 

src/llmq/quorums.cpp

@@ -2,12 +2,13 @@
 // Distributed under the MIT/X11 software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <llmq/quorums.h>
-#include <llmq/commitment.h>
 #include <llmq/blockprocessor.h>
+#include <llmq/commitment.h>
 #include <llmq/dkgsessionmgr.h>
 #include <llmq/options.h>
 #include <llmq/params.h>
+#include <llmq/quorums.h>
+#include <llmq/signhash.h>
 #include <llmq/utils.h>
 
 #include <bls/bls.h>
@@ -36,8 +37,6 @@ static const std::string DB_QUORUM_QUORUM_VVEC = "q_Qqvvec";
 RecursiveMutex cs_data_requests;
 static std::unordered_map<CQuorumDataRequestKey, CQuorumDataRequest, StaticSaltedHasher> mapQuorumDataRequests GUARDED_BY(cs_data_requests);
 
-// forward declaration to avoid circular dependency
-uint256 BuildSignHash(Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, const uint256& msgHash);
 
 static uint256 MakeQuorumKey(const CQuorum& q)
 {
@@ -1279,8 +1278,8 @@ VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain
         return VerifyRecSigStatus::NoQuorum;
     }
 
-    uint256 signHash = BuildSignHash(llmqType, quorum->qc->quorumHash, id, msgHash);
-    const bool ret = sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash);
+    SignHash signHash{llmqType, quorum->qc->quorumHash, id, msgHash};
+    const bool ret = sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get());
     return ret ? VerifyRecSigStatus::Valid : VerifyRecSigStatus::Invalid;
 }
 } // namespace llmq

src/llmq/signhash.cpp

@@ -0,0 +1,24 @@
+// Copyright (c) 2025 The Dash Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <llmq/signhash.h>
+
+#include <hash.h>
+#include <serialize.h>
+
+#include <vector>
+
+namespace llmq {
+
+SignHash::SignHash(Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, const uint256& msgHash)
+{
+    CHashWriter h(SER_GETHASH, 0);
+    h << llmqType;
+    h << quorumHash;
+    h << id;
+    h << msgHash;
+    m_hash = h.GetHash();
+}
+
+} // namespace llmq

src/llmq/signhash.h

@@ -0,0 +1,76 @@
+// Copyright (c) 2025 The Dash Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_LLMQ_SIGNHASH_H
+#define BITCOIN_LLMQ_SIGNHASH_H
+
+#include <llmq/params.h>
+#include <uint256.h>
+
+#include <cstring>
+#include <vector>
+
+#include <saltedhasher.h>
+#include <util/hash_type.h>
+
+namespace llmq {
+
+/**
+ * SignHash is a strongly-typed wrapper for the hash used in LLMQ signing operations.
+ * It encapsulates the hash calculation for quorum signatures, replacing the need for
+ * BuildSignHash function and avoiding circular dependencies.
+ */
+class SignHash : public BaseHash<uint256>
+{
+public:
+    SignHash() = default;
+    using BaseHash<uint256>::BaseHash;
+
+    /**
+     * Constructs a SignHash from the given parameters.
+     * This replaces the previous BuildSignHash function.
+     */
+    SignHash(Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, const uint256& msgHash);
+
+    /**
+     * Get the underlying uint256 hash value.
+     */
+    const uint256& Get() const { return m_hash; }
+
+    // Serialization support
+    template <typename Stream>
+    void Serialize(Stream& s) const
+    {
+        s << m_hash;
+    }
+
+    template <typename Stream>
+    void Unserialize(Stream& s)
+    {
+        s >> m_hash;
+    }
+};
+
+// Salted hasher for llmq::SignHash that reuses the salted hasher for uint256
+struct SignHashSaltedHasher {
+    std::size_t operator()(const SignHash& signHash) const noexcept { return StaticSaltedHasher{}(signHash.Get()); }
+};
+
+} // namespace llmq
+
+// Make SignHash hashable for use in unordered_map
+template <>
+struct std::hash<llmq::SignHash> {
+    std::size_t operator()(const llmq::SignHash& signHash) const noexcept
+    {
+        // Use the first 8 bytes of the hash as the hash value
+        const unsigned char* data = signHash.data();
+        std::size_t result;
+        std::memcpy(&result, data, sizeof(result));
+        return result;
+    }
+};
+
+
+#endif // BITCOIN_LLMQ_SIGNHASH_H

src/llmq/signing.cpp

@@ -6,6 +6,7 @@
 
 #include <llmq/commitment.h>
 #include <llmq/quorums.h>
+#include <llmq/signhash.h>
 #include <llmq/signing_shares.h>
 
 #include <bls/bls_batchverifier.h>
@@ -161,7 +162,7 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig)
 
     // store by signHash
     auto signHash = recSig.buildSignHash();
-    auto k4 = std::make_tuple(std::string("rs_s"), signHash);
+    auto k4 = std::make_tuple(std::string("rs_s"), signHash.Get());
     batch.Write(k4, (uint8_t)1);
 
     // store by current time. Allows fast cleanup of old recSigs
@@ -173,7 +174,7 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig)
     {
         LOCK(cs_cache);
         hasSigForIdCache.insert(std::make_pair(recSig.getLlmqType(), recSig.getId()), true);
-        hasSigForSessionCache.insert(signHash, true);
+        hasSigForSessionCache.insert(signHash.Get(), true);
         hasSigForHashCache.insert(recSig.GetHash(), true);
     }
 }
@@ -190,7 +191,7 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
     auto k1 = std::make_tuple(std::string("rs_r"), recSig.getLlmqType(), recSig.getId());
     auto k2 = std::make_tuple(std::string("rs_r"), recSig.getLlmqType(), recSig.getId(), recSig.getMsgHash());
     auto k3 = std::make_tuple(std::string("rs_h"), recSig.GetHash());
-    auto k4 = std::make_tuple(std::string("rs_s"), signHash);
+    auto k4 = std::make_tuple(std::string("rs_s"), signHash.Get());
     batch.Erase(k1);
     batch.Erase(k2);
     if (deleteHashKey) {
@@ -211,7 +212,7 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
 
     LOCK(cs_cache);
     hasSigForIdCache.erase(std::make_pair(recSig.getLlmqType(), recSig.getId()));
-    hasSigForSessionCache.erase(signHash);
+    hasSigForSessionCache.erase(signHash.Get());
     if (deleteHashKey) {
         hasSigForHashCache.erase(recSig.GetHash());
     }
@@ -469,7 +470,7 @@ void CSigningManager::CollectPendingRecoveredSigsToVerify(
 
             bool alreadyHave = db.HasRecoveredSigForHash(recSig->GetHash());
             if (!alreadyHave) {
-                uniqueSignHashes.emplace(nodeId, recSig->buildSignHash());
+                uniqueSignHashes.emplace(nodeId, recSig->buildSignHash().Get());
                 retSigShares[nodeId].emplace_back(recSig);
             }
             ns.erase(ns.begin());
@@ -553,7 +554,8 @@ bool CSigningManager::ProcessPendingRecoveredSigs(PeerManager& peerman)
             }
 
             const auto& quorum = quorums.at(std::make_pair(recSig->getLlmqType(), recSig->getQuorumHash()));
-            batchVerifier.PushMessage(nodeId, recSig->GetHash(), recSig->buildSignHash(), recSig->sig.Get(), quorum->qc->quorumPublicKey);
+            batchVerifier.PushMessage(nodeId, recSig->GetHash(), recSig->buildSignHash().Get(), recSig->sig.Get(),
+                                      quorum->qc->quorumPublicKey);
             verifyCount++;
         }
     }
@@ -605,7 +607,7 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecovered
         CRecoveredSig otherRecoveredSig;
         if (db.GetRecoveredSigById(llmqType, recoveredSig->getId(), otherRecoveredSig)) {
             auto otherSignHash = otherRecoveredSig.buildSignHash();
-            if (signHash != otherSignHash) {
+            if (signHash.Get() != otherSignHash.Get()) {
                 // this should really not happen, as each masternode is participating in only one vote,
                 // even if it's a member of multiple quorums. so a majority is only possible on one quorum and one msgHash per id
                 LogPrintf("CSigningManager::%s -- conflicting recoveredSig for signHash=%s, id=%s, msgHash=%s, otherSignHash=%s\n", __func__,
@@ -836,20 +838,8 @@ void CSigningManager::WorkThreadMain(PeerManager& peerman)
     }
 }
 
-uint256 CSigBase::buildSignHash() const
-{
-    return BuildSignHash(llmqType, quorumHash, id, msgHash);
-}
+SignHash CSigBase::buildSignHash() const { return SignHash(llmqType, quorumHash, id, msgHash); }
 
-uint256 BuildSignHash(Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, const uint256& msgHash)
-{
-    CHashWriter h(SER_GETHASH, 0);
-    h << llmqType;
-    h << quorumHash;
-    h << id;
-    h << msgHash;
-    return h.GetHash();
-}
 
 bool IsQuorumActive(Consensus::LLMQType llmqType, const CQuorumManager& qman, const uint256& quorumHash)
 {

src/llmq/signing.h

@@ -8,13 +8,14 @@
 #include <bls/bls.h>
 #include <gsl/pointers.h>
 #include <llmq/params.h>
+#include <llmq/signhash.h>
 #include <net_types.h>
 #include <protocol.h>
 #include <random.h>
 #include <saltedhasher.h>
 #include <sync.h>
-#include <util/threadinterrupt.h>
 #include <unordered_lru_cache.h>
+#include <util/threadinterrupt.h>
 
 #include <string_view>
 #include <unordered_map>
@@ -67,7 +68,7 @@ class CSigBase
         return msgHash;
     }
 
-    [[nodiscard]] uint256 buildSignHash() const;
+    [[nodiscard]] SignHash buildSignHash() const;
 };
 
 class CRecoveredSig : virtual public CSigBase
@@ -263,8 +264,6 @@ void IterateNodesRandom(NodesContainer& nodeStates, Continue&& cont, Callback&&
     }
 }
 
-uint256 BuildSignHash(Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, const uint256& msgHash);
-
 bool IsQuorumActive(Consensus::LLMQType llmqType, const CQuorumManager& qman, const uint256& quorumHash);
 
 } // namespace llmq