From a4d894943a334b347cd22c9a1ed2324a00bb742d Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Sun, 16 Apr 2023 12:09:37 -0500 Subject: [PATCH] refactor: use enums instead of random const ints (#5322) ## Issue being fixed or feature implemented Bad coding in governance code ## What was done? Use Enums where possible ## How Has This Been Tested? Make check ## Breaking Changes none ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [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/governance/classes.cpp | 31 ++++++++++++++------------- src/governance/classes.h | 8 +++---- src/governance/governance.cpp | 33 ++++++++++++++-------------- src/governance/object.cpp | 35 ++++++++++++++++-------------- src/governance/object.h | 26 ++++++++++++++--------- src/governance/validators.cpp | 5 +++-- src/qt/governancelist.cpp | 2 +- src/rpc/governance.cpp | 39 ++++++++++++++++------------------ src/zmq/zmqpublishnotifier.cpp | 2 +- 9 files changed, 95 insertions(+), 86 deletions(-) diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index c4e305165c..0d832d5072 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -119,7 +120,7 @@ bool CGovernanceTriggerManager::AddNewTrigger(uint256 nHash) return false; } - pSuperblock->SetStatus(SEEN_OBJECT_IS_VALID); + pSuperblock->SetStatus(SeenObjectStatus::Valid); mapTrigger.insert(std::make_pair(nHash, pSuperblock)); @@ -149,20 +150,20 @@ void CGovernanceTriggerManager::CleanAndRemove() remove = true; } else { pObj = governance->FindGovernanceObject(it->first); - if (!pObj || pObj->GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) { + if (!pObj || pObj->GetObjectType() != GovernanceObject::TRIGGER) { LogPrint(BCLog::GOBJECT, "CGovernanceTriggerManager::CleanAndRemove -- Unknown or non-trigger superblock\n"); - pSuperblock->SetStatus(SEEN_OBJECT_ERROR_INVALID); + pSuperblock->SetStatus(SeenObjectStatus::ErrorInvalid); } - LogPrint(BCLog::GOBJECT, "CGovernanceTriggerManager::CleanAndRemove -- superblock status = %d\n", pSuperblock->GetStatus()); + LogPrint(BCLog::GOBJECT, "CGovernanceTriggerManager::CleanAndRemove -- superblock status = %d\n", ToUnderlying(pSuperblock->GetStatus())); switch (pSuperblock->GetStatus()) { - case SEEN_OBJECT_ERROR_INVALID: - case SEEN_OBJECT_UNKNOWN: + case SeenObjectStatus::ErrorInvalid: + case SeenObjectStatus::Unknown: LogPrint(BCLog::GOBJECT, "CGovernanceTriggerManager::CleanAndRemove -- Unknown or invalid trigger found\n"); remove = true; break; - case SEEN_OBJECT_IS_VALID: - case SEEN_OBJECT_EXECUTED: { + case SeenObjectStatus::Valid: + case SeenObjectStatus::Executed: { LogPrint(BCLog::GOBJECT, "CGovernanceTriggerManager::CleanAndRemove -- Valid trigger found\n"); if (pSuperblock->IsExpired(*governance)) { // update corresponding object @@ -391,7 +392,7 @@ CSuperblock:: CSuperblock() : nGovObjHash(), nBlockHeight(0), - nStatus(SEEN_OBJECT_UNKNOWN), + nStatus(SeenObjectStatus::Unknown), vecPayments() { } @@ -400,7 +401,7 @@ CSuperblock:: CSuperblock(uint256& nHash) : nGovObjHash(nHash), nBlockHeight(0), - nStatus(SEEN_OBJECT_UNKNOWN), + nStatus(SeenObjectStatus::Unknown), vecPayments() { CGovernanceObject* pGovObj = GetGovernanceObject(*governance); @@ -410,15 +411,15 @@ CSuperblock:: } LogPrint(BCLog::GOBJECT, "CSuperblock -- Constructor pGovObj: %s, nObjectType = %d\n", - pGovObj->GetDataAsPlainString(), pGovObj->GetObjectType()); + pGovObj->GetDataAsPlainString(), ToUnderlying(pGovObj->GetObjectType())); - if (pGovObj->GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) { + if (pGovObj->GetObjectType() != GovernanceObject::TRIGGER) { throw std::runtime_error("CSuperblock: Governance Object not a trigger"); } UniValue obj = pGovObj->GetJSONObject(); - if (obj["type"].get_int() != GOVERNANCE_OBJECT_TRIGGER) { + if (obj["type"].get_int() != ToUnderlying(GovernanceObject::TRIGGER)) { throw std::runtime_error("CSuperblock: invalid data type"); } @@ -681,10 +682,10 @@ bool CSuperblock::IsExpired(const CGovernanceManager& governanceManager) const // Other valid triggers are kept for ~1 day only (for mainnet, but no longer than a superblock cycle for other networks). // Everything else is pruned after ~1h (for mainnet, but no longer than a superblock cycle for other networks). switch (nStatus) { - case SEEN_OBJECT_EXECUTED: + case SeenObjectStatus::Executed: nExpirationBlocks = Params().GetConsensus().nSuperblockCycle; break; - case SEEN_OBJECT_IS_VALID: + case SeenObjectStatus::Valid: nExpirationBlocks = std::min(576, Params().GetConsensus().nSuperblockCycle); break; default: diff --git a/src/governance/classes.h b/src/governance/classes.h index 2ae710c34a..94db94666c 100644 --- a/src/governance/classes.h +++ b/src/governance/classes.h @@ -117,7 +117,7 @@ private: uint256 nGovObjHash; int nBlockHeight; - int nStatus; + SeenObjectStatus nStatus; std::vector vecPayments; void ParsePaymentSchedule(const std::string& strPaymentAddresses, const std::string& strPaymentAmounts); @@ -130,11 +130,11 @@ public: static void GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuperblockRet, int& nNextSuperblockRet); static CAmount GetPaymentsLimit(int nBlockHeight); - int GetStatus() const { return nStatus; } - void SetStatus(int nStatusIn) { nStatus = nStatusIn; } + SeenObjectStatus GetStatus() const { return nStatus; } + void SetStatus(SeenObjectStatus nStatusIn) { nStatus = nStatusIn; } // TELL THE ENGINE WE EXECUTED THIS EVENT - void SetExecuted() { nStatus = SEEN_OBJECT_EXECUTED; } + void SetExecuted() { nStatus = SeenObjectStatus::Executed; } CGovernanceObject* GetGovernanceObject(CGovernanceManager& governanceManager); diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index d4c0b37336..fd026b84f5 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -270,7 +270,8 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman return; } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Adding object: hash = %s, type = %d\n", nHash.ToString(), govobj.GetObjectType()); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Adding object: hash = %s, type = %d\n", nHash.ToString(), + ToUnderlying(govobj.GetObjectType())); // INSERT INTO OUR GOVERNANCE OBJECT MEMORY // IF WE HAVE THIS OBJECT ALREADY, WE DON'T WANT ANOTHER COPY @@ -284,9 +285,9 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman // SHOULD WE ADD THIS OBJECT TO ANY OTHER MANAGERS? LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Before trigger block, GetDataAsPlainString = %s, nObjectType = %d\n", - govobj.GetDataAsPlainString(), govobj.GetObjectType()); + govobj.GetDataAsPlainString(), ToUnderlying(govobj.GetObjectType())); - if (govobj.GetObjectType() == GOVERNANCE_OBJECT_TRIGGER && !triggerman.AddNewTrigger(nHash)) { + if (govobj.GetObjectType() == GovernanceObject::TRIGGER && !triggerman.AddNewTrigger(nHash)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString()); objpair.first->second.PrepareDeletion(GetAdjustedTime()); return; @@ -382,7 +383,7 @@ void CGovernanceManager::UpdateCachesAndClean() int64_t nTimeExpired{0}; - if (pObj->GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { + if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) { // keep hashes of deleted proposals forever nTimeExpired = std::numeric_limits::max(); } else { @@ -394,7 +395,7 @@ void CGovernanceManager::UpdateCachesAndClean() mapObjects.erase(it++); } else { // NOTE: triggers are handled via triggerman - if (pObj->GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { + if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) { bool fAllowScript = (VersionBitsState(::ChainActive().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0024, versionbitscache) == ThresholdState::ACTIVE); CProposalValidator validator(pObj->GetDataAsHexString(), fAllowScript); if (!validator.Validate()) { @@ -602,7 +603,7 @@ void CGovernanceManager::SyncSingleObjVotes(CNode& peer, const uint256& nProp, c for (const auto& vote : fileVotes.GetVotes()) { uint256 nVoteHash = vote.GetHash(); - bool onlyVotingKeyAllowed = govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; + bool onlyVotingKeyAllowed = govobj.GetObjectType() == GovernanceObject::PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; if (filter.contains(nVoteHash) || !vote.IsValid(onlyVotingKeyAllowed)) { continue; @@ -653,7 +654,7 @@ void CGovernanceManager::SyncObjects(CNode& peer, CConnman& connman) const continue; } - if (fAllowScript && peer.nVersion < GOVSCRIPT_PROTO_VERSION && govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { + if (fAllowScript && peer.nVersion < GOVSCRIPT_PROTO_VERSION && govobj.GetObjectType() == GovernanceObject::PROPOSAL) { // We know this proposal is valid locally, otherwise we would not store it. // But we don't want to relay it to pre-GOVSCRIPT_PROTO_VERSION peers if payment_address is p2sh // because they won't accept it anyway and will simply ban us eventually. @@ -679,7 +680,7 @@ void CGovernanceManager::SyncObjects(CNode& peer, CConnman& connman) const void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) { - if (govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) return; + if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return; const COutPoint& masternodeOutpoint = govobj.GetMasternodeOutpoint(); auto it = mapLastMasternodeObject.find(masternodeOutpoint); @@ -715,7 +716,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo return true; } - if (govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) { + if (govobj.GetObjectType() != GovernanceObject::TRIGGER) { return true; } @@ -837,7 +838,7 @@ void CGovernanceManager::CheckPostponedObjects(CConnman& connman) const uint256& nHash = it->first; CGovernanceObject& govobj = it->second; - assert(govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER); + assert(govobj.GetObjectType() != GovernanceObject::TRIGGER); std::string strError; bool fMissingConfirmations; @@ -972,7 +973,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& if (mapAskedRecently[nHash].size() >= nPeersPerHashMax) continue; } - if (objPair.second.GetObjectType() == GOVERNANCE_OBJECT_TRIGGER) { + if (objPair.second.GetObjectType() == GovernanceObject::TRIGGER) { vTriggerObjHashes.push_back(nHash); } else { vOtherObjHashes.push_back(nHash); @@ -1076,7 +1077,7 @@ void CGovernanceManager::AddCachedTriggers() for (auto& objpair : mapObjects) { CGovernanceObject& govobj = objpair.second; - if (govobj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) { + if (govobj.GetObjectType() != GovernanceObject::TRIGGER) { continue; } @@ -1109,10 +1110,10 @@ std::string CGovernanceManager::ToString() const for (const auto& objPair : mapObjects) { switch (objPair.second.GetObjectType()) { - case GOVERNANCE_OBJECT_PROPOSAL: + case GovernanceObject::PROPOSAL: nProposalCount++; break; - case GOVERNANCE_OBJECT_TRIGGER: + case GovernanceObject::TRIGGER: nTriggerCount++; break; default: @@ -1137,10 +1138,10 @@ UniValue CGovernanceManager::ToJson() const for (const auto& objpair : mapObjects) { switch (objpair.second.GetObjectType()) { - case GOVERNANCE_OBJECT_PROPOSAL: + case GovernanceObject::PROPOSAL: nProposalCount++; break; - case GOVERNANCE_OBJECT_TRIGGER: + case GovernanceObject::TRIGGER: nTriggerCount++; break; default: diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 36cb519792..9e8d2199b2 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -22,7 +22,7 @@ CGovernanceObject::CGovernanceObject() : cs(), - nObjectType(GOVERNANCE_OBJECT_UNKNOWN), + nObjectType(GovernanceObject::UNKNOWN), nHashParent(), nRevision(0), nTime(0), @@ -49,7 +49,7 @@ CGovernanceObject::CGovernanceObject() : CGovernanceObject::CGovernanceObject(const uint256& nHashParentIn, int nRevisionIn, int64_t nTimeIn, const uint256& nCollateralHashIn, const std::string& strDataHexIn) : cs(), - nObjectType(GOVERNANCE_OBJECT_UNKNOWN), + nObjectType(GovernanceObject::UNKNOWN), nHashParent(nHashParentIn), nRevision(nRevisionIn), nTime(nTimeIn), @@ -184,7 +184,7 @@ bool CGovernanceObject::ProcessVote(const CGovernanceVote& vote, CGovernanceExce nVoteTimeUpdate = nNow; } - bool onlyVotingKeyAllowed = nObjectType == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; + bool onlyVotingKeyAllowed = nObjectType == GovernanceObject::PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; // Finally check that the vote is actually valid (done last because of cost of signature verification) if (!vote.IsValid(onlyVotingKeyAllowed)) { @@ -245,7 +245,7 @@ std::set CGovernanceObject::RemoveInvalidVotes(const COutPoint& mnOutpo return {}; } - auto removedVotes = fileVotes.RemoveInvalidVotes(mnOutpoint, nObjectType == GOVERNANCE_OBJECT_PROPOSAL); + auto removedVotes = fileVotes.RemoveInvalidVotes(mnOutpoint, nObjectType == GovernanceObject::PROPOSAL); if (removedVotes.empty()) { return {}; } @@ -371,7 +371,7 @@ void CGovernanceObject::LoadData() GetData(objResult); LogPrint(BCLog::GOBJECT, "CGovernanceObject::LoadData -- GetDataAsPlainString = %s\n", GetDataAsPlainString()); UniValue obj = GetJSONObject(); - nObjectType = obj["type"].get_int(); + nObjectType = GovernanceObject(obj["type"].get_int()); } catch (std::exception& e) { fUnparsable = true; std::ostringstream ostr; @@ -468,7 +468,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf } switch (nObjectType) { - case GOVERNANCE_OBJECT_PROPOSAL: { + case GovernanceObject::PROPOSAL: { bool fAllowScript = (VersionBitsState(::ChainActive().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0024, versionbitscache) == ThresholdState::ACTIVE); CProposalValidator validator(GetDataAsHexString(), fAllowScript); // Note: It's ok to have expired proposals @@ -484,7 +484,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf } return true; } - case GOVERNANCE_OBJECT_TRIGGER: { + case GovernanceObject::TRIGGER: { if (!fCheckCollateral) { // nothing else we can check here (yet?) return true; @@ -508,7 +508,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf return true; } default: { - strError = strprintf("Invalid object type %d", nObjectType); + strError = strprintf("Invalid object type %d", ToUnderlying(nObjectType)); return false; } } @@ -518,13 +518,16 @@ CAmount CGovernanceObject::GetMinCollateralFee(bool fork_active) const { // Only 1 type has a fee for the moment but switch statement allows for future object types switch (nObjectType) { - case GOVERNANCE_OBJECT_PROPOSAL: - if (fork_active) return GOVERNANCE_PROPOSAL_FEE_TX; - else return GOVERNANCE_PROPOSAL_FEE_TX_OLD; - case GOVERNANCE_OBJECT_TRIGGER: - return 0; - default: - return MAX_MONEY; + case GovernanceObject::PROPOSAL: { + if (fork_active) return GOVERNANCE_PROPOSAL_FEE_TX; + else return GOVERNANCE_PROPOSAL_FEE_TX_OLD; + } + case GovernanceObject::TRIGGER: { + return 0; + } + default: { + return MAX_MONEY; + } } } @@ -687,7 +690,7 @@ void CGovernanceObject::Relay(CConnman& connman) const } int minProtoVersion = MIN_PEER_PROTO_VERSION; - if (nObjectType == GOVERNANCE_OBJECT_PROPOSAL) { + if (nObjectType == GovernanceObject::PROPOSAL) { // We know this proposal is valid locally, otherwise we would not get to the point we should relay it. // But we don't want to relay it to pre-GOVSCRIPT_PROTO_VERSION peers if payment_address is p2sh // because they won't accept it anyway and will simply ban us eventually. diff --git a/src/governance/object.h b/src/governance/object.h index edcb917d3e..416dd2a800 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -26,9 +27,12 @@ extern CCriticalSection cs_main; static constexpr double GOVERNANCE_FILTER_FP_RATE = 0.001; -static constexpr int GOVERNANCE_OBJECT_UNKNOWN = 0; -static constexpr int GOVERNANCE_OBJECT_PROPOSAL = 1; -static constexpr int GOVERNANCE_OBJECT_TRIGGER = 2; +enum class GovernanceObject { + UNKNOWN = 0, + PROPOSAL, + TRIGGER +}; + static constexpr CAmount GOVERNANCE_PROPOSAL_FEE_TX = (1 * COIN); static constexpr CAmount GOVERNANCE_PROPOSAL_FEE_TX_OLD = (5 * COIN); @@ -40,10 +44,12 @@ static constexpr int64_t GOVERNANCE_DELETION_DELAY = 10 * 60; static constexpr int64_t GOVERNANCE_ORPHAN_EXPIRATION_TIME = 10 * 60; // FOR SEEN MAP ARRAYS - GOVERNANCE OBJECTS AND VOTES -static constexpr int SEEN_OBJECT_IS_VALID = 0; -static constexpr int SEEN_OBJECT_ERROR_INVALID = 1; -static constexpr int SEEN_OBJECT_EXECUTED = 3; //used for triggers -static constexpr int SEEN_OBJECT_UNKNOWN = 4; // the default +enum class SeenObjectStatus { + Valid = 0, + ErrorInvalid, + Executed, + Unknown +}; using vote_time_pair_t = std::pair; @@ -99,7 +105,7 @@ private: mutable CCriticalSection cs; /// Object typecode - int nObjectType; + GovernanceObject nObjectType; /// parent object, 0 is root uint256 nHashParent; @@ -175,7 +181,7 @@ public: return nDeletionTime; } - int GetObjectType() const + GovernanceObject GetObjectType() const { return nObjectType; } @@ -295,7 +301,7 @@ public: obj.nTime, obj.nCollateralHash, obj.vchData, - obj.nObjectType, + ToUnderlying(obj.nObjectType), obj.masternodeOutpoint ); if (!(s.GetType() & SER_GETHASH)) { diff --git a/src/governance/validators.cpp b/src/governance/validators.cpp index b6150027fe..22692d7699 100644 --- a/src/governance/validators.cpp +++ b/src/governance/validators.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include @@ -77,8 +78,8 @@ bool CProposalValidator::ValidateType() return false; } - if (nType != GOVERNANCE_OBJECT_PROPOSAL) { - strErrorMessages += strprintf("type is not %d;", GOVERNANCE_OBJECT_PROPOSAL); + if (nType != ToUnderlying(GovernanceObject::PROPOSAL)) { + strErrorMessages += strprintf("type is not %d;", ToUnderlying(GovernanceObject::PROPOSAL)); return false; } diff --git a/src/qt/governancelist.cpp b/src/qt/governancelist.cpp index 3c48d43ec6..9d1ec7524e 100644 --- a/src/qt/governancelist.cpp +++ b/src/qt/governancelist.cpp @@ -355,7 +355,7 @@ void GovernanceList::updateProposalList() clientModel->getAllGovernanceObjects(govObjList); std::vector newProposals; for (const auto& govObj : govObjList) { - if (govObj.GetObjectType() != GOVERNANCE_OBJECT_PROPOSAL) { + if (govObj.GetObjectType() != GovernanceObject::PROPOSAL) { continue; // Skip triggers. } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 79eb21a4a3..1c0fe0a3b0 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -108,7 +108,7 @@ static UniValue gobject_check(const JSONRPCRequest& request) CGovernanceObject govobj(hashParent, nRevision, nTime, uint256(), strDataHex); - if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { + if (govobj.GetObjectType() == GovernanceObject::PROPOSAL) { LOCK(cs_main); bool fAllowScript = (VersionBitsState(::ChainActive().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0024, versionbitscache) == ThresholdState::ACTIVE); CProposalValidator validator(strDataHex, fAllowScript); @@ -183,7 +183,7 @@ static UniValue gobject_prepare(const JSONRPCRequest& request) request.params[2].getValStr(), request.params[3].getValStr(), govobj.GetDataAsPlainString(), govobj.GetHash().ToString()); - if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { + if (govobj.GetObjectType() == GovernanceObject::PROPOSAL) { LOCK(cs_main); bool fAllowScript = (VersionBitsState(::ChainActive().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0024, versionbitscache) == ThresholdState::ACTIVE); CProposalValidator validator(strDataHex, fAllowScript); @@ -192,7 +192,7 @@ static UniValue gobject_prepare(const JSONRPCRequest& request) } } - if (govobj.GetObjectType() == GOVERNANCE_OBJECT_TRIGGER) { + if (govobj.GetObjectType() == GovernanceObject::TRIGGER) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Trigger objects need not be prepared (however only masternodes can create them)"); } @@ -348,7 +348,7 @@ static UniValue gobject_submit(const JSONRPCRequest& request) LogPrint(BCLog::GOBJECT, "gobject_submit -- GetDataAsPlainString = %s, hash = %s, txid = %s\n", govobj.GetDataAsPlainString(), govobj.GetHash().ToString(), txidFee.ToString()); - if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) { + if (govobj.GetObjectType() == GovernanceObject::PROPOSAL) { LOCK(cs_main); bool fAllowScript = (VersionBitsState(::ChainActive().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0024, versionbitscache) == ThresholdState::ACTIVE); CProposalValidator validator(strDataHex, fAllowScript); @@ -358,7 +358,7 @@ static UniValue gobject_submit(const JSONRPCRequest& request) } // Attempt to sign triggers if we are a MN - if (govobj.GetObjectType() == GOVERNANCE_OBJECT_TRIGGER) { + if (govobj.GetObjectType() == GovernanceObject::TRIGGER) { if (fMnFound) { LOCK(activeMasternodeInfoCs); govobj.SetMasternodeOutpoint(activeMasternodeInfo.outpoint); @@ -443,15 +443,13 @@ static UniValue gobject_vote_conf(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid vote outcome. Please use one of the following: 'yes', 'no' or 'abstain'"); } - int govObjType; - { - LOCK(governance->cs); + GovernanceObject govObjType = WITH_LOCK(governance->cs, return [&](){ CGovernanceObject *pGovObj = governance->FindGovernanceObject(hash); if (!pGovObj) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found"); } - govObjType = pGovObj->GetObjectType(); - } + return pGovObj->GetObjectType(); + }()); int nSuccessful = 0; int nFailed = 0; @@ -476,7 +474,7 @@ static UniValue gobject_vote_conf(const JSONRPCRequest& request) CGovernanceVote vote(dmn->collateralOutpoint, hash, eVoteSignal, eVoteOutcome); bool signSuccess = false; - if (govObjType == GOVERNANCE_OBJECT_PROPOSAL && eVoteSignal == VOTE_SIGNAL_FUNDING) { + if (govObjType == GovernanceObject::PROPOSAL && eVoteSignal == VOTE_SIGNAL_FUNDING) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Can't use vote-conf for proposals"); } @@ -727,15 +725,15 @@ static UniValue ListObjects(const std::string& strCachedSignal, const std::strin if (strCachedSignal == "delete" && !govObj.IsSetCachedDelete()) continue; if (strCachedSignal == "endorsed" && !govObj.IsSetCachedEndorsed()) continue; - if (strType == "proposals" && govObj.GetObjectType() != GOVERNANCE_OBJECT_PROPOSAL) continue; - if (strType == "triggers" && govObj.GetObjectType() != GOVERNANCE_OBJECT_TRIGGER) continue; + if (strType == "proposals" && govObj.GetObjectType() != GovernanceObject::PROPOSAL) continue; + if (strType == "triggers" && govObj.GetObjectType() != GovernanceObject::TRIGGER) continue; UniValue bObj(UniValue::VOBJ); bObj.pushKV("DataHex", govObj.GetDataAsHexString()); bObj.pushKV("DataString", govObj.GetDataAsPlainString()); bObj.pushKV("Hash", govObj.GetHash().ToString()); bObj.pushKV("CollateralHash", govObj.GetCollateralHash().ToString()); - bObj.pushKV("ObjectType", govObj.GetObjectType()); + bObj.pushKV("ObjectType", ToUnderlying(govObj.GetObjectType())); bObj.pushKV("CreationTime", govObj.GetCreationTime()); const COutPoint& masternodeOutpoint = govObj.GetMasternodeOutpoint(); if (masternodeOutpoint != COutPoint()) { @@ -869,7 +867,7 @@ static UniValue gobject_get(const JSONRPCRequest& request) objResult.pushKV("DataString", pGovObj->GetDataAsPlainString()); objResult.pushKV("Hash", pGovObj->GetHash().ToString()); objResult.pushKV("CollateralHash", pGovObj->GetCollateralHash().ToString()); - objResult.pushKV("ObjectType", pGovObj->GetObjectType()); + objResult.pushKV("ObjectType", ToUnderlying(pGovObj->GetObjectType())); objResult.pushKV("CreationTime", pGovObj->GetCreationTime()); const COutPoint& masternodeOutpoint = pGovObj->GetMasternodeOutpoint(); if (masternodeOutpoint != COutPoint()) { @@ -1094,15 +1092,14 @@ static UniValue voteraw(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid vote outcome. Please use one of the following: 'yes', 'no' or 'abstain'"); } - int govObjType; - { - LOCK(governance->cs); + GovernanceObject govObjType = WITH_LOCK(governance->cs, return [&](){ CGovernanceObject *pGovObj = governance->FindGovernanceObject(hashGovObj); if (!pGovObj) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found"); } - govObjType = pGovObj->GetObjectType(); - } + return pGovObj->GetObjectType(); + }()); + int64_t nTime = request.params[5].get_int64(); std::string strSig = request.params[6].get_str(); @@ -1123,7 +1120,7 @@ static UniValue voteraw(const JSONRPCRequest& request) vote.SetTime(nTime); vote.SetSignature(vchSig); - bool onlyVotingKeyAllowed = govObjType == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; + bool onlyVotingKeyAllowed = govObjType == GovernanceObject::PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; if (!vote.IsValid(onlyVotingKeyAllowed)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Failure to verify vote."); diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index fd8c619072..ad1d947dbe 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -380,7 +380,7 @@ bool CZMQPublishRawGovernanceVoteNotifier::NotifyGovernanceVote(const std::share bool CZMQPublishRawGovernanceObjectNotifier::NotifyGovernanceObject(const std::shared_ptr& govobj) { uint256 nHash = govobj->GetHash(); - LogPrint(BCLog::ZMQ, "zmq: Publish rawgovernanceobject: hash = %s, type = %d\n", nHash.ToString(), govobj->GetObjectType()); + LogPrint(BCLog::ZMQ, "zmq: Publish rawgovernanceobject: hash = %s, type = %d\n", nHash.ToString(), ToUnderlying(govobj->GetObjectType())); CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss << *govobj; return SendZmqMessage(MSG_RAWGOBJ, &(*ss.begin()), ss.size());