Miscellaneous style-related changes and fix bug

Fix bug in randomPeers function

Author: halfalicious

libdevcore/Common.cpp

@@ -136,7 +136,7 @@ bool isFalse(std::string const& _m)
     return _m == "off" || _m == "no" || _m == "false" || _m == "0";
 }
 
-mt19937_64 g_randomGenerator(random_device{}());
+mt19937_64 g_randomGenerator{random_device{}()};
 
 int randomNumber(int _min, int _max)
 {

libethereum/BlockChain.cpp

@@ -428,13 +428,13 @@ tuple<ImportRoute, bool, unsigned> BlockChain::sync(
     _bq.drain(blocks, _max);
 
     h256s badBlockHashes;
-    tuple<ImportRoute, unsigned> const importResult = sync(blocks, badBlockHashes, _stateDB);
+    std::tuple<ImportRoute, unsigned> const importResult = sync(blocks, badBlockHashes, _stateDB);
     bool const moreBlocks = _bq.doneDrain(badBlockHashes);
-    return {get<0>(importResult), moreBlocks, get<1>(importResult)};
+    return {std::get<0>(importResult), moreBlocks, std::get<1>(importResult)};
 }
 
 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.
@@ -399,7 +399,7 @@ void Client::syncBlockQueue()
     h->propagateNewBlocks(verifiedBlocks);
 
     h256s badBlockHashes;
-    tie(ir, count) = bc().sync(*verifiedBlocks, badBlockHashes, m_stateDB);
+    std::tie(ir, count) = bc().sync(*verifiedBlocks, badBlockHashes, m_stateDB);
     m_syncBlockQueue = m_bq.doneDrain(badBlockHashes);
 
     double elapsed = t.elapsed();
@@ -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

@@ -93,7 +93,7 @@ class EthereumPeerObserver : public EthereumPeerObserverFace
         }
         catch (Exception&)
         {
-            cwarn << "Exception on peer destruciton: "
+            cwarn << "Exception on peer destruction: "
                   << boost::current_exception_diagnostic_information();
         }
     }
@@ -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};
@@ -545,44 +545,43 @@ void EthereumCapability::maintainBlockHashes(h256 const& _currentHash)
     // Send any new block hashes
     auto const detailsFrom = m_chain.details(m_latestBlockHashSent);
     auto const detailsTo = m_chain.details(_currentHash);
-    if (detailsFrom.totalDifficulty < detailsTo.totalDifficulty)
-    {
-        if (diff(detailsFrom.number, detailsTo.number) <= c_maxSendNewBlocksCount)
-        {
-            // don't be sending more than c_maxSendNewBlocksCount "new" block hashes. if there are any more we were probably
-            // waaaay behind.
-            LOG(m_logger) << "Sending new block hashes (current is " << _currentHash << ", was "
-                          << m_latestBlockHashSent << ")";
+    if (detailsFrom.totalDifficulty >= detailsTo.totalDifficulty ||
+        // don't be sending more than c_maxSendNewBlocksCount "new" block hashes. if there are any
+        // more we were probably waaaay behind.
+        diff(detailsFrom.number, detailsTo.number) > c_maxSendNewBlocksCount)
+        return;
 
-            h256s blocks =
-                get<0>(m_chain.treeRoute(m_latestBlockHashSent, _currentHash, false, false, true));
+    LOG(m_logger) << "Sending new block hashes (current is " << _currentHash << ", was "
+                    << m_latestBlockHashSent << ")";
 
-            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)
-                {
-                    ts.appendList(2);
-                    ts.append(b);
-                    ts.append(m_chain.number(b));
-                }
+    h256s const blockHashes =
+        std::get<0>(m_chain.treeRoute(m_latestBlockHashSent, _currentHash, false, false, true));
 
-                auto itPeer = m_peers.find(peerID);
-                if (itPeer != m_peers.end())
-                {
-                    m_host->sealAndSend(peerID, ts);
-                    itPeer->second.clearKnownBlocks();
-                }
-            }
-            if (!peersWithoutBlock.empty())
-                LOG(m_logger) << "Announced " << blocks.size() << " block(s) to "
-                              << peersWithoutBlock.size() << " peers";
+    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, blockHashes.size());
+        for (auto const& bh : blockHashes)
+        {
+            ts.appendList(2);
+            ts.append(bh);
+            ts.append(m_chain.number(bh));
+        }
+
+        auto itPeer = m_peers.find(peerID);
+        if (itPeer != m_peers.end())
+        {
+            m_host->sealAndSend(peerID, ts);
+            itPeer->second.clearKnownBlocks();
         }
-        m_latestBlockHashSent = _currentHash;
     }
+    if (!peersWithoutBlock.empty())
+        LOG(m_logger) << "Announced " << blockHashes.size() << " block(s) to "
+                        << peersWithoutBlock.size() << " peers";
+
+    m_latestBlockHashSent = _currentHash;
 }
 
 bool EthereumCapability::isSyncing() const
@@ -957,16 +956,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);