Feature: Use different datadirs for different signets #29838

1. 
   BrandonOdiwuor commented at 4:25 pm on April 9, 2024: contributor

   Fixes #27494

   When the -signetchallenge argument is provided, the hash-160 of the challenge is appended to the data directory name. This ensures that each signet has its own distinct data directory, following the naming convention signet_XXXXXXX.

   Update:

   When the -signetchallenge argument is provided, the first 8 characters of the hash-160 of the challenge are appended to the data directory name. This ensures that each signet has its own distinct data directory, following the naming convention signet_XXXXXXX.

2. 
   DrahtBot commented at 4:25 pm on April 9, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	1440000bytes, laanwj, RandyMcMillan, tdb3, kristapsk, alfonsoromanz, edilmedeiros, carnhofdaki, pablomartin4btc

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29032 (signet: omit commitment for some trivial challenges by Sjors)
   • #28802 (ArgsManager: support subcommand-specific options by ajtowns)

   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.

3. BrandonOdiwuor force-pushed on Apr 9, 2024
4. 
   DrahtBot commented at 4:30 pm on April 9, 2024: contributor

   🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

   Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   Leave a comment here, if you need help tracking down a confusing failure.

   Debug: https://github.com/bitcoin/bitcoin/runs/23622990227

5. DrahtBot added the label CI failed on Apr 9, 2024
6. in src/common/args.cpp:288 in 9e81922b57 outdated

   283+    const std::string base_data_dir = BaseParams().DataDir();
   284+    const std::string signet_challenge_str = GetArg("-signetchallenge", "");
   285+    if (!signet_challenge_str.empty()) {
   286+        std::vector<uint8_t> signet_challenge = ParseHex(signet_challenge_str);
   287+        const auto hash_160 = Hash160(signet_challenge);
   288+        return fs::PathFromString(base_data_dir + "-" + HexStr(hash_160));
   

   ---

   ---

   

   ajtowns commented at 4:36 pm on April 9, 2024:

   I guess the hash160 will better guarantee uniqueness, but a 40 character string is kind-of long? Would it make sense to use the 8 character HexStr(Params().MessageStart()) value instead, special-casing for when the message start magic number matches the default signet’s 0a03cf40?

   ---

   

   tdb3 commented at 2:08 am on April 14, 2024:

   The approach to use the 8-character message start (i.e. first 4 bytes of the SHA256 hash of the challenge) seems better to me as well. More concise directory lengths with a reasonably low chance of collision between signets.

   nit: Recommend avoiding using - (dash) and instead using something like _ (underscore) when building signet paths to prevent unexpected issues when including custom signet directory paths in command line arguments.

   ---

   

   BrandonOdiwuor commented at 1:01 pm on May 17, 2024:

   I have updated the PR to use the first 8 bytes of the hash160 and replaced the - with _ as suggested
7. 
   1440000bytes commented at 8:16 pm on April 9, 2024: none

   Concept ACK
8. 
   laanwj commented at 9:49 am on April 10, 2024: member

   Concept ACK
9. 
   RandyMcMillan commented at 1:09 pm on April 10, 2024: contributor

   Concept ACK
10. 
    tdb3 commented at 2:09 am on April 14, 2024: contributor

    Concept ACK. Great feature to help prevent conflicting datadir conflicts when switching between signets. Inline comments added.

    Received failures for functional tests feature_signet.py and tool_signet_miner.py.

      0dev@bdev01:~/bitcoin$ test/functional/test_runner.py -u --cachedir=/mnt/tmp/cache --tmpdir=/mnt/tmp test/functional/feature_signet.py test/functional/tool_signet_miner.py 
      1Temporary test directory at /mnt/tmp/test_runner_₿_🏃_20240413_214701
      2Remaining jobs: [feature_signet.py, tool_signet_miner.py --legacy-wallet, tool_signet_miner.py --descriptors]
      31/3 - feature_signet.py failed, Duration: 60 s                                                                          
      4
      5stdout:
      62024-04-14T01:47:02.221000Z TestFramework (INFO): PRNG seed is: 2979480965586364484
      72024-04-14T01:47:02.222000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240413_214701/feature_signet_2
      82024-04-14T01:48:02.321000Z TestFramework (ERROR): Assertion failed
      9Traceback (most recent call last):
     10  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 565, in start_nodes
     11    node.wait_for_rpc_connection()
     12  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 314, in wait_for_rpc_connection
     13    self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
     14  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 188, in _raise_assertion_error
     15    raise AssertionError(self._node_msg(msg))
     16AssertionError: [node 0] Unable to connect to bitcoind after 60s
     17
     18During handling of the above exception, another exception occurred:
     19
     20Traceback (most recent call last):
     21  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
     22    self.setup()
     23  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 302, in setup
     24    self.setup_network()
     25  File "/home/dev/bitcoin/test/functional/feature_signet.py", line 43, in setup_network
     26    self.setup_nodes()
     27  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 418, in setup_nodes
     28    self.start_nodes()
     29  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 568, in start_nodes
     30    self.stop_nodes()
     31  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
     32    node.stop_node(wait=wait, wait_until_stopped=False)
     33  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
     34    self.stop(wait=wait)
     35    ^^^^^^^^^
     36  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
     37    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
     38                                  ^^^^^^^^^^^^^^^^^^^^
     39AssertionError: [node 0] Error: no RPC connection
     402024-04-14T01:48:02.376000Z TestFramework (INFO): Stopping nodes
     41[node 5] Cleaning up leftover process
     42[node 4] Cleaning up leftover process
     43[node 3] Cleaning up leftover process
     44[node 2] Cleaning up leftover process
     45[node 1] Cleaning up leftover process
     46[node 0] Cleaning up leftover process
     47
     48
     49stderr:
     50Traceback (most recent call last):
     51  File "/home/dev/bitcoin/test/functional/feature_signet.py", line 85, in <module>
     52    SignetBasicTest().main()
     53  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 155, in main
     54    exit_code = self.shutdown()
     55                ^^^^^^^^^^^^^^^
     56  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 318, in shutdown
     57    self.stop_nodes()
     58  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
     59    node.stop_node(wait=wait, wait_until_stopped=False)
     60  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
     61    self.stop(wait=wait)
     62    ^^^^^^^^^
     63  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
     64    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
     65                                  ^^^^^^^^^^^^^^^^^^^^
     66AssertionError: [node 0] Error: no RPC connection
     67
     68
     692/3 - tool_signet_miner.py --descriptors failed, Duration: 60 s
     70
     71stdout:
     722024-04-14T01:47:02.220000Z TestFramework (INFO): PRNG seed is: 1471356017149591392
     732024-04-14T01:47:02.221000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240413_214701/tool_signet_miner_0
     742024-04-14T01:48:02.317000Z TestFramework (ERROR): Assertion failed
     75Traceback (most recent call last):
     76  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 565, in start_nodes
     77    node.wait_for_rpc_connection()
     78  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 314, in wait_for_rpc_connection
     79    self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
     80  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 188, in _raise_assertion_error
     81    raise AssertionError(self._node_msg(msg))
     82AssertionError: [node 0] Unable to connect to bitcoind after 60s
     83
     84During handling of the above exception, another exception occurred:
     85
     86Traceback (most recent call last):
     87  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
     88    self.setup()
     89  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 302, in setup
     90    self.setup_network()
     91  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 396, in setup_network
     92    self.setup_nodes()
     93  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 418, in setup_nodes
     94    self.start_nodes()
     95  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 568, in start_nodes
     96    self.stop_nodes()
     97  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
     98    node.stop_node(wait=wait, wait_until_stopped=False)
     99  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    100    self.stop(wait=wait)
    101    ^^^^^^^^^
    102  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    103    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
    104                                  ^^^^^^^^^^^^^^^^^^^^
    105AssertionError: [node 0] Error: no RPC connection
    1062024-04-14T01:48:02.370000Z TestFramework (INFO): Stopping nodes
    107[node 0] Cleaning up leftover process
    108
    109
    110stderr:
    111Traceback (most recent call last):
    112  File "/home/dev/bitcoin/test/functional/tool_signet_miner.py", line 65, in <module>
    113    SignetMinerTest().main()
    114  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 155, in main
    115    exit_code = self.shutdown()
    116                ^^^^^^^^^^^^^^^
    117  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 318, in shutdown
    118    self.stop_nodes()
    119  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    120    node.stop_node(wait=wait, wait_until_stopped=False)
    121  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    122    self.stop(wait=wait)
    123    ^^^^^^^^^
    124  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    125    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
    126                                  ^^^^^^^^^^^^^^^^^^^^
    127AssertionError: [node 0] Error: no RPC connection
    128
    129
    130Remaining jobs: [tool_signet_miner.py --legacy-wallet]
    1313/3 - tool_signet_miner.py --legacy-wallet failed, Duration: 61 s
    132
    133stdout:
    1342024-04-14T01:47:02.211000Z TestFramework (INFO): PRNG seed is: 4742527368365497737
    1352024-04-14T01:47:02.212000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240413_214701/tool_signet_miner_1
    1362024-04-14T01:48:02.317000Z TestFramework (ERROR): Assertion failed
    137Traceback (most recent call last):
    138  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 565, in start_nodes
    139    node.wait_for_rpc_connection()
    140  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 314, in wait_for_rpc_connection
    141    self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
    142  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 188, in _raise_assertion_error
    143    raise AssertionError(self._node_msg(msg))
    144AssertionError: [node 0] Unable to connect to bitcoind after 60s
    145
    146During handling of the above exception, another exception occurred:
    147
    148Traceback (most recent call last):
    149  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
    150    self.setup()
    151  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 302, in setup
    152    self.setup_network()
    153  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 396, in setup_network
    154    self.setup_nodes()
    155  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 418, in setup_nodes
    156    self.start_nodes()
    157  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 568, in start_nodes
    158    self.stop_nodes()
    159  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    160    node.stop_node(wait=wait, wait_until_stopped=False)
    161  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    162    self.stop(wait=wait)
    163    ^^^^^^^^^
    164  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    165    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
    166                                  ^^^^^^^^^^^^^^^^^^^^
    167AssertionError: [node 0] Error: no RPC connection
    1682024-04-14T01:48:02.370000Z TestFramework (INFO): Stopping nodes
    169[node 0] Cleaning up leftover process
    170
    171
    172stderr:
    173Traceback (most recent call last):
    174  File "/home/dev/bitcoin/test/functional/tool_signet_miner.py", line 65, in <module>
    175    SignetMinerTest().main()
    176  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 155, in main
    177    exit_code = self.shutdown()
    178                ^^^^^^^^^^^^^^^
    179  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 318, in shutdown
    180    self.stop_nodes()
    181  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    182    node.stop_node(wait=wait, wait_until_stopped=False)
    183  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    184    self.stop(wait=wait)
    185    ^^^^^^^^^
    186  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    187    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
    188                                  ^^^^^^^^^^^^^^^^^^^^
    189AssertionError: [node 0] Error: no RPC connection
    190
    191
    192
    193TEST                                 | STATUS    | DURATION
    194
    195feature_signet.py                    | ✖ Failed  | 60 s
    196tool_signet_miner.py --descriptors   | ✖ Failed  | 60 s
    197tool_signet_miner.py --legacy-wallet | ✖ Failed  | 61 s
    198
    199ALL                                  | ✖ Failed  | 181 s (accumulated) 
    200Runtime: 61 s
    201
    202dev@bdev01:~/bitcoin$ 
    

11. 
    kristapsk commented at 6:14 am on April 14, 2024: contributor

    Concept ACK
12. 
    alfonsoromanz commented at 4:13 pm on April 15, 2024: contributor

    Concept ACK
13. 
    Sjors commented at 6:54 am on April 18, 2024: member

    I use both the default signet and at least one custom one, so having separate directories would be quite useful. Shorter would be nice.

    Ideally we would also support different config file sections and startup flags, using the same convention:

    0src/bitcoind -chain=signet_xxxx
    

    0[signet]
    1prune=1000
    2
    3[signet_xxx]
    4signetchallenge=51
    5signetseednode=...
    

14. 
    RandyMcMillan commented at 3:58 pm on April 22, 2024: contributor

    i concur with @tdb3 - use underscores for signet paths.
15. 
    RandyMcMillan commented at 4:09 pm on April 22, 2024: contributor

    I concur with @Sjors.
16. 
    RandyMcMillan commented at 4:13 pm on April 22, 2024: contributor

    Ensure the signet path pre/postfix is shared with the wallet so it can find the appropriate wallets for each signet. Treating the global signet as “just another signet” ought to simplify the wallet logic as well as avoiding a bug where the wallet doesn’t distinguish between global signet and custom signets.
17. 
    RandyMcMillan commented at 4:39 pm on April 22, 2024: contributor

    You may be able to allow cascading signet configurations by having a user configured “prefix” setting. This may also translate better to a gui dialogue later on as well (hot swapping signets from the gui).

    [signet] prune=1000

    prefix=<short_string1> signetchallenge=1234abc… signetseednode=…

    prefix=<short_string2> signetchallenge=5678def… signetseednode=…

    note: consider supporting forward slashes in the signet path - allowing more control on where the data lands in disk.

18. BrandonOdiwuor force-pushed on May 15, 2024
19. 
    edilmedeiros commented at 4:01 am on May 16, 2024: contributor

    Concept ACK.

    Code looks good, but not tested yet.

    Any way to add a functional test for this?

20. BrandonOdiwuor force-pushed on May 17, 2024
21. BrandonOdiwuor force-pushed on May 17, 2024
22. BrandonOdiwuor marked this as ready for review on May 17, 2024
23. BrandonOdiwuor requested review from ajtowns on May 17, 2024
24. 
    BrandonOdiwuor commented at 1:03 pm on May 17, 2024: contributor

    @Sjors and @RandyMcMillan I love the idea of the signet_XXXX config section and would like to work on it if there is a lot of interest in a follow-up PR
25. BrandonOdiwuor requested review from tdb3 on May 17, 2024
26. DrahtBot removed the label CI failed on May 17, 2024
27. BrandonOdiwuor force-pushed on May 22, 2024
28. 
    carnhofdaki commented at 8:48 am on May 27, 2024: contributor

    Concept ACK
29. BrandonOdiwuor requested review from edilmedeiros on May 29, 2024
30. 
    pablomartin4btc commented at 4:11 pm on July 5, 2024: member

    Concept ACK and light tACK 50f4ca2363e008aacb9f8a8aa1bb7abba04c0b33

    Tried different challenges and custom -datadir option.

31. 
    tdb3 commented at 1:25 am on September 4, 2024: contributor

    This is a great feature. Would love to see it get close to merge. Looks like the CI logs might be gone? Maybe this CI could be nudged to run again. Not sure if this CI failure is related to the one seen a few months ago related to macOS.
32. Use distinct data dirs for each signet

    If the -signetchallenge argument is provided,
    the hash-160 (first 8 characters) of the challenge is appended to the datadir name,
    resulting in unique data directories for each signet, i.e., signet_XXXXXXX.

    f1b40b4de4
33. test: Add test for diffrent signet data directories

    - Added a test to verify the Signet data directory when -signetchallenge=51 is correctly set to 'signet_da1745e9'.
    - Added a test to verify the main Signet data directory is correctly set to 'signet'.

    32c3a732c1
34. BrandonOdiwuor force-pushed on Sep 12, 2024
35. BrandonOdiwuor requested review from pablomartin4btc on Sep 12, 2024
36. DrahtBot added the label CI failed on Sep 13, 2024
37. DrahtBot removed the label CI failed on Sep 15, 2024

Contributors
BrandonOdiwuor DrahtBot ajtowns tdb3 1440000bytes laanwj RandyMcMillan kristapsk alfonsoromanz Sjors edilmedeiros carnhofdaki pablomartin4btc

Review Requested
ajtowns edilmedeiros tdb3 pablomartin4btc