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.

Note: The behaviour of the default signet remains unchanged

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 & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/29838.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	jsarenik
Concept ACK	laanwj, RandyMcMillan, tdb3, kristapsk, alfonsoromanz, edilmedeiros, pablomartin4btc, luisschwab, zaidmstrr, achow101, 1440000bytes
Stale ACK	carnhofdaki, yuvicc

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:

#33201 (Add functional test for IPC interface by sipa)
#30437 (ipc: add bitcoin-mine test program by ryanofsky)
#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

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.

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.

carnhofdaki commented at 8:25 am on December 4, 2024: contributor

Concept NACK.

I use a simple script setting datadir according to the current working directory. If the change would be backward-compatible I don’t care. But running multiple custom signets and never needed to make a mess. The proposed naming convention (signet_XXXXXXX) seems like a mess to me.

edilmedeiros commented at 1:26 pm on December 4, 2024: contributor

I believe everyone running multiple signets will have a workaround for this. Mine is to maintain different datadirs around which, in practice, is pretty much the same of what’s being proposed here.

A different naming scheme for signet actually makes sense since there’s no signet (as in mainnet), but potentially infinite signets serving different purposes. I can’t see how this being automatically handled by the tool instead of exposing the problem to the user would be called a mess.

carnhofdaki commented at 7:36 am on December 5, 2024: contributor

@edilmedeiros Yes, my harsh word is not being helpful. Just ignore it.

But regarding one network like mainnet there is a signet which anyone can find at mempool.space/signet or just simply running bitcoind -signet. Yes it is really something like Signet6 (similar to Testnets) but there is one Signet which was started by the original author of the sources and is maintained (blocks are mined+signed) by another Bitcoin Core developer.

Some more related text (by original author mostly) is at https://en.bitcoin.it/wiki/Signet

yuvicc commented at 4:41 pm on January 2, 2025: contributor

Concept ACK

It’s a pretty required feature!! I will test it soon!!

yuvicc approved

yuvicc commented at 1:16 pm on January 4, 2025: contributor

Tested ACK f1b40b4de460173e927ebe93cdb5f4ad2ac02859

I tried testing using ./src/bitcoind -signetchallenge=512102ee856c56a5aaadd1656f849bafa4c9dacc86a2878fe546c6189185f842ae2c1851ae
All the test(functional) passes!!

zaidmstrr commented at 7:44 pm on February 18, 2025: contributor

Concept ACK A very much-needed feature when you’re working with different signet chains.

jsarenik commented at 6:58 pm on February 20, 2025: none

How will the default signet directory name be here with this patch?

Could it possibly stay signet for the default Signet? I think the advantages of one default Signet are huge, one of them being the fact you can use mempool.space/signet. There is now a working faucet at alt.signetfaucet.com.

yuvicc commented at 7:17 pm on February 20, 2025: contributor

@jsarenik read the PR description #29838#issue-2233837348

jsarenik commented at 8:54 am on February 21, 2025: none

@jsarenik read the PR description #29838#issue-2233837348

Yes, it does not seem to honor the non-custom signet and leave it as signet. NACK

DrahtBot added the label CI failed on Mar 17, 2025

in test/functional/tool_signet_miner.py:51 in 32c3a732c1 outdated

47@@ -48,10 +48,11 @@ def run_test(self):
48         # generate block with signet miner tool
49         base_dir = self.config["environment"]["SRCDIR"]
50         signet_miner_path = os.path.join(base_dir, "contrib", "signet", "miner")
51+        # breakpoint()

maflcko commented at 7:42 pm on March 17, 2025:

stray leftover debug statement?

jsarenik commented at 7:06 am on March 18, 2025:

@maflcko Apologies for my previous deleted stray comment. I was out of context “thanks” to GitHub app on Android.

BrandonOdiwuor commented at 8:18 am on March 19, 2025:

@maflcko fixed

maflcko commented at 7:43 pm on March 17, 2025: member

are you still working on this?

BrandonOdiwuor force-pushed on Mar 19, 2025

BrandonOdiwuor commented at 8:17 am on March 19, 2025: contributor

rebased and removed the leftover debug statement

