chore: Post v19 cleanup (#5622)

## Issue being fixed or feature implemented
Now that v19 is buried we can enforce basic bls scheme usage in
governance and coinjoin and drop some extra code we used for backwards
compatibility.

## What was done?
pls see individual commits

## How Has This Been Tested?
run tests, sync and mix on testnet

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

src/bls/bls.cpp

@@ -118,6 +118,11 @@ CBLSPublicKey CBLSSecretKey::GetPublicKey() const
 }
 
 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()) {
         return {};
@@ -125,7 +130,7 @@ CBLSSignature CBLSSecretKey::Sign(const uint256& hash) const
 
     CBLSSignature sigRet;
     try {
-        sigRet.impl = Scheme(bls::bls_legacy_scheme.load())->Sign(impl, bls::Bytes(hash.begin(), hash.size()));
+        sigRet.impl = Scheme(specificLegacyScheme)->Sign(impl, bls::Bytes(hash.begin(), hash.size()));
         sigRet.fValid = true;
     } catch (...) {
         sigRet.fValid = false;

src/bls/bls.h

@@ -285,6 +285,7 @@ public:
 
     [[nodiscard]] CBLSPublicKey GetPublicKey() const;
     [[nodiscard]] CBLSSignature Sign(const uint256& hash) const;
+    [[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool specificLegacyScheme) const;
 };
 
 class CBLSPublicKey : public CBLSWrapper<bls::G1Element, BLS_CURVE_PUBKEY_SIZE, CBLSPublicKey>

src/coinjoin/coinjoin.cpp

@@ -41,31 +41,28 @@ bool CCoinJoinEntry::AddScriptSig(const CTxIn& txin)
     return false;
 }
 
-uint256 CCoinJoinQueue::GetSignatureHash(bool legacy) const
+uint256 CCoinJoinQueue::GetSignatureHash() const
 {
-    int version = legacy ? COINJOIN_PROTX_HASH_PROTO_VERSION - 1 : PROTOCOL_VERSION;
+    return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
-    return SerializeHash(*this, SER_GETHASH, version);
 }
 
 bool CCoinJoinQueue::Sign()
 {
     if (!fMasternodeMode) return false;
 
-    bool legacy_bls_scheme = !llmq::utils::IsV19Active(::ChainActive().Tip());
+    uint256 hash = GetSignatureHash();
-    uint256 hash = GetSignatureHash(legacy_bls_scheme);
+    CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, false));
-    CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash));
     if (!sig.IsValid()) {
         return false;
     }
-    vchSig = sig.ToByteVector(legacy_bls_scheme);
+    vchSig = sig.ToByteVector(false);
 
     return true;
 }
 
 bool CCoinJoinQueue::CheckSignature(const CBLSPublicKey& blsPubKey) const
 {
-    bool legacy_bls_scheme = !llmq::utils::IsV19Active(::ChainActive().Tip());
+    if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) {
-    if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(legacy_bls_scheme))) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinQueue::CheckSignature -- VerifyInsecure() failed\n");
         return false;
     }
@@ -90,31 +87,28 @@ bool CCoinJoinQueue::IsTimeOutOfBounds(int64_t current_time) const
            nTime - current_time > COINJOIN_QUEUE_TIMEOUT;
 }
 
-uint256 CCoinJoinBroadcastTx::GetSignatureHash(bool legacy) const
+uint256 CCoinJoinBroadcastTx::GetSignatureHash() const
 {
-    int version = legacy ? COINJOIN_PROTX_HASH_PROTO_VERSION - 1 : PROTOCOL_VERSION;
+    return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
-    return SerializeHash(*this, SER_GETHASH, version);
 }
 
 bool CCoinJoinBroadcastTx::Sign()
 {
     if (!fMasternodeMode) return false;
 
-    bool legacy_bls_scheme = !llmq::utils::IsV19Active(::ChainActive().Tip());
+    uint256 hash = GetSignatureHash();
-    uint256 hash = GetSignatureHash(legacy_bls_scheme);
+    CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, false));
-    CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash));
     if (!sig.IsValid()) {
         return false;
     }
-    vchSig = sig.ToByteVector(legacy_bls_scheme);
+    vchSig = sig.ToByteVector(false);
 
     return true;
 }
 
 bool CCoinJoinBroadcastTx::CheckSignature(const CBLSPublicKey& blsPubKey) const
 {
-    bool legacy_bls_scheme = !llmq::utils::IsV19Active(::ChainActive().Tip());
+    if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) {
-    if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(legacy_bls_scheme))) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinBroadcastTx::CheckSignature -- VerifyInsecure() failed\n");
         return false;
     }

