Skip to content
This repository was archived by the owner on Oct 28, 2021. It is now read-only.

Commit 6e423ff

Browse files
halfalicioushalfalicious
committed
Address PR feedback
Address multiple issues raised in PR: * Replace lineWidth() getter with static constexpr unsigned c_lineWidth * Replace Datagram type -> name table lookup with overridden function in each Datagram subclass (e.g. PingNode, Pong) which returns the subclass name * Move from cnetdetails to LOG(m_logger) when logging datagram traffic (decreased verbosity and less overhead) * Remove interfaces arg from WebThreeDirect ctor: The interfaces arg is used in the ctor to determine whether or not the client should be configured / started, but we only ever pass 1 interface name (eth) to WebThreeDirect so the argument is unnecessary. * Minor code "fit and finish" modifications (e.g. move constants to anonymous namespace)
1 parent 7056b6f commit 6e423ff

File tree

14 files changed

+98
-134
lines changed

14 files changed

+98
-134
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,12 @@ add_subdirectory(libwebthree)
123123
add_subdirectory(libweb3jsonrpc)
124124

125125
add_subdirectory(aleth)
126-
add_subdirectory(aleth-bootnode)
127126

128127
if (TOOLS)
129128
add_subdirectory(aleth-key)
130129
add_subdirectory(aleth-vm)
131130
add_subdirectory(rlp)
131+
add_subdirectory(aleth-bootnode)
132132
endif()
133133

134134
if (TESTS)

‎aleth-bootnode/main.cpp‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,25 @@ using namespace dev;
3030
using namespace dev::p2p;
3131
using namespace std;
3232

33+
namespace
34+
{
35+
string const c_programName = "aleth-bootnode";
36+
string const c_networkConfigFileName = c_programName + "-network.rlp";
37+
} // namespace
38+
3339
int main(int argc, char** argv)
3440
{
3541
setDefaultOrCLocale();
3642

37-
string const ProgramName = "aleth-bootnode";
38-
string const NetworkConfigFileName = ProgramName + "-network.rlp";
39-
40-
/// Networking params.
41-
string listenIP;
42-
unsigned short listenPort = c_defaultIPPort;
43-
string publicIP;
44-
bool upnp = true;
45-
46-
po::options_description generalOptions("GENERAL OPTIONS", lineWidth());
43+
po::options_description generalOptions("GENERAL OPTIONS", c_lineWidth);
4744
auto addGeneralOption = generalOptions.add_options();
4845
addGeneralOption("help,h", "Show this help message and exit\n");
4946

5047
LoggingOptions loggingOptions;
5148
po::options_description loggingProgramOptions(
52-
createLoggingProgramOptions(lineWidth(), loggingOptions));
49+
createLoggingProgramOptions(c_lineWidth, loggingOptions));
5350

54-
po::options_description clientNetworking("NETWORKING", lineWidth());
51+
po::options_description clientNetworking("NETWORKING", c_lineWidth);
5552
auto addNetworkingOption = clientNetworking.add_options();
5653
#if ETH_MINIUPNPC
5754
addNetworkingOption(
@@ -83,9 +80,9 @@ int main(int argc, char** argv)
8380
if (vm.count("help"))
8481
{
8582
cout << "NAME:\n"
86-
<< " " << ProgramName << "\n"
83+
<< " " << c_programName << "\n"
8784
<< "USAGE:\n"
88-
<< " " << ProgramName << " [options]\n\n";
85+
<< " " << c_programName << " [options]\n\n";
8986
cout << generalOptions << clientNetworking << loggingProgramOptions;
9087
return 0;
9188
}
@@ -123,15 +120,15 @@ int main(int argc, char** argv)
123120

124121
setupLogging(loggingOptions);
125122
if (loggingOptions.verbosity > 0)
126-
cout << EthGrayBold << ProgramName << ", a C++ Ethereum bootnode implementation" EthReset
123+
cout << EthGrayBold << c_programName << ", a C++ Ethereum bootnode implementation" EthReset
127124
<< "\n";
128125

129126
auto netPrefs = publicIP.empty() ? NetworkConfig(listenIP, listenPort, upnp) :
130127
NetworkConfig(publicIP, listenIP, listenPort, upnp);
131-
auto netData = contents(getDataDir() / fs::path(NetworkConfigFileName));
128+
auto netData = contents(getDataDir() / fs::path(c_networkConfigFileName));
132129

133-
// TODO: Compose client version
134-
Host h(ProgramName, netPrefs, &netData);
130+
// Pass in empty client version since it's not used in the discovery process
131+
Host h("", netPrefs, &netData);
135132
h.start();
136133

137134
cout << "Node ID: " << h.enode() << endl;
@@ -148,7 +145,7 @@ int main(int argc, char** argv)
148145

149146
netData = h.saveNetwork();
150147
if (!netData.empty())
151-
writeFile(getDataDir() / fs::path(NetworkConfigFileName), &netData);
148+
writeFile(getDataDir() / fs::path(c_networkConfigFileName), &netData);
152149

153150
return 0;
154151
}