BrandonOdiwuor commented at 8:25 am on March 19, 2025: contributor

@jsarenik @carnhofdaki the behaviour default signet is not changed. I have updated the PR description to include this in case it was unclear.

This patch only affects custom signets with signetchallenge and the goal was to run those on custom directories signet_XXXXXXXX so as not to interfere with the default signet which runs on signet directory if you run multiple signets

jsarenik commented at 9:59 am on March 19, 2025: none

@jsarenik @carnhofdaki the behaviour default signet is not changed. I have updated the PR description to include this in case it was unclear.

This patch only affects custom signets with signetchallenge and the goal was to run those on custom directories signet_XXXXXXXX so as not to interfere with the default signet which runs on signet directory if you run multiple signets

Thank you for this clarification.

utACK abe8639

DrahtBot requested review from yuvicc on Mar 19, 2025

DrahtBot requested review from laanwj on Mar 19, 2025

DrahtBot requested review from zaidmstrr on Mar 19, 2025

carnhofdaki commented at 10:01 am on March 19, 2025: contributor

utACK abe8639

maflcko commented at 4:40 pm on March 19, 2025: member

This patch only affects custom signets with signetchallenge and the goal was to run those on custom directories signet_XXXXXXXX so as not to interfere with the default signet which runs on signet directory if you run multiple signets

Not sure if this is true, looking at the current code.

If someone provides the default signet challenge via -signetchallenge, it will pick a custom dir, no? Please add a test for this as well.

Also, the CI fails currently.

DrahtBot added the label Needs rebase on Mar 20, 2025

BrandonOdiwuor commented at 9:40 am on March 21, 2025: contributor

This patch only affects custom signets with signetchallenge and the goal was to run those on custom directories signet_XXXXXXXX so as not to interfere with the default signet which runs on signet directory if you run multiple signets

Not sure if this is true, looking at the current code.

If someone provides the default signet challenge via -signetchallenge, it will pick a custom dir, no? Please add a test for this as well. @maflcko The assumption was, providing default signet challenge is not so useful as it would result in the loss of default the signet configurations, hence did not include the check

https://github.com/bitcoin/bitcoin/blob/2db00278ea571d0f734609f9fefecfa75c16ee34/src/kernel/chainparams.cpp#L393-L408

BrandonOdiwuor force-pushed on Mar 21, 2025

DrahtBot removed the label Needs rebase on Mar 21, 2025

BrandonOdiwuor commented at 1:16 pm on March 21, 2025: contributor

Also, the CI fails currently. @maflcko rebased and fixed the ci error

DrahtBot removed the label CI failed on Mar 21, 2025

in src/common/args.h:217 in a11fa9078f outdated

