From 44055fb7b768e8d8d4893ec48820382b9e5b87e5 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 19 Oct 2023 19:33:44 +0300 Subject: [PATCH] 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 | 7 +++++- src/bls/bls.h | 1 + src/coinjoin/coinjoin.cpp | 30 ++++++++++--------------- src/coinjoin/coinjoin.h | 27 +++++----------------- src/governance/governance.cpp | 11 ++------- src/governance/object.cpp | 12 ++++------ src/governance/vote.cpp | 12 ++++------ src/llmq/utils.cpp | 6 ----- src/llmq/utils.h | 1 - src/test/evo_deterministicmns_tests.cpp | 2 +- src/version.h | 6 ----- 11 files changed, 35 insertions(+), 80 deletions(-) diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index 639f901c73..48ea4a7bf2 100644 --- a/src/bls/bls.cpp +++ b/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; diff --git a/src/bls/bls.h b/src/bls/bls.h index 714da5934e..1f84bda112 100644 --- a/src/bls/bls.h +++ b/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 diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index f64aedc304..dd1d8943d9 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/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, version); + return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION); } bool CCoinJoinQueue::Sign() { if (!fMasternodeMode) return false; - bool legacy_bls_scheme = !llmq::utils::IsV19Active(::ChainActive().Tip()); - uint256 hash = GetSignatureHash(legacy_bls_scheme); - CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash)); + uint256 hash = GetSignatureHash(); + CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, false)); 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(legacy_bls_scheme))) { + if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { 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, version); + return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION); } bool CCoinJoinBroadcastTx::Sign() { if (!fMasternodeMode) return false; - bool legacy_bls_scheme = !llmq::utils::IsV19Active(::ChainActive().Tip()); - uint256 hash = GetSignatureHash(legacy_bls_scheme); - CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash)); + uint256 hash = GetSignatureHash(); + CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, false)); 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(legacy_bls_scheme))) { + if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { LogPrint(BCLog::COINJOIN, "CCoinJoinBroadcastTx::CheckSignature -- VerifyInsecure() failed\n"); return false; } diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index efe19c724e..30fe8f9f44 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -109,11 +109,7 @@ public: SERIALIZE_METHODS(CCoinJoinStatusUpdate, obj) { - READWRITE(obj.nSessionID, obj.nState); - if (s.GetVersion() <= COINJOIN_SU_PROTO_VERSION) { - READWRITE(obj.nEntriesCount); - } - READWRITE(obj.nStatusUpdate, obj.nMessageID); + READWRITE(obj.nSessionID, obj.nState, obj.nStatusUpdate, obj.nMessageID); } }; @@ -219,20 +215,13 @@ public: SERIALIZE_METHODS(CCoinJoinQueue, obj) { - READWRITE(obj.nDenom); - - if (s.GetVersion() < COINJOIN_PROTX_HASH_PROTO_VERSION) { - READWRITE(obj.masternodeOutpoint); - } else { - READWRITE(obj.m_protxHash); - } - READWRITE(obj.nTime, obj.fReady); + READWRITE(obj.nDenom, obj.m_protxHash, 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); - - if (s.GetVersion() < COINJOIN_PROTX_HASH_PROTO_VERSION) { - READWRITE(obj.masternodeOutpoint); - } else { - READWRITE(obj.m_protxHash); - } + READWRITE(obj.tx, 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; diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index ddf5515ed7..47de1cd86b 100644 --- a/src/governance/governance.cpp +++ b/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 - // and CGovernanceVote::CheckSignature no longer needs to use ::ChainActive() - LOCK2(cs_main, cs); + LOCK(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 - // and CGovernanceVote::CheckSignature no longer needs to use ::ChainActive() - LOCK2(cs_main, cs); + LOCK(cs); auto curMNList = deterministicMNManager->GetListAtChainTip(); auto diff = lastMNListForVotingKeys->BuildDiff(curMNList); diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 01dfb8b00c..5df0674bcf 100644 --- a/src/governance/object.cpp +++ b/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()); - bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime; - vchSig = sig.ToByteVector(is_bls_legacy_scheme); + vchSig = sig.ToByteVector(false); return true; } bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const { CBLSSignature sig; - const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip()); - bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime; - sig.SetByteVector(vchSig, is_bls_legacy_scheme); - if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) { + sig.SetByteVector(vchSig, false); + if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), false)) { LogPrintf("CGovernanceObject::CheckSignature -- VerifyInsecure() failed\n"); return false; } diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index c5b3a3e1c0..76b761fa81 100644 --- a/src/governance/vote.cpp +++ b/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()); - bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime; - vchSig = sig.ToByteVector(is_bls_legacy_scheme); + vchSig = sig.ToByteVector(false); return true; } bool CGovernanceVote::CheckSignature(const CBLSPublicKey& pubKey) const { CBLSSignature sig; - const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip()); - bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime; - sig.SetByteVector(vchSig, is_bls_legacy_scheme); - if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) { + sig.SetByteVector(vchSig, false); + if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), false)) { LogPrintf("CGovernanceVote::CheckSignature -- VerifyInsecure() failed\n"); return false; } diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 2ab78a0efb..b865d61ec4 100644 --- a/src/llmq/utils.cpp +++ b/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); diff --git a/src/llmq/utils.h b/src/llmq/utils.h index add4cff095..7446f2c637 100644 --- a/src/llmq/utils.h +++ b/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); diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 4e1ca09a2a..e33571cc12 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/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); diff --git a/src/version.h b/src/version.h index 0c4eccf6af..ce0893530e 100644 --- a/src/version.h +++ b/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;