From fe0ebb3c0439564dd83636eb93e5830ef27b364d Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Fri, 11 Feb 2022 23:15:26 +0700 Subject: [PATCH] refactor: fix numerous compilation warnings (#4682) * style: use clang-tidy style named parameters * refactor: make IsTimeOutOfBounds testable by having current time be a parameter * style: use x-> not (*x). * refactor: make SelectCoinsGroupedByAddresses return a vector, remove out param previous semantics was return false if the vecTally vector was empty. Now we just let the caller check if it is empty or not * refactor: fix some sign-compare warnings * refactor: consistently pre-declare stuff as struct / class inline with underlying type * refactor: don't return const bool * refactor: use ref to string * refactor: use = default for CompactTallyItem * refactor: adjust "initialization" ordering * refactor: adjust how we handle negatives in GetProjectedMNPayees, use std::min * refactor: don't bind a reference to a temporary value * refactor: use a ref * refactor: ensure attempt in SelectMemberForRecovery is non-negative. * refactor: remove unused this capture * refactor: fix numerous sign-compare warnings * refactor: more consistently use size_t, use empty() --- src/bls/bls_worker.cpp | 6 +++--- src/coinjoin/client.cpp | 12 ++++++------ src/coinjoin/coinjoin.cpp | 13 +++++++------ src/coinjoin/coinjoin.h | 2 +- src/coinjoin/server.cpp | 8 ++++---- src/evo/cbtx.cpp | 4 ++-- src/evo/deterministicmns.cpp | 5 +++-- src/interfaces/chain.h | 2 +- src/interfaces/wallet.cpp | 4 ++-- src/llmq/blockprocessor.cpp | 2 +- src/llmq/commitment.cpp | 8 ++++---- src/llmq/dkgsession.cpp | 10 +++++----- src/llmq/instantsend.cpp | 4 ++-- src/llmq/quorums.cpp | 2 +- src/llmq/signing_shares.cpp | 12 ++++++------ src/llmq/signing_shares.h | 2 +- src/masternode/payments.h | 2 +- src/masternode/utils.cpp | 2 +- src/net_processing.cpp | 2 +- src/prevector.h | 2 +- src/qt/governancelist.cpp | 2 +- src/qt/guiutil.cpp | 6 +++--- src/qt/guiutil.h | 2 +- src/rpc/masternode.cpp | 4 ++-- src/rpc/rpcquorums.cpp | 2 +- src/serialize.h | 2 +- src/stacktraces.cpp | 2 +- src/util/system.cpp | 2 +- src/validation.h | 2 +- src/validationinterface.h | 2 +- src/wallet/init.cpp | 2 +- src/wallet/rpcdump.cpp | 2 +- src/wallet/rpcwallet.cpp | 4 ++-- src/wallet/test/coinjoin_tests.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 31 ++++++++++++++++-------------- src/wallet/wallet.cpp | 22 ++++++++++----------- src/wallet/wallet.h | 9 +++------ src/zmq/zmqabstractnotifier.h | 2 +- 38 files changed, 102 insertions(+), 102 deletions(-) diff --git a/src/bls/bls_worker.cpp b/src/bls/bls_worker.cpp index 373a01b785..277f76badc 100644 --- a/src/bls/bls_worker.cpp +++ b/src/bls/bls_worker.cpp @@ -87,7 +87,7 @@ bool CBLSWorker::GenerateContributions(int quorumThreshold, const BLSIdVector& i std::vector> futures; futures.reserve((quorumThreshold / batchSize + ids.size() / batchSize) + 2); - for (size_t i = 0; i < quorumThreshold; i += batchSize) { + for (size_t i = 0; i < size_t(quorumThreshold); i += batchSize) { size_t start = i; size_t count = std::min(batchSize, quorumThreshold - start); auto f = [&, start, count](int threadId) { @@ -152,10 +152,10 @@ struct Aggregator : public std::enable_shared_from_this> { bool _parallel, ctpl::thread_pool& _workerPool, DoneCallback _doneCallback) : + inputVec(std::make_shared>(count)), parallel(_parallel), workerPool(_workerPool), - doneCallback(std::move(_doneCallback)), - inputVec(std::make_shared>(count)) + doneCallback(std::move(_doneCallback)) { for (size_t i = 0; i < count; i++) { (*inputVec)[i] = pointer(_inputVec[start + i]); diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 10f3f153e5..2686ea6e34 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -1055,7 +1055,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, } // skip next mn payments winners - if (dmn->pdmnState->nLastPaidHeight + mnList.GetValidMNsCount() < mnList.GetHeight() + winners_to_skip) { + if (dmn->pdmnState->nLastPaidHeight + int(mnList.GetValidMNsCount()) < mnList.GetHeight() + winners_to_skip) { LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::JoinExistingQueue -- skipping winner, masternode=%s\n", dmn->proTxHash.ToString()); continue; } @@ -1327,7 +1327,7 @@ bool CCoinJoinClientSession::PrepareDenominate(int nMinRounds, int nMaxRounds, s // NOTE: No need to randomize order of inputs because they were // initially shuffled in CWallet::SelectTxDSInsByDenomination already. - int nSteps{0}; + size_t nSteps{0}; vecPSInOutPairsRet.clear(); // Try to add up to COINJOIN_ENTRY_MAX_SIZE of every needed denomination @@ -1388,8 +1388,8 @@ bool CCoinJoinClientSession::MakeCollateralAmounts() // We still want to consume a lot of inputs to avoid creating only smaller denoms though. // Knowing that each CTxIn is at least 148 B big, 400 inputs should take 400 x ~148 B = ~60 kB. // This still leaves more than enough room for another data of typical MakeCollateralAmounts tx. - std::vector vecTally; - if (!mixingWallet.SelectCoinsGroupedByAddresses(vecTally, false, false, true, 400)) { + std::vector vecTally = mixingWallet.SelectCoinsGroupedByAddresses(false, false, true, 400); + if (vecTally.empty()) { LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::MakeCollateralAmounts -- SelectCoinsGroupedByAddresses can't find any inputs!\n"); return false; } @@ -1571,8 +1571,8 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate) // We still want to consume a lot of inputs to avoid creating only smaller denoms though. // Knowing that each CTxIn is at least 148 B big, 400 inputs should take 400 x ~148 B = ~60 kB. // This still leaves more than enough room for another data of typical CreateDenominated tx. - std::vector vecTally; - if (!mixingWallet.SelectCoinsGroupedByAddresses(vecTally, true, true, true, 400)) { + std::vector vecTally = mixingWallet.SelectCoinsGroupedByAddresses(true, true, true, 400); + if (vecTally.empty()) { LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::CreateDenominated -- SelectCoinsGroupedByAddresses can't find any inputs!\n"); return false; } diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 95f0d384ae..c28b2666a8 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -80,9 +80,10 @@ bool CCoinJoinQueue::Relay(CConnman& connman) return true; } -bool CCoinJoinQueue::IsTimeOutOfBounds() const +bool CCoinJoinQueue::IsTimeOutOfBounds(int64_t current_time) const { - return GetAdjustedTime() - nTime > COINJOIN_QUEUE_TIMEOUT || nTime - GetAdjustedTime() > COINJOIN_QUEUE_TIMEOUT; + return current_time - nTime > COINJOIN_QUEUE_TIMEOUT || + nTime - current_time > COINJOIN_QUEUE_TIMEOUT; } uint256 CCoinJoinBroadcastTx::GetSignatureHash() const @@ -129,7 +130,7 @@ bool CCoinJoinBroadcastTx::IsValidStructure() const if (tx->vin.size() != tx->vout.size()) { return false; } - if (tx->vin.size() < CCoinJoin::GetMinPoolParticipants()) { + if (tx->vin.size() < size_t(CCoinJoin::GetMinPoolParticipants())) { return false; } if (tx->vin.size() > CCoinJoin::GetMaxPoolParticipants() * COINJOIN_ENTRY_MAX_SIZE) { @@ -167,8 +168,8 @@ void CCoinJoinBaseManager::CheckQueue() // check mixing queue objects for timeouts auto it = vecCoinJoinQueue.begin(); while (it != vecCoinJoinQueue.end()) { - if ((*it).IsTimeOutOfBounds()) { - LogPrint(BCLog::COINJOIN, "CCoinJoinBaseManager::%s -- Removing a queue (%s)\n", __func__, (*it).ToString()); + if (it->IsTimeOutOfBounds()) { + LogPrint(BCLog::COINJOIN, "CCoinJoinBaseManager::%s -- Removing a queue (%s)\n", __func__, it->ToString()); it = vecCoinJoinQueue.erase(it); } else { ++it; @@ -348,7 +349,7 @@ bool CCoinJoin::IsCollateralValid(const CTransaction& txCollateral) { LOCK(cs_main); CValidationState validationState; - if (!AcceptToMemoryPool(mempool, validationState, MakeTransactionRef(txCollateral), nullptr /* pfMissingInputs */, false /* bypass_limits */, DEFAULT_MAX_RAW_TX_FEE /* nAbsurdFee */, true /* fDryRun */)) { + if (!AcceptToMemoryPool(mempool, validationState, MakeTransactionRef(txCollateral), /*pfMissingInputs=*/nullptr, /*bypass_limits=*/false, /*nAbsurdFee=*/DEFAULT_MAX_RAW_TX_FEE, /*test_accept=*/true)) { LogPrint(BCLog::COINJOIN, "CCoinJoin::IsCollateralValid -- didn't pass AcceptToMemoryPool()\n"); return false; } diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index af7d68c6d1..554e8a63ba 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -230,7 +230,7 @@ public: bool Relay(CConnman& connman); /// Check if a queue is too old or too far into the future - bool IsTimeOutOfBounds() const; + bool IsTimeOutOfBounds(int64_t current_time = GetAdjustedTime()) const; std::string ToString() const { diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 550eb2d7bf..58a3009382 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -276,7 +276,7 @@ void CCoinJoinServer::CheckPool(CConnman& connman) LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- entries count %lu\n", GetEntriesCount()); // If we have an entry for each collateral, then create final tx - if (nState == POOL_STATE_ACCEPTING_ENTRIES && GetEntriesCount() == vecSessionCollaterals.size()) { + if (nState == POOL_STATE_ACCEPTING_ENTRIES && size_t(GetEntriesCount()) == vecSessionCollaterals.size()) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- FINALIZE TRANSACTIONS\n"); CreateFinalTransaction(connman); return; @@ -431,10 +431,10 @@ void CCoinJoinServer::ChargeFees(CConnman& connman) const if (vecOffendersCollaterals.empty()) return; //mostly offending? Charge sometimes - if ((int)vecOffendersCollaterals.size() >= vecSessionCollaterals.size() - 1 && GetRandInt(100) > 33) return; + if (vecOffendersCollaterals.size() >= vecSessionCollaterals.size() - 1 && GetRandInt(100) > 33) return; //everyone is an offender? That's not right - if ((int)vecOffendersCollaterals.size() >= vecSessionCollaterals.size()) return; + if (vecOffendersCollaterals.size() >= vecSessionCollaterals.size()) return; //charge one of the offenders randomly Shuffle(vecOffendersCollaterals.begin(), vecOffendersCollaterals.end(), FastRandomContext()); @@ -582,7 +582,7 @@ bool CCoinJoinServer::AddEntry(CConnman& connman, const CCoinJoinEntry& entry, P { if (!fMasternodeMode) return false; - if (GetEntriesCount() >= vecSessionCollaterals.size()) { + if (size_t(GetEntriesCount()) >= vecSessionCollaterals.size()) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: entries is full!\n", __func__); nMessageIDRet = ERR_ENTRIES_FULL; return false; diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index e001f4d2b5..b69d433996 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -215,7 +215,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre auto qcHash = ::SerializeHash(qc.commitment); const auto& llmq_params = llmq::GetLLMQParams(qc.commitment.llmqType); auto& v = qcHashes[llmq_params.type]; - if (v.size() == llmq_params.signingActiveQuorumCount) { + if (v.size() == size_t(llmq_params.signingActiveQuorumCount)) { // we pop the last entry, which is actually the oldest quorum as GetMinedAndActiveCommitmentsUntilBlock // returned quorums in reversed order. This pop and later push can only work ONCE, but we rely on the // fact that a block can only contain a single commitment for one LLMQ type @@ -223,7 +223,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre } v.emplace_back(qcHash); hashCount++; - if (v.size() > llmq_params.signingActiveQuorumCount) { + if (v.size() > uint64_t(llmq_params.signingActiveQuorumCount)) { return state.DoS(100, false, REJECT_INVALID, "excess-quorums-calc-cbtx-quorummerkleroot"); } } diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 5eefe91ea6..8f0002c0e6 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -230,9 +230,10 @@ CDeterministicMNCPtr CDeterministicMNList::GetMNPayee() const std::vector CDeterministicMNList::GetProjectedMNPayees(int nCount) const { - if (nCount > GetValidMNsCount()) { - nCount = GetValidMNsCount(); + if (nCount < 0 ) { + return {}; } + nCount = std::min(nCount, int(GetValidMNsCount())); std::vector result; result.reserve(nCount); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 95d019587a..a0a54124ea 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -29,7 +29,7 @@ enum class MemPoolRemovalReason; namespace llmq { class CChainLockSig; -class CInstantSendLock; +struct CInstantSendLock; } // namespace llmq typedef std::shared_ptr CTransactionRef; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 2a5fe32d8e..d3bcc3d03e 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -556,12 +556,12 @@ public: std::unique_ptr handleInstantLockReceived(InstantLockReceivedFn fn) override { return MakeHandler(m_wallet->NotifyISLockReceived.connect( - [fn, this]() { fn(); })); + [fn]() { fn(); })); } std::unique_ptr handleChainLockReceived(ChainLockReceivedFn fn) override { return MakeHandler(m_wallet->NotifyChainLockReceived.connect( - [fn, this](int chainLockHeight) { fn(chainLockHeight); })); + [fn](int chainLockHeight) { fn(chainLockHeight); })); } std::unique_ptr handleWatchOnlyChanged(WatchOnlyChangedFn fn) override { diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index d38d93ccf7..d24f19ad2e 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -459,7 +459,7 @@ std::vector CQuorumBlockProcessor::GetMinedCommitmentsUntilB } uint32_t nMinedHeight = std::numeric_limits::max() - be32toh(std::get<2>(curKey)); - if (nMinedHeight > pindex->nHeight) { + if (nMinedHeight > uint32_t(pindex->nHeight)) { break; } diff --git a/src/llmq/commitment.cpp b/src/llmq/commitment.cpp index 1aa58c21e4..bc7f5ae750 100644 --- a/src/llmq/commitment.cpp +++ b/src/llmq/commitment.cpp @@ -69,7 +69,7 @@ bool CFinalCommitment::Verify(const CBlockIndex* pQuorumBaseBlockIndex, bool che } auto members = CLLMQUtils::GetAllQuorumMembers(llmq_params, pQuorumBaseBlockIndex); - for (size_t i = members.size(); i < llmq_params.size; i++) { + for (size_t i = members.size(); i < size_t(llmq_params.size); i++) { if (validMembers[i]) { LogPrintfFinalCommitment("invalid validMembers bitset. bit %d should not be set\n", i); return false; @@ -122,11 +122,11 @@ bool CFinalCommitment::VerifyNull() const bool CFinalCommitment::VerifySizes(const Consensus::LLMQParams& params) const { - if (signers.size() != params.size) { + if (signers.size() != size_t(params.size)) { LogPrintfFinalCommitment("invalid signers.size=%d\n", signers.size()); return false; } - if (validMembers.size() != params.size) { + if (validMembers.size() != size_t(params.size)) { LogPrintfFinalCommitment("invalid signers.size=%d\n", signers.size()); return false; } @@ -144,7 +144,7 @@ bool CheckLLMQCommitment(const CTransaction& tx, const CBlockIndex* pindexPrev, return state.DoS(100, false, REJECT_INVALID, "bad-qc-version"); } - if (qcTx.nHeight != pindexPrev->nHeight + 1) { + if (qcTx.nHeight != uint32_t(pindexPrev->nHeight + 1)) { return state.DoS(100, false, REJECT_INVALID, "bad-qc-height"); } diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index 26d131a438..5ffd35968e 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -98,7 +98,7 @@ bool CDKGSession::Init(const CBlockIndex* _pQuorumBaseBlockIndex, const std::vec CDKGLogger logger(*this, __func__); - if (mns.size() < params.minSize) { + if (mns.size() < size_t(params.minSize)) { logger.Batch("not enough members (%d < %d), aborting init", mns.size(), params.minSize); return false; } @@ -213,7 +213,7 @@ bool CDKGSession::PreVerifyMessage(const CDKGContribution& qc, bool& retBan) con retBan = true; return false; } - if (qc.vvec->size() != params.threshold) { + if (qc.vvec->size() != size_t(params.threshold)) { logger.Batch("invalid verification vector length"); retBan = true; return false; @@ -626,7 +626,7 @@ void CDKGSession::VerifyAndJustify(CDKGPendingMessages& pendingMessages) if (m->bad) { continue; } - if (m->badMemberVotes.size() >= params.dkgBadVotesThreshold) { + if (m->badMemberVotes.size() >= size_t(params.dkgBadVotesThreshold)) { logger.Batch("%s marked as bad as %d other members voted for this", m->dmn->proTxHash.ToString(), m->badMemberVotes.size()); MarkBadMember(m->idx); continue; @@ -1053,7 +1053,7 @@ bool CDKGSession::PreVerifyMessage(const CDKGPrematureCommitment& qc, bool& retB return false; } - for (size_t i = members.size(); i < params.size; i++) { + for (size_t i = members.size(); i < size_t(params.size); i++) { if (qc.validMembers[i]) { retBan = true; logger.Batch("invalid validMembers bitset. bit %d should not be set", i); @@ -1187,7 +1187,7 @@ std::vector CDKGSession::FinalizeCommitments() std::vector finalCommitments; for (const auto& p : commitmentsMap) { auto& cvec = p.second; - if (cvec.size() < params.minSize) { + if (cvec.size() < size_t(params.minSize)) { // commitment was signed by a minority continue; } diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp index f83596109e..090c419c2d 100644 --- a/src/llmq/instantsend.cpp +++ b/src/llmq/instantsend.cpp @@ -190,7 +190,7 @@ std::unordered_map CInstantSen break; } uint32_t nHeight = std::numeric_limits::max() - be32toh(std::get<1>(curKey)); - if (nHeight > nUntilHeight) { + if (nHeight > uint32_t(nUntilHeight)) { break; } @@ -234,7 +234,7 @@ void CInstantSendDb::RemoveArchivedInstantSendLocks(int nUntilHeight) break; } uint32_t nHeight = std::numeric_limits::max() - be32toh(std::get<1>(curKey)); - if (nHeight > nUntilHeight) { + if (nHeight > uint32_t(nUntilHeight)) { break; } diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index daca6f80c2..ad29222fc5 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -674,7 +674,7 @@ void CQuorumManager::ProcessMessage(CNode* pFrom, const std::string& strCommand, // Check if request has ENCRYPTED_CONTRIBUTIONS data if (request.GetDataMask() & CQuorumDataRequest::ENCRYPTED_CONTRIBUTIONS) { - if (WITH_LOCK(pQuorum->cs, return pQuorum->quorumVvec->size() != pQuorum->params.threshold)) { + if (WITH_LOCK(pQuorum->cs, return pQuorum->quorumVvec->size() != size_t(pQuorum->params.threshold))) { errorHandler("No valid quorum verification vector available", 0); // Don't bump score because we asked for it return; } diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 4b9d69b5f5..823c34920d 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -336,7 +336,7 @@ bool CSigSharesManager::ProcessMessageSigSesAnn(const CNode* pfrom, const CSigSe bool CSigSharesManager::VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv) { - return inv.inv.size() == GetLLMQParams(llmqType).size; + return inv.inv.size() == size_t(GetLLMQParams(llmqType).size); } bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode* pfrom, const CSigSharesInv& inv) @@ -737,7 +737,7 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CConnma } size_t sigShareCount = sigShares.CountForSignHash(sigShare.GetSignHash()); - if (sigShareCount >= quorum->params.threshold) { + if (sigShareCount >= size_t(quorum->params.threshold)) { canTryRecovery = true; } } @@ -766,14 +766,14 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& sigSharesForRecovery.reserve((size_t) quorum->params.threshold); idsForRecovery.reserve((size_t) quorum->params.threshold); - for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < quorum->params.threshold; ++it) { + for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < size_t(quorum->params.threshold); ++it) { auto& sigShare = it->second; sigSharesForRecovery.emplace_back(sigShare.sigShare.Get()); idsForRecovery.emplace_back(quorum->members[sigShare.quorumMember]->proTxHash); } // check if we can recover the final signature - if (sigSharesForRecovery.size() < quorum->params.threshold) { + if (sigSharesForRecovery.size() < size_t(quorum->params.threshold)) { return; } } @@ -809,9 +809,9 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& quorumSigningManager->ProcessRecoveredSig(rs); } -CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256 &id, int attempt) +CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256 &id, size_t attempt) { - assert(attempt < quorum->members.size()); + assert(size_t(attempt) < quorum->members.size()); std::vector> v; v.reserve(quorum->members.size()); diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 840d1d6204..2bc1b156e7 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -398,7 +398,7 @@ public: void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override; - static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256& id, int attempt); + static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256& id, size_t attempt); private: // all of these return false when the currently processed message should be aborted (as each message actually contains multiple messages) diff --git a/src/masternode/payments.h b/src/masternode/payments.h index 25672f76e0..7787ade6e1 100644 --- a/src/masternode/payments.h +++ b/src/masternode/payments.h @@ -13,7 +13,7 @@ class CMasternodePayments; class CBlock; class CTransaction; -class CMutableTransaction; +struct CMutableTransaction; class CTxOut; /// TODO: all 4 functions do not belong here really, they should be refactored/moved somewhere (main.cpp ?) diff --git a/src/masternode/utils.cpp b/src/masternode/utils.cpp index e70f2c4813..fe05273a18 100644 --- a/src/masternode/utils.cpp +++ b/src/masternode/utils.cpp @@ -31,7 +31,7 @@ void CMasternodeUtils::ProcessMasternodeConnections(CConnman& connman) nonMasternodeCount++; } }); - if (nonMasternodeCount < connman.GetMaxOutboundNodeCount()) { + if (nonMasternodeCount < int(connman.GetMaxOutboundNodeCount())) { return; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5f5360fa66..333892e376 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3919,7 +3919,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr bool found = false; const std::vector &allMessages = getAllNetMessageTypes(); - for (const std::string msg : allMessages) { + for (const std::string& msg : allMessages) { if(msg == strCommand) { found = true; break; diff --git a/src/prevector.h b/src/prevector.h index b263985b81..7c2d339ede 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -555,7 +555,7 @@ public: // This avoids internal use of std::copy and operator++ on the iterators and instead allows efficient memcpy/memmove if (std::is_trivially_constructible::value) { auto s = e - b; - if (v.size() != s) { + if (v.size() != size_t(s)) { v.resize(s); } if (!v.empty()) { diff --git a/src/qt/governancelist.cpp b/src/qt/governancelist.cpp index 11e36ec3f4..d9a6060d41 100644 --- a/src/qt/governancelist.cpp +++ b/src/qt/governancelist.cpp @@ -235,7 +235,7 @@ void ProposalModel::reconcile(const std::vector& proposals) std::vector keep_index(m_data.count(), false); for (const auto proposal : proposals) { bool found = false; - for (unsigned int i = 0; i < m_data.count(); ++i) { + for (int i = 0; i < m_data.count(); ++i) { if (m_data.at(i)->hash() == proposal->hash()) { found = true; keep_index.at(i) = true; diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 8597073c13..afcb58f37f 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -979,7 +979,7 @@ const QString getDefaultTheme() return defaultTheme; } -const bool isValidTheme(const QString& strTheme) +bool isValidTheme(const QString& strTheme) { return strTheme == defaultTheme || strTheme == darkThemePrefix || strTheme == traditionalTheme; } @@ -1626,14 +1626,14 @@ std::vector getSupportedWeights() QFont::Weight supportedWeightFromIndex(int nIndex) { auto vecWeights = getSupportedWeights(); - assert(vecWeights.size() > nIndex); + assert(vecWeights.size() > uint64_t(nIndex)); return vecWeights[nIndex]; } int supportedWeightToIndex(QFont::Weight weight) { auto vecWeights = getSupportedWeights(); - for (int index = 0; index < vecWeights.size(); ++index) { + for (uint64_t index = 0; index < vecWeights.size(); ++index) { if (weight == vecWeights[index]) { return index; } diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 425fe7791d..9a214d6b5d 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -291,7 +291,7 @@ namespace GUIUtil const QString getDefaultTheme(); /** Check if the given theme name is valid or not */ - const bool isValidTheme(const QString& strTheme); + bool isValidTheme(const QString& strTheme); /** Sets the stylesheet of the whole app and updates it if the related css files has been changed and -debug-ui mode is active. */ diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index f80b927539..f97b2e9af4 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -439,7 +439,7 @@ static UniValue masternode_payments(const JSONRPCRequest& request) // A temporary vector which is used to sort results properly (there is no "reverse" in/for UniValue) std::vector vecPayments; - while (vecPayments.size() < std::abs(nCount) && pindex != nullptr) { + while (vecPayments.size() < uint64_t(std::abs(nCount)) && pindex != nullptr) { CBlock block; if (!ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { @@ -454,7 +454,7 @@ static UniValue masternode_payments(const JSONRPCRequest& request) continue; } CAmount nValueIn{0}; - for (const auto txin : tx->vin) { + for (const auto& txin : tx->vin) { CTransactionRef txPrev; uint256 blockHashTmp; GetTransaction(txin.prevout.hash, txPrev, Params().GetConsensus(), blockHashTmp); diff --git a/src/rpc/rpcquorums.cpp b/src/rpc/rpcquorums.cpp index 525c71c04b..ccf73e7bc7 100644 --- a/src/rpc/rpcquorums.cpp +++ b/src/rpc/rpcquorums.cpp @@ -543,7 +543,7 @@ static UniValue quorum_selectquorum(const JSONRPCRequest& request) ret.pushKV("quorumHash", quorum->qc->quorumHash.ToString()); UniValue recoveryMembers(UniValue::VARR); - for (int i = 0; i < quorum->params.recoveryMembers; i++) { + for (size_t i = 0; i < size_t(quorum->params.recoveryMembers); i++) { auto dmn = llmq::quorumSigSharesManager->SelectMemberForRecovery(quorum, id, i); recoveryMembers.push_back(dmn->proTxHash.ToString()); } diff --git a/src/serialize.h b/src/serialize.h index 21e2488c76..4c4c38f95c 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -513,7 +513,7 @@ void ReadFixedVarIntsBitSet(Stream& s, std::vector& vec, size_t size) break; } int32_t idx = last + offset; - if (idx >= size) { + if (idx >= int32_t(size)) { throw std::ios_base::failure("out of bounds index"); } if (last != -1 && idx <= last) { diff --git a/src/stacktraces.cpp b/src/stacktraces.cpp index 619d1d8747..f6c083d0dd 100644 --- a/src/stacktraces.cpp +++ b/src/stacktraces.cpp @@ -109,7 +109,7 @@ static std::string GetExeFileName() if (len < 0) { return ""; } - if (len < buf.size()) { + if (len < int64_t(buf.size())) { return std::string(buf.begin(), buf.begin() + len); } buf.resize(buf.size() * 2); diff --git a/src/util/system.cpp b/src/util/system.cpp index 4609b263d5..53ca09d26c 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1229,7 +1229,7 @@ void RenameThreadPool(ctpl::thread_pool& tp, const char* baseName) // `doneCnt` should be at least `futures.size()` if tp size was increased (for whatever reason), // or at least `tp.size()` if tp size was decreased and queue was cleared // (which can happen on `stop()` if we were not fast enough to get all jobs to their threads). - } while (doneCnt < futures.size() && doneCnt < tp.size()); + } while (uint64_t(doneCnt) < futures.size() && doneCnt < tp.size()); cond->notify_all(); diff --git a/src/validation.h b/src/validation.h index 83e6cb2b12..f9ff6bcbff 100644 --- a/src/validation.h +++ b/src/validation.h @@ -43,7 +43,7 @@ class CScriptCheck; class CBlockPolicyEstimator; class CTxMemPool; class CValidationState; -class PrecomputedTransactionData; +struct PrecomputedTransactionData; struct ChainTxData; struct DisconnectedBlockTransactions; diff --git a/src/validationinterface.h b/src/validationinterface.h index 435260d308..04762d98dc 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -31,7 +31,7 @@ enum class MemPoolRemovalReason; namespace llmq { class CChainLockSig; - class CInstantSendLock; + struct CInstantSendLock; class CRecoveredSig; } // namespace llmq diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 47d7d2a999..072518e561 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -255,7 +255,7 @@ void WalletInit::Construct(InitInterfaces& interfaces) const void WalletInit::AutoLockMasternodeCollaterals() const { // we can't do this before DIP3 is fully initialized - for (const auto pwallet : GetWallets()) { + for (const auto& pwallet : GetWallets()) { pwallet->AutoLockMasternodeCollaterals(); } } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index ea99893b82..5c4ef2fc0f 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -821,7 +821,7 @@ UniValue importelectrumwallet(const JSONRPCRequest& request) file.close(); pwallet->ShowProgress("", 100); // hide progress dialog in GUI - const uint32_t tip_height = locked_chain->getHeight().value_or(-1); + const int32_t tip_height = locked_chain->getHeight().value_or(std::numeric_limits::max()); // Whether to perform rescan after import int nStartHeight = 0; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c06d2f75db..fc1190e6f3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3855,7 +3855,7 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request) // Find all addresses that have the given label UniValue ret(UniValue::VOBJ); std::set addresses; - for (const std::pair& item : pwallet->mapAddressBook) { + for (const std::pair item : pwallet->mapAddressBook) { if (item.second.name == label) { std::string address = EncodeDestination(item.first); // CWallet::mapAddressBook is not expected to contain duplicate @@ -3921,7 +3921,7 @@ static UniValue listlabels(const JSONRPCRequest& request) // Add to a set to sort by label name, then insert into Univalue array std::set label_set; - for (const std::pair& entry : pwallet->mapAddressBook) { + for (const std::pair entry : pwallet->mapAddressBook) { if (purpose.empty() || entry.second.purpose == purpose) { label_set.insert(entry.second.name); } diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index ae9562fef9..0e8048c4a7 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -97,7 +97,7 @@ public: BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, state)); AddTxToChain(tx->GetHash()); for (size_t n = 0; n < tx->vout.size(); ++n) { - if (nChangePosRet != -1 && n == nChangePosRet) { + if (nChangePosRet != -1 && int(n) == nChangePosRet) { // Skip the change output to only return the requested coins continue; } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b197d6ddfe..d8c52ba0a5 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -598,7 +598,7 @@ public: std::vector vecOutpoints; size_t n; for (n = 0; n < tx->vout.size(); ++n) { - if (nChangePosRet != -1 && n == nChangePosRet) { + if (nChangePosRet != -1 && int(n) == nChangePosRet) { // Skip the change output to only return the requested coins continue; } @@ -632,7 +632,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) {13, {{100001, true}}} }; assert(mapTestCases.size() == mapExpected.size()); - for (int i = 0; i < mapTestCases.size(); ++i) { + for (size_t i = 0; i < mapTestCases.size(); ++i) { if (!CreateTransaction(mapTestCases.at(i), mapExpected.at(i).first, mapExpected.at(i).second)) { std::cout << strprintf("CreateTransactionTest failed at: %d - %d\n", nTestId, i) << std::endl; } @@ -858,10 +858,10 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) // Just to create nCount output recipes to use in tests below std::vector> vecOutputEntries{{5000, false}}; auto createOutputEntries = [&](int nCount) { - while (vecOutputEntries.size() <= nCount) { + while (vecOutputEntries.size() <= size_t(nCount)) { vecOutputEntries.push_back(vecOutputEntries.back()); } - if (vecOutputEntries.size() > nCount) { + if (vecOutputEntries.size() > size_t(nCount)) { int nDiff = vecOutputEntries.size() - nCount; vecOutputEntries.erase(vecOutputEntries.begin(), vecOutputEntries.begin() + nDiff); } @@ -904,13 +904,15 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup // Check initial balance from one mature coinbase transaction. BOOST_CHECK_EQUAL(wallet->GetAvailableBalance(), 500 * COIN); - std::vector vecTally; - BOOST_CHECK(wallet->SelectCoinsGroupedByAddresses(vecTally, false /*fSkipDenominated*/, false /*fAnonymizable*/, - false /*fSkipUnconfirmed*/, 100/*nMaxOupointsPerAddress*/)); - BOOST_CHECK_EQUAL(vecTally.size(), 1); - BOOST_CHECK_EQUAL(vecTally.at(0).nAmount, 500 * COIN); - BOOST_CHECK_EQUAL(vecTally.at(0).vecInputCoins.size(), 1); - vecTally.clear(); + { + std::vector vecTally = wallet->SelectCoinsGroupedByAddresses(/*fSkipDenominated=*/false, + /*fAnonymizable=*/false, + /*fSkipUnconfirmed=*/false, + /*nMaxOupointsPerAddress=*/100); + BOOST_CHECK_EQUAL(vecTally.size(), 1); + BOOST_CHECK_EQUAL(vecTally.at(0).nAmount, 500 * COIN); + BOOST_CHECK_EQUAL(vecTally.at(0).vecInputCoins.size(), 1); + } // Create two conflicting transactions, add one to the wallet and mine the other one. CTransactionRef tx1; @@ -946,14 +948,15 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup // Committed tx is the one that should be marked as "conflicting". // Make sure that available balance and SelectCoinsGroupedByAddresses results match. - BOOST_CHECK(wallet->SelectCoinsGroupedByAddresses(vecTally, false /*fSkipDenominated*/, false /*fAnonymizable*/, - false /*fSkipUnconfirmed*/, 100/*nMaxOupointsPerAddress*/)); + const auto vecTally = wallet->SelectCoinsGroupedByAddresses(/*fSkipDenominated=*/false, + /*fAnonymizable=*/false, + /*fSkipUnconfirmed=*/false, + /*nMaxOupointsPerAddress=*/100); BOOST_CHECK_EQUAL(vecTally.size(), 2); BOOST_CHECK_EQUAL(vecTally.at(0).vecInputCoins.size(), 1); BOOST_CHECK_EQUAL(vecTally.at(1).vecInputCoins.size(), 1); BOOST_CHECK_EQUAL(vecTally.at(0).nAmount + vecTally.at(1).nAmount, (500 + 499) * COIN); BOOST_CHECK_EQUAL(wallet->GetAvailableBalance(), (500 + 499) * COIN); - vecTally.clear(); } BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 79ec2bb140..17394bd5f3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2773,8 +2773,8 @@ CAmount CWallet::GetAnonymizableBalance(bool fSkipDenominated, bool fSkipUnconfi { if (!CCoinJoinClientOptions::IsEnabled()) return 0; - std::vector vecTally; - if(!SelectCoinsGroupedByAddresses(vecTally, fSkipDenominated, true, fSkipUnconfirmed)) return 0; + std::vector vecTally = SelectCoinsGroupedByAddresses(fSkipDenominated, true, fSkipUnconfirmed); + if (vecTally.empty()) return 0; CAmount nTotal = 0; @@ -3351,7 +3351,7 @@ static uint32_t GetLocktimeForNewTransaction(interfaces::Chain& chain, interface return locktime; } -bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTallyRet, bool fSkipDenominated, bool fAnonymizable, bool fSkipUnconfirmed, int nMaxOupointsPerAddress) const +std::vector CWallet::SelectCoinsGroupedByAddresses(bool fSkipDenominated, bool fAnonymizable, bool fSkipUnconfirmed, int nMaxOupointsPerAddress) const { auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -3362,14 +3362,12 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa // This should only be used if nMaxOupointsPerAddress was NOT specified. if(nMaxOupointsPerAddress == -1 && fAnonymizable && fSkipUnconfirmed) { if(fSkipDenominated && fAnonymizableTallyCachedNonDenom) { - vecTallyRet = vecAnonymizableTallyCachedNonDenom; - LogPrint(BCLog::SELECTCOINS, "SelectCoinsGroupedByAddresses - using cache for non-denom inputs %d\n", vecTallyRet.size()); - return vecTallyRet.size() > 0; + LogPrint(BCLog::SELECTCOINS, "SelectCoinsGroupedByAddresses - using cache for non-denom inputs %d\n", vecAnonymizableTallyCachedNonDenom.size()); + return vecAnonymizableTallyCachedNonDenom; } if(!fSkipDenominated && fAnonymizableTallyCached) { - vecTallyRet = vecAnonymizableTallyCached; - LogPrint(BCLog::SELECTCOINS, "SelectCoinsGroupedByAddresses - using cache for all inputs %d\n", vecTallyRet.size()); - return vecTallyRet.size() > 0; + LogPrint(BCLog::SELECTCOINS, "SelectCoinsGroupedByAddresses - using cache for all inputs %d\n", vecAnonymizableTallyCached.size()); + return vecAnonymizableTallyCached; } } @@ -3399,7 +3397,7 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa if(!(mine & filter)) continue; auto itTallyItem = mapTally.find(txdest); - if (nMaxOupointsPerAddress != -1 && itTallyItem != mapTally.end() && itTallyItem->second.vecInputCoins.size() >= nMaxOupointsPerAddress) continue; + if (nMaxOupointsPerAddress != -1 && itTallyItem != mapTally.end() && int64_t(itTallyItem->second.vecInputCoins.size()) >= nMaxOupointsPerAddress) continue; if(IsSpent(*locked_chain, outpoint.hash, i) || IsLockedCoin(outpoint.hash, i)) continue; @@ -3427,7 +3425,7 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa // construct resulting vector // NOTE: vecTallyRet is "sorted" by txdest (i.e. address), just like mapTally - vecTallyRet.clear(); + std::vector vecTallyRet; for (const auto& item : mapTally) { if(fAnonymizable && item.second.nAmount < nSmallestDenom) continue; vecTallyRet.push_back(item.second); @@ -3453,7 +3451,7 @@ bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTa LogPrint(BCLog::SELECTCOINS, "%s", strMessage); /* Continued */ } - return vecTallyRet.size() > 0; + return vecTallyRet; } bool CWallet::SelectDenominatedAmounts(CAmount nValueMax, std::set& setAmountsRet) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 06e273f71f..93861b9f3b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -112,12 +112,9 @@ enum WalletFeature struct CompactTallyItem { CTxDestination txdest; - CAmount nAmount; + CAmount nAmount{0}; std::vector vecInputCoins; - CompactTallyItem() - { - nAmount = 0; - } + CompactTallyItem() = default; }; enum WalletFlags : uint64_t { @@ -825,7 +822,7 @@ public: bool SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector& vecTxDSInRet); bool SelectDenominatedAmounts(CAmount nValueMax, std::set& setAmountsRet) const; - bool SelectCoinsGroupedByAddresses(std::vector& vecTallyRet, bool fSkipDenominated = true, bool fAnonymizable = true, bool fSkipUnconfirmed = true, int nMaxOupointsPerAddress = -1) const; + std::vector SelectCoinsGroupedByAddresses(bool fSkipDenominated = true, bool fAnonymizable = true, bool fSkipUnconfirmed = true, int nMaxOupointsPerAddress = -1) const; bool HasCollateralInputs(bool fOnlyConfirmed = true) const; int CountInputsWithAmount(CAmount nInputAmount) const; diff --git a/src/zmq/zmqabstractnotifier.h b/src/zmq/zmqabstractnotifier.h index bb39301f08..5148158647 100644 --- a/src/zmq/zmqabstractnotifier.h +++ b/src/zmq/zmqabstractnotifier.h @@ -20,7 +20,7 @@ typedef std::shared_ptr CTransactionRef; namespace llmq { class CChainLockSig; - class CInstantSendLock; + struct CInstantSendLock; class CRecoveredSig; } // namespace llmq