From 64a153d634d0e23f5cab7e5a12ff0b530352ceb8 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 1 Dec 2023 16:59:14 +0700 Subject: [PATCH 1/6] refactor: untie governance/object and wallet implementation --- src/Makefile.am | 2 + src/governance/common.cpp | 73 +++++++++++++++++++++++++++++ src/governance/common.h | 93 +++++++++++++++++++++++++++++++++++++ src/governance/object.cpp | 97 ++++++++++----------------------------- src/governance/object.h | 51 +++----------------- src/interfaces/wallet.h | 1 + src/rpc/governance.cpp | 11 +++-- src/wallet/wallet.cpp | 9 ++-- src/wallet/wallet.h | 11 ++--- src/wallet/walletdb.cpp | 6 +-- src/wallet/walletdb.h | 8 +++- 11 files changed, 223 insertions(+), 139 deletions(-) create mode 100644 src/governance/common.cpp create mode 100644 src/governance/common.h diff --git a/src/Makefile.am b/src/Makefile.am index 4cd8c18efa1e4..f181677e72113 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -187,6 +187,7 @@ BITCOIN_CORE_H = \ dsnotificationinterface.h \ governance/governance.h \ governance/classes.h \ + governance/common.h \ governance/exceptions.h \ governance/object.h \ governance/validators.h \ @@ -702,6 +703,7 @@ libbitcoin_common_a_SOURCES = \ core_read.cpp \ core_write.cpp \ deploymentinfo.cpp \ + governance/common.cpp \ key.cpp \ key_io.cpp \ merkleblock.cpp \ diff --git a/src/governance/common.cpp b/src/governance/common.cpp new file mode 100644 index 0000000000000..81d1b3f1c0c76 --- /dev/null +++ b/src/governance/common.cpp @@ -0,0 +1,73 @@ +// Copyright (c) 2014-2023 The Dash Core developers +// Distributed under the MIT/X11 software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include + +namespace Governance +{ + +Object::Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& nCollateralHash, const std::string& strDataHex) : + hashParent{nHashParent}, + revision{nRevision}, + time{nTime}, + collateralHash{nCollateralHash}, + masternodeOutpoint{}, + vchSig{}, + vchData{ParseHex(strDataHex)} +{ +} + +uint256 Object::GetHash() const +{ + // Note: doesn't match serialization + + // CREATE HASH OF ALL IMPORTANT PIECES OF DATA + + CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); + ss << hashParent; + ss << revision; + ss << time; + ss << HexStr(vchData); + ss << masternodeOutpoint << uint8_t{} << 0xffffffff; // adding dummy values here to match old hashing + ss << vchSig; + // fee_tx is left out on purpose + + return ss.GetHash(); +} + +UniValue Object::ToJson() const +{ + UniValue obj(UniValue::VOBJ); + obj.pushKV("objectHash", GetHash().ToString()); + obj.pushKV("parentHash", hashParent.ToString()); + obj.pushKV("collateralHash", collateralHash.ToString()); + obj.pushKV("createdAt", time); + obj.pushKV("revision", revision); + UniValue data; + if (!data.read(GetDataAsPlainString())) { + data.clear(); + data.setObject(); + data.pushKV("plain", GetDataAsPlainString()); + } + data.pushKV("hex", GetDataAsHexString()); + obj.pushKV("data", data); + return obj; +} + +std::string Object::GetDataAsHexString() const +{ + return HexStr(vchData); +} + +std::string Object::GetDataAsPlainString() const +{ + return std::string(vchData.begin(), vchData.end()); +} + +} // namespace Governance diff --git a/src/governance/common.h b/src/governance/common.h new file mode 100644 index 0000000000000..a68bff24ae1c1 --- /dev/null +++ b/src/governance/common.h @@ -0,0 +1,93 @@ +// Copyright (c) 2014-2023 The Dash Core developers +// Distributed under the MIT/X11 software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_GOVERNANCE_COMMON_H +#define BITCOIN_GOVERNANCE_COMMON_H + +#include +#include + +#include + +#include +#include + +/** + * This module is a public interface of governance module that can be used + * in other components such as wallet + */ + +class UniValue; + +enum class GovernanceObject : int { + UNKNOWN = 0, + PROPOSAL, + TRIGGER +}; +template<> struct is_serializable_enum : std::true_type {}; + +namespace Governance +{ +class Object +{ + Object() : + type{GovernanceObject::UNKNOWN}, + hashParent{}, + revision{0}, + time{0}, + collateralHash{}, + vchData{} + { + } + + Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& nCollateralHash, const std::string& strDataHex); + + UniValue ToJson() const; + + uint256 GetHash() const; + + std::string GetDataAsHexString() const; + std::string GetDataAsPlainString() const; + + /// Object typecode + GovernanceObject type; + + /// parent object, 0 is root + uint256 hashParent; + + /// object revision in the system + int revision; + + /// time this object was created + int64_t time; + + /// fee-tx + uint256 collateralHash; + + /// Masternode info for signed objects + COutPoint masternodeOutpoint; + std::vector vchSig; + + /// Data field - can be used for anything + std::vector vchData; + + SERIALIZE_METHODS(Object, obj) + { + READWRITE( + obj.hashParent, + obj.revision, + obj.time, + obj.collateralHash, + obj.vchData, + obj.type, + obj.masternodeOutpoint + ); + if (!(s.GetType() & SER_GETHASH)) { + READWRITE(obj.vchSig); + } + } +}; + +} // namespace Governance +#endif diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 61b48096bbc64..912903f8a53b5 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -24,15 +24,8 @@ CGovernanceObject::CGovernanceObject() : cs(), - nObjectType(GovernanceObject::UNKNOWN), - nHashParent(), - nRevision(0), - nTime(0), + m_obj{}, nDeletionTime(0), - nCollateralHash(), - vchData(), - masternodeOutpoint(), - vchSig(), fCachedLocalValidity(false), strLocalValidityError(), fCachedFunding(false), @@ -51,15 +44,8 @@ CGovernanceObject::CGovernanceObject() : CGovernanceObject::CGovernanceObject(const uint256& nHashParentIn, int nRevisionIn, int64_t nTimeIn, const uint256& nCollateralHashIn, const std::string& strDataHexIn) : cs(), - nObjectType(GovernanceObject::UNKNOWN), - nHashParent(nHashParentIn), - nRevision(nRevisionIn), - nTime(nTimeIn), + m_obj{nHashParentIn, nRevisionIn, nTimeIn, nCollateralHashIn, strDataHexIn}, nDeletionTime(0), - nCollateralHash(nCollateralHashIn), - vchData(ParseHex(strDataHexIn)), - masternodeOutpoint(), - vchSig(), fCachedLocalValidity(false), strLocalValidityError(), fCachedFunding(false), @@ -78,15 +64,8 @@ CGovernanceObject::CGovernanceObject(const uint256& nHashParentIn, int nRevision CGovernanceObject::CGovernanceObject(const CGovernanceObject& other) : cs(), - nObjectType(other.nObjectType), - nHashParent(other.nHashParent), - nRevision(other.nRevision), - nTime(other.nTime), + m_obj{other.m_obj}, nDeletionTime(other.nDeletionTime), - nCollateralHash(other.nCollateralHash), - vchData(other.vchData), - masternodeOutpoint(other.masternodeOutpoint), - vchSig(other.vchSig), fCachedLocalValidity(other.fCachedLocalValidity), strLocalValidityError(other.strLocalValidityError), fCachedFunding(other.fCachedFunding), @@ -186,7 +165,7 @@ bool CGovernanceObject::ProcessVote(const CGovernanceVote& vote, CGovernanceExce nVoteTimeUpdate = nNow; } - bool onlyVotingKeyAllowed = nObjectType == GovernanceObject::PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; + bool onlyVotingKeyAllowed = m_obj.type == 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)) { @@ -247,7 +226,7 @@ std::set CGovernanceObject::RemoveInvalidVotes(const COutPoint& mnOutpo return {}; } - auto removedVotes = fileVotes.RemoveInvalidVotes(mnOutpoint, nObjectType == GovernanceObject::PROPOSAL); + auto removedVotes = fileVotes.RemoveInvalidVotes(mnOutpoint, m_obj.type == GovernanceObject::PROPOSAL); if (removedVotes.empty()) { return {}; } @@ -278,20 +257,7 @@ std::set CGovernanceObject::RemoveInvalidVotes(const COutPoint& mnOutpo uint256 CGovernanceObject::GetHash() const { - // Note: doesn't match serialization - - // CREATE HASH OF ALL IMPORTANT PIECES OF DATA - - CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); - ss << nHashParent; - ss << nRevision; - ss << nTime; - ss << GetDataAsHexString(); - ss << masternodeOutpoint << uint8_t{} << 0xffffffff; // adding dummy values here to match old hashing - ss << vchSig; - // fee_tx is left out on purpose - - return ss.GetHash(); + return m_obj.GetHash(); } uint256 CGovernanceObject::GetDataHash() const @@ -309,7 +275,7 @@ uint256 CGovernanceObject::GetSignatureHash() const void CGovernanceObject::SetMasternodeOutpoint(const COutPoint& outpoint) { - masternodeOutpoint = outpoint; + m_obj.masternodeOutpoint = outpoint; } bool CGovernanceObject::Sign(const CBLSSecretKey& key) @@ -318,14 +284,14 @@ bool CGovernanceObject::Sign(const CBLSSecretKey& key) if (!sig.IsValid()) { return false; } - vchSig = sig.ToByteVector(false); + m_obj.vchSig = sig.ToByteVector(false); return true; } bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const { CBLSSignature sig; - sig.SetByteVector(vchSig, false); + sig.SetByteVector(m_obj.vchSig, false); if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), false)) { LogPrintf("CGovernanceObject::CheckSignature -- VerifyInsecure() failed\n"); return false; @@ -341,7 +307,7 @@ bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const UniValue CGovernanceObject::GetJSONObject() const { UniValue obj(UniValue::VOBJ); - if (vchData.empty()) { + if (m_obj.vchData.empty()) { return obj; } @@ -369,7 +335,7 @@ UniValue CGovernanceObject::GetJSONObject() const void CGovernanceObject::LoadData() { - if (vchData.empty()) { + if (m_obj.vchData.empty()) { return; } @@ -379,7 +345,7 @@ void CGovernanceObject::LoadData() GetData(objResult); LogPrint(BCLog::GOBJECT, "CGovernanceObject::LoadData -- GetDataAsPlainString = %s\n", GetDataAsPlainString()); UniValue obj = GetJSONObject(); - nObjectType = GovernanceObject(obj["type"].get_int()); + m_obj.type = GovernanceObject(obj["type"].get_int()); } catch (std::exception& e) { fUnparsable = true; std::ostringstream ostr; @@ -417,36 +383,19 @@ void CGovernanceObject::GetData(UniValue& objResult) const * -------------------------------------------------------- * */ - std::string CGovernanceObject::GetDataAsHexString() const { - return HexStr(vchData); + return m_obj.GetDataAsHexString(); } std::string CGovernanceObject::GetDataAsPlainString() const { - return std::string(vchData.begin(), vchData.end()); + return m_obj.GetDataAsPlainString(); } UniValue CGovernanceObject::ToJson() const { - UniValue obj(UniValue::VOBJ); - obj.pushKV("objectHash", GetHash().ToString()); - obj.pushKV("parentHash", nHashParent.ToString()); - obj.pushKV("collateralHash", GetCollateralHash().ToString()); - obj.pushKV("createdAt", GetCreationTime()); - obj.pushKV("revision", nRevision); - UniValue data; - if (!data.read(GetDataAsPlainString())) { - data.clear(); - data.setObject(); - data.pushKV("plain", GetDataAsPlainString()); - data.pushKV("hex", GetDataAsHexString()); - } else { - data.pushKV("hex", GetDataAsHexString()); - } - obj.pushKV("data", data); - return obj; + return m_obj.ToJson(); } void CGovernanceObject::UpdateLocalValidity() @@ -475,7 +424,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf return false; } - switch (nObjectType) { + switch (m_obj.type) { case GovernanceObject::PROPOSAL: { CProposalValidator validator(GetDataAsHexString()); // Note: It's ok to have expired proposals @@ -499,8 +448,8 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf auto mnList = deterministicMNManager->GetListAtChainTip(); - std::string strOutpoint = masternodeOutpoint.ToStringShort(); - auto dmn = mnList.GetMNByCollateral(masternodeOutpoint); + std::string strOutpoint = m_obj.masternodeOutpoint.ToStringShort(); + auto dmn = mnList.GetMNByCollateral(m_obj.masternodeOutpoint); if (!dmn) { strError = "Failed to find Masternode by UTXO, missing masternode=" + strOutpoint; return false; @@ -515,7 +464,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf return true; } default: { - strError = strprintf("Invalid object type %d", ToUnderlying(nObjectType)); + strError = strprintf("Invalid object type %d", ToUnderlying(m_obj.type)); return false; } } @@ -524,7 +473,7 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf CAmount CGovernanceObject::GetMinCollateralFee() const { // Only 1 type has a fee for the moment but switch statement allows for future object types - switch (nObjectType) { + switch (m_obj.type) { case GovernanceObject::PROPOSAL: { return GOVERNANCE_PROPOSAL_FEE_TX; } @@ -547,9 +496,9 @@ bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingC // RETRIEVE TRANSACTION IN QUESTION uint256 nBlockHash; - CTransactionRef txCollateral = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, nCollateralHash, Params().GetConsensus(), nBlockHash); + CTransactionRef txCollateral = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, m_obj.collateralHash, Params().GetConsensus(), nBlockHash); if (!txCollateral) { - strError = strprintf("Can't find collateral tx %s", nCollateralHash.ToString()); + strError = strprintf("Can't find collateral tx %s", m_obj.collateralHash.ToString()); LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError); return false; } @@ -694,7 +643,7 @@ void CGovernanceObject::Relay(CConnman& connman) const } int minProtoVersion = MIN_PEER_PROTO_VERSION; - if (nObjectType == GovernanceObject::PROPOSAL) { + if (m_obj.type == 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 497f62e50b269..142b9e4d9f872 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -5,11 +5,11 @@ #ifndef BITCOIN_GOVERNANCE_OBJECT_H #define BITCOIN_GOVERNANCE_OBJECT_H +#include #include #include #include #include -#include #include @@ -26,13 +26,6 @@ extern RecursiveMutex cs_main; static constexpr double GOVERNANCE_FILTER_FP_RATE = 0.001; -enum class GovernanceObject : int { - UNKNOWN = 0, - PROPOSAL, - TRIGGER -}; -template<> struct is_serializable_enum : std::true_type {}; - static constexpr CAmount GOVERNANCE_PROPOSAL_FEE_TX = (1 * COIN); @@ -104,30 +97,11 @@ class CGovernanceObject /// critical section to protect the inner data structures mutable RecursiveMutex cs; - /// Object typecode - GovernanceObject nObjectType; - - /// parent object, 0 is root - uint256 nHashParent; - - /// object revision in the system - int nRevision; - - /// time this object was created - int64_t nTime; + Governance::Object m_obj; /// time this object was marked for deletion int64_t nDeletionTime; - /// fee-tx - uint256 nCollateralHash; - - /// Data field - can be used for anything - std::vector vchData; - - /// Masternode info for signed objects - COutPoint masternodeOutpoint; - std::vector vchSig; /// is valid by blockchain bool fCachedLocalValidity; @@ -173,7 +147,7 @@ class CGovernanceObject int64_t GetCreationTime() const { - return nTime; + return m_obj.time; } int64_t GetDeletionTime() const @@ -183,17 +157,17 @@ class CGovernanceObject GovernanceObject GetObjectType() const { - return nObjectType; + return m_obj.type; } const uint256& GetCollateralHash() const { - return nCollateralHash; + return m_obj.collateralHash; } const COutPoint& GetMasternodeOutpoint() const { - return masternodeOutpoint; + return m_obj.masternodeOutpoint; } bool IsSetCachedFunding() const @@ -296,18 +270,7 @@ class CGovernanceObject SERIALIZE_METHODS(CGovernanceObject, obj) { // SERIALIZE DATA FOR SAVING/LOADING OR NETWORK FUNCTIONS - READWRITE( - obj.nHashParent, - obj.nRevision, - obj.nTime, - obj.nCollateralHash, - obj.vchData, - obj.nObjectType, - obj.masternodeOutpoint - ); - if (!(s.GetType() & SER_GETHASH)) { - READWRITE(obj.vchSig); - } + READWRITE(obj.m_obj); if (s.GetType() & SER_DISK) { // Only include these for the disk file format READWRITE(obj.nDeletionTime, obj.fExpired, obj.mapCurrentMNVotes, obj.fileVotes); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 734c8e0e995cd..92be583100c9e 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -29,6 +29,7 @@ class CFeeRate; class CJClientManager; class CKey; class CWallet; +class UniValue; enum class FeeReason; enum class TransactionError; enum isminetype : unsigned int; diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index da2bda6942bd7..a0e06fa07020a 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -227,7 +228,7 @@ static UniValue gobject_prepare(const JSONRPCRequest& request) throw JSONRPCError(RPC_INTERNAL_ERROR, err); } - if (!wallet->WriteGovernanceObject({hashParent, nRevision, nTime, tx->GetHash(), strDataHex})) { + if (!wallet->WriteGovernanceObject(Governance::Object{hashParent, nRevision, nTime, tx->GetHash(), strDataHex})) { throw JSONRPCError(RPC_INTERNAL_ERROR, "WriteGovernanceObject failed"); } @@ -267,11 +268,11 @@ static UniValue gobject_list_prepared(const JSONRPCRequest& request) } // Get a list of all prepared governance objects stored in the wallet LOCK(wallet->cs_wallet); - std::vector vecObjects = wallet->GetGovernanceObjects(); + std::vector vecObjects = wallet->GetGovernanceObjects(); // Sort the vector by the object creation time/hex data - std::sort(vecObjects.begin(), vecObjects.end(), [](const CGovernanceObject* a, const CGovernanceObject* b) { - bool fGreater = a->GetCreationTime() > b->GetCreationTime(); - bool fEqual = a->GetCreationTime() == b->GetCreationTime(); + std::sort(vecObjects.begin(), vecObjects.end(), [](const Governance::Object* a, const Governance::Object* b) { + bool fGreater = a->time > b->time; + bool fEqual = a->time == b->time; bool fHexGreater = a->GetDataAsHexString() > b->GetDataAsHexString(); return fGreater || (fEqual && fHexGreater); }); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2d24a589edcd9..c67cb20248016 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -45,7 +45,6 @@ #include #include #include -#include #include #include @@ -5091,23 +5090,23 @@ void CWallet::notifyChainLock(const CBlockIndex* pindexChainLock, const std::sha NotifyChainLockReceived(pindexChainLock->nHeight); } -bool CWallet::LoadGovernanceObject(const CGovernanceObject& obj) +bool CWallet::LoadGovernanceObject(const Governance::Object& obj) { AssertLockHeld(cs_wallet); return m_gobjects.emplace(obj.GetHash(), obj).second; } -bool CWallet::WriteGovernanceObject(const CGovernanceObject& obj) +bool CWallet::WriteGovernanceObject(const Governance::Object& obj) { AssertLockHeld(cs_wallet); WalletBatch batch(GetDatabase()); return batch.WriteGovernanceObject(obj) && LoadGovernanceObject(obj); } -std::vector CWallet::GetGovernanceObjects() +std::vector CWallet::GetGovernanceObjects() { AssertLockHeld(cs_wallet); - std::vector vecObjects; + std::vector vecObjects; vecObjects.reserve(m_gobjects.size()); for (auto& obj : m_gobjects) { vecObjects.push_back(&obj.second); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1fcc65952c79f..ad3e9335ffaaa 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -8,6 +8,7 @@ #define BITCOIN_WALLET_WALLET_H #include +#include #include #include #include @@ -27,8 +28,6 @@ #include #include -#include - #include #include #include @@ -828,7 +827,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati const std::string& GetName() const { return m_name; } // Map from governance object hash to governance object, they are added by gobject_prepare. - std::map m_gobjects; + std::map m_gobjects; typedef std::map MasterKeyMap; MasterKeyMap mapMasterKeys; @@ -1278,11 +1277,11 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void notifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr& clsig) override; /** Load a CGovernanceObject into m_gobjects. */ - bool LoadGovernanceObject(const CGovernanceObject& obj); + bool LoadGovernanceObject(const Governance::Object& obj); /** Store a CGovernanceObject in the wallet database. This should only be used by governance objects that are created by this wallet via `gobject prepare`. */ - bool WriteGovernanceObject(const CGovernanceObject& obj); + bool WriteGovernanceObject(const Governance::Object& obj); /** Returns a vector containing pointers to the governance objects in m_gobjects */ - std::vector GetGovernanceObjects(); + std::vector GetGovernanceObjects(); /** * Blocks until the wallet state is up-to-date to /at least/ the current diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index ddaa6cb9499c5..6038cef7d2268 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -8,7 +8,7 @@ #include #include -#include +#include #include #include #include @@ -216,7 +216,7 @@ bool WalletBatch::WriteCoinJoinSalt(const uint256& salt) return WriteIC(DBKeys::COINJOIN_SALT, salt); } -bool WalletBatch::WriteGovernanceObject(const CGovernanceObject& obj) +bool WalletBatch::WriteGovernanceObject(const Governance::Object& obj) { return WriteIC(std::make_pair(DBKeys::G_OBJECT, obj.GetHash()), obj, false); } @@ -493,7 +493,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } } else if (strType == DBKeys::G_OBJECT) { uint256 nObjectHash; - CGovernanceObject obj; + Governance::Object obj; ssKey >> nObjectHash; ssValue >> obj; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index b7863fc660605..7026b131a6875 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -30,7 +30,6 @@ static const bool DEFAULT_FLUSHWALLET = true; struct CBlockLocator; -class CGovernanceObject; class CHDChain; class CHDPubKey; class CKeyPool; @@ -41,6 +40,11 @@ class CWalletTx; class uint160; class uint256; +namespace Governance +{ + class Object; +} // namespace Governance + /** Error statuses for the wallet database */ enum class DBErrors { @@ -200,7 +204,7 @@ class WalletBatch bool WriteCoinJoinSalt(const uint256& salt); /** Write a CGovernanceObject to the database */ - bool WriteGovernanceObject(const CGovernanceObject& obj); + bool WriteGovernanceObject(const Governance::Object& obj); /// Write destination data key,value tuple to database bool WriteDestData(const std::string &address, const std::string &key, const std::string &value); From 4de30f4b61e3c2df6764c3ad2e4559fa72dccbe4 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 19 Dec 2023 15:49:52 +0700 Subject: [PATCH 2/6] refactor: a default constructor for Governance::Object --- src/governance/common.h | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/governance/common.h b/src/governance/common.h index a68bff24ae1c1..ae053d850413a 100644 --- a/src/governance/common.h +++ b/src/governance/common.h @@ -31,15 +31,8 @@ namespace Governance { class Object { - Object() : - type{GovernanceObject::UNKNOWN}, - hashParent{}, - revision{0}, - time{0}, - collateralHash{}, - vchData{} - { - } +public: + Object() = default; Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& nCollateralHash, const std::string& strDataHex); @@ -51,23 +44,23 @@ class Object std::string GetDataAsPlainString() const; /// Object typecode - GovernanceObject type; + GovernanceObject type{GovernanceObject::UNKNOWN}; /// parent object, 0 is root - uint256 hashParent; + uint256 hashParent{}; /// object revision in the system - int revision; + int revision{0}; /// time this object was created - int64_t time; + int64_t time{0}; /// fee-tx - uint256 collateralHash; + uint256 collateralHash{}; /// Masternode info for signed objects COutPoint masternodeOutpoint; - std::vector vchSig; + std::vector vchSig{}; /// Data field - can be used for anything std::vector vchData; From 7e13727738390eed8293d76d65a538e8d3a71407 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 1 Dec 2023 19:26:25 +0700 Subject: [PATCH 3/6] refactor: drop circular dependency validationinterface <-> governance/object --- src/coinjoin/coinjoin.cpp | 1 + src/governance/governance.cpp | 3 ++- src/governance/object.h | 5 +++++ src/validationinterface.cpp | 4 ++-- src/validationinterface.h | 10 +++++++--- src/zmq/zmqabstractnotifier.cpp | 2 +- src/zmq/zmqabstractnotifier.h | 8 ++++++-- src/zmq/zmqnotificationinterface.cpp | 2 +- src/zmq/zmqnotificationinterface.h | 2 +- src/zmq/zmqpublishnotifier.cpp | 8 ++++---- src/zmq/zmqpublishnotifier.h | 11 ++++++++--- test/lint/lint-circular-dependencies.sh | 1 - 12 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index ebb7f967bafeb..09072bbf8908c 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index e4ed64355212c..7d741cceae89b 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -333,7 +334,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman CheckOrphanVotes(govobj, connman); // SEND NOTIFICATION TO SCRIPT/ZMQ - GetMainSignals().NotifyGovernanceObject(std::make_shared(govobj)); + GetMainSignals().NotifyGovernanceObject(std::make_shared(govobj.Object())); } void CGovernanceManager::UpdateCachesAndClean() diff --git a/src/governance/object.h b/src/governance/object.h index 142b9e4d9f872..da4c990a5cba0 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -145,6 +145,11 @@ class CGovernanceObject // Public Getter methods + const Governance::Object& Object() const + { + return m_obj; + } + int64_t GetCreationTime() const { return m_obj.time; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index ed014cb7529f4..a293dc9dad51c 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -7,12 +7,12 @@ #include #include +#include #include #include #include #include -#include #include #include #include @@ -295,7 +295,7 @@ void CMainSignals::NotifyGovernanceVote(const std::shared_ptrGetHash().ToString()); } -void CMainSignals::NotifyGovernanceObject(const std::shared_ptr& object) { +void CMainSignals::NotifyGovernanceObject(const std::shared_ptr& object) { auto event = [object, this] { m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyGovernanceObject(object); }); }; diff --git a/src/validationinterface.h b/src/validationinterface.h index 90a5729239fea..25368aae98438 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -20,13 +20,17 @@ struct CBlockLocator; class CConnman; class CValidationInterface; class CGovernanceVote; -class CGovernanceObject; class CDeterministicMNList; class CDeterministicMNListDiff; class uint256; class CScheduler; enum class MemPoolRemovalReason; +namespace Governance +{ + class Object; +} + namespace llmq { class CChainLockSig; struct CInstantSendLock; @@ -162,7 +166,7 @@ class CValidationInterface { virtual void NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr& islock) {} virtual void NotifyChainLock(const CBlockIndex* pindex, const std::shared_ptr& clsig) {} virtual void NotifyGovernanceVote(const std::shared_ptr& vote) {} - virtual void NotifyGovernanceObject(const std::shared_ptr& object) {} + virtual void NotifyGovernanceObject(const std::shared_ptr& object) {} virtual void NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& currentTx, const CTransactionRef& previousTx) {} virtual void NotifyRecoveredSig(const std::shared_ptr& sig) {} virtual void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff) {} @@ -229,7 +233,7 @@ class CMainSignals { void NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr& islock); void NotifyChainLock(const CBlockIndex* pindex, const std::shared_ptr& clsig); void NotifyGovernanceVote(const std::shared_ptr& vote); - void NotifyGovernanceObject(const std::shared_ptr& object); + void NotifyGovernanceObject(const std::shared_ptr& object); void NotifyInstantSendDoubleSpendAttempt(const CTransactionRef ¤tTx, const CTransactionRef &previousTx); void NotifyRecoveredSig(const std::shared_ptr &sig); void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff); diff --git a/src/zmq/zmqabstractnotifier.cpp b/src/zmq/zmqabstractnotifier.cpp index 33bc248c3bdd0..799bf859b1b42 100644 --- a/src/zmq/zmqabstractnotifier.cpp +++ b/src/zmq/zmqabstractnotifier.cpp @@ -38,7 +38,7 @@ bool CZMQAbstractNotifier::NotifyGovernanceVote(const std::shared_ptr & /*object*/) +bool CZMQAbstractNotifier::NotifyGovernanceObject(const std::shared_ptr & /*object*/) { return true; } diff --git a/src/zmq/zmqabstractnotifier.h b/src/zmq/zmqabstractnotifier.h index 16a2db9df72a8..1d22ae7e9987c 100644 --- a/src/zmq/zmqabstractnotifier.h +++ b/src/zmq/zmqabstractnotifier.h @@ -10,13 +10,17 @@ #include class CBlockIndex; -class CGovernanceObject; class CGovernanceVote; class CTransaction; class CZMQAbstractNotifier; typedef std::shared_ptr CTransactionRef; +namespace Governance +{ + class Object; +} //namespace Governance + namespace llmq { class CChainLockSig; struct CInstantSendLock; @@ -58,7 +62,7 @@ class CZMQAbstractNotifier virtual bool NotifyTransaction(const CTransaction &transaction); virtual bool NotifyTransactionLock(const CTransactionRef& transaction, const std::shared_ptr& islock); virtual bool NotifyGovernanceVote(const std::shared_ptr& vote); - virtual bool NotifyGovernanceObject(const std::shared_ptr& object); + virtual bool NotifyGovernanceObject(const std::shared_ptr& object); virtual bool NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& currentTx, const CTransactionRef& previousTx); virtual bool NotifyRecoveredSig(const std::shared_ptr& sig); diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 073ba4ae9d9da..6db7ca667afb6 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -198,7 +198,7 @@ void CZMQNotificationInterface::NotifyGovernanceVote(const std::shared_ptr &object) +void CZMQNotificationInterface::NotifyGovernanceObject(const std::shared_ptr &object) { TryForEachAndRemoveFailed(notifiers, [&object](CZMQAbstractNotifier* notifier) { return notifier->NotifyGovernanceObject(object); diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index c17101fefb6f0..8b380f32ae1c4 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -33,7 +33,7 @@ class CZMQNotificationInterface final : public CValidationInterface void NotifyChainLock(const CBlockIndex *pindex, const std::shared_ptr& clsig) override; void NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr& islock) override; void NotifyGovernanceVote(const std::shared_ptr& vote) override; - void NotifyGovernanceObject(const std::shared_ptr& object) override; + void NotifyGovernanceObject(const std::shared_ptr& object) override; void NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& currentTx, const CTransactionRef& previousTx) override; void NotifyRecoveredSig(const std::shared_ptr& sig) override; diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index fd10dce13c871..c08ae206fec65 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -11,7 +11,7 @@ #include #include -#include +#include #include #include @@ -245,7 +245,7 @@ bool CZMQPublishHashGovernanceVoteNotifier::NotifyGovernanceVote(const std::shar return SendZmqMessage(MSG_HASHGVOTE, data, 32); } -bool CZMQPublishHashGovernanceObjectNotifier::NotifyGovernanceObject(const std::shared_ptr& object) +bool CZMQPublishHashGovernanceObjectNotifier::NotifyGovernanceObject(const std::shared_ptr& object) { uint256 hash = object->GetHash(); LogPrint(BCLog::ZMQ, "zmq: Publish hashgovernanceobject %s\n", hash.GetHex()); @@ -378,10 +378,10 @@ bool CZMQPublishRawGovernanceVoteNotifier::NotifyGovernanceVote(const std::share return SendZmqMessage(MSG_RAWGVOTE, &(*ss.begin()), ss.size()); } -bool CZMQPublishRawGovernanceObjectNotifier::NotifyGovernanceObject(const std::shared_ptr& govobj) +bool CZMQPublishRawGovernanceObjectNotifier::NotifyGovernanceObject(const std::shared_ptr& govobj) { uint256 nHash = govobj->GetHash(); - LogPrint(BCLog::ZMQ, "zmq: Publish rawgovernanceobject: hash = %s to %s, type = %d\n", nHash.ToString(), this->address, ToUnderlying(govobj->GetObjectType())); + LogPrint(BCLog::ZMQ, "zmq: Publish rawgovernanceobject: hash = %s to %s, type = %d\n", nHash.ToString(), this->address, ToUnderlying(govobj->type)); CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss << *govobj; return SendZmqMessage(MSG_RAWGOBJ, &(*ss.begin()), ss.size()); diff --git a/src/zmq/zmqpublishnotifier.h b/src/zmq/zmqpublishnotifier.h index 6fc488eba4e67..68c64314d69ba 100644 --- a/src/zmq/zmqpublishnotifier.h +++ b/src/zmq/zmqpublishnotifier.h @@ -9,7 +9,12 @@ class CBlockIndex; class CGovernanceVote; -class CGovernanceObject; + +namespace Governance +{ + class Object; +} //namespace Governance + class CZMQAbstractPublishNotifier : public CZMQAbstractNotifier { @@ -63,7 +68,7 @@ class CZMQPublishHashGovernanceVoteNotifier : public CZMQAbstractPublishNotifier class CZMQPublishHashGovernanceObjectNotifier : public CZMQAbstractPublishNotifier { public: - bool NotifyGovernanceObject(const std::shared_ptr& object) override; + bool NotifyGovernanceObject(const std::shared_ptr& object) override; }; class CZMQPublishHashInstantSendDoubleSpendNotifier : public CZMQAbstractPublishNotifier @@ -123,7 +128,7 @@ class CZMQPublishRawGovernanceVoteNotifier : public CZMQAbstractPublishNotifier class CZMQPublishRawGovernanceObjectNotifier : public CZMQAbstractPublishNotifier { public: - bool NotifyGovernanceObject(const std::shared_ptr& object) override; + bool NotifyGovernanceObject(const std::shared_ptr& object) override; }; class CZMQPublishRawInstantSendDoubleSpendNotifier : public CZMQAbstractPublishNotifier diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index 3f91423783606..0e96325ed9c05 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -97,7 +97,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "spork -> validation -> spork" "governance/governance -> validation -> governance/governance" "evo/deterministicmns -> validationinterface -> governance/vote -> evo/deterministicmns" - "governance/object -> validationinterface -> governance/object" "governance/vote -> validation -> validationinterface -> governance/vote" "llmq/signing -> masternode/node -> validationinterface -> llmq/signing" "evo/mnhftx -> validation -> evo/mnhftx" From 2bb6361dc0b8fbbc95af7e52732007d2febd0dab Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 1 Dec 2023 19:36:24 +0700 Subject: [PATCH 4/6] cleanup: removed unused definitions from governance/object.h --- src/governance/object.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/governance/object.h b/src/governance/object.h index da4c990a5cba0..58cabc9d9b520 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -17,8 +17,6 @@ class CBLSSecretKey; class CBLSPublicKey; class CNode; -class CGovernanceManager; -class CGovernanceTriggerManager; class CGovernanceObject; class CGovernanceVote; From 4083fff0b2af82624ebd1ee1050e89247eff2847 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 1 Dec 2023 19:31:08 +0700 Subject: [PATCH 5/6] refactor: drop circular dependency governance/object <-> governance/validators --- src/governance/validators.cpp | 2 +- test/lint/lint-circular-dependencies.sh | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/governance/validators.cpp b/src/governance/validators.cpp index 22692d7699b04..aaeaf9d5502f3 100644 --- a/src/governance/validators.cpp +++ b/src/governance/validators.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT/X11 software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include +#include #include #include diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index 0e96325ed9c05..bd899e9ae9ee9 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -34,7 +34,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "governance/governance -> governance/object -> governance/governance" "governance/governance -> masternode/sync -> governance/governance" "governance/governance -> net_processing -> governance/governance" - "governance/object -> governance/validators -> governance/object" "llmq/quorums -> llmq/utils -> llmq/quorums" "llmq/blockprocessor -> net_processing -> llmq/blockprocessor" "llmq/chainlocks -> llmq/instantsend -> llmq/chainlocks" From b2ad5302ce81180336e6f3ae097860e7d7903f74 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 12 Dec 2023 03:58:08 +0700 Subject: [PATCH 6/6] refactor: simplify comparator in rpc/governance --- src/rpc/governance.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index a0e06fa07020a..f8744b28c5189 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -271,16 +271,13 @@ static UniValue gobject_list_prepared(const JSONRPCRequest& request) std::vector vecObjects = wallet->GetGovernanceObjects(); // Sort the vector by the object creation time/hex data std::sort(vecObjects.begin(), vecObjects.end(), [](const Governance::Object* a, const Governance::Object* b) { - bool fGreater = a->time > b->time; - bool fEqual = a->time == b->time; - bool fHexGreater = a->GetDataAsHexString() > b->GetDataAsHexString(); - return fGreater || (fEqual && fHexGreater); + if (a->time != b->time) return a->time < b->time; + return a->GetDataAsHexString() < b->GetDataAsHexString(); }); UniValue jsonArray(UniValue::VARR); - auto it = vecObjects.rbegin() + std::max(0, vecObjects.size() - nCount); - while (it != vecObjects.rend()) { - jsonArray.push_back((*it++)->ToJson()); + for (auto it = vecObjects.begin() + std::max(0, vecObjects.size() - nCount); it != vecObjects.end(); ++it) { + jsonArray.push_back((*it)->ToJson()); } return jsonArray;