By tolerating unknown extra rpcauth parameters (and ignoring the rpcauth), we can ensure a limited forward compatibility by not forcing users to downgrade their config file to switch back to older versions (perhaps temporarily).
Same as #20548, but without the additional effort to explain the situation at runtime.
259@@ -260,7 +260,6 @@ static bool InitRPCAuthentication()
260 g_rpcauth.push_back(fields);
261 } else {
262 LogPrintf("Invalid -rpcauth argument.\n");
0 LogPrintf("Ignoring invalid -rpcauth argument.\n");
I prefer failing here to be honest. But I guess my dislike for ignoring invalid options is well known.
In #20461 (comment) I’ve suggested a -strict (or similar) option.
I mean, if the entire problem here is that users running older bitcoin core might run into this error when upgrading and be confused, I think the error needs to be clearer. All in all it will result in removing cruft from their configuration. Even more, they will likely also be alerted to mistakes they made. Possibly critical security mistakes. (…why would one add entries on purpose just to be ignored?)
I’ve suggested a -strict (or similar) option.
Not sure this kind of meta-configuration makes sense. It just adds more code paths where it’s better to make a principled decision. And I think bitcoin core should be strict and unambigious about input handling.
By tolerating unknown extra rpcauth parameters (and ignoring the rpcauth), we can ensure a limited forward compatibility by not forcing users to downgrade their config file to switch back to older versions (perhaps temporarily).
For the past couple releases, hasn’t Bitcoin Core refused to start on unknown configuration settings?
$ bitcoind -not-a-real-setting
Error: Error parsing command line arguments: Invalid parameter -not-a-real-setting
Future releases, or spinoffs, are likely to support new configuration settings that old releases will not, so any users including those settings in their configuration files will need to edit those files anyway to use an old or baseline release. Rejecting unknown parameters seems like a simple and logically consistent extension of this previous policy.
The policy of failing on unknown settings is quite beneficial at eliminating the annoyance of figuring out that your configuration is broken and then restarting the node—which is something that can take a long time (e.g. if you have a high db cache or are using an underpowered computer) and which inexperienced users might not know how to do (or might do wrong, e.g. sending SIGKILL rather than SIGTERM, which could result in a need to reindex).
Based on my own misadventures, wrongly configuring a node is a much more common problem than wanting to use a future/spinoff configuration file with an old/baseline node, so I think we should optimize for helping users fix misconfigurations as fast and painlessly as possible.
For the past couple releases, hasn’t Bitcoin Core refused to start on unknown configuration settings?
On the command line only, not in config files where they would reasonably be expected.
On the command line only, not in config files
Confirmed. That seems to me like an unnecessary half measure. If it’s planned to keep it that way, there may indeed be merit in a -strict option that also validates options in the configuration file.
rpcport causes failure outside of network sections. That’s not quite the same as it’s blacklisting known-meaningless settings rather than whitelisting just known-meaningful ones, but in general becoming more strict in configuration files too makes sense.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
🐙 This pull request conflicts with the target branch and needs rebase.
Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.