refactor: enforce passing bls scheme for each call of sign by BLS

dashpay · knst · Dec 13, 2024 · Dec 13, 2024 · Dec 24, 2024 · Dec 23, 2024
commit ce18dcd2ac64e4b11d640253e2f0b3ff655885a2
diff --git a/src/bench/bls.cpp b/src/bench/bls.cpp
@@ -29,12 +29,12 @@ static void BuildTestVectors(size_t count, size_t invalidCount,
         secKeys[i].MakeNewKey();
         pubKeys[i] = secKeys[i].GetPublicKey();
         msgHashes[i] = GetRandHash();
-        sigs[i] = secKeys[i].Sign(msgHashes[i]);
+        sigs[i] = secKeys[i].Sign(msgHashes[i], false);
 
         if (invalid[i]) {
             CBLSSecretKey s;
             s.MakeNewKey();
-            sigs[i] = s.Sign(msgHashes[i]);
+            sigs[i] = s.Sign(msgHashes[i], false);
         }
     }
 }
@@ -71,8 +71,8 @@ static void BLS_SignatureAggregate_Normal(benchmark::Bench& bench)
     CBLSSecretKey secKey1, secKey2;
     secKey1.MakeNewKey();
     secKey2.MakeNewKey();
-    CBLSSignature sig1 = secKey1.Sign(hash);
-    CBLSSignature sig2 = secKey2.Sign(hash);
+    CBLSSignature sig1 = secKey1.Sign(hash, false);
+    CBLSSignature sig2 = secKey2.Sign(hash, false);
 
     // Benchmark.
     bench.run([&] {
@@ -89,7 +89,7 @@ static void BLS_Sign_Normal(benchmark::Bench& bench)
     // Benchmark.
     bench.minEpochIterations(100).run([&] {
         uint256 hash = GetRandHash();
-        sig = secKey.Sign(hash);
+        sig = secKey.Sign(hash, false);
     });
 }
 

diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp
@@ -117,11 +117,6 @@ CBLSPublicKey CBLSSecretKey::GetPublicKey() const
     return pubKey;
 }
 
