Command line arguments (when Bitcoin Core is started from the command line)
QT settings (only for bitcoin-qt, not bitcoind or the utilities)
Settings are also sometimes updated within the code. Usually this happens
if some user-provided config implies other config. For example, if the
user starts Bitcoin Core with -connect config to connect to specific peers,
then by default -listen will be switched off, so the node won’t accept
incoming connections
(code).
See ForceSetArg(), SoftSetArg() and SoftSetBoolArg() for other places
where configuration is updated in the code.
Since PR 11862, the
bitcoin.conf file has allowed network-specific sections. Different config
can be specified for mainnet, testnet and regtest in the same config file by
using section headings.
QT settings are used for GUI-only persistent configuration. For example,
the window location and whether the application is minimized to the tray is
persisted between sessions in the QT settings. These settings are saved in the
windows registry or platform specific config files.
QT settings also store application configuration that can be updated
in the GUI, such as whether to prune or to use tor. This config is saved
but is only applied when running bitcoin-qt, not when running bitcoind.
Generally, QT settings override command-line arguments, which override
bitcoin.conf configuration.
There are a lot of quirks in the way that configuration is parsed and merged
(eg command-line argument precedence is treated differently from config file
precedence, some command-line arguments are ignored, etc). ryanofsky lists
these quirks in the PR.
PR 15869 added test coverage
for settings merging to prevent new code from introducing regressions.
<fjahr> Not sure I was really efficient, I just re-read the code many times until I felt I had a good understanding. I felt the tests were really helpful in getting confidence in the code. As I said I cherry-picked the regression test to verify since it was last in the commit history.
<jonatack> The test was fun to tweak and did a bit of manual testing. The most time-consuming was reading up on the context and the many idiosyncracies of the current behavior, which ryanofsky already kindly
<jnewbery> ariard: yeah, I think that's really the only safe way to do it. Time-consuming but the only way to be sure it is indeed a non-behaviour change.
<jonatack> I like that a unit test was added instead functional ones. We sometimes see cases where functional tests are written out of ease where unit tests could have been added instead.
<michaelfolkson> So <ryanofsky> for that test in #15869 you literally sketched out all the combinations of changing settings in the config and QT and in different orders? And the test checks the resulting behavior is the same pre and post PR?
<ryanofsky> "auto Function() -> ReturnType" is a weird way to write "ReturnType Function()" which you are forced to do when the return type depends on the function arguments
<jnewbery> if a templated function has rvalue references in its arguments, then we get forwarding references: if an lvalue is provided, then the reference is an lvalue, and if an rvalue is provided, the reference is an rvalue
<ryanofsky> That's right I think. There are special rules for templates combined with && arguments. But if you want a template argument to accept any argument const or nonconst lvalue or rvalue you can use && to do that
<jnewbery> I think that now ArgsManagerHelper has been reduced to just two functions, we should just get rid of it entirely and move those to be private functions of ArgsManager
<jnewbery> lightlike: I asked aj that question in the original PR. His response "These functions could just be private member functions of ArgsManager, but that would mean bumping util.h every time any of them need to be changed, which causes a lot of unnecessary recompilation." (https://github.com/bitcoin/bitcoin/pull/11862#discussion_r173870835)
<jnewbery> next question: What is the univalue library, and how is it used in Bitcoin Core? How is it used in this PR? What are your thoughts about that?
<ajonas> univalue lib is a universal type that encapsulates a JSON value. Used for communication with external utilities through the RPC interface (exclusively until #15935?)
<jnewbery> on that point, let's go to the last question. 15935 adds a persistent read/write config file to the data directory. Have there been any other attempts to do this? Which do you prefer?
<fjahr> I did not have time to through either PRs but I feel people regularly stumble over how/where args are defined. So a single place makes sense to me. There was also this PR recently where the configs should be printed out in the logs. It shows that it's a pain for people.
<michaelfolkson> I don't like the idea of multiple of .conf files if I'm understood it correctly. As a user I just want to see the latest settings, I don't care the path those settings followed to the latest settings
<jnewbery> Talkless: I see you're reviewing the PR right now (thanks!) Are you able to use github's 'review' feature rather than leave individual comments to spare people's notifications a bit? :)