src/coinjoin/coinjoin.h

@@ -109,11 +109,7 @@ public:
 
     SERIALIZE_METHODS(CCoinJoinStatusUpdate, obj)
     {
-        READWRITE(obj.nSessionID, obj.nState);
+        READWRITE(obj.nSessionID, obj.nState, obj.nStatusUpdate, obj.nMessageID);
-        if (s.GetVersion() <= COINJOIN_SU_PROTO_VERSION) {
-            READWRITE(obj.nEntriesCount);
-        }
-        READWRITE(obj.nStatusUpdate, obj.nMessageID);
     }
 };
 
@@ -219,20 +215,13 @@ public:
 
     SERIALIZE_METHODS(CCoinJoinQueue, obj)
     {
-        READWRITE(obj.nDenom);
+        READWRITE(obj.nDenom, obj.m_protxHash, obj.nTime, obj.fReady);
-
-        if (s.GetVersion() < COINJOIN_PROTX_HASH_PROTO_VERSION) {
-            READWRITE(obj.masternodeOutpoint);
-        } else {
-            READWRITE(obj.m_protxHash);
-        }
-        READWRITE(obj.nTime, obj.fReady);
         if (!(s.GetType() & SER_GETHASH)) {
             READWRITE(obj.vchSig);
         }
     }
 
