Miscellaneous style-related changes and fix bug

Fix bug in randomPeers function

Author: halfalicious

libethereum/BlockChain.cpp

@@ -434,7 +434,7 @@ tuple<ImportRoute, bool, unsigned> BlockChain::sync(
 }
 
 tuple<ImportRoute, unsigned> BlockChain::sync(
-    VerifiedBlocks const& _blocks, h256s& _badBlockHashes, OverlayDB const& _stateDB)
+    VerifiedBlocks const& _blocks, h256s& o_badBlockHashes, OverlayDB const& _stateDB)
 {
     h256s fresh;
     h256s dead;
@@ -463,9 +463,9 @@ tuple<ImportRoute, unsigned> BlockChain::sync(
             catch (dev::eth::UnknownParent const&)
             {
                 cwarn << "ODD: Import queue contains block with unknown parent.";// << LogTag::Error << boost::current_exception_diagnostic_information();
-                                                                                 // NOTE: don't reimport since the queue should guarantee everything in the right order.
-                                                                                 // Can't continue - chain bad.
-                _badBlockHashes.push_back(block.verified.info.hash());
+                // NOTE: don't reimport since the queue should guarantee everything in the right order.
+                // Can't continue - chain bad.
+                o_badBlockHashes.push_back(block.verified.info.hash());
             }
             catch (dev::eth::FutureTime const&)
             {
@@ -480,12 +480,12 @@ tuple<ImportRoute, unsigned> BlockChain::sync(
             }
             catch (Exception& ex)
             {
-                //              cnote << "Exception while importing block. Someone (Jeff? That you?) seems to be giving us dodgy blocks!";// << LogTag::Error << diagnostic_information(ex);
+                // cnote << "Exception while importing block. Someone (Jeff? That you?) seems to be giving us dodgy blocks!";// << LogTag::Error << diagnostic_information(ex);
                 if (m_onBad)
                     m_onBad(ex);
                 // NOTE: don't reimport since the queue should guarantee everything in the right order.
                 // Can't continue - chain  bad.
-                _badBlockHashes.push_back(block.verified.info.hash());
+                o_badBlockHashes.push_back(block.verified.info.hash());
             }
         } while (false);
     }

libethereum/BlockChain.h

@@ -111,7 +111,7 @@ class BlockChain
     /// @returns a tuple with two members - the first (ImportRoute) contains fresh blocks, dead
     /// blocks and imported transactions. The second contains the imported block count.
     std::tuple<ImportRoute, unsigned> sync(
-        VerifiedBlocks const& _blocks, h256s& _badBlockHashes, OverlayDB const& _stateDB);
+        VerifiedBlocks const& _blocks, h256s& o_badBlockHashes, OverlayDB const& _stateDB);
 
     /// Attempt to import the given block directly into the BlockChain and sync with the state DB.
     /// @returns the block hashes of any blocks that came into/went out of the canonical block chain.

libethereum/Client.cpp