-CBLSSignature CBLSSecretKey::Sign(const uint256& hash) const
-{
-    return Sign(hash, bls::bls_legacy_scheme.load());
-}
-
 CBLSSignature CBLSSecretKey::Sign(const uint256& hash, const bool specificLegacyScheme) const
 {
     if (!IsValid()) {

diff --git a/src/bls/bls.h b/src/bls/bls.h
@@ -275,7 +275,6 @@ class CBLSSecretKey : public CBLSWrapper<bls::PrivateKey, BLS_CURVE_SECKEY_SIZE,
     bool SecretKeyShare(Span<CBLSSecretKey> msk, const CBLSId& id);
 
     [[nodiscard]] CBLSPublicKey GetPublicKey() const;
-    [[nodiscard]] CBLSSignature Sign(const uint256& hash) const;
     [[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool specificLegacyScheme) const;
 };
 

diff --git a/src/bls/bls_worker.cpp b/src/bls/bls_worker.cpp
@@ -762,7 +762,7 @@ bool CBLSWorker::VerifyVerificationVectors(Span<BLSVerificationVectorPtr> vvecs)
 void CBLSWorker::AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash, const CBLSWorker::SignDoneCallback& doneCallback)
 {
     workerPool.push([secKey, msgHash, doneCallback](int threadId) {
-        doneCallback(secKey.Sign(msgHash));
+        doneCallback(secKey.Sign(msgHash, bls::bls_legacy_scheme.load()));
     });
 }
 

diff --git a/src/evo/mnauth.cpp b/src/evo/mnauth.cpp
@@ -51,7 +51,7 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternode
 
     mnauth.proRegTxHash = mn_activeman.GetProTxHash();
 
-    mnauth.sig = mn_activeman.Sign(signHash);
+    mnauth.sig = mn_activeman.Sign(signHash, bls::bls_legacy_scheme.load());
 
     LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- Sending MNAUTH, peer=%d\n", __func__, peer.GetId());
     connman.PushMessage(&peer, CNetMsgMaker(peer.GetCommonVersion()).Make(NetMsgType::MNAUTH, mnauth));

diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp
@@ -84,7 +84,8 @@ CDKGSession::CDKGSession(const CBlockIndex* pQuorumBaseBlockIndex, const Consens
     m_mn_metaman(mn_metaman),
     m_mn_activeman(mn_activeman),
     m_sporkman(sporkman),
-    m_quorum_base_block_index{pQuorumBaseBlockIndex}
+    m_quorum_base_block_index{pQuorumBaseBlockIndex},
+    m_use_legacy_bls{!DeploymentActiveAfter(m_quorum_base_block_index, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}
 {
 }
 
@@ -215,7 +216,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages, PeerMa
 
     logger.Batch("encrypted contributions. time=%d", t1.count());
 
-    qc.sig = m_mn_activeman->Sign(qc.GetSignHash());
+    qc.sig = m_mn_activeman->Sign(qc.GetSignHash(), m_use_legacy_bls);
 
     logger.Flush();
 
@@ -527,7 +528,7 @@ void CDKGSession::SendComplaint(CDKGPendingMessages& pendingMessages, PeerManage
 
     logger.Batch("sending complaint. badCount=%d, complaintCount=%d", badCount, complaintCount);
 
-    qc.sig = m_mn_activeman->Sign(qc.GetSignHash());
+    qc.sig = m_mn_activeman->Sign(qc.GetSignHash(), m_use_legacy_bls);
 
     logger.Flush();
 
@@ -721,7 +722,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, PeerMa
         return;
     }
 
-    qj.sig = m_mn_activeman->Sign(qj.GetSignHash());
+    qj.sig = m_mn_activeman->Sign(qj.GetSignHash(), m_use_legacy_bls);
 
     logger.Flush();
 
@@ -1011,19 +1012,17 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages, PeerManag
         (*commitmentHash.begin())++;
     }
 
-    qc.sig = m_mn_activeman->Sign(commitmentHash);
-    qc.quorumSig = skShare.Sign(commitmentHash);
+    qc.sig = m_mn_activeman->Sign(commitmentHash, m_use_legacy_bls);
+    qc.quorumSig = skShare.Sign(commitmentHash, m_use_legacy_bls);
 
     if (lieType == 3) {
-        const bool is_bls_legacy = bls::bls_legacy_scheme.load();
-        std::vector<uint8_t> buf = qc.sig.ToByteVector(is_bls_legacy);
+        std::vector<uint8_t> buf = qc.sig.ToByteVector(m_use_legacy_bls);
         buf[5]++;
-        qc.sig.SetByteVector(buf, is_bls_legacy);
+        qc.sig.SetByteVector(buf, m_use_legacy_bls);
     } else if (lieType == 4) {
-        const bool is_bls_legacy = bls::bls_legacy_scheme.load();
-        std::vector<uint8_t> buf = qc.quorumSig.ToByteVector(is_bls_legacy);
+        std::vector<uint8_t> buf = qc.quorumSig.ToByteVector(m_use_legacy_bls);
         buf[5]++;
-        qc.quorumSig.SetByteVector(buf, is_bls_legacy);
+        qc.quorumSig.SetByteVector(buf, m_use_legacy_bls);
     }
 
     t3.stop();

diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h
@@ -288,6 +288,7 @@ class CDKGSession
     const CSporkManager& m_sporkman;
 
     const CBlockIndex* const m_quorum_base_block_index;
+    bool m_use_legacy_bls;
     int quorumIndex{0};
 
 private:

diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp
@@ -1541,7 +1541,7 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
     CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
     uint256 signHash = sigShare.buildSignHash();
 
-    sigShare.sigShare.Set(skShare.Sign(signHash), bls::bls_legacy_scheme.load());
+    sigShare.sigShare.Set(skShare.Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load());
     if (!sigShare.sigShare.Get().IsValid()) {
         LogPrintf("CSigSharesManager::%s -- failed to sign sigShare. signHash=%s, id=%s, msgHash=%s, time=%s\n", __func__,
                   signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), t.count());

diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp
@@ -275,12 +275,6 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESEncryptedObject<CBL
 template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjects<CBLSSecretKey>& obj, size_t idx,
                                                 CBLSSecretKey& ret_obj, int version) const;
 
-[[nodiscard]] CBLSSignature CActiveMasternodeManager::Sign(const uint256& hash) const
-{
-    AssertLockNotHeld(cs);
-    return WITH_READ_LOCK(cs, return m_info.blsKeyOperator.Sign(hash));
-}
-
 [[nodiscard]] CBLSSignature CActiveMasternodeManager::Sign(const uint256& hash, const bool is_legacy) const
 {
     AssertLockNotHeld(cs);

diff --git a/src/masternode/node.h b/src/masternode/node.h
@@ -65,7 +65,6 @@ class CActiveMasternodeManager final : public CValidationInterface
     template <template <typename> class EncryptedObj, typename Obj>
     [[nodiscard]] bool Decrypt(const EncryptedObj<Obj>& obj, size_t idx, Obj& ret_obj, int version) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    [[nodiscard]] CBLSSignature Sign(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
     [[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool is_legacy) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     /* TODO: Reconsider external locking */

diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp
@@ -307,13 +307,14 @@ static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxP
     }
 }
 
-template<typename SpecialTxPayload>
-static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxPayload& payload, const CBLSSecretKey& key)
+template <typename SpecialTxPayload>
+static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxPayload& payload,
+                                       const CBLSSecretKey& key, bool use_legacy)
 {
     UpdateSpecialTxInputsHash(tx, payload);
 
     uint256 hash = ::SerializeHash(payload);
-    payload.sig = key.Sign(hash);
+    payload.sig = key.Sign(hash, use_legacy);
 }
 
 static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit = true)
