Skip to content

Commit 75d6cf4

Browse files
refactor: leverage SignHash type throughout LLMQ signing code
Extends usage of the SignHash type to improve type safety: - CSigBase::buildSignHash() now returns SignHash instead of uint256 - Added serialization support for database storage compatibility - Added std::hash specialization for use in unordered_map - Use SignHash directly where possible, only converting to uint256 at API boundaries (BLS verification, database operations) This provides better type safety while maintaining clean abstraction boundaries - the BLS library continues to work with uint256 as it should not depend on higher-level LLMQ types. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 851e9bc commit 75d6cf4

File tree

8 files changed

+51
-24
lines changed

8 files changed

+51
-24
lines changed

src/evo/assetlocktx.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ bool CAssetUnlockPayload::VerifySig(const llmq::CQuorumManager& qman, const uint
146146

147147
const uint256 requestId = ::SerializeHash(std::make_pair(ASSETUNLOCK_REQUESTID_PREFIX, index));
148148

149-
if (const uint256 signHash = llmq::SignHash(llmqType, quorum->qc->quorumHash, requestId, msgHash).Get();
150-
quorumSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash)) {
149+
if (const llmq::SignHash signHash(llmqType, quorum->qc->quorumHash, requestId, msgHash);
150+
quorumSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get())) {
151151
return true;
152152
}
153153

src/evo/mnhftx.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ bool MNHFTx::Verify(const llmq::CQuorumManager& qman, const uint256& quorumHash,
103103
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-missing-quorum");
104104
}
105105

106-
const uint256 signHash = llmq::SignHash(llmqType, quorum->qc->quorumHash, requestId, msgHash).Get();
107-
if (!sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash)) {
106+
const llmq::SignHash signHash(llmqType, quorum->qc->quorumHash, requestId, msgHash);
107+
if (!sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get())) {
108108
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-invalid");
109109
}
110110

src/llmq/quorums.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,8 +1278,8 @@ VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain
12781278
return VerifyRecSigStatus::NoQuorum;
12791279
}
12801280

1281-
uint256 signHash = SignHash(llmqType, quorum->qc->quorumHash, id, msgHash).Get();
1282-
const bool ret = sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash);
1281+
SignHash signHash(llmqType, quorum->qc->quorumHash, id, msgHash);
1282+
const bool ret = sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get());
12831283
return ret ? VerifyRecSigStatus::Valid : VerifyRecSigStatus::Invalid;
12841284
}
12851285
} // namespace llmq

src/llmq/signhash.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <llmq/params.h>
99
#include <uint256.h>
1010

11+
#include <cstring>
1112
#include <vector>
1213

1314
#include <util/hash_type.h>
@@ -34,8 +35,33 @@ class SignHash : public BaseHash<uint256> {
3435
* Get the underlying uint256 hash value.
3536
*/
3637
const uint256& Get() const { return m_hash; }
38+
39+
// Serialization support
40+
template<typename Stream>
41+
void Serialize(Stream& s) const
42+
{
43+
s << m_hash;
44+
}
45+
46+
template<typename Stream>
47+
void Unserialize(Stream& s)
48+
{
49+
s >> m_hash;
50+
}
3751
};
3852

3953
} // namespace llmq
4054

55+
// Make SignHash hashable for use in unordered_map
56+
template<>
57+
struct std::hash<llmq::SignHash> {
58+
std::size_t operator()(const llmq::SignHash& signHash) const noexcept {
59+
// Use the first 8 bytes of the hash as the hash value
60+
const unsigned char* data = signHash.data();
61+
std::size_t result;
62+
std::memcpy(&result, data, sizeof(result));
63+
return result;
64+
}
65+
};
66+
4167
#endif // BITCOIN_LLMQ_SIGNHASH_H

src/llmq/signing.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig)
162162

163163
// store by signHash
164164
auto signHash = recSig.buildSignHash();
165-
auto k4 = std::make_tuple(std::string("rs_s"), signHash);
165+
auto k4 = std::make_tuple(std::string("rs_s"), signHash.Get());
166166
batch.Write(k4, (uint8_t)1);
167167

168168
// store by current time. Allows fast cleanup of old recSigs
@@ -174,7 +174,7 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig)
174174
{
175175
LOCK(cs_cache);
176176
hasSigForIdCache.insert(std::make_pair(recSig.getLlmqType(), recSig.getId()), true);
177-
hasSigForSessionCache.insert(signHash, true);
177+
hasSigForSessionCache.insert(signHash.Get(), true);
178178
hasSigForHashCache.insert(recSig.GetHash(), true);
179179
}
180180
}
@@ -212,7 +212,7 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
212212

213213
LOCK(cs_cache);
214214
hasSigForIdCache.erase(std::make_pair(recSig.getLlmqType(), recSig.getId()));
215-
hasSigForSessionCache.erase(signHash);
215+
hasSigForSessionCache.erase(signHash.Get());
216216
if (deleteHashKey) {
217217
hasSigForHashCache.erase(recSig.GetHash());
218218
}
@@ -470,7 +470,7 @@ void CSigningManager::CollectPendingRecoveredSigsToVerify(
470470

471471
bool alreadyHave = db.HasRecoveredSigForHash(recSig->GetHash());
472472
if (!alreadyHave) {
473-
uniqueSignHashes.emplace(nodeId, recSig->buildSignHash());
473+
uniqueSignHashes.emplace(nodeId, recSig->buildSignHash().Get());
474474
retSigShares[nodeId].emplace_back(recSig);
475475
}
476476
ns.erase(ns.begin());
@@ -554,7 +554,7 @@ bool CSigningManager::ProcessPendingRecoveredSigs(PeerManager& peerman)
554554
}
555555

