From e8b426cabe9044957d6582df3636f5d1a1f4df1b Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 19 Jun 2023 04:06:54 +0700 Subject: [PATCH 1/5] refactor: remove global usage of bls's version from SetHex --- src/bls/bls.h | 5 ----- src/rpc/evo.cpp | 20 ++++++++++------- src/rpc/quorums.cpp | 50 ++++++++++++++++++++++++++++++++---------- src/test/bls_tests.cpp | 12 +++++----- 4 files changed, 56 insertions(+), 31 deletions(-) diff --git a/src/bls/bls.h b/src/bls/bls.h index d16914c5683c..7ab23b960140 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -160,11 +160,6 @@ class CBLSWrapper return IsValid(); } - bool SetHexStr(const std::string& str) - { - return SetHexStr(str, bls::bls_legacy_scheme.load()); - } - inline void Serialize(CSizeComputer& s) const { s.seek(SerSize); diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index c82d079fc4b3..0417765f7599 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -184,7 +184,7 @@ static CKeyID ParsePubKeyIDFromAddress(const std::string& strAddress, const std: return ToKeyID(*pkhash); } -static CBLSPublicKey ParseBLSPubKey(const std::string& hexKey, const std::string& paramName, bool specific_legacy_bls_scheme = false) +static CBLSPublicKey ParseBLSPubKey(const std::string& hexKey, const std::string& paramName, bool specific_legacy_bls_scheme) { CBLSPublicKey pubKey; if (!pubKey.SetHexStr(hexKey, specific_legacy_bls_scheme)) { @@ -193,10 +193,11 @@ static CBLSPublicKey ParseBLSPubKey(const std::string& hexKey, const std::string return pubKey; } -static CBLSSecretKey ParseBLSSecretKey(const std::string& hexKey, const std::string& paramName) +static CBLSSecretKey ParseBLSSecretKey(const std::string& hexKey, const std::string& paramName, bool specific_legacy_bls_scheme) { CBLSSecretKey secKey; - if (!secKey.SetHexStr(hexKey)) { + + if (!secKey.SetHexStr(hexKey, specific_legacy_bls_scheme)) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a valid BLS secret key", paramName)); } return secKey; @@ -899,7 +900,8 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques EnsureWalletIsUnlocked(wallet.get()); - bool isV19active = llmq::utils::IsV19Active(WITH_LOCK(cs_main, return ::ChainActive().Tip();)); + const bool isV19active = llmq::utils::IsV19Active(WITH_LOCK(cs_main, return ::ChainActive().Tip();)); + const bool is_bls_legacy = !isV19active; if (isHPMNrequested && !isV19active) { throw JSONRPCError(RPC_INVALID_REQUEST, "HPMN aren't allowed yet"); } @@ -912,7 +914,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques throw std::runtime_error(strprintf("invalid network address %s", request.params[1].get_str())); } - CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[2].get_str(), "operatorKey"); + CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[2].get_str(), "operatorKey", is_bls_legacy); size_t paramIdx = 3; if (isHPMNrequested) { @@ -1139,11 +1141,13 @@ static UniValue protx_revoke(const JSONRPCRequest& request) EnsureWalletIsUnlocked(wallet.get()); + const bool isV19active = llmq::utils::IsV19Active(WITH_LOCK(cs_main, return ::ChainActive().Tip();)); + const bool is_bls_legacy = !isV19active; CProUpRevTx ptx; - ptx.nVersion = CProUpRevTx::GetVersion(llmq::utils::IsV19Active(::ChainActive().Tip())); + ptx.nVersion = CProUpRevTx::GetVersion(isV19active); ptx.proTxHash = ParseHashV(request.params[0], "proTxHash"); - CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey"); + CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey", is_bls_legacy); if (!request.params[2].isNull()) { int32_t nReason = ParseInt32V(request.params[2], "reason"); @@ -1701,11 +1705,11 @@ static UniValue bls_fromsecret(const JSONRPCRequest& request) { bls_fromsecret_help(request); - CBLSSecretKey sk = ParseBLSSecretKey(request.params[0].get_str(), "secretKey"); bool bls_legacy_scheme = !llmq::utils::IsV19Active(::ChainActive().Tip()); if (!request.params[1].isNull()) { bls_legacy_scheme = ParseBoolV(request.params[1], "bls_legacy_scheme"); } + CBLSSecretKey sk = ParseBLSSecretKey(request.params[0].get_str(), "secretKey", bls_legacy_scheme); UniValue ret(UniValue::VOBJ); ret.pushKV("secret", sk.ToString()); ret.pushKV("public", sk.GetPublicKey().ToString(bls_legacy_scheme)); diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 1a4d8675f015..c77908aa3c7b 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -576,8 +576,9 @@ static UniValue quorum_sigs_cmd(const JSONRPCRequest& request) return obj; } } else if (cmd == "quorumverify") { + const bool use_bls_legacy = bls::bls_legacy_scheme.load(); CBLSSignature sig; - if (!sig.SetHexStr(request.params[3].get_str())) { + if (!sig.SetHexStr(request.params[3].get_str(), use_bls_legacy)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); } @@ -874,20 +875,37 @@ static UniValue verifychainlock(const JSONRPCRequest& request) const uint256 nBlockHash(ParseHashV(request.params[0], "blockHash")); - CBLSSignature sig; - if (!sig.SetHexStr(request.params[1].get_str())) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); - } - int nBlockHeight; + CBlockIndex* pIndex{nullptr}; if (request.params[2].isNull()) { - const CBlockIndex* pIndex = WITH_LOCK(cs_main, return g_chainman.m_blockman.LookupBlockIndex(nBlockHash)); + pIndex = WITH_LOCK(cs_main, return g_chainman.m_blockman.LookupBlockIndex(nBlockHash)); if (pIndex == nullptr) { throw JSONRPCError(RPC_INTERNAL_ERROR, "blockHash not found"); } nBlockHeight = pIndex->nHeight; } else { nBlockHeight = ParseInt32V(request.params[2], "blockHeight"); + LOCK(cs_main); + if (nBlockHeight < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); + } + if (nBlockHeight <= ::ChainActive().Height()) { + pIndex = ::ChainActive()[nBlockHeight]; + } + } + + CBLSSignature sig; + if (pIndex) { + bool use_legacy_signature = !llmq::utils::IsV19Active(pIndex); + if (!sig.SetHexStr(request.params[1].get_str(), use_legacy_signature)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); + } + } else { + if (!sig.SetHexStr(request.params[1].get_str(), false) && + !sig.SetHexStr(request.params[1].get_str(), true) + ) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); + } } LLMQContext& llmq_ctx = EnsureLLMQContext(request.context); @@ -917,11 +935,6 @@ static UniValue verifyislock(const JSONRPCRequest& request) uint256 id(ParseHashV(request.params[0], "id")); uint256 txid(ParseHashV(request.params[1], "txid")); - CBLSSignature sig; - if (!sig.SetHexStr(request.params[2].get_str())) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); - } - if (g_txindex) { g_txindex->BlockUntilSyncedToCurrentChain(); } @@ -958,6 +971,19 @@ static UniValue verifyislock(const JSONRPCRequest& request) } } + CBLSSignature sig; + if (pindexMined != nullptr) { + const bool use_bls_legacy = !llmq::utils::IsV19Active(pindexMined); + if (!sig.SetHexStr(request.params[2].get_str(), use_bls_legacy)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); + } + } else { + if (!sig.SetHexStr(request.params[2].get_str(), false) && + !sig.SetHexStr(request.params[2].get_str(), true)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); + } + } + LLMQContext& llmq_ctx = EnsureLLMQContext(request.context); auto llmqType = llmq::utils::GetInstantSendLLMQType(*llmq_ctx.qman, pBlockIndex); diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index 7516f47f8495..45c0430c1381 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -64,16 +64,16 @@ void FuncSetHexStr(const bool legacy_scheme) CBLSSecretKey sk; std::string strValidSecret = "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f"; // Note: invalid string passed to SetHexStr() should cause it to fail and reset key internal data - BOOST_CHECK(sk.SetHexStr(strValidSecret)); - BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1g")); // non-hex + BOOST_CHECK(sk.SetHexStr(strValidSecret, legacy_scheme)); + BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1g", legacy_scheme)); // non-hex BOOST_CHECK(!sk.IsValid()); BOOST_CHECK(sk == CBLSSecretKey()); // Try few more invalid strings - BOOST_CHECK(sk.SetHexStr(strValidSecret)); - BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e")); // hex but too short + BOOST_CHECK(sk.SetHexStr(strValidSecret, legacy_scheme)); + BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e", legacy_scheme)); // hex but too short BOOST_CHECK(!sk.IsValid()); - BOOST_CHECK(sk.SetHexStr(strValidSecret)); - BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20")); // hex but too long + BOOST_CHECK(sk.SetHexStr(strValidSecret, legacy_scheme)); + BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20", legacy_scheme)); // hex but too long BOOST_CHECK(!sk.IsValid()); return; From 51c152b35960f4e5b78250334599832c581dc040 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 19 Jun 2023 04:29:40 +0700 Subject: [PATCH 2/5] refactor: remove SetByteVector's without "is legacy" flag --- src/bls/bls.h | 7 +------ src/llmq/dkgsession.cpp | 5 +++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/bls/bls.h b/src/bls/bls.h index 7ab23b960140..53b32d9307d6 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -59,7 +59,7 @@ class CBLSWrapper explicit CBLSWrapper() = default; explicit CBLSWrapper(const std::vector& vecBytes) : CBLSWrapper() { - SetByteVector(vecBytes); + SetByteVector(vecBytes, bls::bls_legacy_scheme.load()); } CBLSWrapper(const CBLSWrapper& ref) = default; @@ -119,11 +119,6 @@ class CBLSWrapper cachedHash.SetNull(); } - void SetByteVector(const std::vector& vecBytes) - { - SetByteVector(vecBytes, bls::bls_legacy_scheme.load()); - } - std::vector ToByteVector(const bool specificLegacyScheme) const { if (!fValid) { diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index e9d95c508495..ace54680bb2d 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -989,14 +989,15 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages) qc.sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(commitmentHash)); qc.quorumSig = skShare.Sign(commitmentHash); + const bool is_bls_legacy = bls::bls_legacy_scheme.load(); if (lieType == 3) { std::vector buf = qc.sig.ToByteVector(); buf[5]++; - qc.sig.SetByteVector(buf); + qc.sig.SetByteVector(buf, is_bls_legacy); } else if (lieType == 4) { std::vector buf = qc.quorumSig.ToByteVector(); buf[5]++; - qc.quorumSig.SetByteVector(buf); + qc.quorumSig.SetByteVector(buf, is_bls_legacy); } t3.stop(); From b5904d9352b90be3a1c3f6521413339935d1aaf0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 19 Jun 2023 04:32:56 +0700 Subject: [PATCH 3/5] refactor: pass flag isLegacy to `ToByteVector` whenever possible instead using global variable --- src/governance/object.cpp | 4 +++- src/governance/vote.cpp | 4 +++- src/llmq/dkgsession.cpp | 7 ++++--- src/llmq/dkgsession.h | 1 + 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 2ee21f823eb1..552d0d31a8df 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -308,7 +308,9 @@ bool CGovernanceObject::Sign(const CBLSSecretKey& key) if (!sig.IsValid()) { return false; } - vchSig = sig.ToByteVector(); + const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip()); + bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime; + vchSig = sig.ToByteVector(is_bls_legacy_scheme); return true; } diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index cea35b32d0a9..c5b3a3e1c024 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -230,7 +230,9 @@ bool CGovernanceVote::Sign(const CBLSSecretKey& key) if (!sig.IsValid()) { return false; } - vchSig = sig.ToByteVector(); + const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip()); + bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime; + vchSig = sig.ToByteVector(is_bls_legacy_scheme); return true; } diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index ace54680bb2d..24b32b70e9dd 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -989,13 +989,14 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages) qc.sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(commitmentHash)); qc.quorumSig = skShare.Sign(commitmentHash); - const bool is_bls_legacy = bls::bls_legacy_scheme.load(); if (lieType == 3) { - std::vector buf = qc.sig.ToByteVector(); + const bool is_bls_legacy = bls::bls_legacy_scheme.load(); + std::vector buf = qc.sig.ToByteVector(is_bls_legacy); buf[5]++; qc.sig.SetByteVector(buf, is_bls_legacy); } else if (lieType == 4) { - std::vector buf = qc.quorumSig.ToByteVector(); + const bool is_bls_legacy = bls::bls_legacy_scheme.load(); + std::vector buf = qc.quorumSig.ToByteVector(is_bls_legacy); buf[5]++; qc.quorumSig.SetByteVector(buf, is_bls_legacy); } diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index f051afe7fb34..bea6b441c8d0 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -7,6 +7,7 @@ #include +#include #include #include From c5b55d3558a78de3b12d1b7549697f36208b692a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 19 Jun 2023 14:48:21 +0700 Subject: [PATCH 4/5] refactor: removed using bls.h header from txmempool.h txmempool.h is widely used header and the dependency bls.h should not be spread to all usages of mempool. It also noticeable improve compilation speed of project if bls.h has been changed. --- src/txmempool.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/txmempool.h b/src/txmempool.h index 718246f7c7ed..0d37a188cba7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -38,6 +37,11 @@ class CBlockIndex; class CChainState; extern RecursiveMutex cs_main; +// Forward declation for CBLSLazyPublicKey: +template class CBLSLazyWrapper; +class CBLSPublicKey; +using CBLSLazyPublicKey = CBLSLazyWrapper; + /** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */ static const uint32_t MEMPOOL_HEIGHT = 0x7FFFFFFF; From 308e5641cc3598b08563227e2117a918e4b4362c Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 20 Jun 2023 18:34:17 +0700 Subject: [PATCH 5/5] refactor: simplification of rpc/quorums.cpp bcs pindex can't be nullptr +partiall revert "refactor: simplification of rpc/quorums.cpp bcs pindex can't be nullptr" in some cases - can! --- src/rpc/quorums.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index c77908aa3c7b..4b8e6a35782a 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -961,7 +961,7 @@ static UniValue verifyislock(const JSONRPCRequest& request) signHeight = pindexMined->nHeight; } - CBlockIndex* pBlockIndex; + CBlockIndex* pBlockIndex{nullptr}; { LOCK(cs_main); if (signHeight == -1) { @@ -971,17 +971,12 @@ static UniValue verifyislock(const JSONRPCRequest& request) } } + CHECK_NONFATAL(pBlockIndex != nullptr); + CBLSSignature sig; - if (pindexMined != nullptr) { - const bool use_bls_legacy = !llmq::utils::IsV19Active(pindexMined); - if (!sig.SetHexStr(request.params[2].get_str(), use_bls_legacy)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); - } - } else { - if (!sig.SetHexStr(request.params[2].get_str(), false) && - !sig.SetHexStr(request.params[2].get_str(), true)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); - } + const bool use_bls_legacy = !llmq::utils::IsV19Active(pBlockIndex); + if (!sig.SetHexStr(request.params[2].get_str(), use_bls_legacy)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); } LLMQContext& llmq_ctx = EnsureLLMQContext(request.context);