@@ -43,9 +43,9 @@ static_assert(BOOST_VERSION >= 106400, "Wrong boost headers version");
 
 namespace
 {
-constexpr unsigned c_syncMin = 1;
-constexpr unsigned c_syncMax = 1000;
-constexpr double c_targetDuration = 1;
+constexpr unsigned c_syncMinBlockCount = 1;
+constexpr unsigned c_syncMaxBlockCount = 1000;
+constexpr double c_targetDurationS = 1;
 
 std::string filtersToString(h256Hash const& _fs)
 {
@@ -389,7 +389,7 @@ void Client::syncBlockQueue()
     unsigned count;
     Timer t;
 
-    shared_ptr<VerifiedBlocks> verifiedBlocks = make_shared<VerifiedBlocks>();
+    std::shared_ptr<VerifiedBlocks> verifiedBlocks = std::make_shared<VerifiedBlocks>();
     m_bq.drain(*verifiedBlocks, m_syncAmount);
 
     // Propagate new blocks to peers before importing them into the chain.
@@ -410,10 +410,10 @@ void Client::syncBlockQueue()
                       << (count / elapsed) << " blocks/s) in #" << bc().number();
     }
 
-    if (elapsed > c_targetDuration * 1.1 && count > c_syncMin)
-        m_syncAmount = max(c_syncMin, count * 9 / 10);
-    else if (count == m_syncAmount && elapsed < c_targetDuration * 0.9 && m_syncAmount < c_syncMax)
-        m_syncAmount = min(c_syncMax, m_syncAmount * 11 / 10 + 1);
+    if (elapsed > c_targetDurationS * 1.1 && count > c_syncMinBlockCount)
+        m_syncAmount = max(c_syncMinBlockCount, count * 9 / 10);
+    else if (count == m_syncAmount && elapsed < c_targetDurationS * 0.9 && m_syncAmount < c_syncMaxBlockCount)
+        m_syncAmount = min(c_syncMaxBlockCount, m_syncAmount * 11 / 10 + 1);
     if (ir.liveBlocks.empty())
         return;
     onChainChanged(ir);

libethereum/EthereumCapability.cpp

@@ -388,7 +388,7 @@ class EthereumHostData : public EthereumHostDataFace
 
 std::vector<NodeID> randomPeers(std::vector<NodeID> const& _peers, size_t _count)
 {
-    if (_peers.size() >= _count || _peers.empty())
+    if (_peers.size() <= _count || _peers.empty())
         return _peers;
 
     std::vector<NodeID> peers{_peers};
@@ -554,20 +554,20 @@ void EthereumCapability::maintainBlockHashes(h256 const& _currentHash)
             LOG(m_logger) << "Sending new block hashes (current is " << _currentHash << ", was "
                           << m_latestBlockHashSent << ")";
 
-            h256s blocks =
+            h256s blockHashes =
                 get<0>(m_chain.treeRoute(m_latestBlockHashSent, _currentHash, false, false, true));
 
             auto const peersWithoutBlock = selectPeers(
                 [&](EthereumPeer const& _peer) { return !_peer.isBlockKnown(_currentHash); });
             for (NodeID const& peerID : peersWithoutBlock)
             {
                 RLPStream ts;
-                m_host->prep(peerID, name(), ts, NewBlockHashesPacket, blocks.size());
-                for (auto const& b : blocks)
+                m_host->prep(peerID, name(), ts, NewBlockHashesPacket, blockHashes.size());
+                for (auto const& bh : blockHashes)
                 {
                     ts.appendList(2);
-                    ts.append(b);
-                    ts.append(m_chain.number(b));
+                    ts.append(bh);
+                    ts.append(m_chain.number(bh));
                 }
 
                 auto itPeer = m_peers.find(peerID);
@@ -578,7 +578,7 @@ void EthereumCapability::maintainBlockHashes(h256 const& _currentHash)
                 }
             }
             if (!peersWithoutBlock.empty())
-                LOG(m_logger) << "Announced " << blocks.size() << " block(s) to "
+                LOG(m_logger) << "Announced " << blockHashes.size() << " block(s) to "
                               << peersWithoutBlock.size() << " peers";
         }
         m_latestBlockHashSent = _currentHash;
@@ -957,16 +957,17 @@ void EthereumCapability::removeSentTransactions(std::vector<h256> const& _txHash
     }
 }
 
-void EthereumCapability::propagateNewBlocks(shared_ptr<VerifiedBlocks const> const& _newBlocks)
+void EthereumCapability::propagateNewBlocks(std::shared_ptr<VerifiedBlocks const> const& _newBlocks)
 {
     // Safe to call isSyncing() from a non-network thread since the underlying variable is
     // std::atomic
     if (_newBlocks->empty() || isSyncing())
         return;
 
     m_host->postWork([this, _newBlocks]() {
-        // Verify that we're not too far behind - we perform this check on the network thread rather
-        // than before posting the work to simplify the synchronization story
+        // Verify that we're not too far behind - we perform this check on the network thread to
+        // simplify the synchronization story (since otherwise we'd need to synchronize access to
+        // m_latestBlockSent)
         auto const latestHash = _newBlocks->back().verified.info.hash();
         auto const detailsFrom = m_chain.details(m_latestBlockSent);
         auto const detailsTo = m_chain.details(latestHash);