‎aleth-vm/main.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ int main(int argc, char** argv)
107107
Ethash::init();
108108
NoProof::init();
109109

110-
po::options_description transactionOptions("Transaction options", lineWidth());
110+
po::options_description transactionOptions("Transaction options", c_lineWidth);
111111
string const gasLimitDescription =
112112
"<n> Block gas limit (default: " + to_string(maxBlockGasLimit()) + ").";
113113
auto addTransactionOption = transactionOptions.add_options();
@@ -126,20 +126,20 @@ int main(int argc, char** argv)
126126
addTransactionOption("code", po::value<string>(),
127127
"<d> Contract code <d>. Makes transaction a call to this contract");
128128

129-
po::options_description networkOptions("Network options", lineWidth());
129+
po::options_description networkOptions("Network options", c_lineWidth);
130130
networkOptions.add_options()("network", po::value<string>(),
131131
"Main|Ropsten|Homestead|Frontier|Byzantium|Constantinople\n");
132132

133-
po::options_description optionsForTrace("Options for trace", lineWidth());
133+
po::options_description optionsForTrace("Options for trace", c_lineWidth);
134134
auto addTraceOption = optionsForTrace.add_options();
135135
addTraceOption("flat", "Minimal whitespace in the JSON.");
136136
addTraceOption("mnemonics", "Show instruction mnemonics in the trace (non-standard).\n");
137137

138138
LoggingOptions loggingOptions;
139139
po::options_description loggingProgramOptions(
140-
createLoggingProgramOptions(lineWidth(), loggingOptions));
140+
createLoggingProgramOptions(c_lineWidth, loggingOptions));
141141

142-
po::options_description generalOptions("General options", lineWidth());
142+
po::options_description generalOptions("General options", c_lineWidth);
143143
auto addGeneralOption = generalOptions.add_options();
144144
addGeneralOption("version,v", "Show the version and exit.");
145145
addGeneralOption("help,h", "Show this help message and exit.");
@@ -159,7 +159,7 @@ int main(int argc, char** argv)
159159

160160
po::options_description allowedOptions(
161161
"Usage ethvm <options> [trace|stats|output|test] (<file>|-)");
162-
allowedOptions.add(vmProgramOptions(lineWidth()))
162+
allowedOptions.add(vmProgramOptions(c_lineWidth))
163163
.add(networkOptions)
164164
.add(optionsForTrace)
165165
.add(loggingProgramOptions)

‎aleth/main.cpp‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ int main(int argc, char** argv)
214214
fs::path configPath;
215215
string configJSON;
216216

217-
po::options_description clientDefaultMode("CLIENT MODE (default)", lineWidth());
217+
po::options_description clientDefaultMode("CLIENT MODE (default)", c_lineWidth);
218218
auto addClientOption = clientDefaultMode.add_options();
219219
addClientOption("mainnet", "Use the main network protocol");
220220
addClientOption("ropsten", "Use the Ropsten testnet");
@@ -243,7 +243,7 @@ int main(int argc, char** argv)
243243
addClientOption("password", po::value<string>()->value_name("<password>"),
244244
"Give a password for a private key\n");
245245

