refactor: a bunch of trivial-ish code improvements

src/bls/bls_worker.cpp

@@ -124,7 +124,7 @@ bool CBLSWorker::GenerateContributions(int quorumThreshold, Span<CBLSId> ids, BL
 // input vector is stored. This means that the input vector must stay alive for the whole lifetime of the Aggregator
 template <typename T>
 struct Aggregator : public std::enable_shared_from_this<Aggregator<T>> {
-    size_t batchSize{16};
+    const size_t BATCH_SIZE{16};
     std::shared_ptr<std::vector<const T*> > inputVec;
 
     bool parallel;
@@ -164,7 +164,7 @@ struct Aggregator : public std::enable_shared_from_this<Aggregator<T>> {
     // If parallel=true, then this will return fast, otherwise this will block until aggregation is done
     void Start()
     {
-        size_t batchCount = (inputVec->size() + batchSize - 1) / batchSize;
+        size_t batchCount = (inputVec->size() + BATCH_SIZE - 1) / BATCH_SIZE;
 
         if (!parallel) {
             if (inputVec->size() == 1) {
@@ -191,8 +191,8 @@ struct Aggregator : public std::enable_shared_from_this<Aggregator<T>> {
         // increment wait counter as otherwise the first finished async aggregation might signal that we're done
         IncWait();
         for (size_t i = 0; i < batchCount; i++) {
-            size_t start = i * batchSize;
-            size_t count = std::min(batchSize, inputVec->size() - start);
+            size_t start = i * BATCH_SIZE;
+            size_t count = std::min(BATCH_SIZE, inputVec->size() - start);
             AsyncAggregateAndPushAggQueue(inputVec, start, count, false);
         }
         // this will decrement the wait counter and in most cases NOT finish, as async work is still in progress
@@ -272,24 +272,24 @@ struct Aggregator : public std::enable_shared_from_this<Aggregator<T>> {
             throw;
         }
 
-        if (++aggQueueSize >= batchSize) {
+        if (++aggQueueSize >= BATCH_SIZE) {
             // we've collected enough intermediate results to form a new batch.
             std::shared_ptr<std::vector<const T*> > newBatch;
             {
                 std::unique_lock<std::mutex> l(m);
-                if (aggQueueSize < batchSize) {
+                if (aggQueueSize < BATCH_SIZE) {
                     // some other worker thread grabbed this batch
                     return;
                 }
-                newBatch = std::make_shared<std::vector<const T*> >(batchSize);
+                newBatch = std::make_shared<std::vector<const T*>>(BATCH_SIZE);
                 // collect items for new batch
-                for (size_t i = 0; i < batchSize; i++) {
+                for (size_t i = 0; i < BATCH_SIZE; i++) {
                     T* p = nullptr;
                     bool s = aggQueue.pop(p);
                     assert(s);
                     (*newBatch)[i] = p;
                 }
-                aggQueueSize -= batchSize;
+                aggQueueSize -= BATCH_SIZE;
             }
 
             // push new batch to work queue. del=true this time as these items are intermediate results and need to be deleted

src/flat-database.h

@@ -21,8 +21,7 @@ template<typename T>
 class CFlatDB
 {
 private:
-
-    enum ReadResult {
+    enum class ReadResult {
         Ok,
         FileError,
         HashReadError,
@@ -82,7 +81,7 @@ class CFlatDB
         CAutoFile filein(file, SER_DISK, CLIENT_VERSION);
         if (filein.IsNull()) {
             // It is not actually error, maybe it's a first initialization of core.
-            return FileError;
+            return ReadResult::FileError;
         }
 
         // use file size to size memory buffer
@@ -102,7 +101,7 @@ class CFlatDB
         }
         catch (std::exception &e) {
             error("%s: Deserialize or I/O error - %s", __func__, e.what());
-            return HashReadError;
+            return ReadResult::HashReadError;
         }
         filein.fclose();
 
@@ -113,7 +112,7 @@ class CFlatDB
         if (hashIn != hashTmp)
         {
             error("%s: Checksum mismatch, data corrupted", __func__);
-            return IncorrectHash;
+            return ReadResult::IncorrectHash;
         }
 
 
@@ -127,7 +126,7 @@ class CFlatDB
             if (strMagicMessage != strMagicMessageTmp)
             {
                 error("%s: Invalid magic message", __func__);
-                return IncorrectMagicMessage;
+                return ReadResult::IncorrectMagicMessage;
             }
 
 
@@ -138,7 +137,7 @@ class CFlatDB
             if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp)))
             {
                 error("%s: Invalid network magic number", __func__);
-                return IncorrectMagicNumber;
+                return ReadResult::IncorrectMagicNumber;
             }
 
             // de-serialize data into T object
@@ -147,28 +146,25 @@ class CFlatDB
         catch (std::exception &e) {
             objToLoad.Clear();
             error("%s: Deserialize or I/O error - %s", __func__, e.what());
-            return IncorrectFormat;
+            return ReadResult::IncorrectFormat;
         }
 
         LogPrintf("Loaded info from %s  %dms\n", strFilename, GetTimeMillis() - nStart);
         LogPrintf("     %s\n", objToLoad.ToString());
 
-        return Ok;
+        return ReadResult::Ok;
     }
 
     [[nodiscard]] bool Read(T& objToLoad)
     {
         ReadResult readResult = CoreRead(objToLoad);
-        if (readResult == FileError)
+        if (readResult == ReadResult::FileError)
             LogPrintf("Missing file %s, will try to recreate\n", strFilename);
-        else if (readResult != Ok)
-        {
+        else if (readResult != ReadResult::Ok) {
             LogPrintf("ERROR: CFlatDB::Read Error reading %s: ", strFilename);
-            if(readResult == IncorrectFormat)
-            {
+            if (readResult == ReadResult::IncorrectFormat) {
                 LogPrintf("%s: Magic is ok but data has invalid format, will try to recreate\n", __func__);
-            }
-            else {
+            } else {
                 LogPrintf("%s: File format is unknown or invalid, please fix it manually\n", __func__);
                 // program should exit with an error
                 return false;

src/governance/classes.cpp

@@ -20,21 +20,6 @@
 
 #include <univalue.h>
 
-// SPLIT UP STRING BY DELIMITER
-std::vector<std::string> SplitBy(const std::string& strCommand, const std::string& strDelimit)
-{
-    std::vector<std::string> vParts = SplitString(strCommand, strDelimit);
-
-    for (int q = 0; q < (int)vParts.size(); q++) {
-        if (strDelimit.find(vParts[q]) != std::string::npos) {
-            vParts.erase(vParts.begin() + q);
-            --q;
-        }
-    }
-
-    return vParts;
-}
-
 CAmount ParsePaymentAmount(const std::string& strAmount)
 {
     CAmount nAmount = 0;
@@ -196,23 +181,20 @@ void CSuperblock::ParsePaymentSchedule(const std::string& strPaymentAddresses, c
 {
     // SPLIT UP ADDR/AMOUNT STRINGS AND PUT IN VECTORS
 
-    std::vector<std::string> vecParsed1;
-    std::vector<std::string> vecParsed2;
-    std::vector<std::string> vecParsed3;
-    vecParsed1 = SplitBy(strPaymentAddresses, "|");
-    vecParsed2 = SplitBy(strPaymentAmounts, "|");
-    vecParsed3 = SplitBy(strProposalHashes, "|");
+    const auto vecPaymentAddresses = SplitString(strPaymentAddresses, "|");
+    const auto vecPaymentAmounts = SplitString(strPaymentAmounts, "|");
+    const auto vecProposalHashes = SplitString(strProposalHashes, "|");
 
     // IF THESE DON'T MATCH, SOMETHING IS WRONG
 
-    if (vecParsed1.size() != vecParsed2.size() || vecParsed1.size() != vecParsed3.size()) {
+    if (vecPaymentAddresses.size() != vecPaymentAmounts.size() || vecPaymentAddresses.size() != vecProposalHashes.size()) {
         std::ostringstream ostr;
         ostr << "CSuperblock::ParsePaymentSchedule -- Mismatched payments, amounts and proposalHashes";
         LogPrintf("%s\n", ostr.str());
         throw std::runtime_error(ostr.str());
     }
 
-    if (vecParsed1.empty()) {
+    if (vecPaymentAddresses.empty()) {
         std::ostringstream ostr;
         ostr << "CSuperblock::ParsePaymentSchedule -- Error no payments";
         LogPrintf("%s\n", ostr.str());
@@ -225,26 +207,28 @@ void CSuperblock::ParsePaymentSchedule(const std::string& strPaymentAddresses, c
       AMOUNTS = [AMOUNT1|2|3|4|5|6]
     */
 
-    for (int i = 0; i < (int)vecParsed1.size(); i++) {
-        CTxDestination dest = DecodeDestination(vecParsed1[i]);
+    for (int i = 0; i < (int)vecPaymentAddresses.size(); i++) {
+        CTxDestination dest = DecodeDestination(vecPaymentAddresses[i]);
         if (!IsValidDestination(dest)) {
             std::ostringstream ostr;
-            ostr << "CSuperblock::ParsePaymentSchedule -- Invalid Dash Address : " << vecParsed1[i];
+            ostr << "CSuperblock::ParsePaymentSchedule -- Invalid Dash Address : " << vecPaymentAddresses[i];
             LogPrintf("%s\n", ostr.str());
             throw std::runtime_error(ostr.str());
         }
 
-        CAmount nAmount = ParsePaymentAmount(vecParsed2[i]);
+        CAmount nAmount = ParsePaymentAmount(vecPaymentAmounts[i]);
 
         uint256 proposalHash;
-        if (!ParseHashStr(vecParsed3[i], proposalHash)){
+        if (!ParseHashStr(vecProposalHashes[i], proposalHash)) {
             std::ostringstream ostr;
-            ostr << "CSuperblock::ParsePaymentSchedule -- Invalid proposal hash : " << vecParsed3[i];
+            ostr << "CSuperblock::ParsePaymentSchedule -- Invalid proposal hash : " << vecProposalHashes[i];
             LogPrintf("%s\n", ostr.str());
             throw std::runtime_error(ostr.str());
         }
 
-        LogPrint(BCLog::GOBJECT, "CSuperblock::ParsePaymentSchedule -- i = %d, amount string = %s, nAmount = %lld, proposalHash = %s\n", i, vecParsed2[i], nAmount, proposalHash.ToString());
+        LogPrint(BCLog::GOBJECT, /* Continued */
+                 "CSuperblock::ParsePaymentSchedule -- i = %d, amount string = %s, nAmount = %lld, proposalHash = %s\n",
+                 i, vecPaymentAmounts[i], nAmount, proposalHash.ToString());
 
         CGovernancePayment payment(dest, nAmount, proposalHash);
         if (payment.IsValid()) {

src/governance/governance.cpp

@@ -548,14 +548,10 @@ std::vector<CGovernanceVote> CGovernanceManager::GetCurrentVotes(const uint256&
         vote_rec_t voteRecord;
         if (!govobj.GetCurrentMNVotes(mnpair.first, voteRecord)) continue;
 
-        for (const auto& voteInstancePair : voteRecord.mapInstances) {
-            int signal = voteInstancePair.first;
-            int outcome = voteInstancePair.second.eOutcome;
-            int64_t nCreationTime = voteInstancePair.second.nCreationTime;
-
-            CGovernanceVote vote = CGovernanceVote(mnpair.first, nParentHash, (vote_signal_enum_t)signal, (vote_outcome_enum_t)outcome);
-            vote.SetTime(nCreationTime);
-
+        for (const auto& [signal, vote_instance] : voteRecord.mapInstances) {
+            CGovernanceVote vote = CGovernanceVote(mnpair.first, nParentHash, (vote_signal_enum_t)signal,
+                                                   vote_instance.eOutcome);
+            vote.SetTime(vote_instance.nCreationTime);
             vecResult.push_back(vote);
         }
     }

src/governance/object.cpp

@@ -90,7 +90,7 @@ bool CGovernanceObject::ProcessVote(CMasternodeMetaMan& mn_metaman, CGovernanceM
         exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_WARNING);
         return false;
     }
-    if (eSignal > MAX_SUPPORTED_VOTE_SIGNAL) {
+    if (eSignal < VOTE_SIGNAL_NONE || eSignal >= VOTE_SIGNAL_UNKNOWN) {
         std::ostringstream ostr;
         ostr << "CGovernanceObject::ProcessVote -- Unsupported vote signal: " << CGovernanceVoting::ConvertSignalToString(vote.GetSignal());
         LogPrintf("%s\n", ostr.str());

src/governance/vote.cpp

@@ -85,27 +85,13 @@ vote_signal_enum_t CGovernanceVoting::ConvertVoteSignal(const std::string& strVo
     return it->second;
 }
 
-CGovernanceVote::CGovernanceVote() :
-    fValid(true),
-    fSynced(false),
-    nVoteSignal(int(VOTE_SIGNAL_NONE)),
-    masternodeOutpoint(),
-    nParentHash(),
-    nVoteOutcome(int(VOTE_OUTCOME_NONE)),
-    nTime(0),
-    vchSig()
-{
-}
-
-CGovernanceVote::CGovernanceVote(const COutPoint& outpointMasternodeIn, const uint256& nParentHashIn, vote_signal_enum_t eVoteSignalIn, vote_outcome_enum_t eVoteOutcomeIn) :
-    fValid(true),
-    fSynced(false),
-    nVoteSignal(eVoteSignalIn),
+CGovernanceVote::CGovernanceVote(const COutPoint& outpointMasternodeIn, const uint256& nParentHashIn,
+                                 vote_signal_enum_t eVoteSignalIn, vote_outcome_enum_t eVoteOutcomeIn) :
     masternodeOutpoint(outpointMasternodeIn),
     nParentHash(nParentHashIn),
     nVoteOutcome(eVoteOutcomeIn),
-    nTime(GetAdjustedTime()),
-    vchSig()
+    nVoteSignal(eVoteSignalIn),
+    nTime(GetAdjustedTime())
 {
     UpdateHash();
 }
@@ -211,15 +197,15 @@ bool CGovernanceVote::IsValid(const CDeterministicMNList& tip_mn_list, bool useV
         return false;
     }
 
-    // support up to MAX_SUPPORTED_VOTE_SIGNAL, can be extended
-    if (nVoteSignal > MAX_SUPPORTED_VOTE_SIGNAL) {
-        LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- Client attempted to vote on invalid signal(%d) - %s\n", nVoteSignal, GetHash().ToString());
+    if (nVoteSignal < VOTE_SIGNAL_NONE || nVoteSignal >= VOTE_SIGNAL_UNKNOWN) {
+        LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- Client attempted to vote on invalid signal(%d) - %s\n",
+                 nVoteSignal, GetHash().ToString());
         return false;
     }
 
-    // 0=none, 1=yes, 2=no, 3=abstain. Beyond that reject votes
-    if (nVoteOutcome > 3) {
-        LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- Client attempted to vote on invalid outcome(%d) - %s\n", nVoteSignal, GetHash().ToString());
+    if (nVoteOutcome < VOTE_OUTCOME_NONE || nVoteOutcome >= VOTE_OUTCOME_UNKNOWN) {
+        LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- Client attempted to vote on invalid outcome(%d) - %s\n",
+                 nVoteOutcome, GetHash().ToString());
         return false;
     }
 

src/governance/vote.h

@@ -18,24 +18,25 @@ class CKeyID;
 class PeerManager;
 
 // INTENTION OF MASTERNODES REGARDING ITEM
-enum vote_outcome_enum_t : uint8_t {
-    VOTE_OUTCOME_NONE      = 0,
-    VOTE_OUTCOME_YES       = 1,
-    VOTE_OUTCOME_NO        = 2,
-    VOTE_OUTCOME_ABSTAIN   = 3
+enum vote_outcome_enum_t : int {
+    VOTE_OUTCOME_NONE = 0,
+    VOTE_OUTCOME_YES,
+    VOTE_OUTCOME_NO,
+    VOTE_OUTCOME_ABSTAIN,
+    VOTE_OUTCOME_UNKNOWN
 };
-
+template<> struct is_serializable_enum<vote_outcome_enum_t> : std::true_type {};
 
 // SIGNAL VARIOUS THINGS TO HAPPEN:
-enum vote_signal_enum_t : uint8_t {
-    VOTE_SIGNAL_NONE       = 0,
-    VOTE_SIGNAL_FUNDING    = 1, //   -- fund this object for it's stated amount
-    VOTE_SIGNAL_VALID      = 2, //   -- this object checks out in sentinel engine
-    VOTE_SIGNAL_DELETE     = 3, //   -- this object should be deleted from memory entirely
-    VOTE_SIGNAL_ENDORSED   = 4, //   -- officially endorsed by the network somehow (delegation)
+enum vote_signal_enum_t : int {
+    VOTE_SIGNAL_NONE = 0,
+    VOTE_SIGNAL_FUNDING,  //   -- fund this object for it's stated amount
+    VOTE_SIGNAL_VALID,    //   -- this object checks out in sentinel engine
+    VOTE_SIGNAL_DELETE,   //   -- this object should be deleted from memory entirely
+    VOTE_SIGNAL_ENDORSED, //   -- officially endorsed by the network somehow (delegation)
+    VOTE_SIGNAL_UNKNOWN
 };
-
-static constexpr int MAX_SUPPORTED_VOTE_SIGNAL = VOTE_SIGNAL_ENDORSED;
+template<> struct is_serializable_enum<vote_signal_enum_t> : std::true_type {};
 
 /**
 * Governance Voting
@@ -63,32 +64,26 @@ class CGovernanceVote
     friend bool operator<(const CGovernanceVote& vote1, const CGovernanceVote& vote2);
 
 private:
-    bool fValid;     //if the vote is currently valid / counted
-    bool fSynced;    //if we've sent this to our peers
-    int nVoteSignal; // see VOTE_ACTIONS above
     COutPoint masternodeOutpoint;
     uint256 nParentHash;
-    int nVoteOutcome; // see VOTE_OUTCOMES above
-    int64_t nTime;
+    vote_outcome_enum_t nVoteOutcome{VOTE_OUTCOME_NONE};
+    vote_signal_enum_t nVoteSignal{VOTE_SIGNAL_NONE};
+    int64_t nTime{0};
     std::vector<unsigned char> vchSig;
 
     /** Memory only. */
-    const uint256 hash;
+    const uint256 hash{0};
     void UpdateHash() const;
 
 public:
-    CGovernanceVote();
+    CGovernanceVote() = default;
     CGovernanceVote(const COutPoint& outpointMasternodeIn, const uint256& nParentHashIn, vote_signal_enum_t eVoteSignalIn, vote_outcome_enum_t eVoteOutcomeIn);
 
-    bool IsValid() const { return fValid; }
-
-    bool IsSynced() const { return fSynced; }
-
     int64_t GetTimestamp() const { return nTime; }
 
-    vote_signal_enum_t GetSignal() const { return vote_signal_enum_t(nVoteSignal); }
+    vote_signal_enum_t GetSignal() const { return nVoteSignal; }
 
-    vote_outcome_enum_t GetOutcome() const { return vote_outcome_enum_t(nVoteOutcome); }
+    vote_outcome_enum_t GetOutcome() const { return nVoteOutcome; }
 
     const uint256& GetParentHash() const { return nParentHash; }