Remove --private option from aleth

[halfalicious]

Aleth's --private option is unnecessary and should be removed - it sets some chain parameters to certain values, but in order to use a private chain one must also specify a chain configuration json file (via the --config parameter) and this config file must also include values for those chain parameters.

The private option's usage is logged here:

aleth/aleth/main.cpp

Line 211 in 523c965

addClientOption("private", po::value<string>()->value_name("<name>"), "Use a private chain");

The private option is parsed here:

aleth/aleth/main.cpp

Lines 626 to 635 in ad6f54f

	if (vm.count("private"))
	try
	{
	privateChain = vm["private"].as<string>();
	}
	catch (...)
	{
	cerr << "Bad " << "--private" << " option: " << vm["private"].as<string>() << "\n";
	return AlethErrors::BadPrivateOption;
	}

Chain parameters are then set to certain values (based on whether or not --private was supplied) here:

aleth/aleth/main.cpp

Lines 698 to 703 in ad6f54f

	if (!privateChain.empty())
	{
	chainParams.extraData = sha3(privateChain).asBytes();
	chainParams.difficulty = chainParams.minimumDifficulty;
	chainParams.gasLimit = u256(1) << 32;
	}

The chain configuration file is processed here:

aleth/aleth/main.cpp

Lines 681 to 694 in ad6f54f

	if (!configJSON.empty())
	{
	try
	{
	chainParams = chainParams.loadConfig(configJSON, {}, configPath);
	chainConfigIsSet = true;
	}
	catch (...)
	{
	cerr << "provided configuration is not well formatted\n";
	cerr << "sample: \n" << genesisInfo(eth::Network::MainNetworkTest) << "\n";
	return AlethErrors::Success;
	}
	}

[twinstar26]

To resolve this issue,

I am planning to remove

aleth/aleth/main.cpp

Line 211 in 523c965

addClientOption("private", po::value<string>()->value_name("<name>"), "Use a private chain");

so that "private" key-value pair is not created in map "vm".

Then I am going to move the contents of

aleth/aleth/main.cpp

Lines 626 to 635 in 523c965

	if (vm.count("private"))
	try
	{
	privateChain = vm["private"].as<string>();
	}
	catch (...)
	{
	cerr << "Bad " << "--private" << " option: " << vm["private"].as<string>() << "\n";
	return AlethErrors::BadPrivateOption;
	}

to

aleth/aleth/main.cpp

Lines 468 to 486 in 523c965

	if (vm.count("config"))
	{
	try
	{
	configPath = vm["config"].as<string>();
	configJSON = contentsString(configPath.string());
	if (configJSON.empty())
	{
	cerr << "Config file not found or empty (" << configPath.string() << ")\n";
	return AlethErrors::ConfigFileEmptyOrNotFound;
	}
	}
	catch (...)
	{
	cerr << "Bad --config option: " << vm["config"].as<string>() << "\n";
	return AlethErrors::BadConfigOption;
	}
	}

with appropriate condition checking, but the string "privateChain" which used to contain the value of key "private" from map "vm" will now remain NULL.
So is this information going to be included in "configJSON"? Or we can modify

aleth/aleth/main.cpp

Line 213 in 523c965

addClientOption("config", po::value<string>()->value_name("<file>"),

to "value()->multitoken()" so as to obtain --config value as well as --private value (if it exists).

Please pardon my noviceness as this is my first open source issue.

[twinstar26]

@halfalicious

[halfalicious]

Thank you for your interest in fixing this issue @twinstar26 !

I don't think the multitoken change is needed - you should also remove the privateChain string definition and update the following places where it's checked accordingly:

aleth/aleth/main.cpp

Line 759 in 32bb833

netPrefs.discovery = (privateChain.empty() && !disableDiscovery) || enableDiscovery;

and

aleth/aleth/main.cpp

Line 1029 in 32bb833

if (bootstrap && privateChain.empty())

So after your changes, --private won't be recognized and one will have to explicitly disable discovery (--no-discovery) and/or disable attempting to peer with the official Ethereum bootnodes (--no-bootstrap) when they wish to run a private chain without discovery (the only way to add peers will be via --peerset or RPC).

Also, if you haven't already, please review the documentation for contributors: https://github.com/ethereum/aleth/blob/master/CONTRIBUTING.md

Let me know if you have any questions and thanks again! 😃

[twinstar26]

@halfalicious thanks for the warmness...

I have created a pull request.
And I have a suggestion.

Suggestion 1: The definition of "enableDiscovery" is redundant throughout the repo. So should I remove it (considering any future use of that bool variable)??

aleth/aleth/main.cpp

Line 152 in 32bb833

bool enableDiscovery = false;

[halfalicious]

@halfalicious thanks for the warmness...

I have created a pull request.
And I have a suggestion.

Suggestion 1: The definition of "enableDiscovery" is redundant throughout the repo. So should I remove it (considering any future use of that bool variable)??

aleth/aleth/main.cpp

Line 152 in 32bb833
bool enableDiscovery = false;

Thank you for calling this out 😄 You're correct, this can be removed

[halfalicious]

@twinstar26 : Thanks for your contribution! 😄