246-
po::options_description clientTransacting("CLIENT TRANSACTING", lineWidth());
246+
po::options_description clientTransacting("CLIENT TRANSACTING", c_lineWidth);
247247
auto addTransactingOption = clientTransacting.add_options();
248248
addTransactingOption("ask", po::value<u256>()->value_name("<wei>"),
249249
("Set the minimum ask gas price under which no transaction will be mined (default: " +
@@ -256,15 +256,15 @@ int main(int argc, char** argv)
256256
addTransactingOption("unsafe-transactions",
257257
"Allow all transactions to proceed without verification; EXTREMELY UNSAFE\n");
258258

259-
po::options_description clientMining("CLIENT MINING", lineWidth());
259+
po::options_description clientMining("CLIENT MINING", c_lineWidth);
260260
auto addMininigOption = clientMining.add_options();
261261
addMininigOption("address,a", po::value<Address>()->value_name("<addr>"),
262262
"Set the author (mining payout) address (default: auto)");
263263
addMininigOption("mining,m", po::value<string>()->value_name("<on/off/number>"),
264264
"Enable mining; optionally for a specified number of blocks (default: off)");
265265
addMininigOption("extra-data", po::value<string>(), "Set extra data for the sealed blocks\n");
266266

267-
po::options_description clientNetworking("CLIENT NETWORKING", lineWidth());
267+
po::options_description clientNetworking("CLIENT NETWORKING", c_lineWidth);
268268
auto addNetworkingOption = clientNetworking.add_options();
269269
addNetworkingOption("bootstrap,b",
270270
"Connect to the default Ethereum peer servers (default unless --no-discovery used)");
@@ -308,7 +308,7 @@ int main(int argc, char** argv)
308308
addNetworkingOption("pin", "Only accept or connect to trusted peers\n");
309309

310310
std::string snapshotPath;
311-
po::options_description importExportMode("IMPORT/EXPORT MODES", lineWidth());
311+
po::options_description importExportMode("IMPORT/EXPORT MODES", c_lineWidth);
312312
auto addImportExportOption = importExportMode.add_options();
313313
addImportExportOption(
314314
"import,I", po::value<string>()->value_name("<file>"), "Import blocks from file");
@@ -334,18 +334,18 @@ int main(int argc, char** argv)
334334

335335
LoggingOptions loggingOptions;
336336
po::options_description loggingProgramOptions(
337-
createLoggingProgramOptions(lineWidth(), loggingOptions));
337+
createLoggingProgramOptions(c_lineWidth, loggingOptions));
338338

339-
po::options_description generalOptions("GENERAL OPTIONS", lineWidth());
339+
po::options_description generalOptions("GENERAL OPTIONS", c_lineWidth);
340340
auto addGeneralOption = generalOptions.add_options();
341341
addGeneralOption("data-dir,d", po::value<string>()->value_name("<path>"),
342342
("Load configuration files and keystore from path (default: " + getDataDir().string() + ")").c_str());
343343
addGeneralOption("version,V", "Show the version and exit");
344344
addGeneralOption("help,h", "Show this help message and exit\n");
345345

346-
po::options_description vmOptions = vmProgramOptions(lineWidth());
347-
po::options_description dbOptions = db::databaseProgramOptions(lineWidth());
348-
po::options_description minerOptions = MinerCLI::createProgramOptions(lineWidth());
346+
po::options_description vmOptions = vmProgramOptions(c_lineWidth);
347+
po::options_description dbOptions = db::databaseProgramOptions(c_lineWidth);
348+
po::options_description minerOptions = MinerCLI::createProgramOptions(c_lineWidth);
349349

350350
po::options_description allowedOptions("Allowed options");
351351
allowedOptions.add(clientDefaultMode)
@@ -767,13 +767,12 @@ int main(int argc, char** argv)
767767
netPrefs.pin = vm.count("pin") != 0;
768768

769769
auto nodesState = contents(getDataDir() / fs::path("network.rlp"));
770-
auto caps = set<string>{"eth"};
771770

772771
if (testingMode)
773772
chainParams.allowFutureBlocks = true;
774773

775774
dev::WebThreeDirect web3(WebThreeDirect::composeClientVersion("aleth"), db::databasePath(),
776-
snapshotPath, chainParams, withExisting, caps, netPrefs, &nodesState, testingMode);
775+
snapshotPath, chainParams, withExisting, netPrefs, &nodesState, testingMode);
777776

778777
if (!extraData.empty())
779778
web3.ethereum()->setExtraData(extraData);
@@ -933,12 +932,12 @@ int main(int argc, char** argv)
933932
web3.setPeerStretch(peerStretch);
934933
std::shared_ptr<eth::TrivialGasPricer> gasPricer =
935934
make_shared<eth::TrivialGasPricer>(askPrice, bidPrice);
936-
eth::Client* c = web3.ethereum();
937-
c->setGasPricer(gasPricer);
938-
c->setSealer(miner.minerType());
939-
c->setAuthor(author);
935+
Client& c = *(web3.ethereum());
936+
c.setGasPricer(gasPricer);
937+
c.setSealer(miner.minerType());
938+
c.setAuthor(author);
940939
if (networkID != NoNetworkID)
941-
c->setNetworkId(networkID);
940+
c.setNetworkId(networkID);
942941

