Skip to content

Commit f2859c3

Browse files
committed
Merge bitcoin/bitcoin#25727: util, config: error on startup if conf or reindex are set in config file
deba6fe test: update feature_config_args.py (josibake) 2e3826c util: warn if reindex is used in conf (josibake) 5e744f4 util: disallow setting conf in bitcoin.conf (josibake) Pull request description: In help from `bitcoind -h` it specifes that `conf` can only be used from the commandline. However, if `conf` is set in a `bitcoin.conf` file, there is no error and from reading the logs it seems as if the `conf=<other file>` is being used, despite it being ignored. To recreate, you can setup a `bitcoin.conf` file in the default directory, add `conf=<some other file>.conf` and in the separate config file set whichever config value you want and verify that it is being ignored. alternatively, if you set `includeconf=<some other file>.conf` , your config in `<some other file>` will be picked up. This PR fixes this by having the node error when reading the config file if `conf=` is set. Additionally, it was mentioned in a recent [PR review club](https://bitcoincore.reviews/24858) that if `reindex=1` is set in the config file, the node will reindex on every startup, which is undesirable: ```irc 17:14 <larryruane> michaelfolkson: Reindex is requested by the user (node operator) as a configuration option (command line or in the config file, tho you probably would never put it in the file, or else it would reindex on every startup!) ``` This PR also has a commit to warn if `reindex=1` is set in the config file. ACKs for top commit: hebasto: ACK deba6fe, tested on Ubuntu 22.04. aureleoules: tACK deba6fe ryanofsky: Code review ACK deba6fe. Tree-SHA512: 619fd0aa14e98af1166d6beb92651f5ba3f10d38b8ee132957f094f19c3a37313d9f4d7be2e4019f3fc9a2ca5fa42d03eb539ad820e27efec7ee58a26eb520b1
2 parents 6da4564 + deba6fe commit f2859c3

File tree

2 files changed

+37
-0
lines changed

2 files changed

+37
-0
lines changed

src/util/system.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,20 @@ static bool GetConfigOptions(std::istream& stream, const std::string& filepath,
922922
return true;
923923
}
924924

925+
bool IsConfSupported(KeyInfo& key, std::string& error) {
926+
if (key.name == "conf") {
927+
error = "conf cannot be set in the configuration file; use includeconf= if you want to include additional config files";
928+
return false;
929+
}
930+
if (key.name == "reindex") {
931+
// reindex can be set in a config file but it is strongly discouraged as this will cause the node to reindex on
932+
// every restart. Allow the config but throw a warning
933+
LogPrintf("Warning: reindex=1 is set in the configuration file, which will significantly slow down startup. Consider removing or commenting out this option for better performance, unless there is currently a condition which makes rebuilding the indexes necessary\n");
934+
return true;
935+
}
936+
return true;
937+
}
938+
925939
bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys)
926940
{
927941
LOCK(cs_args);
@@ -932,6 +946,7 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file
932946
for (const std::pair<std::string, std::string>& option : options) {
933947
KeyInfo key = InterpretKey(option.first);
934948
std::optional<unsigned int> flags = GetArgFlags('-' + key.name);
949+
if (!IsConfSupported(key, error)) return false;
935950
if (flags) {
936951
std::optional<util::SettingsValue> value = InterpretValue(key, &option.second, *flags, error);
937952
if (!value) {

test/functional/feature_config_args.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,44 @@ def set_test_params(self):
2020
self.disable_autoconnect = False
2121

2222
def test_config_file_parser(self):
23+
self.log.info('Test config file parser')
2324
self.stop_node(0)
2425

26+
# Check that startup fails if conf= is set in bitcoin.conf or in an included conf file
27+
bad_conf_file_path = os.path.join(self.options.tmpdir, 'node0', 'bitcoin_bad.conf')
28+
util.write_config(bad_conf_file_path, n=0, chain='', extra_config=f'conf=some.conf\n')
29+
conf_in_config_file_err = 'Error: Error reading configuration file: conf cannot be set in the configuration file; use includeconf= if you want to include additional config files'
30+
self.nodes[0].assert_start_raises_init_error(
31+
extra_args=[f'-conf={bad_conf_file_path}'],
32+
expected_msg=conf_in_config_file_err,
33+
)
2534
inc_conf_file_path = os.path.join(self.nodes[0].datadir, 'include.conf')
2635
with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf:
2736
conf.write(f'includeconf={inc_conf_file_path}\n')
37+
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
38+
conf.write('conf=some.conf\n')
39+
self.nodes[0].assert_start_raises_init_error(
40+
expected_msg=conf_in_config_file_err,
41+
)
2842

2943
self.nodes[0].assert_start_raises_init_error(
3044
expected_msg='Error: Error parsing command line arguments: Invalid parameter -dash_cli=1',
3145
extra_args=['-dash_cli=1'],
3246
)
3347
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
3448
conf.write('dash_conf=1\n')
49+
3550
with self.nodes[0].assert_debug_log(expected_msgs=['Ignoring unknown configuration value dash_conf']):
3651
self.start_node(0)
3752
self.stop_node(0)
3853

54+
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
55+
conf.write('reindex=1\n')
56+
57+
with self.nodes[0].assert_debug_log(expected_msgs=['Warning: reindex=1 is set in the configuration file, which will significantly slow down startup. Consider removing or commenting out this option for better performance, unless there is currently a condition which makes rebuilding the indexes necessary']):
58+
self.start_node(0)
59+
self.stop_node(0)
60+
3961
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
4062
conf.write('-dash=1\n')
4163
self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 1: -dash=1, options in configuration file must be specified without leading -')

0 commit comments

Comments
 (0)