-    [[nodiscard]] uint256 GetSignatureHash(bool legacy) const;
+    [[nodiscard]] uint256 GetSignatureHash() const;
     /** Sign this mixing transaction
      *  return true if all conditions are met:
      *     1) we have an active Masternode,
@@ -292,13 +281,7 @@ public:
 
     SERIALIZE_METHODS(CCoinJoinBroadcastTx, obj)
     {
-        READWRITE(obj.tx);
+        READWRITE(obj.tx, obj.m_protxHash);
-
-        if (s.GetVersion() < COINJOIN_PROTX_HASH_PROTO_VERSION) {
-            READWRITE(obj.masternodeOutpoint);
-        } else {
-            READWRITE(obj.m_protxHash);
-        }
 
         if (!(s.GetType() & SER_GETHASH)) {
             READWRITE(obj.vchSig);
@@ -319,7 +302,7 @@ public:
         return *this != CCoinJoinBroadcastTx();
     }
 
-    [[nodiscard]] uint256 GetSignatureHash(bool legacy) const;
+    [[nodiscard]] uint256 GetSignatureHash() const;
 
     bool Sign();
     [[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;

src/governance/governance.cpp

@@ -828,9 +828,7 @@ void CGovernanceManager::SyncSingleObjVotes(CNode& peer, const uint256& nProp, c
 
     LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing single object to peer=%d, nProp = %s\n", __func__, peer.GetId(), nProp.ToString());
 
-    // TODO: drop cs_main here when v19 activation is buried
+    LOCK(cs);
-    // and CGovernanceVote::CheckSignature no longer needs to use ::ChainActive()
-    LOCK2(cs_main, cs);
 
     // single valid object and its valid votes
     auto it = mapObjects.find(nProp);
@@ -1021,9 +1019,6 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo
 
 bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman)
 {
-    // TODO: drop cs_main here when v19 activation is buried
-    // and CGovernanceVote::CheckSignature no longer needs to use ::ChainActive()
-    LOCK(cs_main);
     ENTER_CRITICAL_SECTION(cs)
     uint256 nHashVote = vote.GetHash();
     uint256 nHashGovobj = vote.GetParentHash();
@@ -1487,9 +1482,7 @@ void CGovernanceManager::RemoveInvalidVotes()
         return;
     }
 
-    // TODO: drop cs_main here when v19 activation is buried
+    LOCK(cs);
-    // and CGovernanceVote::CheckSignature no longer needs to use ::ChainActive()
-    LOCK2(cs_main, cs);
 
     auto curMNList = deterministicMNManager->GetListAtChainTip();
     auto diff = lastMNListForVotingKeys->BuildDiff(curMNList);

src/governance/object.cpp

@@ -312,23 +312,19 @@ void CGovernanceObject::SetMasternodeOutpoint(const COutPoint& outpoint)
 
 bool CGovernanceObject::Sign(const CBLSSecretKey& key)
 {
-    CBLSSignature sig = key.Sign(GetSignatureHash());
+    CBLSSignature sig = key.Sign(GetSignatureHash(), false);
     if (!sig.IsValid()) {
         return false;
     }
-    const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip());
+    vchSig = sig.ToByteVector(false);
-    bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime;
-    vchSig = sig.ToByteVector(is_bls_legacy_scheme);
     return true;
 }
 
 bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const
 {
     CBLSSignature sig;
-    const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip());
+    sig.SetByteVector(vchSig, false);
-    bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime;
+    if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), false)) {
-    sig.SetByteVector(vchSig, is_bls_legacy_scheme);
-    if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) {
         LogPrintf("CGovernanceObject::CheckSignature -- VerifyInsecure() failed\n");
         return false;
     }

src/governance/vote.cpp

@@ -226,23 +226,19 @@ bool CGovernanceVote::CheckSignature(const CKeyID& keyID) const
 
 bool CGovernanceVote::Sign(const CBLSSecretKey& key)
 {
-    CBLSSignature sig = key.Sign(GetSignatureHash());
+    CBLSSignature sig = key.Sign(GetSignatureHash(), false);
     if (!sig.IsValid()) {
         return false;
     }
-    const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip());
+    vchSig = sig.ToByteVector(false);
-    bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime;
-    vchSig = sig.ToByteVector(is_bls_legacy_scheme);
     return true;
 }
 
 bool CGovernanceVote::CheckSignature(const CBLSPublicKey& pubKey) const
 {
     CBLSSignature sig;
-    const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip());
+    sig.SetByteVector(vchSig, false);
-    bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime;
+    if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), false)) {
-    sig.SetByteVector(vchSig, is_bls_legacy_scheme);
-    if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) {
         LogPrintf("CGovernanceVote::CheckSignature -- VerifyInsecure() failed\n");
         return false;
     }

src/llmq/utils.cpp

@@ -711,12 +711,6 @@ bool IsV19Active(const CBlockIndex* pindex)
     return pindex->nHeight + 1 >= Params().GetConsensus().V19Height;
 }
 
-const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex)
-{
-    assert(pindex);
-    return pindex->GetAncestor(Params().GetConsensus().V19Height);
-}
-
 bool IsV20Active(const CBlockIndex* pindex)
 {
     assert(pindex);

src/llmq/utils.h

@@ -74,7 +74,6 @@ Consensus::LLMQType GetInstantSendLLMQType(const CQuorumManager& qman, const CBl
 Consensus::LLMQType GetInstantSendLLMQType(bool deterministic);
 bool IsDIP0024Active(const CBlockIndex* pindex);
 bool IsV19Active(const CBlockIndex* pindex);
-const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex);
 bool IsV20Active(const CBlockIndex* pindex);
 bool IsMNRewardReallocationActive(const CBlockIndex* pindex);
 ThresholdState GetMNRewardReallocationState(const CBlockIndex* pindex);

src/test/evo_deterministicmns_tests.cpp

@@ -376,7 +376,7 @@ void FuncV19Activation(TestChainSetup& setup)
     }
 
     // check mn list/diff
-    const CBlockIndex* v19_index = llmq::utils::V19ActivationIndex(::ChainActive().Tip());
+    const CBlockIndex* v19_index = ::ChainActive().Tip()->GetAncestor(Params().GetConsensus().V19Height);
     auto v19_list = deterministicMNManager->GetListForBlock(v19_index);
     dummy_diff = v19_list.BuildDiff(tip_list);
     dummmy_list = v19_list.ApplyDiff(::ChainActive().Tip(), dummy_diff);

src/version.h

@@ -37,15 +37,9 @@ static const int GOVSCRIPT_PROTO_VERSION = 70221;
 //! ADDRV2 was introduced in this version
 static const int ADDRV2_PROTO_VERSION = 70223;
 
-//! CCoinJoinStatusUpdate bug fix was introduced in this version
-static const int COINJOIN_SU_PROTO_VERSION = 70224;
-
 //! BLS scheme was introduced in this version
 static const int BLS_SCHEME_PROTO_VERSION = 70225;
 
-//! DSQ and DSTX started using protx hash in this version
-static const int COINJOIN_PROTX_HASH_PROTO_VERSION = 70226;
-
 //! Masternode type was introduced in this version
 static const int DMN_TYPE_PROTO_VERSION = 70227;