Feature: Use different datadirs for different signets #29838

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	ekzyis
Concept ACK	laanwj, RandyMcMillan, tdb3, kristapsk, alfonsoromanz, edilmedeiros, pablomartin4btc, luisschwab, zaidmstrr, achow101, 1440000bytes
Stale ACK	carnhofdaki, yuvicc, jsarenik

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

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

<sub>Debug: https://github.com/bitcoin/bitcoin/runs/23622990227</sub>

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.

dev@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 
Temporary test directory at /mnt/tmp/test_runner_₿_🏃_20240413_214701
Remaining jobs: [feature_signet.py, tool_signet_miner.py --legacy-wallet, tool_signet_miner.py --descriptors]
1/3 - feature_signet.py failed, Duration: 60 s                                                                          

stdout:
2024-04-14T01:47:02.221000Z TestFramework (INFO): PRNG seed is: 2979480965586364484
2024-04-14T01:47:02.222000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240413_214701/feature_signet_2
2024-04-14T01:48:02.321000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 565, in start_nodes
    node.wait_for_rpc_connection()
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 314, in wait_for_rpc_connection
    self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 188, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Unable to connect to bitcoind after 60s

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
    self.setup()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 302, in setup
    self.setup_network()
  File "/home/dev/bitcoin/test/functional/feature_signet.py", line 43, in setup_network
    self.setup_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 418, in setup_nodes
    self.start_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 568, in start_nodes
    self.stop_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection
2024-04-14T01:48:02.376000Z TestFramework (INFO): Stopping nodes
[node 5] Cleaning up leftover process
[node 4] Cleaning up leftover process
[node 3] Cleaning up leftover process
[node 2] Cleaning up leftover process
[node 1] Cleaning up leftover process
[node 0] Cleaning up leftover process


stderr:
Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/feature_signet.py", line 85, in <module>
    SignetBasicTest().main()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 155, in main
    exit_code = self.shutdown()
                ^^^^^^^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 318, in shutdown
    self.stop_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection


2/3 - tool_signet_miner.py --descriptors failed, Duration: 60 s

stdout:
2024-04-14T01:47:02.220000Z TestFramework (INFO): PRNG seed is: 1471356017149591392
2024-04-14T01:47:02.221000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240413_214701/tool_signet_miner_0
2024-04-14T01:48:02.317000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 565, in start_nodes
    node.wait_for_rpc_connection()
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 314, in wait_for_rpc_connection
    self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 188, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Unable to connect to bitcoind after 60s

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
    self.setup()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 302, in setup
    self.setup_network()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 396, in setup_network
    self.setup_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 418, in setup_nodes
    self.start_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 568, in start_nodes
    self.stop_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection
2024-04-14T01:48:02.370000Z TestFramework (INFO): Stopping nodes
[node 0] Cleaning up leftover process


stderr:
Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/tool_signet_miner.py", line 65, in <module>
    SignetMinerTest().main()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 155, in main
    exit_code = self.shutdown()
                ^^^^^^^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 318, in shutdown
    self.stop_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection


Remaining jobs: [tool_signet_miner.py --legacy-wallet]
3/3 - tool_signet_miner.py --legacy-wallet failed, Duration: 61 s

stdout:
2024-04-14T01:47:02.211000Z TestFramework (INFO): PRNG seed is: 4742527368365497737
2024-04-14T01:47:02.212000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240413_214701/tool_signet_miner_1
2024-04-14T01:48:02.317000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 565, in start_nodes
    node.wait_for_rpc_connection()
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 314, in wait_for_rpc_connection
    self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 188, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Unable to connect to bitcoind after 60s

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
    self.setup()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 302, in setup
    self.setup_network()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 396, in setup_network
    self.setup_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 418, in setup_nodes
    self.start_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 568, in start_nodes
    self.stop_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection
2024-04-14T01:48:02.370000Z TestFramework (INFO): Stopping nodes
[node 0] Cleaning up leftover process


