Various small cleanups (#2761)

* Fix remaining `print`s in tests * use AssertLockHeld(cs) instead of relying on comments * actually use `clsig` in `EnforceBestChainLock()` * fix log output in `EnforceBestChainLock()` * drop comments
dashpay · Mar 11, 2019 · fbf0dcb · fbf0dcb
1 parent 588eb30
commit fbf0dcb
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 11 deletions.
diff --git a/qa/rpc-tests/autoix-mempool.py b/qa/rpc-tests/autoix-mempool.py
@@ -127,13 +127,13 @@ def run_test(self):
         self.wait_for_sporks_same()
         self.mine_quorum()
 
-        print("Test old InstantSend")
+        self.log.info("Test old InstantSend")
         self.test_auto();
 
         self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0)
         self.wait_for_sporks_same()
 
-        print("Test new InstantSend")
+        self.log.info("Test new InstantSend")
         self.test_auto(True);
 
     def test_auto(self, new_is = False):

diff --git a/qa/rpc-tests/p2p-autoinstantsend.py b/qa/rpc-tests/p2p-autoinstantsend.py
@@ -121,13 +121,13 @@ def run_test(self):
         self.wait_for_sporks_same()
         self.mine_quorum()
 
-        print("Test old InstantSend")
+        self.log.info("Test old InstantSend")
         self.test_auto();
 
         self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0)
         self.wait_for_sporks_same()
 
-        print("Test new InstantSend")
+        self.log.info("Test new InstantSend")
         self.test_auto(True);
 
     def test_auto(self, new_is = False):

diff --git a/qa/rpc-tests/p2p-instantsend.py b/qa/rpc-tests/p2p-instantsend.py
@@ -25,13 +25,13 @@ def run_test(self):
         self.wait_for_sporks_same()
         self.mine_quorum()
 
-        print("Test old InstantSend")
+        self.log.info("Test old InstantSend")
         self.test_doublespend()
 
         self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0)
         self.wait_for_sporks_same()
 
-        print("Test new InstantSend")
+        self.log.info("Test new InstantSend")
         self.test_doublespend()
 
     def test_doublespend(self):

diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp
@@ -428,7 +428,7 @@ void CChainLocksHandler::EnforceBestChainLock()
                     continue;
                 }
                 LogPrintf("CChainLocksHandler::%s -- CLSIG (%s) invalidates block %s\n",
-                          __func__, bestChainLockWithKnownBlock.ToString(), jt->second->GetBlockHash().ToString());
+                          __func__, clsig.ToString(), jt->second->GetBlockHash().ToString());
                 DoInvalidateBlock(jt->second, false);
             }
 
@@ -439,7 +439,7 @@ void CChainLocksHandler::EnforceBestChainLock()
 
     CValidationState state;
     if (!ActivateBestChain(state, Params())) {
-        LogPrintf("CChainLocksHandler::UpdatedBlockTip -- ActivateBestChain failed: %s\n", FormatStateMessage(state));
+        LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state));
         // This should not have happened and we are in a state were it's not safe to continue anymore
         assert(false);
     }

diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp
@@ -790,9 +790,10 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256&
     quorumSigningManager->ProcessRecoveredSig(-1, rs, quorum, connman);
 }
 
-// cs must be held
 void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map<NodeId, std::unordered_map<uint256, CSigSharesInv, StaticSaltedHasher>>& sigSharesToRequest)
 {
+    AssertLockHeld(cs);
+
     int64_t now = GetTimeMillis();
     const size_t maxRequestsForNode = 32;
 
@@ -885,9 +886,10 @@ void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map<NodeId, std
     }
 }
 
-// cs must be held
 void CSigSharesManager::CollectSigSharesToSend(std::unordered_map<NodeId, std::unordered_map<uint256, CBatchedSigShares, StaticSaltedHasher>>& sigSharesToSend)
 {
+    AssertLockHeld(cs);
+
     for (auto& p : nodeStates) {
         auto nodeId = p.first;
         auto& nodeState = p.second;
@@ -936,9 +938,10 @@ void CSigSharesManager::CollectSigSharesToSend(std::unordered_map<NodeId, std::u
     }
 }
 
-// cs must be held
 void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map<NodeId, std::unordered_map<uint256, CSigSharesInv, StaticSaltedHasher>>& sigSharesToAnnounce)
 {
+    AssertLockHeld(cs);
+
     std::unordered_set<std::pair<Consensus::LLMQType, uint256>, StaticSaltedHasher> quorumNodesPrepared;
 
     this->sigSharesToAnnounce.ForEach([&](const SigShareKey& sigShareKey, bool) {