From 3133be10f9f3cc3d5999230a580d0ec5c7c7c3b2 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 20 Feb 2024 21:22:37 +0700 Subject: [PATCH] test: multiple linter warnings to suppress or fix (#5880) ## Issue being fixed or feature implemented On my local kubuntu linters have way too much spam ## What was done? See each commit ## How Has This Been Tested? Run locally. Amount of warnings decreased from thousands to fewer amount. Excluding typos, they are: ``` src/coinjoin/client.cpp:1420:5: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm] src/coinjoin/client.cpp:1426:5: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm] src/coinjoin/client.cpp:655:26: warning: Consider using std::copy_if algorithm instead of a raw loop. [useStlAlgorithm] src/coinjoin/server.cpp:593:33: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm] src/coinjoin/server.cpp:630:106: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm] src/governance/governance.cpp:1057:9: warning: C-style pointer casting [cstyleCast] src/governance/governance.cpp:1068:9: warning: C-style pointer casting [cstyleCast] src/governance/governance.cpp:1079:13: warning: C-style pointer casting [cstyleCast] src/governance/governance.cpp:1086:9: warning: C-style pointer casting [cstyleCast] src/governance/governance.cpp:1094:9: warning: C-style pointer casting [cstyleCast] src/governance/governance.cpp:1099:5: warning: C-style pointer casting [cstyleCast] src/governance/governance.cpp:1486:34: warning: Consider using std::copy_if algorithm instead of a raw loop. [useStlAlgorithm] src/llmq/commitment.cpp:102:5: warning: Consider using std::all_of or std::none_of algorithm instead of a raw loop. [useStlAlgorithm] src/llmq/instantsend.cpp:820:38: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm] src/llmq/quorums.cpp:831:102: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm] src/llmq/quorums.h:300:17: warning: C-style pointer casting [cstyleCast] src/llmq/quorums.h:301:17: warning: C-style pointer casting [cstyleCast] src/llmq/quorums.h:302:17: warning: C-style pointer casting [cstyleCast] src/llmq/quorums.h:303:17: warning: C-style pointer casting [cstyleCast] src/spork.cpp:119:58: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm] src/statsd_client.cpp:234:63: warning: C-style pointer casting [cstyleCast] Advice not applicable in this specific case? Add an exception by updating IGNORED_WARNINGS in test/lint/lint-cppcheck-dash.sh ^---- failure generated from test/lint/lint-cppcheck-dash.sh Consider install flake8-cached for cached flake8 results. test/functional/data/invalid_txs.py: error: Source file found twice under different module names: "invalid_txs" and "data.invalid_txs" test/functional/data/invalid_txs.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info test/functional/data/invalid_txs.py: note: Common resolutions include: a) adding `__init__.py` somewhere, b) using `--explicit-package-bases` or adjusting MYPYPATH Found 1 error in 1 file (errors prevented further checking) ^---- failure generated from test/lint/lint-python.s ``` ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone --- doc/release-notes-5742.md | 2 +- src/evo/cbtx.cpp | 2 +- src/evo/mnhftx.h | 6 +++--- src/evo/simplifiedmns.cpp | 9 ++------- src/evo/simplifiedmns.h | 2 +- src/governance/classes.cpp | 2 +- src/llmq/chainlocks.cpp | 6 +++--- src/llmq/context.cpp | 2 +- src/llmq/signing_shares.cpp | 2 +- src/masternode/utils.cpp | 2 +- test/functional/feature_dip3_v19.py | 2 +- test/functional/feature_llmq_connections.py | 4 ++-- test/functional/feature_llmq_simplepose.py | 2 +- test/lint/lint-cppcheck-dash.sh | 2 +- test/lint/lint-python.sh | 3 ++- 15 files changed, 22 insertions(+), 26 deletions(-) diff --git a/doc/release-notes-5742.md b/doc/release-notes-5742.md index 6fa741c293..2b8c6b7262 100644 --- a/doc/release-notes-5742.md +++ b/doc/release-notes-5742.md @@ -1,4 +1,4 @@ RPC changes ----------- -RPC `gettxchainlocks` will also return the status `mempool` indicating wether the transaction is in the mempool or not. +RPC `gettxchainlocks` will also return the status `mempool` indicating whether the transaction is in the mempool or not. diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index b67d8439b7..7bd3777ea8 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -175,7 +175,7 @@ using QcIndexedHashMap = std::map diff --git a/src/evo/mnhftx.h b/src/evo/mnhftx.h index 73983a5540..6f14ac8653 100644 --- a/src/evo/mnhftx.h +++ b/src/evo/mnhftx.h @@ -101,7 +101,7 @@ private: // versionBit <-> height unordered_lru_cache mnhfCache GUARDED_BY(cs_cache) {MNHFCacheSize}; - // This cache is used only for v20 activation to avoid double lock throught VersionBitsConditionChecker::SignalHeight + // This cache is used only for v20 activation to avoid double lock through VersionBitsConditionChecker::SignalHeight VersionBitsCache v20_activation GUARDED_BY(cs_cache); public: explicit CMNHFManager(CEvoDB& evoDb); @@ -116,8 +116,8 @@ public: /** * Every undo block should be processed when Tip() is updated by calling of CMNHFManager::UndoBlock - * This function actually does nothing at the moment, because status of ancester block is already know. - * Altough it should be still called to do some sanity checks + * This function actually does nothing at the moment, because status of ancestor block is already know. + * Although it should be still called to do some sanity checks */ bool UndoBlock(const CBlock& block, const CBlockIndex* const pindex); diff --git a/src/evo/simplifiedmns.cpp b/src/evo/simplifiedmns.cpp index 4d8c1b3852..18dedb9ddd 100644 --- a/src/evo/simplifiedmns.cpp +++ b/src/evo/simplifiedmns.cpp @@ -183,7 +183,7 @@ bool CSimplifiedMNListDiff::BuildQuorumsDiff(const CBlockIndex* baseBlockIndex, return true; } -bool CSimplifiedMNListDiff::BuildQuorumChainlockInfo(const CBlockIndex* blockIndex) +void CSimplifiedMNListDiff::BuildQuorumChainlockInfo(const CBlockIndex* blockIndex) { // Group quorums (indexes corresponding to entries of newQuorums) per CBlockIndex containing the expected CL signature in CbTx. // We want to avoid to load CbTx now, as more than one quorum will target the same block: hence we want to load CbTxs once per block (heavy operation). @@ -220,8 +220,6 @@ bool CSimplifiedMNListDiff::BuildQuorumChainlockInfo(const CBlockIndex* blockInd it_sig->second.insert(idx_set.begin(), idx_set.end()); } } - - return true; } UniValue CSimplifiedMNListDiff::ToJson(bool extended) const @@ -363,10 +361,7 @@ bool BuildSimplifiedMNListDiff(const uint256& baseBlockHash, const uint256& bloc } if (DeploymentActiveAfter(blockIndex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) { - if (!mnListDiffRet.BuildQuorumChainlockInfo(blockIndex)) { - errorRet = strprintf("failed to build quorums chainlocks info"); - return false; - } + mnListDiffRet.BuildQuorumChainlockInfo(blockIndex); } // TODO store coinbase TX in CBlockIndex diff --git a/src/evo/simplifiedmns.h b/src/evo/simplifiedmns.h index 4abde7c39d..de6a21515a 100644 --- a/src/evo/simplifiedmns.h +++ b/src/evo/simplifiedmns.h @@ -164,7 +164,7 @@ public: bool BuildQuorumsDiff(const CBlockIndex* baseBlockIndex, const CBlockIndex* blockIndex, const llmq::CQuorumBlockProcessor& quorum_block_processor); - bool BuildQuorumChainlockInfo(const CBlockIndex* blockIndex); + void BuildQuorumChainlockInfo(const CBlockIndex* blockIndex); [[nodiscard]] UniValue ToJson(bool extended = false) const; }; diff --git a/src/governance/classes.cpp b/src/governance/classes.cpp index 6ee0c2b7f7..7096edb4b0 100644 --- a/src/governance/classes.cpp +++ b/src/governance/classes.cpp @@ -407,7 +407,7 @@ CSuperblock:: nStatus(SeenObjectStatus::Unknown), vecPayments() { - CGovernanceObject* pGovObj = GetGovernanceObject(*governance); + const CGovernanceObject* pGovObj = GetGovernanceObject(*governance); if (!pGovObj) { throw std::runtime_error("CSuperblock: Failed to find Governance Object"); diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index 16fcfe7da6..59ac16b659 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -424,7 +424,7 @@ CChainLocksHandler::BlockTxs::mapped_type CChainLocksHandler::GetBlockTxs(const uint32_t blockTime; { LOCK(cs_main); - auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(blockHash); + const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(blockHash); CBlock block; if (!ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { return nullptr; @@ -640,7 +640,7 @@ void CChainLocksHandler::Cleanup() } for (auto it = blockTxs.begin(); it != blockTxs.end(); ) { - auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(it->first); + const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(it->first); if (InternalHasChainLock(pindex->nHeight, pindex->GetBlockHash())) { for (const auto& txid : *it->second) { txFirstSeenTime.erase(txid); @@ -659,7 +659,7 @@ void CChainLocksHandler::Cleanup() // tx has vanished, probably due to conflicts it = txFirstSeenTime.erase(it); } else if (!hashBlock.IsNull()) { - auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(hashBlock); + const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(hashBlock); if (m_chainstate.m_chain.Tip()->GetAncestor(pindex->nHeight) == pindex && m_chainstate.m_chain.Height() - pindex->nHeight >= 6) { // tx got confirmed >= 6 times, so we can stop keeping track of it it = txFirstSeenTime.erase(it); diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 6d96d77208..b62943c1df 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -54,7 +54,7 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CEvoDB& evo } LLMQContext::~LLMQContext() { - // LLMQContext doesn't own these objects, but still need to care of them for consistancy: + // LLMQContext doesn't own these objects, but still need to care of them for consistency: llmq::quorumInstantSendManager.reset(); llmq::chainLocksHandler.reset(); llmq::quorumManager.reset(); diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 0af6c79283..8667fdcff8 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -152,7 +152,7 @@ CSigSharesNodeState::Session* CSigSharesNodeState::GetSessionByRecvId(uint32_t s bool CSigSharesNodeState::GetSessionInfoByRecvId(uint32_t sessionId, SessionInfo& retInfo) { - auto* s = GetSessionByRecvId(sessionId); + const auto* s = GetSessionByRecvId(sessionId); if (s == nullptr) { return false; } diff --git a/src/masternode/utils.cpp b/src/masternode/utils.cpp index 9cc6341dff..651264d3f8 100644 --- a/src/masternode/utils.cpp +++ b/src/masternode/utils.cpp @@ -29,7 +29,7 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, const CMasternodeSync& m // Don't disconnect masternode connections when we have less then the desired amount of outbound nodes int nonMasternodeCount = 0; - connman.ForEachNode(CConnman::AllNodes, [&](CNode* pnode) { + connman.ForEachNode(CConnman::AllNodes, [&](const CNode* pnode) { if ((!pnode->IsInboundConn() && !pnode->IsFeelerConn() && !pnode->IsManualConn() && diff --git a/test/functional/feature_dip3_v19.py b/test/functional/feature_dip3_v19.py index 189702337e..4800bd79b1 100755 --- a/test/functional/feature_dip3_v19.py +++ b/test/functional/feature_dip3_v19.py @@ -142,7 +142,7 @@ class DIP3V19Test(DashTestFramework): self.wait_until(lambda: self.nodes[node_idx].getconnectioncount() == 0) self.connect_nodes(node_idx, 0) self.sync_all(self.nodes) - self.log.info(f"Succesfully revoked={revoke_protx}") + self.log.info(f"Successfully revoked={revoke_protx}") for mn in self.mninfo: if mn.proTxHash == revoke_protx: self.mninfo.remove(mn) diff --git a/test/functional/feature_llmq_connections.py b/test/functional/feature_llmq_connections.py index e8bdc3a015..98c75b1e0c 100755 --- a/test/functional/feature_llmq_connections.py +++ b/test/functional/feature_llmq_connections.py @@ -86,7 +86,7 @@ class LLMQConnections(DashTestFramework): # Since we IS quorums are mined only using dip24 (rotation) we need to enable rotation, and continue tests on llmq_test_dip0024 for connections. - self.log.info("check that old masternode conections are dropped") + self.log.info("check that old masternode connections are dropped") removed = False for mn in self.mninfo: if len(mn.node.quorum("memberof", mn.proTxHash)) > 0: @@ -102,7 +102,7 @@ class LLMQConnections(DashTestFramework): break assert removed # no way we removed none - self.log.info("check that inter-quorum masternode conections are added") + self.log.info("check that inter-quorum masternode connections are added") added = False for mn in self.mninfo: if len(mn.node.quorum("memberof", mn.proTxHash)) > 0: diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index e5b9f2882a..c0d927fe39 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -94,7 +94,7 @@ class LLMQSimplePoSeTest(DashTestFramework): def mine_quorum_no_check(self, expected_good_nodes, mninfos_online): # Unlike in mine_quorum we skip most of the checks and only care about - # nodes moving forward from phase to phase and the fact that the quorum is actualy mined. + # nodes moving forward from phase to phase and the fact that the quorum is actually mined. self.log.info("Mining a quorum with no checks") nodes = [self.nodes[0]] + [mn.node for mn in mninfos_online] diff --git a/test/lint/lint-cppcheck-dash.sh b/test/lint/lint-cppcheck-dash.sh index b6776c9897..908820cb68 100755 --- a/test/lint/lint-cppcheck-dash.sh +++ b/test/lint/lint-cppcheck-dash.sh @@ -112,7 +112,7 @@ then mkdir $CPPCHECK_DIR fi WARNINGS=$(echo "${FILES}" | \ - xargs cppcheck --enable=all --inline-suppr --cppcheck-build-dir=$CPPCHECK_DIR -j "$(getconf _NPROCESSORS_ONLN)" --language=c++ --std=c++17 --template=gcc -D__cplusplus -DENABLE_WALLET -DCLIENT_VERSION_BUILD -DCLIENT_VERSION_IS_RELEASE -DCLIENT_VERSION_MAJOR -DCLIENT_VERSION_MINOR -DCOPYRIGHT_YEAR -DDEBUG -DCHAR_BIT=8 -I src/ -q 2>&1 | sort -u | \ + xargs cppcheck --enable=all --inline-suppr --suppress=missingIncludeSystem --cppcheck-build-dir=$CPPCHECK_DIR -j "$(getconf _NPROCESSORS_ONLN)" --language=c++ --std=c++17 --template=gcc -D__cplusplus -DENABLE_WALLET -DCLIENT_VERSION_BUILD -DCLIENT_VERSION_IS_RELEASE -DCLIENT_VERSION_MAJOR -DCLIENT_VERSION_MINOR -DCOPYRIGHT_YEAR -DDEBUG -DCHAR_BIT=8 -I src/ -q 2>&1 | sort -u | \ grep -E "${ENABLED_CHECKS_REGEXP}" | \ grep -vE "${IGNORED_WARNINGS_REGEXP}" | \ grep -E "${FILES_REGEXP}") diff --git a/test/lint/lint-python.sh b/test/lint/lint-python.sh index 213b94a45c..38931ad2b1 100755 --- a/test/lint/lint-python.sh +++ b/test/lint/lint-python.sh @@ -27,7 +27,8 @@ enabled=( E272 # multiple spaces before keyword E273 # tab after keyword E274 # tab before keyword - E275 # missing whitespace after keyword + # TODO: enable it after bitcoin/bitcoin#26257 - too many warnings with newer flake + #E275 # missing whitespace after keyword E304 # blank lines found after function decorator E306 # expected 1 blank line before a nested definition E401 # multiple imports on one line