Docs: Create default bitcoin.conf file on startup #13761

leishman · 2018-07-26T02:28:26Z

Fixes #10746. If no -conf flag is passed and no bitcoin.conf file exists in the datadir, then a bitcoin.conf template will be created in the datadir. This conf file will have no configurations set, only explanatory comments. The default bitcoin.conf template was copied from contrib/debian/examples/bitcoin.conf.

The lack of a default config file historically causes a lot of confusion for new users because bitcoind will print Using config file $DATADIR/bitcoin.conf on startup even if no file exists.

Curious if error handling is being done properly here or if file permissions need to be considered. I don't think a failure to create this file should terminate the process.

DrahtBot · 2018-07-26T04:10:59Z

Note to reviewers: This pull request conflicts with the following ones:

Use std::unordered_set instead of set in blockfilter interface #14074 (Use std::unordered_set instead of set in blockfilter interface by jimpo)
[Logging] Only log "Using config file PATH_TO_bitcoin.conf" message on startup if conf file exists #14057 ([Logging] Only log "Using config file PATH_TO_bitcoin.conf" message on startup if conf file exists by leishman)
-masterdatadir for datadir bootstrapping #13746 (-masterdatadir for datadir bootstrapping by kallewoof)
Check for datadir after the config files were read #13621 (Check for datadir after the config files were read by Flowdalic)

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.

ken2812221

You should add the filenameto src/Makefile.am

bitcoin/src/Makefile.am

Line 92 in 29b4ee6

BITCOIN_CORE_H = \

ken2812221 · 2018-07-26T04:17:37Z

src/bitcoin.conf

@@ -0,0 +1,147 @@
+R"(##


What's The R and ( for?

ken2812221 · 2018-07-26T04:17:54Z

src/bitcoin.conf

+#min=1
+
+# Minimize to the system tray
+#minimizetotray=1)"


What's the ) for ?

ken2812221 · 2018-07-26T04:19:42Z

src/bitcoind.cpp

+// default bitcoin.conf file template
+const char* g_default_conf_file =
+#include <bitcoin.conf>
+    ;


I don't think this is a good way to do this. I'd prefer rename to bitcoin_conf.h and move const char ... into that file.

The R() is for being able to #include the file. But I like your idea better. I will update. Thanks for the suggestions.

Also sorry for not explaining further. R() is for multiline string literal: https://stackoverflow.com/a/20508617/2302781

Fixes bitcoin#10746. If no -conf flag is passed and no bitcoin.conf file exists in the datadir, then a bitcoin.conf template will be created in the datadir. This conf file will have no configurations set, only explanatory comments. The default bitcoin.conf template was copied from contrib/debian/examples/bitcoin.conf.

leishman · 2018-07-26T05:47:00Z

@ken2812221 updated!

jonasschnelli · 2018-07-26T09:59:02Z

I'm conceptually unsure if the daemon binary should take care of the configuration management (if even supporting a template make sense). This seems to be better address on the package manager level.

leishman · 2018-07-26T16:37:33Z

I can understand that point of view, however I think that many users are accustomed to config files being generated automatically for them from a binary. When people see that the datadir has been generated they are confused about why there is no config file in there. At the very least we should prevent the daemon output from saying that it is using a conf file that doesn't exist.

On a higher level, I'm hoping to start contributing more to Bitcoin Core and I'd like to start with some changes that improve usability for new users. It can be quite intimidating for people.

sipa · 2018-07-27T00:40:22Z

If bitcoind is able to create a datadir, at least I think there should be no problem with it creating a default config file in that directory at the same time.

leishman · 2018-07-27T07:03:32Z

I've updated the PR to create the template config file immediately after creation of the datadir.

laanwj · 2018-07-30T11:20:50Z

I'm conceptually unsure if the daemon binary should take care of the configuration management (if even supporting a template make sense). This seems to be better address on the package manager level.

I tend to agree. Daemons should not write their own configuration file.
We can package a default configuration file, but I don't think bitcoin.conf should be created.
(in many cases this is not even possible as the configuration is in a read-only location)
(so NACK from me, see also my argumentation in #10746)

laanwj · 2018-07-30T11:54:40Z

If bitcoind is able to create a datadir, at least I think there should be no problem with it creating a default config file in that directory at the same time.

What about a compromise: always create a bitcoin.conf.example in the data directory. The user can decide to use it by copying it to bitcoin.conf (whatever its location) and editing it, or simply ignore it.

leishman · 2018-07-30T13:19:25Z

What about a compromise: always create a bitcoin.conf.example in the data directory. The user can decide to use it by copying it to bitcoin.conf (whatever its location) and editing it, or simply ignore it.

I like this. I also think we should disable the startup logging message saying a conf file is being used, unless it is actually true. If there is no conf file, then we can print a message instructing the user how/where to create one. What do you think?

…actually exists

leishman · 2018-08-01T01:22:23Z

I've updated things so that a bitcoin.conf.example file is created and we only tell the user a conf file is being used if this file actually exists at the time of startup

practicalswift · 2018-08-02T08:02:06Z

src/conf_file.h

+##            Quick Primer on addnode vs connect            ##
+##  Let's say for instance you use addnode=4.2.2.4          ##
+##  addnode will connect you to and tell you about the      ##
+##    nodes connected to 4.2.2.4.  In addition it will tell ##


Please left-indent this entire block and increase the total line length to say 78 chars. That will make it easier to read.

@practicalswift I copied this from contrib/debian/examples/bitcoin.conf. I'm happy to reformat the file and make these changes, but do you think they should be made in both places?

practicalswift · 2018-08-02T08:03:05Z

src/conf_file.h

+##  all of them to open lots of connections.  Instead       ##
+##  'connect' them all to one node that is port forwarded   ##
+##  and has lots of connections.                            ##
+##       Thanks goes to [Noodle] on Freenode.               ##


Please remove the greeting :-)