@@ -1054,7 +1055,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques
 
     FundSpecialTx(*wallet, tx, ptx, feeSource);
 
-    SignSpecialTxPayloadByHash(tx, ptx, keyOperator);
+    SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active);
     SetTxPayload(tx, ptx);
 
     return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
@@ -1254,7 +1255,7 @@ static RPCHelpMan protx_revoke()
         throw JSONRPCError(RPC_INTERNAL_ERROR, "No payout or fee source addresses found, can't revoke");
     }
 
-    SignSpecialTxPayloadByHash(tx, ptx, keyOperator);
+    SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active);
     SetTxPayload(tx, ptx);
 
     return SignAndSendSpecialTx(request, chain_helper, chainman, tx);

diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp
@@ -24,8 +24,8 @@ void FuncSign(const bool legacy_scheme)
     uint256 msgHash1 = uint256::ONE;
     uint256 msgHash2 = uint256::TWO;
 
-    auto sig1 = sk1.Sign(msgHash1);
-    auto sig2 = sk2.Sign(msgHash1);
+    auto sig1 = sk1.Sign(msgHash1, legacy_scheme);
+    auto sig2 = sk2.Sign(msgHash1, legacy_scheme);
     BOOST_CHECK(sig1.VerifyInsecure(sk1.GetPublicKey(), msgHash1));
     BOOST_CHECK(!sig1.VerifyInsecure(sk1.GetPublicKey(), msgHash2));
     BOOST_CHECK(!sig2.VerifyInsecure(sk1.GetPublicKey(), msgHash1));
@@ -44,7 +44,7 @@ void FuncSerialize(const bool legacy_scheme)
     uint256 msgHash = uint256::ONE;
 
     sk.MakeNewKey();
-    CBLSSignature sig1 = sk.Sign(msgHash);
+    CBLSSignature sig1 = sk.Sign(msgHash, legacy_scheme);
     ds2 << sig1;
     ds3 << CBLSSignatureVersionWrapper(const_cast<CBLSSignature&>(sig1), !legacy_scheme);
 
@@ -102,7 +102,7 @@ void FuncKeyAgg(const bool legacy_scheme)
     uint256 msgHash1 = uint256::ONE;
     uint256 msgHash2 = uint256::TWO;
 
-    auto sig = ag_sk.Sign(msgHash1);
+    auto sig = ag_sk.Sign(msgHash1, legacy_scheme);
     BOOST_CHECK(sig.VerifyInsecure(ag_pk, msgHash1));
     BOOST_CHECK(!sig.VerifyInsecure(ag_pk, msgHash2));
 }
@@ -144,7 +144,7 @@ void FuncKeyAggVec(const bool legacy_scheme)
     uint256 msgHash1 = uint256::ONE;
     uint256 msgHash2 = uint256::TWO;
 
-    auto sig = ag_sk.Sign(msgHash1);
+    auto sig = ag_sk.Sign(msgHash1, legacy_scheme);
     BOOST_CHECK(sig.VerifyInsecure(ag_pk, msgHash1));
     BOOST_CHECK(!sig.VerifyInsecure(ag_pk, msgHash2));
 }
@@ -170,7 +170,7 @@ void FuncSigAggSub(const bool legacy_scheme)
         vec_pks.push_back(sk.GetPublicKey());
         hash = GetRandHash();
         vec_hashes.push_back(hash);
