Feature: Use different datadirs for different signets

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

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.

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, luisschwab

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.

BrandonOdiwuor force-pushed on Apr 9, 2024

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

DrahtBot added the label CI failed on Apr 9, 2024

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

1440000bytes commented at 8:16 pm on April 9, 2024: none

Concept ACK

laanwj commented at 9:49 am on April 10, 2024: member

Concept ACK

RandyMcMillan commented at 1:09 pm on April 10, 2024: contributor

Concept ACK

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$

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

Concept ACK

alfonsoromanz commented at 4:13 pm on April 15, 2024: contributor

Concept ACK

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=...

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

i concur with @tdb3 - use underscores for signet paths.

RandyMcMillan commented at 4:09 pm on April 22, 2024: contributor

I concur with @Sjors.

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.

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.

BrandonOdiwuor force-pushed on May 15, 2024

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?

BrandonOdiwuor force-pushed on May 17, 2024

BrandonOdiwuor marked this as ready for review on May 17, 2024

BrandonOdiwuor requested review from ajtowns on May 17, 2024

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

BrandonOdiwuor requested review from tdb3 on May 17, 2024

DrahtBot removed the label CI failed on May 17, 2024

BrandonOdiwuor force-pushed on May 22, 2024

carnhofdaki commented at 8:48 am on May 27, 2024: contributor

Concept ACK

BrandonOdiwuor requested review from edilmedeiros on May 29, 2024

pablomartin4btc commented at 4:11 pm on July 5, 2024: member

Concept ACK and light tACK 50f4ca2363e008aacb9f8a8aa1bb7abba04c0b33

Tried different challenges and custom -datadir option.

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.

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

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

BrandonOdiwuor force-pushed on Sep 12, 2024

BrandonOdiwuor requested review from pablomartin4btc on Sep 12, 2024

DrahtBot added the label CI failed on Sep 13, 2024

DrahtBot removed the label CI failed on Sep 15, 2024

luisschwab commented at 2:30 am on October 3, 2024: contributor

Concept ACK.

Great feature, not being able to do this is a PITA.

Feature: Use different datadirs for different signets #29838

Update:

Code Coverage

Reviews

Conflicts