util: Check if specified config file cannot be opened #22622

1. 
   n-thumann commented at 11:07 AM on August 4, 2021: contributor

   Fixes #22612. When running e.g. ./src/bitcoind -datadir=/tmp/bitcoin -regtest -conf=/tmp/bitcoin/regtest/bitcoin.conf and the specified config cannot be opened (doesn't exist, permission denied, ...), the initialization silently uses the default config.

   As voidburn already noted:

   I can't think of a situation in which a config file is specified explicitly (in the startup options, as per service unit linked above), but inaccessible, where the fail condition should be to keep booting using defaults instead.

   With this patch applied, the initialization will fail immediately, if the specified config file cannot be opened. If no config file is explicitly specified, the behavior is unchanged. This not only affects bitcoind, but also bitcoin-cli and bitcoin-qt.

   In the example below the datadir is accessible, but the config file is not due to insufficient permissions:

   $ ./src/bitcoind -datadir=/tmp/bitcoin -regtest --debug=1 -conf=/tmp/bitcoin/regtest/bitcoin.conf
   Error: Error reading configuration file: specified config file "/tmp/bitcoin/regtest/bitcoin.conf" could not be opened.
   

2. util: Check if specified config file cannot be opened 6bb54708e6
3. fanquake added the label Utils/log/libs on Aug 4, 2021
4. test: Check if specified config file cannot be opened 127b4608e9
5. n-thumann force-pushed on Aug 4, 2021
6. 
   0xB10C commented at 6:56 PM on August 4, 2021: member

   ACK 127b4608e9dbb8217c74c9332e82fcec8c326fa8

   I agree that failing to read an explicitly specified config file should fail initialization.

   Tested that starting with a non-openable config file throws an error.

7. MarcoFalke requested review from ryanofsky on Aug 4, 2021
8. n-thumann renamed this:
   Check config file cannot be opened
   util: Check if specified config file cannot be opened
   on Aug 4, 2021
9. 
   practicalswift commented at 9:16 PM on August 4, 2021: contributor

   Concept ACK

10. 
    Zero-1729 commented at 12:16 PM on August 5, 2021: contributor

    tACK 127b4608e9dbb8217c74c9332e82fcec8c326fa8

    Tested on macOS v11.4

    Patch works as described. I got the following error from testing both cases: config file doesn't exist and config file is inaccessible (due to permissions):

    $ bitcoind -datadir=/tmp/bitcoin -regtest -conf=/tmp/bitcoin/regtest/bitcoin.conf
    Error: Error reading configuration file: specified config file "/tmp/bitcoin/regtest/bitcoin.conf" could not be opened.
    

11. 
    theStack commented at 2:09 PM on August 5, 2021: member

    Concept ACK

12. NikhilBartwal approved
13. 
    NikhilBartwal commented at 8:12 PM on August 8, 2021: contributor

    Concept ACK. Makes sense to fail the initial startup if the config is inaccessible instead of using the default one (which the user might/might not want)

    Tested on Manjaro Linux v21.1.0 . The PR patch works as described :)

14. theStack approved
15. 
    theStack commented at 8:21 PM on August 9, 2021: member

    Tested ACK 127b4608e9dbb8217c74c9332e82fcec8c326fa8

    Checked that starting src/bitcoind -conf=doesnt_exist leads to the newly introduced error message (running OpenBSD 6.9 here). Also tested that running the extended test in the second commit fails on master branch.

16. MarcoFalke merged this on Aug 23, 2021
17. MarcoFalke closed this on Aug 23, 2021

    ---

18. sidhujag referenced this in commit 8b35ac35c4 on Aug 23, 2021
19. DrahtBot locked this on Aug 23, 2022

Contributors

n-thumann0xB10CpracticalswiftZero-1729theStackNikhilBartwal

Review Requested
ryanofsky