943942
auto renderFullAddress = [&](Address const& _a) -> std::string
944943
{
@@ -1041,18 +1040,12 @@ int main(int argc, char** argv)
10411040
signal(SIGTERM, &ExitHandler::exitHandler);
10421041
signal(SIGINT, &ExitHandler::exitHandler);
10431042

1044-
if (c)
1045-
{
1046-
unsigned n = c->blockChain().details().number;
1047-
if (mining)
1048-
c->startSealing();
1043+
unsigned n = c.blockChain().details().number;
1044+
if (mining)
1045+
c.startSealing();
10491046

1050-
while (!exitHandler.shouldExit())
1051-
stopSealingAfterXBlocks(c, n, mining);
1052-
}
1053-
else
1054-
while (!exitHandler.shouldExit())
1055-
this_thread::sleep_for(chrono::milliseconds(1000));
1047+
while (!exitHandler.shouldExit())
1048+
stopSealingAfterXBlocks(&c, n, mining);
10561049

10571050
if (jsonrpcIpcServer.get())
10581051
jsonrpcIpcServer->StopListening();

‎libdevcore/Common.cpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,6 @@ void setDefaultOrCLocale()
126126
#endif
127127
}
128128

129-
static unsigned const c_lineWidth = 160;
130-
131-
unsigned lineWidth()
132-
{
133-
return c_lineWidth;
134-
}
135-
136129
bool ExitHandler::s_shouldExit = false;
137130

138131
bool isTrue(std::string const& _m)

‎libdevcore/Common.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ int64_t utcTime();
304304

305305
void setDefaultOrCLocale();
306306

307-
unsigned lineWidth();
307+
static constexpr unsigned c_lineWidth = 160;
308308

309309
class ExitHandler
310310
{

‎libp2p/NodeTable.cpp‎

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,30 +32,6 @@ namespace
3232
BOOST_LOG_INLINE_GLOBAL_LOGGER_CTOR_ARGS(g_discoveryWarnLogger,
3333
boost::log::sources::severity_channel_logger_mt<>,
3434
(boost::log::keywords::severity = 0)(boost::log::keywords::channel = "discov"))
35-
36-
struct DatagramTypeTableEntry
37-
{
38-
char const* name;
39-
uint8_t type;
40-
};
41-
42-
DatagramTypeTableEntry datagramTypesTable[] = {
43-
{"PING", PingNode::type},
44-
{"PONG", Pong::type},
45-
{"FINDNODE", FindNode::type},
46-
{"NEIGHBORS", Neighbours::type}
47-
};
48-
49-
string datagramNameFromType(uint8_t _t)
50-
{
51-
for (auto const& entry : datagramTypesTable)
52-
{
53-
if (entry.type == _t)
54-
return entry.name;
55-
}
56-
return "UNKNOWN";
57-
}
58-
5935
} // namespace
6036

6137
inline bool operator==(
@@ -422,7 +398,7 @@ void NodeTable::onPacketReceived(
422398
return;
423399
}
424400

425-
cnetdetails << "Received " << datagramNameFromType(packet->packetType()) << " from " << packet->sourceid << "@" << _from.address() << ":" << _from.port();
401+
LOG(m_logger) << "Received " << packet->typeName() << " from " << packet->sourceid << "@" << _from.address() << ":" << _from.port();
426402
switch (packet->packetType())
427403
{
428404
case Pong::type:
@@ -506,7 +482,7 @@ void NodeTable::onPacketReceived(
506482
for (unsigned offset = 0; offset < nearest.size(); offset += nlimit)
507483
{
508484
Neighbours out(_from, nearest, offset, nlimit);
509-
cnetdetails << "Sending " << datagramNameFromType(out.packetType()) << " to " << out.sourceid << "@" << _from.address() << ":" << _from.port();
485+
LOG(m_logger) << "Sending " << packet->typeName() << " to " << out.sourceid << "@" << _from.address() << ":" << _from.port();
510486
out.sign(m_secret);
511487
if (out.data.size() > 1280)
512488
cnetlog << "Sending truncated datagram, size: " << out.data.size();
@@ -523,7 +499,7 @@ void NodeTable::onPacketReceived(
523499
addNode(Node(in.sourceid, in.source));
524500

525501
Pong p(in.source);
526-
cnetdetails << "Sending " << datagramNameFromType(p.packetType()) << " to " << in.sourceid << "@" << _from.address() << ":" << _from.port();
502+
LOG(m_logger) << "Sending " << packet->typeName() << " to " << in.sourceid << "@" << _from.address() << ":" << _from.port();
527503
p.echo = in.echo;
528504
p.sign(m_secret);
529505
m_socketPointer->send(p);

0 commit comments

Comments
 (0)