Minor code cleanup

Change capability background work intervals from constexpr in unnamed namespace to static constexpr class members since this makes more conceptual sense given that the values are exposed via a function (backgroundWorkInterval()), updated some license messages, removed unnecessary class names when creating CapDesc instances, changed Host run timer from deadline timer to steady timer so we can use std::chrono, and added some comments.

Author: halfalicious

libethereum/EthereumCapability.cpp

@@ -22,15 +22,15 @@ using namespace dev::eth;
 char const* const EthereumCapability::s_stateNames[static_cast<int>(SyncState::Size)] = {
     "NotSynced", "Idle", "Waiting", "Blocks", "State"};
 
-constexpr chrono::milliseconds c_backgroundWorkInterval{1000};
+chrono::milliseconds constexpr EthereumCapability::s_backgroundWorkInterval;
 
 namespace
 {
-constexpr unsigned c_maxSendTransactions = 256;
-constexpr unsigned c_maxHeadersToSend = 1024;
-constexpr unsigned c_maxIncomingNewHashes = 1024;
-constexpr unsigned c_peerTimeoutSeconds = 10;
-constexpr int c_minBlockBroadcastPeers = 4;
+static constexpr unsigned c_maxSendTransactions = 256;
+static constexpr unsigned c_maxHeadersToSend = 1024;
+static constexpr unsigned c_maxIncomingNewHashes = 1024;
+static constexpr unsigned c_peerTimeoutSeconds = 10;
+static constexpr int c_minBlockBroadcastPeers = 4;
 
 string toString(Asking _a)
 {
@@ -402,14 +402,13 @@ EthereumCapability::EthereumCapability(shared_ptr<p2p::CapabilityHostFace> _host
 
 chrono::milliseconds EthereumCapability::backgroundWorkInterval() const
 {
-    return c_backgroundWorkInterval;
+    return s_backgroundWorkInterval;
 }
 
 void EthereumCapability::onStarting()
 {
     m_backgroundWorkEnabled = true;
-    m_host->scheduleCapabilityBackgroundWork(
-        p2p::CapDesc{name(), version()}, [this]() { doBackgroundWork(); });
+    m_host->scheduleCapabilityBackgroundWork({name(), version()}, [this]() { doBackgroundWork(); });
 }
 
 void EthereumCapability::onStopping()
@@ -437,7 +436,7 @@ void EthereumCapability::reset()
 
     // reset() can be called from RPC handling thread,
     // but we access m_latestBlockSent and m_transactionsSent only from the network thread
-    m_host->postCapabilityWork(p2p::CapDesc{name(), version()}, [this]() {
+    m_host->postCapabilityWork({name(), version()}, [this]() {
         m_latestBlockSent = h256();
         m_transactionsSent.clear();
     });
@@ -482,7 +481,8 @@ void EthereumCapability::doBackgroundWork()
     }
 
     if (m_backgroundWorkEnabled)
-        m_host->scheduleCapabilityBackgroundWork(make_pair(name(), version()), [this]() { doBackgroundWork(); });
+        m_host->scheduleCapabilityBackgroundWork(
+            {name(), version()}, [this]() { doBackgroundWork(); });
 }
 
 void EthereumCapability::maintainTransactions()
@@ -645,7 +645,7 @@ SyncStatus EthereumCapability::status() const
 void EthereumCapability::onTransactionImported(
     ImportResult _ir, h256 const& _h, h512 const& _nodeId)
 {
-    m_host->postCapabilityWork(p2p::CapDesc{name(), version()}, [this, _ir, _h, _nodeId]() {
+    m_host->postCapabilityWork({name(), version()}, [this, _ir, _h, _nodeId]() {
         auto itPeerStatus = m_peers.find(_nodeId);
         if (itPeerStatus == m_peers.end())
             return;

libethereum/EthereumCapability.h

@@ -132,6 +132,7 @@ class EthereumCapability : public p2p::CapabilityFace
 
 private:
     static char const* const s_stateNames[static_cast<int>(SyncState::Size)];
+    static constexpr std::chrono::milliseconds s_backgroundWorkInterval{1000};
 
     std::vector<NodeID> selectPeers(
         std::function<bool(EthereumPeer const&)> const& _predicate) const;

libethereum/WarpCapability.cpp

@@ -16,10 +16,12 @@ namespace dev
 {
 namespace eth
 {
+
+chrono::milliseconds constexpr WarpCapability::s_backgroundWorkInterval;
+
 namespace
 {
 static size_t const c_freePeerBufferSize = 32;
-constexpr chrono::milliseconds c_backgroundWorkInterval{1000};
 
 bool validateManifest(RLP const& _manifestRlp)
 {
@@ -315,14 +317,13 @@ WarpCapability::WarpCapability(shared_ptr<p2p::CapabilityHostFace> _host,
 
 chrono::milliseconds WarpCapability::backgroundWorkInterval() const
 {
-    return c_backgroundWorkInterval;
+    return s_backgroundWorkInterval;
 }
 
 void WarpCapability::onStarting()
 {
     m_backgroundWorkEnabled = true;
-    m_host->scheduleCapabilityBackgroundWork(
-        p2p::CapDesc{name(), version()}, [this]() { doBackgroundWork(); });
+    m_host->scheduleCapabilityBackgroundWork({name(), version()}, [this]() { doBackgroundWork(); });
 }
 
 void WarpCapability::onStopping()
@@ -351,7 +352,7 @@ void WarpCapability::doBackgroundWork()
 
     if (m_backgroundWorkEnabled)
         m_host->scheduleCapabilityBackgroundWork(
-            p2p::CapDesc{name(), version()}, [this]() { doBackgroundWork(); });
+            {name(), version()}, [this]() { doBackgroundWork(); });
 }
 
 void WarpCapability::onConnect(NodeID const& _peerID, u256 const& /* _peerCapabilityVersion */)

libethereum/WarpCapability.h

@@ -105,6 +105,8 @@ class WarpCapability : public p2p::CapabilityFace
     void disablePeer(NodeID const& _peerID, std::string const& _problem);
 
 private:
+    static constexpr std::chrono::milliseconds s_backgroundWorkInterval{1000};
+
     std::shared_ptr<WarpPeerObserverFace> createPeerObserver(
         boost::filesystem::path const& _snapshotDownloadPath);
 

libp2p/Capability.h

@@ -1,19 +1,6 @@
-/*
-    This file is part of cpp-ethereum.
-
-    cpp-ethereum is free software: you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation, either version 3 of the License, or
-    (at your option) any later version.
-
-    cpp-ethereum is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with cpp-ethereum.  If not, see <http://www.gnu.org/licenses/>.
-*/
+// Aleth: Ethereum C++ client, tools and libraries.
+// Copyright 2019 Aleth Authors.
+// Licensed under the GNU General Public License, Version 3.
 
 #pragma once
 
@@ -23,8 +10,7 @@ namespace dev
 {
 namespace p2p
 {
-
-    DEV_SIMPLE_EXCEPTION(UnknownCapability);
+DEV_SIMPLE_EXCEPTION(CapabilityNotRegistered);
 
 /**
  * @brief The Capability interface.

libp2p/Common.cpp

@@ -130,6 +130,11 @@ string reasonOf(DisconnectReason _r)
     }
 }
 
+ostream& operator<<(ostream& _out, CapDesc const& _capDesc)
+{
+    return _out << _capDesc.first << ": " << _capDesc.second;
+}
+
 void NodeIPEndpoint::streamRLP(RLPStream& _s, RLPAppend _append) const
 {
     if (_append == StreamList)

libp2p/Common.h

@@ -114,6 +114,8 @@ using CapDesc = std::pair<std::string, unsigned>;
 using CapDescSet = std::set<CapDesc>;
 using CapDescs = std::vector<CapDesc>;
 
+std::ostream& operator<<(std::ostream& _out, CapDesc const& _capDesc);
+
 /*
  * Used by Host to pass negotiated information about a connection to a
  * new Peer Session; PeerSessionInfo is then maintained by Session and can

libp2p/Host.cpp

@@ -27,13 +27,13 @@ using namespace dev::p2p;
 namespace
 {
 /// Interval at which Host::run will call keepAlivePeers to ping peers.
-constexpr chrono::seconds c_keepAliveInterval = chrono::seconds(30);
+static constexpr chrono::seconds c_keepAliveInterval{30};
 
 /// Disconnect timeout after failure to respond to keepAlivePeers ping.
-constexpr chrono::seconds c_keepAliveTimeOut = chrono::seconds(1);
+static constexpr chrono::seconds c_keepAliveTimeOut{1};
 
 /// Interval which m_runTimer is run when network is connected.
-constexpr unsigned int c_runTimerIntervalMs = 100;
+static constexpr chrono::milliseconds c_runTimerInterval{100};
 }  // namespace
 
 HostNodeTableHandler::HostNodeTableHandler(Host& _host): m_host(_host) {}
@@ -732,7 +732,7 @@ void Host::run(boost::system::error_code const& _ec)
         return;
 
     auto runcb = [this](boost::system::error_code const& error) { run(error); };
-    m_runTimer.expires_from_now(boost::posix_time::milliseconds(c_runTimerIntervalMs));
+    m_runTimer.expires_from_now(c_runTimerInterval);
     m_runTimer.async_wait(runcb);
 }
 
@@ -1029,10 +1029,13 @@ void Host::scheduleCapabilityBackgroundWork(CapDesc const& _capDesc, function<vo
 {
     auto cap = m_capabilities.find(_capDesc);
     if (cap == m_capabilities.end())
-        BOOST_THROW_EXCEPTION(UnknownCapability());
+    {
+        LOG(m_logger) << "Capability not registered: " << _capDesc;
+        BOOST_THROW_EXCEPTION(CapabilityNotRegistered());
+    }
 
     auto timer = cap->second.backgroundWorkTimer.get();
-    timer->expires_from_now(chrono::milliseconds(cap->second.capability->backgroundWorkInterval()));
+    timer->expires_from_now(cap->second.capability->backgroundWorkInterval());
     timer->async_wait([_f](boost::system::error_code _ec) {
         if (!_ec)
             _f();
@@ -1042,7 +1045,10 @@ void Host::scheduleCapabilityBackgroundWork(CapDesc const& _capDesc, function<vo
 void Host::postCapabilityWork(CapDesc const& _capDesc, function<void()> _f)
 {
     if (m_capabilities.find(_capDesc) == m_capabilities.end())
-        BOOST_THROW_EXCEPTION(UnknownCapability());
-    
+    {
+        LOG(m_logger) << "Capability not registered" << _capDesc;
+        BOOST_THROW_EXCEPTION(CapabilityNotRegistered());
+    }
+
     m_ioService.post(_f);
 }

libp2p/Host.h

@@ -231,8 +231,11 @@ class Host: public Worker
     void forEachPeer(
         std::string const& _capabilityName, std::function<bool(NodeID const&)> _f) const;
 
+    /// Schedule a capability's background work loop on the network thread
     void scheduleCapabilityBackgroundWork(CapDesc const& _capDesc, std::function<void()> _f);
-    void postCapabilityWork(CapDesc const& _capdesc, std::function<void()> _f);
+
+    /// Execute capability work on the network thread
+    void postCapabilityWork(CapDesc const& _capDesc, std::function<void()> _f);
 
     std::shared_ptr<CapabilityHostFace> capabilityHost() const { return m_capabilityHost; }
 
@@ -320,7 +323,7 @@ class Host: public Worker
     bi::tcp::acceptor m_tcp4Acceptor;										///< Listening acceptor.
 
     /// Timer which, when network is running, calls run() every c_timerInterval ms.
-    io::deadline_timer m_runTimer;
+    ba::steady_timer m_runTimer;
 
     std::set<Peer*> m_pendingPeerConns;									/// Used only by connect(Peer&) to limit concurrently connecting to same node. See connect(shared_ptr<Peer>const&).
 

test/unittests/libp2p/capability.cpp

@@ -15,11 +15,6 @@ using namespace std;
 using namespace dev;
 using namespace dev::p2p;
 
-namespace
-{
-chrono::milliseconds constexpr c_backgroundWorkInterval{1000};
-}
-
 class TestCapability : public CapabilityFace, public Worker
 {
 public:
@@ -30,7 +25,7 @@ class TestCapability : public CapabilityFace, public Worker
     unsigned messageCount() const override { return UserPacket + 1; }
     chrono::milliseconds backgroundWorkInterval() const override
     {
-        return c_backgroundWorkInterval;
+        return s_backgroundWorkInterval;
     }
 
     void onStarting() override {}
@@ -75,6 +70,8 @@ class TestCapability : public CapabilityFace, public Worker
         return {cnt, checksum};
     }
 
+    static chrono::milliseconds constexpr s_backgroundWorkInterval{1000};
+
     Host const& m_host;
     unordered_map<NodeID, int> m_cntReceivedMessages;
     unordered_map<NodeID, int> m_testSums;