Signet challenges are currently only available via getblocktemplate RPC.
getblockchaininfo and getmininginfo both provide inadequate information to distinguish signets. Since these are the RPCs used to determine the current network, they should also provide the signet challenge for signets.
Test coverage is included in test/functional/feature_signet.py.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31531.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | Sjors, zaidmstrr, sipa |
| Concept ACK | pinheadmz, i-am-yuvi, BrandonOdiwuor, kallewoof |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
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.
"rules"
30 self.setup_clean_chain = True
31- shared_args1 = ["-signetchallenge=51"] # OP_TRUE
32+
33+ self.signet_challenge1 = '51' # OP_TRUE
34+ # signet 2 uses the default challenge
35+ self.signet_challenge2 = '512103ad5e0edad18cb1f0fc0d28a3d4f1f3e445640337489abb10404f2d1e086be430210359ef5021964fe22d6f8e05b2463c9540ce96883fe3b278760f048f5189f2e6c452ae'
SIGNET_DEFAULT_CHALLENGE = ... at the top.
28 self.chain = "signet"
29 self.num_nodes = 6
30 self.setup_clean_chain = True
31- shared_args1 = ["-signetchallenge=51"] # OP_TRUE
32+
33+ self.signet_challenge1 = '51' # OP_TRUE
self.signet_challenges an array, which is a more common pattern in the tests.
95+ assert_equal(mining_info3['chain'], 'signet')
96+ assert_equal(mining_info3['signet_challenge'], self.signet_challenge3)
97
98 self.generate(self.nodes[0], 1, sync_fun=self.no_op)
99
100+
@A-Manning thanks, can you squash the commits?
cc @maflcko
50+
51+ self.signets = [
52+ SignetParams(challenge='51'), # OP_TRUE
53+ SignetParams(), # default challenge
54+ # default challenge as a 2-of-2, which means it should fail
55+ SignetParams(challenge='522103ad5e0edad18cb1f0fc0d28a3d4f1f3e445640337489abb10404f2d1e086be430210359ef5021964fe22d6f8e05b2463c9540ce96883fe3b278760f048f5189f2e6c452ae')
57
58 self.extra_args = [
59- shared_args1, shared_args1,
60- shared_args2, shared_args2,
61- shared_args3, shared_args3,
62+ self.signets[0].shared_args, self.signets[0].shared_args,
Makes no sense to me. I read through the entire thread in #18267 where this was added, and it looks like I put it there right at the end before it was merged, so it may have slipped under the radar.
Anyway, my suggestion: remove the duplication. Try the tests. If the tests fail, we know this at least has a purpose.
79- assert_equal(mining_info['chain'], 'signet')
80- assert 'currentblocktx' not in mining_info
81- assert 'currentblockweight' not in mining_info
82- assert_equal(mining_info['networkhashps'], Decimal('0'))
83- assert_equal(mining_info['pooledtx'], 0)
84+ # mining info for node on signet 0
check_getmininginfo helper?
78- assert_equal(mining_info['chain'], 'signet')
79- assert 'currentblocktx' not in mining_info
80- assert 'currentblockweight' not in mining_info
81- assert_equal(mining_info['networkhashps'], Decimal('0'))
82- assert_equal(mining_info['pooledtx'], 0)
83+ def check_getmininginfo(node_idx, signet_idx):
Thanks for adding the helper. But now it’s inconsistent with the check_getblockchaininfo helper. No strong preference for which style to use.
Maybe make it def check_getmininginfo(self, node_idx, signet_idx): and check_getblockchaininfo(self, node_idx, signet_idx) and put them above run_test.
run_test in ecaa786cc103cf7cc63ae899ec13d81a54e2fd1e.
check_getmininginfo makes specific assertions on values for fields such as blocks, which may change during a test. It is best to keep it as localized as possible, since it is unlikely to be broadly useful.
Concept ACK ecaa786cc103cf7cc63ae899ec13d81a54e2fd1e
Both the RPC seems to be giving enough information about the signet challenges!! Thanks @A-Manning for this!!
I have used custom signet(OP_TRUE) on Linux x86_64:
getblockchaininfo:
0{
1 "chain": "signet",
2 "blocks": 0,
3 "headers": 0,
4 "bestblockhash": "00000008819873e925422c1ff0f99f7cc9bbb232af63a077a480a3633bee1ef6",
5 "difficulty": 0.001126515290698186,
6 "time": 1598918400,
7 "mediantime": 1598918400,
8 "verificationprogress": 1,
9 "initialblockdownload": true,
10 "chainwork": "000000000000000000000000000000000000000000000000000000000049d414",
11 "size_on_disk": 293,
12 "pruned": false,
13 "signet_challenge": "512102ee856c56a5aaadd1656f849bafa4c9dacc86a2878fe546c6189185f842ae2c1851ae",
14 "warnings": [
15 "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
16 ]
17}
getmininginfo:
0{
1 "blocks": 0,
2 "difficulty": 0.001126515290698186,
3 "networkhashps": 0,
4 "pooledtx": 0,
5 "chain": "signet",
6 "signet_challenge": "512102ee856c56a5aaadd1656f849bafa4c9dacc86a2878fe546c6189185f842ae2c1851ae",
7 "warnings": [
8 "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
9 ]
10}
The test also seems to pass:
0./build/test/functional/test_runner.py feature_signet.py
1Temporary test directory at /tmp/test_runner_₿_🏃_20241221_004806
2Remaining jobs: [feature_signet.py]
31/1 - feature_signet.py passed, Duration: 1 s
4
5TEST | STATUS | DURATION
6
7feature_signet.py | ✓ Passed | 1 s
8
9ALL | ✓ Passed | 1 s (accumulated)
10Runtime: 1 s
1306@@ -1307,6 +1307,7 @@ RPCHelpMan getblockchaininfo()
1307 {RPCResult::Type::NUM, "pruneheight", /*optional=*/true, "height of the last block pruned, plus one (only present if pruning is enabled)"},
1308 {RPCResult::Type::BOOL, "automatic_pruning", /*optional=*/true, "whether automatic pruning is enabled (only present if pruning is enabled)"},
1309 {RPCResult::Type::NUM, "prune_target_size", /*optional=*/true, "the target size used by pruning (only present if automatic pruning is enabled)"},
1310+ {RPCResult::Type::STR_HEX, "signet_challenge", /*optional=*/true, "the block challenge (aka. block script), in hexadecimal (only present if the current network is a signet)"},
-signetchallenge, perhaps use that here as well.
signet_challenge is consistent with getblocktemplate
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/mining.cpp#L660
argument_name whereas for startup arguments we use -argumentname.
423@@ -424,6 +424,7 @@ static RPCHelpMan getmininginfo()
424 {RPCResult::Type::NUM, "networkhashps", "The network hashes per second"},
425 {RPCResult::Type::NUM, "pooledtx", "The size of the mempool"},
426 {RPCResult::Type::STR, "chain", "current network name (" LIST_CHAIN_NAMES ")"},
427+ {RPCResult::Type::STR_HEX, "signet_challenge", /*optional=*/true, "The block challenge (aka. block script), in hexadecimal (only present if the current network is a signet)"},
-signetchallenge.
signet_challenge is consistent with getblocktemplate
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/mining.cpp#L660
getblockchaininfo and getmininginfo RPC includes signet_challenge field. I also manually reviewed the new code for any errors and missing statements.
The updated test test/functional/feature_signet.py also works fine.