practicalswift · 2018-08-02T08:05:21Z

src/conf_file.h

+# If not, you must set rpcuser and rpcpassword to secure the JSON-RPC api. The first
+# method(DEPRECATED) is to set this pair for the server and client:
+#rpcuser=Ulysseys
+#rpcpassword=YourSuperGreatPasswordNumber_DO_NOT_USE_THIS_OR_YOU_WILL_GET_ROBBED_385593


Perhaps the 385593 part could be longer and randomized?

part could be longer and randomized?

What do you mean by this? This is an example password that we'd never want anyone to use, and actually comes from the sample bitcoin.conf on the wiki.

@fanquake Yes I know but randomizing would make it slighly less painful for newcomers to make the mistake of uncommenting as-is. Perhaps that is overkill though.

I think it's overkill. If anything we should remove the term 'Number' from the example pw so people don't think it must be limited to a numeric value.

OK, makes sense!

Sjors · 2018-08-09T18:54:58Z

Afaik you can't download that binary separately from the "official" source, and the release signatures don't cover individual files either: https://bitcoincore.org/en/download/

leishman · 2018-08-09T19:10:57Z

They are hosted here: https://bitcoincore.org/bin/bitcoin-core-0.16.2/

And when installing the bitcoind package on ubuntu: sudo apt-get install bitcoind this does not setup the ~/.bitcoin directory for you. This directory is created upon execution of the bitcoind binary. So it's unclear to me where a static file, like the default conf, could possibly be included. What am I missing?

sipa · 2018-08-09T19:28:27Z

It's also not possible. When you do a system wide installation of bitcoind it doesn't know which user(s) are going to run it.

Sjors · 2018-08-09T19:29:08Z

@leishman these are packages with multiple files, which you can add files to by tweaking the deploy scripts. Similarly, it should be possible for the ubuntu install script to put this template file somewhere, since it also manages put the icon in /usr/share.

leishman · 2018-08-09T20:12:36Z

@sipa so do you think the approach I have taken in this PR is reasonable?

Sjors · 2018-08-10T11:11:18Z

To clarify my position: I prefer a more elegant solution, but I'm OK with doing that later.

I do however recommend adding a functional test to check if this file is indeed created.

leishman · 2018-08-11T23:13:24Z

@Sjors ok I'll add in that test

leishman · 2018-08-17T19:20:34Z

@Sjors After digging into the testing framework it looks like testing for this behavior is actually quite tricky. The current functionality only creates a bitcoin.conf.example file if the default datadir is used and doesn't already exist. In the testing framework, a custom test-specific datadir is always used in order to not pollute the global filesystem of the testing machine.

Sjors · 2018-08-20T16:50:02Z

@jnewbery is there already a test which checks that the datadir is created when necessary? If so, perhaps that test can be modified to also check the presence of bitcoin.conf.example?

jnewbery · 2018-08-24T20:46:43Z

is there already a test which checks that the datadir is created when necessary?

I don't think so. See initialize_datadir() in util.py and the places it's called. There are various places where the files in the datadir or the datadir itself is removed. See here for example:

bitcoin/test/functional/feature_blocksdir.py

Line 22 in 55c18a4

initialize_datadir(self.options.tmpdir, 0)

leishman · 2018-08-24T21:30:08Z

Yeah, currently the test framework always puts the datadirs in a custom path deep in the system /tmp directory. This prevents polluting the $HOME path of the machine running the tests. But because of this, my change is basically untestable, unless we change the way the test framework works or hack something ugly.

Testing default datadir creation could be a testing feature we add in a later PR.

leishman · 2018-08-24T22:38:11Z

Pulling out logging update into a separate PR: #14057

jnewbery · 2018-08-27T13:32:52Z

The current functionality only creates a bitcoin.conf.example file if the default datadir is used and doesn't already exist.

... my change is basically untestable

What do you think about changing the conditional here: https://github.com/bitcoin/bitcoin/pull/13761/files#diff-b7702a084eb00fb47f9800fd68271951R791 to check whether the directory is empty instead of whether the directory exists (ie create the wallets subdirectory and example conf file when a non-existent or empty datadir is used).

