Feature: Use different datadirs for different signets #29838

BrandonOdiwuor · 2024-04-09T16:25:32Z

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 · 2024-04-09T16:25:35Z

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

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:

#29432 (Stratum v2 Template Provider (take 3) by Sjors)
#29032 (signet: fixing mining for OP_TRUE challenge 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.

DrahtBot · 2024-04-09T16:30:13Z

🚧 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}

1440000bytes · 2024-04-09T20:16:19Z

Concept ACK

laanwj · 2024-04-10T09:49:13Z

Concept ACK

ajtowns · 2024-04-09T16:36:49Z

src/common/args.cpp

+    if (!signet_challenge_str.empty()) {
+        std::vector<uint8_t> signet_challenge = ParseHex(signet_challenge_str);
+        const auto hash_160 = Hash160(signet_challenge);
+        return fs::PathFromString(base_data_dir + "-" + HexStr(hash_160));


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?

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.

I have updated the PR to use the first 8 bytes of the hash160 and replaced the - with _ as suggested

RandyMcMillan · 2024-04-10T13:09:20Z

Concept ACK

tdb3

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$

tdb3 · 2024-04-14T02:08:12Z

src/common/args.cpp

+    if (!signet_challenge_str.empty()) {
+        std::vector<uint8_t> signet_challenge = ParseHex(signet_challenge_str);
+        const auto hash_160 = Hash160(signet_challenge);
+        return fs::PathFromString(base_data_dir + "-" + HexStr(hash_160));


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.

kristapsk · 2024-04-14T06:14:55Z

Concept ACK

alfonsoromanz · 2024-04-15T16:13:34Z

Concept ACK

Sjors · 2024-04-18T06:54:27Z

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 · 2024-04-22T15:58:58Z

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

RandyMcMillan · 2024-04-22T16:09:09Z

I concur with @Sjors.

RandyMcMillan · 2024-04-22T16:13:30Z

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 · 2024-04-22T16:39:48Z

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.

edilmedeiros

Concept ACK.

Code looks good, but not tested yet.

Any way to add a functional test for this?

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.

BrandonOdiwuor · 2024-05-17T13:03:51Z

@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

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

carnhofdaki · 2024-05-27T08:48:05Z

Concept ACK

BrandonOdiwuor force-pushed the signet-datadirs branch from 64d3828 to 9e81922 Compare April 9, 2024 16:30

DrahtBot added the CI failed label Apr 9, 2024

DrahtBot mentioned this pull request Apr 10, 2024

ArgsManager: support subcommand-specific options #28802

Open

ajtowns reviewed Apr 10, 2024

View reviewed changes

tdb3 reviewed Apr 14, 2024

View reviewed changes

BrandonOdiwuor force-pushed the signet-datadirs branch from 9e81922 to 9f8b438 Compare May 15, 2024 16:12

edilmedeiros reviewed May 16, 2024

View reviewed changes

BrandonOdiwuor force-pushed the signet-datadirs branch from 9f8b438 to 8c3c484 Compare May 17, 2024 12:20

Use distinct data dirs for each signet

ac1ccd5

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.

BrandonOdiwuor force-pushed the signet-datadirs branch from 8c3c484 to ac1ccd5 Compare May 17, 2024 12:56

BrandonOdiwuor marked this pull request as ready for review May 17, 2024 12:57

BrandonOdiwuor requested a review from ajtowns May 17, 2024 12:57

BrandonOdiwuor requested a review from tdb3 May 17, 2024 13:04

DrahtBot removed the CI failed label May 17, 2024

test: Add test for diffrent signet data directories

50f4ca2

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

BrandonOdiwuor force-pushed the signet-datadirs branch from a0b1d77 to 50f4ca2 Compare May 22, 2024 09:53

DrahtBot mentioned this pull request May 24, 2024

signet: fixing mining for OP_TRUE challenge #29032

Open

BrandonOdiwuor requested a review from edilmedeiros May 29, 2024 14:11

BrandonOdiwuor mentioned this pull request May 30, 2024

Enhance signet chain configuration in bitcoin.conf #30203

Draft

DrahtBot mentioned this pull request May 30, 2024

Stratum v2 Template Provider (take 3) #29432

Draft

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Use different datadirs for different signets #29838

Feature: Use different datadirs for different signets #29838

BrandonOdiwuor commented Apr 9, 2024 •

edited

DrahtBot commented Apr 9, 2024 •

edited

DrahtBot commented Apr 9, 2024

1440000bytes commented Apr 9, 2024

laanwj commented Apr 10, 2024

ajtowns Apr 9, 2024

tdb3 Apr 14, 2024

BrandonOdiwuor May 17, 2024

RandyMcMillan commented Apr 10, 2024

tdb3 left a comment

tdb3 Apr 14, 2024

kristapsk commented Apr 14, 2024

alfonsoromanz commented Apr 15, 2024

Sjors commented Apr 18, 2024

RandyMcMillan commented Apr 22, 2024

RandyMcMillan commented Apr 22, 2024

RandyMcMillan commented Apr 22, 2024 •

edited

RandyMcMillan commented Apr 22, 2024 •

edited

edilmedeiros left a comment

BrandonOdiwuor commented May 17, 2024

carnhofdaki commented May 27, 2024

Feature: Use different datadirs for different signets #29838

Are you sure you want to change the base?

Feature: Use different datadirs for different signets #29838

Conversation

BrandonOdiwuor commented Apr 9, 2024 • edited

Update:

DrahtBot commented Apr 9, 2024 • edited

Code Coverage

Reviews

Conflicts

DrahtBot commented Apr 9, 2024

1440000bytes commented Apr 9, 2024

laanwj commented Apr 10, 2024

ajtowns Apr 9, 2024

Choose a reason for hiding this comment

tdb3 Apr 14, 2024

Choose a reason for hiding this comment

BrandonOdiwuor May 17, 2024

Choose a reason for hiding this comment

RandyMcMillan commented Apr 10, 2024

tdb3 left a comment

Choose a reason for hiding this comment

tdb3 Apr 14, 2024

Choose a reason for hiding this comment

kristapsk commented Apr 14, 2024

alfonsoromanz commented Apr 15, 2024

Sjors commented Apr 18, 2024

RandyMcMillan commented Apr 22, 2024

RandyMcMillan commented Apr 22, 2024

RandyMcMillan commented Apr 22, 2024 • edited

RandyMcMillan commented Apr 22, 2024 • edited

edilmedeiros left a comment

Choose a reason for hiding this comment

BrandonOdiwuor commented May 17, 2024

carnhofdaki commented May 27, 2024

BrandonOdiwuor commented Apr 9, 2024 •

edited

DrahtBot commented Apr 9, 2024 •

edited

RandyMcMillan commented Apr 22, 2024 •

edited

RandyMcMillan commented Apr 22, 2024 •

edited