stderr:
Traceback (most recent call last):
  File "/home/dev/bitcoin/test/functional/tool_signet_miner.py", line 65, in <module>
    SignetMinerTest().main()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 155, in main
    exit_code = self.shutdown()
                ^^^^^^^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 318, in shutdown
    self.stop_nodes()
  File "/home/dev/bitcoin/test/functional/test_framework/test_framework.py", line 583, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 375, in stop_node
    self.stop(wait=wait)
    ^^^^^^^^^
  File "/home/dev/bitcoin/test/functional/test_framework/test_node.py", line 205, in __getattr__
    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
                                  ^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Error: no RPC connection



TEST                                 | STATUS    | DURATION

feature_signet.py                    | ✖ Failed  | 60 s
tool_signet_miner.py --descriptors   | ✖ Failed  | 60 s
tool_signet_miner.py --legacy-wallet | ✖ Failed  | 61 s

ALL                                  | ✖ Failed  | 181 s (accumulated) 
Runtime: 61 s

dev@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:

src/bitcoind -chain=signet_xxxx

[signet]
prune=1000

[signet_xxx]
signetchallenge=51
signetseednode=...

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

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

BrandonOdiwuor force-pushed on Jun 12, 2025

DrahtBot added the label Needs rebase on Sep 5, 2025

in src/common/args.cpp:293 in 6bcf96c4a4 outdated

 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 12: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

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

achow101 commented at 9:50 PM on November 17, 2025: member

Are you still working on this? This has needed rebase for a few months now.

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.

57c6762607

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

bf2d4bf988

BrandonOdiwuor force-pushed on Nov 30, 2025

DrahtBot removed the label Needs rebase on Nov 30, 2025

DrahtBot added the label CI failed on Nov 30, 2025

maflcko commented at 10:18 AM on December 5, 2025: member

Could turn into draft, while CI is red?

BrandonOdiwuor marked this as a draft on Dec 6, 2025

in test/functional/test_framework/test_framework.py:504 in bf2d4bf988

 499 | @@ -499,11 +500,21 @@ def bin_dir_from_version(version):
 500 |                  i,
 501 |                  get_datadir_path(self.options.tmpdir, i),
 502 |                  **init)
 503 | +            if self.chain == "signet":
 504 | +                test_node_i.signet_chain = self.get_signet_chain(args)

ekzyis commented at 2:00 AM on February 3, 2026:

I wonder if this code to set signet_chain should be moved into the TestNode constructor so signet_chain is always initialized and can be relied upon?

Or maybe even replace signet_chain with a chain_dir property like this:

     def chain_path(self) -> Path:
-        return self.datadir_path / self.get_chain()
+        return self.datadir_path / self.chain_dir
+
+ [@property](/bitcoin-bitcoin/contributor/property/)
+    def chain_dir(self) -> str:
+        if self.chain == "signet":
+            for arg in self.extra_args:
+                if arg.startswith('-signetchallenge'):
+                    signetchallenge = arg.split('=')[1]
+                    data_dir_suffix = hash160(bytes.fromhex(signetchallenge)).hex()[0:8]
+                    return self.datadir_path / f"signet_{data_dir_suffix}"
+        return self.chain

(see 70064ab in my fork)

I think self.chain_dir is more clear than self.chain vs self.get_chain().

ekzyis commented at 2:44 AM on February 3, 2026: none

tACK bf2d4bf988 I ran all the functional tests

As with rpc_url and get_auth_cookie, I think delete_cookie_file in test_node.py:287 also needs to be passed self.get_chain() now. I tried to find other cases by searching for variations of datadir_path but didn't find any.

I also agree with @RandyMcMillan: I don't think we need hash160 here.

I left a separate comment about self.get_chain(). I think the naming is confusing given that self.chain also exists. @BrandonOdiwuor, are you still planning to work on this? If not, I’d be happy to continue your work with proper credit in a new PR based on this one, unless there's a different preferred etiquette.

DrahtBot requested review from jsarenik on Feb 3, 2026

maflcko commented at 10:50 AM on February 9, 2026: member

Closing for now. This has had failing CI (or needed rebase) for almost half a year.

Let's move discussion back to the issue. Also, a new pull can be created, or this one can be re-opened. Though, without any progress there seems little value in keeping this open.

maflcko closed this on Feb 9, 2026