That makes more sense to me, and would be testable in the existing test framework.

DrahtBot · 2018-09-10T14:15:58Z

Needs rebase

DrahtBot · 2018-12-12T19:23:16Z

There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened.

…tcoin.conf" message on startup if conf file exists 946107a Only log "Using PATH_TO_bitcoin.conf" message on startup if conf file exists. (Alexander Leishman) Pull request description: Currently we log a message indicating that a bitcoin.conf file is being used even if one does not exist. This PR changes the logic to: **If config file does not exist and no -conf flag passed, log:** `Config file: FILE_PATH (not found, skipping)`. Where `FILE_PATH` is the default or the path passed in with the `-conf` flag. **If config file does not exist and -conf flag passed with incorrect path, log warning:** `Warning: The specified config file FILE_PATH does not exist` **If config file exists, log**: `Config file: FILE_PATH` Note: This is a (modified) subset of changes introduced in bitcoin#13761 which creates a default example config file. I think it makes sense to extract this small bit out into a separate PR. Tree-SHA512: be0f0ae6a0c9041e2d6acb54d2563bbcc79786fb2f8bf9a963fe01bc54cd4e388b89079fde1eb79f7f17099776428e5e984bf7107590a3d1ecfc0562dbc6e3f5

leishman force-pushed the write-default-config-file branch 4 times, most recently from 04be012 to a517c47 Compare July 26, 2018 03:12

ken2812221 suggested changes Jul 26, 2018

View reviewed changes

DrahtBot mentioned this pull request Jul 26, 2018

Check for datadir after the config files were read #13621

Closed

leishman force-pushed the write-default-config-file branch from 1f46306 to 6249702 Compare July 26, 2018 04:55

DrahtBot mentioned this pull request Jul 26, 2018

Utils and libraries: Fix #13371 - move umask operation earlier in AppInit() #13389

Closed

leishman force-pushed the write-default-config-file branch from fb4194d to f2df814 Compare July 26, 2018 05:09

leishman force-pushed the write-default-config-file branch from 35ae6b3 to 08def4b Compare July 26, 2018 05:46

leishman added 2 commits July 26, 2018 23:30

move file creation to same function that creates datadir

77f10e6

revert changes to bitcoind.cpp

345425d

DrahtBot mentioned this pull request Jul 27, 2018

-masterdatadir for datadir bootstrapping #13746

Closed

create bitcoin.conf.example and only log on startup if the conf file …

d88246a

…actually exists

This was referenced Jul 31, 2018

Test for Windows encoding issue #13787

Merged

[bugfix] Fix encoding issue for Windows #13426

Closed

practicalswift reviewed Aug 2, 2018

View reviewed changes

leishman mentioned this pull request Aug 24, 2018

[Logging] Only log "Using config file PATH_TO_bitcoin.conf" message on startup if conf file exists #14057

Merged

DrahtBot mentioned this pull request Aug 26, 2018

Use std::unordered_set instead of set in blockfilter interface #14074

Merged

DrahtBot added the Needs rebase label Sep 10, 2018

Sjors mentioned this pull request Sep 25, 2018

gui: Add dynamic wallets support #13100

Closed

DrahtBot added the Up for grabs label Dec 12, 2018

DrahtBot closed this Dec 12, 2018

laanwj removed the Needs rebase label Oct 24, 2019

adamjonas mentioned this pull request Jan 6, 2021

Provide skeleton bitcoin.conf during datadir generation #19641

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

fanquake removed the Up for grabs label Jan 2, 2022

Docs: Create default bitcoin.conf file on startup #13761

Docs: Create default bitcoin.conf file on startup #13761

Uh oh!

Conversation

leishman commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leishman commented Jul 26, 2018

Uh oh!

jonasschnelli commented Jul 26, 2018

Uh oh!

leishman commented Jul 26, 2018

Uh oh!

sipa commented Jul 27, 2018

Uh oh!

leishman commented Jul 27, 2018

Uh oh!

laanwj commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leishman commented Jul 30, 2018

Uh oh!

leishman commented Aug 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors commented Aug 9, 2018

Uh oh!

leishman commented Aug 9, 2018

Uh oh!

sipa commented Aug 9, 2018

Uh oh!

Sjors commented Aug 9, 2018

Uh oh!

leishman commented Aug 9, 2018

Uh oh!

Sjors commented Aug 10, 2018

Uh oh!

leishman commented Aug 11, 2018

Uh oh!

leishman commented Aug 17, 2018

Uh oh!

Sjors commented Aug 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

leishman commented Jul 26, 2018 •

edited

Loading

DrahtBot commented Jul 26, 2018 •

edited

Loading

laanwj commented Jul 30, 2018 •

edited

Loading

laanwj commented Jul 30, 2018 •

edited

Loading

practicalswift Aug 2, 2018 •

edited

Loading

practicalswift Aug 2, 2018 •

edited

Loading

Sjors commented Aug 20, 2018 •

edited

Loading