-        CBLSSignature sig_i = sk.Sign(hash);
+        CBLSSignature sig_i = sk.Sign(hash, legacy_scheme);
         vec_sigs.push_back(sig_i);
         if (i == 0) {
             // first sig is assigned directly
@@ -221,7 +221,7 @@ void FuncSigAggSecure(const bool legacy_scheme)
     for (int i = 0; i < count; i++) {
         sk.MakeNewKey();
         vec_pks.push_back(sk.GetPublicKey());
-        vec_sigs.push_back(sk.Sign(hash));
+        vec_sigs.push_back(sk.Sign(hash, legacy_scheme));
     }
 
     auto sec_agg_sig = CBLSSignature::AggregateSecure(vec_sigs, vec_pks, hash);
@@ -264,19 +264,20 @@ struct Message
 
 static void AddMessage(std::vector<Message>& vec, uint32_t sourceId, uint32_t msgId, uint8_t msgHash, bool valid)
 {
+    bool legacy_scheme = bls::bls_legacy_scheme.load();
     Message m;
     m.sourceId = sourceId;
     m.msgId = msgId;
     m.msgHash = uint256(msgHash);
     m.sk.MakeNewKey();
     m.pk = m.sk.GetPublicKey();
-    m.sig = m.sk.Sign(m.msgHash);
+    m.sig = m.sk.Sign(m.msgHash, legacy_scheme);
     m.valid = valid;
 
     if (!valid) {
         CBLSSecretKey tmp;
         tmp.MakeNewKey();
-        m.sig = tmp.Sign(m.msgHash);
+        m.sig = tmp.Sign(m.msgHash, legacy_scheme);
     }
 
     vec.emplace_back(m);
@@ -381,7 +382,7 @@ void FuncThresholdSignature(const bool legacy_scheme)
 
     CBLSSecretKey thr_sk = v_threshold_sks[0];
     CBLSPublicKey thr_pk = v_threshold_pks[0];
-    CBLSSignature thr_sig = thr_sk.Sign(hash);
+    CBLSSignature thr_sig = thr_sk.Sign(hash, legacy_scheme);
 
     std::vector<CBLSId> v_size_ids;
     std::vector<CBLSSecretKey> v_size_sk_shares;
@@ -398,7 +399,7 @@ void FuncThresholdSignature(const bool legacy_scheme)
         std::vector<CBLSSignature> v_share_sigs;
         std::vector<CBLSId> v_share_ids;
         for ([[maybe_unused]] const auto j : irange::range(m_shares)) {
-            v_share_sigs.emplace_back(v_size_sk_shares[j].Sign(hash));
+            v_share_sigs.emplace_back(v_size_sk_shares[j].Sign(hash, legacy_scheme));
             BOOST_CHECK(v_share_sigs.back().VerifyInsecure(v_size_pk_shares[j], hash));
             v_share_ids.push_back(v_size_ids[j]);
         }

diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp
@@ -132,7 +132,7 @@ static CMutableTransaction CreateProUpServTx(const CChain& active_chain, const C
     tx.nType = TRANSACTION_PROVIDER_UPDATE_SERVICE;
     FundTransaction(active_chain, tx, utxos, GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())), 1 * COIN, coinbaseKey);
     proTx.inputsHash = CalcTxInputsHash(CTransaction(tx));
-    proTx.sig = operatorKey.Sign(::SerializeHash(proTx));
+    proTx.sig = operatorKey.Sign(::SerializeHash(proTx), bls::bls_legacy_scheme);
     SetTxPayload(tx, proTx);
     SignTransaction(mempool, tx, coinbaseKey);
 
@@ -171,7 +171,7 @@ static CMutableTransaction CreateProUpRevTx(const CChain& active_chain, const CT
     tx.nType = TRANSACTION_PROVIDER_UPDATE_REVOKE;
     FundTransaction(active_chain, tx, utxos, GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())), 1 * COIN, coinbaseKey);
     proTx.inputsHash = CalcTxInputsHash(CTransaction(tx));
-    proTx.sig = operatorKey.Sign(::SerializeHash(proTx));
+    proTx.sig = operatorKey.Sign(::SerializeHash(proTx), bls::bls_legacy_scheme);
     SetTxPayload(tx, proTx);
     SignTransaction(mempool, tx, coinbaseKey);
 

diff --git a/src/test/evo_mnhf_tests.cpp b/src/test/evo_mnhf_tests.cpp
@@ -60,9 +60,10 @@ BOOST_AUTO_TEST_CASE(verify_mnhf_specialtx_tests)
     BOOST_CHECK(ag_sk.IsValid());
     BOOST_CHECK(ag_pk.IsValid());
 
+    const bool use_legacy{false};
     uint256 verHash = uint256S(ToString(bit));
-    auto sig = ag_sk.Sign(verHash);
-    BOOST_CHECK(sig.VerifyInsecure(ag_pk, verHash));
+    auto sig = ag_sk.Sign(verHash, use_legacy);
+    BOOST_CHECK(sig.VerifyInsecure(ag_pk, verHash, use_legacy));
 
     auto& chainman = Assert(m_node.chainman);
     auto& qman = *Assert(m_node.llmq_ctx)->qman;