211@@ -212,6 +212,14 @@ class ArgsManager
212      */
213     std::optional<const Command> GetCommand() const;
214 
215+    /**
216+     * Get signet data directory path.
217+     * If a signet-challange argument is provided, it is used in constructing the directory path.

flack commented at 1:44 pm on March 21, 2025:

nit: challange => challenge

BrandonOdiwuor commented at 2:33 pm on March 21, 2025:

fixed

BrandonOdiwuor force-pushed on Mar 21, 2025

jsarenik commented at 8:45 am on March 22, 2025: none

utACK 6913904

yuvicc commented at 9:36 am on May 8, 2025: contributor

re-ACK 69139049194e55a7fd99b7125b9afccb8585922d

in test/functional/feature_signet.py:106 in 6913904919 outdated

101@@ -101,6 +102,12 @@ def check_getmininginfo(node_idx, signet_idx):
102 
103         assert_equal(self.nodes[4].submitblock(signet_blocks[0]), 'bad-signet-blksig')
104 
105+        self.log.info("Test that the signet data directory with -signetchallenge=51 is 'signet_da1745e9'")
106+        assert_equal(path.basename(self.nodes[0].chain_path), "signet_da1745e9")

achow101 commented at 9:57 pm on June 2, 2025:

In 69139049194e55a7fd99b7125b9afccb8585922d “test: Add test for diffrent signet data directories”

This test is not sufficient. All it checks is that the test framework has computed the expected datadir. It does not check that the directory even exists, or that the node is using that as the data directory.

I think this will need to read the debug log to see that it has the expected Using data directory line, but it would also probably be better if the node itself could report the current datadir via rpc. A bit of a hack for that would be getrpcinfo’s logpath.

BrandonOdiwuor commented at 8:19 am on June 12, 2025:

I have rebased and added more tests to check that the data directory exists and is being used

achow101 commented at 9:58 pm on June 2, 2025: member

Concept ACK

DrahtBot added the label Needs rebase on Jun 3, 2025

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.

bead5266f5

BrandonOdiwuor force-pushed on Jun 12, 2025

DrahtBot removed the label Needs rebase on Jun 12, 2025

in test/functional/feature_signet.py:110 in 4e5ada313c outdated

101@@ -101,6 +102,23 @@ def check_getmininginfo(node_idx, signet_idx):
102 
103         assert_equal(self.nodes[4].submitblock(signet_blocks[0]), 'bad-signet-blksig')
104 
105+        def assert_node_datadir(node, expected_dirname):
106+            datadir = node.chain_path
107+            self.log.info(f"Checking node datadir: {datadir}")
108+            assert_equal(path.basename(datadir), expected_dirname)
109+
110+            assert datadir.is_dir() # check if the direcory exists

maflcko commented at 8:59 am on June 12, 2025:

direcory -> directory [spelling]

BrandonOdiwuor commented at 9:09 am on June 12, 2025:

fixed

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

6bcf96c4a4

BrandonOdiwuor force-pushed on Jun 12, 2025

DrahtBot added the label Needs rebase on Sep 5, 2025

DrahtBot commented at 5:48 pm on September 5, 2025: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

in src/common/args.cpp:289 in 6bcf96c4a4

284+{
285+    const std::string base_data_dir = BaseParams().DataDir();
286+    const std::string signet_challenge_str = GetArg("-signetchallenge", "");
287+    if (!signet_challenge_str.empty()) {
288+        std::vector<uint8_t> signet_challenge = ParseHex(signet_challenge_str);
289+        const auto data_dir_suffix = HexStr(Hash160(signet_challenge)).substr(0, 8);

RandyMcMillan commented at 11:28 am on September 23, 2025:

By NOT taking the Hash160 of the signet_challenge - the substring will be familiar to the user. This IMO is preferred.

example: The user may have multiple signet challenges - it is better to preserve the “finger print” of the signet challenge so that the user can easily identify the related datadir when browsing the file structure manually.

additionally: I have a proposal which displays the signet_challenge(finger print) in the gui.

RandyMcMillan commented at 11:35 am on September 23, 2025:

the collision concern isnt a huge issue IMO. Keeping the appended “finger print” familiar to the user is preferred.

RandyMcMillan commented at 4:10 pm on September 23, 2025: contributor

When displayed in the gui - a consistent finger print makes more sense to the user.

switch-chain-signet-fingerprint

kallewoof commented at 0:23 am on September 24, 2025: contributor

Nice. Should vanity grind that hash when/if we ever switch default signet challenge.

jsarenik commented at 6:36 am on September 24, 2025: none

Nice. Should vanity grind that hash when/if we ever switch default signet challenge.

As far as I understand this change does not influence the default signet only custom ones.

See #29838 (comment) for my total misunderstanding:)

jsarenik commented at 8:45 am on September 24, 2025: none

utACK 6bcf96c

DrahtBot requested review from achow101 on Sep 24, 2025

DrahtBot requested review from RandyMcMillan on Sep 24, 2025

DrahtBot requested review from yuvicc on Sep 24, 2025

maflcko removed the label Needs rebase on Sep 24, 2025

DrahtBot added the label Needs rebase on Sep 24, 2025

DrahtBot commented at 9:00 am on September 24, 2025: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

RandyMcMillan commented at 2:16 pm on September 24, 2025: contributor

@kallewoof - I am sure you can offer improvements on

https://github.com/bitcoin-core/gui/pull/896

Tested on top of this PR

Feature: Use different datadirs for different signets #29838

Update:

Code Coverage & Benchmarks

Reviews

Conflicts