556556
const auto& quorum = quorums.at(std::make_pair(recSig->getLlmqType(), recSig->getQuorumHash()));
557-
batchVerifier.PushMessage(nodeId, recSig->GetHash(), recSig->buildSignHash(), recSig->sig.Get(), quorum->qc->quorumPublicKey);
557+
batchVerifier.PushMessage(nodeId, recSig->GetHash(), recSig->buildSignHash().Get(), recSig->sig.Get(), quorum->qc->quorumPublicKey);
558558
verifyCount++;
559559
}
560560
}
@@ -606,7 +606,7 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecovered
606606
CRecoveredSig otherRecoveredSig;
607607
if (db.GetRecoveredSigById(llmqType, recoveredSig->getId(), otherRecoveredSig)) {
608608
auto otherSignHash = otherRecoveredSig.buildSignHash();
609-
if (signHash != otherSignHash) {
609+
if (signHash.Get() != otherSignHash.Get()) {
610610
// this should really not happen, as each masternode is participating in only one vote,
611611
// even if it's a member of multiple quorums. so a majority is only possible on one quorum and one msgHash per id
612612
LogPrintf("CSigningManager::%s -- conflicting recoveredSig for signHash=%s, id=%s, msgHash=%s, otherSignHash=%s\n", __func__,
@@ -837,9 +837,9 @@ void CSigningManager::WorkThreadMain(PeerManager& peerman)
837837
}
838838
}
839839

840-
uint256 CSigBase::buildSignHash() const
840+
SignHash CSigBase::buildSignHash() const
841841
{
842-
return SignHash(llmqType, quorumHash, id, msgHash).Get();
842+
return SignHash(llmqType, quorumHash, id, msgHash);
843843
}
844844

845845

src/llmq/signing.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <bls/bls.h>
99
#include <gsl/pointers.h>
1010
#include <llmq/params.h>
11+
#include <llmq/signhash.h>
1112
#include <net_types.h>
1213
#include <protocol.h>
1314
#include <random.h>
@@ -67,7 +68,7 @@ class CSigBase
6768
return msgHash;
6869
}
6970

70-
[[nodiscard]] uint256 buildSignHash() const;
71+
[[nodiscard]] SignHash buildSignHash() const;
7172
};
7273

7374
class CRecoveredSig : virtual public CSigBase

src/llmq/signing_shares.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ namespace llmq
2828
{
2929
void CSigShare::UpdateKey()
3030
{
31-
key.first = this->buildSignHash();
31+
key.first = this->buildSignHash().Get();
3232
key.second = quorumMember;
3333
}
3434

@@ -126,9 +126,9 @@ CSigSharesNodeState::Session& CSigSharesNodeState::GetOrCreateSessionFromShare(c
126126
CSigSharesNodeState::Session& CSigSharesNodeState::GetOrCreateSessionFromAnn(const llmq::CSigSesAnn& ann)
127127
{
128128
auto signHash = ann.buildSignHash();
129-
auto& s = sessions[signHash];
129+
auto& s = sessions[signHash.Get()];
130130
if (s.announced.inv.empty()) {
131-
InitSession(s, signHash, ann);
131+
InitSession(s, signHash.Get(), ann);
132132
}
133133
return s;
134134
}
@@ -831,7 +831,7 @@ void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& q
831831
// verification because this is unbatched and thus slow verification that happens here.
832832
if (((recoveredSigsCounter++) % 100) == 0) {
833833
auto signHash = rs->buildSignHash();
834-
bool valid = recoveredSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash);
834+
bool valid = recoveredSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get());
835835
if (!valid) {
836836
// this should really not happen as we have verified all signature shares before
837837
LogPrintf("CSigSharesManager::%s -- own recovered signature is invalid. id=%s, msgHash=%s\n", __func__,
@@ -1557,7 +1557,7 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
15571557
}
15581558

15591559
CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
1560-
uint256 signHash = sigShare.buildSignHash();
1560+
uint256 signHash = sigShare.buildSignHash().Get();
15611561

15621562
// TODO: This one should be SIGN by QUORUM key, not by OPERATOR key
15631563
// see TODO in CDKGSession::FinalizeSingleCommitment for details
@@ -1593,7 +1593,7 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
15931593
}
15941594

15951595
CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
1596-
uint256 signHash = sigShare.buildSignHash();
1596+
uint256 signHash = sigShare.buildSignHash().Get();
15971597

15981598
sigShare.sigShare.Set(skShare.Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load());
15991599
if (!sigShare.sigShare.Get().IsValid()) {
@@ -1640,7 +1640,7 @@ void CSigSharesManager::ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus
16401640
MessageProcessingResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
16411641
{
16421642
LOCK(cs);
1643-
RemoveSigSharesForSession(recoveredSig.buildSignHash());
1643+
RemoveSigSharesForSession(recoveredSig.buildSignHash().Get());
16441644
return {};
16451645
}
16461646

src/rpc/quorums.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,8 +608,8 @@ static RPCHelpMan quorum_verify()
608608
throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found");
609609
}
610610

611-
uint256 signHash = llmq::SignHash(llmqType, quorum->qc->quorumHash, id, msgHash).Get();
612-
return sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash);
611+
llmq::SignHash signHash(llmqType, quorum->qc->quorumHash, id, msgHash);
612+
return sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get());
613613
},
614614
};
615615
}

0 commit comments

Comments
 (0)