cli: Parse -signetchallenge like bitcoind #36689

pull ekzyis wants to merge 2 commits into bitcoin:master from ekzyis:bitcoin-cli-signetchallenge-validation changing 6 files +57 −23

ekzyis commented at 7:14 PM on October 28, 2026: contributor
bitcoind returns an useful error when invalid hex is passed to -signetchallenge:
```
$ bitcoind -signet -signetchallenge=z
Error: -signetchallenge must be hex, not 'z'
```
The error returned by bitcoin-cli only indicates that the RPC cookie wasn't found. It doesn't hint at invalid hex:
```
$ bitcoin-cli -signet -signetchallenge=z getblockcount
error: Authorization failed: ...
Is your -signetchallenge correct for custom signets?
```
To have consistent validation of signet args in bitcoind and bitcoin-cli, we now parse -signetchallenge in a new ReadSigNetChallengeArg() function which is called in ReadSigNetDir() and ReadSigNetArgs().

Addresses #34566 (review)

I didn't add a release note because I don't think this change of an error message warrants one.
refactor: Rename signet helpers to SigNet casing 38998ad410
DrahtBot commented at 7:15 PM on October 28, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36689
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36689.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36689#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

Concept ACK Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
ekzyis renamed this:
~~Bitcoin cli signetchallenge validation~~
cli: Parse -signetchallenge like bitcoind
on Oct 28, 2026
DrahtBot added the label Scripts and tools on Oct 28, 2026
ValeraFinebits commented at 4:58 PM on October 29, 2026: none

Optional test suggestion: set signetchallenge=51 in the [signet] config section and also pass -signetchallenge=51 on the command line, then assert the multiple-values error.

This would cover the GetArg() -> GetArgs() change across configuration sources, using valid, identical values. Running this case with RPC unavailable would also verify that the diagnostic does not depend on a running node.

Type	Reviewers
Concept ACK	Sjors

cli: Parse -signetchallenge like bitcoind

bitcoind returns an useful error when invalid hex is passed to
-signetchallenge:

```
$ bitcoind -signet -signetchallenge=z
Error: -signetchallenge must be hex, not 'z'
```

The error returned by bitcoin-cli only indicates that the RPC cookie
wasn't found. It doesn't hint at invalid hex:

```
$ bitcoin-cli -signet -signetchallenge=z getblockcount
error: Authorization failed: ...
Is your -signetchallenge correct for custom signets?
```

To have consistent validation of signet args in bitcoind and
bitcoin-cli, we now parse -signetchallenge in a new
ReadSigNetChallengeArg() function which is called in ReadSigNetDir() and
ReadSigNetArgs().

cb7bb77a10

ekzyis force-pushed on Oct 29, 2026
ekzyis commented at 6:41 PM on October 29, 2026: contributor

Optional test suggestion: set signetchallenge=51 in the [signet] config section and also pass -signetchallenge=51 on the command line, then assert the multiple-values error.

This would cover the GetArg() -> GetArgs() change across configuration sources, using valid, identical values. Running this case with RPC unavailable would also verify that the diagnostic does not depend on a running node.

Thanks! I replaced the CLI test which passes the same invalid value twice (-signetchallenge=abc) with a test that uses signetchallenge=51 from the config and passes the same valid value (-signetchallenge=51) in cb7bb77a10, as you suggested.
in test/functional/feature_signet.py:175 in cb7bb77a10
```
 170 | +        conf = conf_path.read_text()
 171 | +        conf_path.write_text(conf + "signetchallenge=51\n")
 172 | +        # command line and config file values are not merged, but rejected as multiple values
 173 | +        assert_raises_process_error(1, "-signetchallenge cannot be multiple values.", node.cli("-signetchallenge=51").getblockcount)
 174 | +        # undo changes to bitcoin.conf
 175 | +        conf_path.write_text(conf)
```
ekzyis commented at 7:05 PM on October 29, 2026:

I checked if we're using patterns like here which could be refactored with context managers, but there's only one similar case I've found at interface_bitcoin_cli.py:205 where we comment out rpcport in the config before running some tests and restoring it.

In the other cases, we're either restarting the node with the new changes (so undo is undesired) or node.replace_in_config() is more ergonomic.

So in case someone has more context than me to still consider this a good idea: This and similar patterns could be refactored with a context manager like this:

with extra_config(node, "signetchallenge=51"): assert_raises_process_error(1, "-signetchallenge cannot be multiple values.", node.cli("-signetchallenge=51").getblockcount)
Sjors commented at 8:14 AM on October 30, 2026: member

Concept ACK

Consider splitting cb7bb77a10fbccc16b90310a25d3f9f2d348bf9f into a pure refactor to extract the helper(s) you need, followed by the change itself.

I suggest dropping the rename in 38998ad4104b1dca4db4dd039b0ddb5e7ea766fe. It's going to invite bike-shedding (and personally I prefer Signet over SigNet).

Contributors

Labels

Linked (view graph)

#34566 feature: Use different datadirs for different signets

cli: Parse -signetchallenge like bitcoind #36689

External sites

Reviews