#36689 cli: Parse -signetchallenge like bitcoind

ReviewabilityReviewsAgreementSize
#36689 ekzyis cli: Parse -signetchallenge like bitcoindReady0Positive32 + 21 tests
  • Validate -signetchallenge arguments immediately in bitcoin-cli rather than failing later during RPC auth
  • Give clear error messages when custom signet challenges contain invalid hex or duplicate values
  • Ready to review
  • Concept approval without stated reasons (Sjors)
  • Author addressed suggestions to add a test case and drop a helper rename (ValeraFinebits, Sjors)
  • 32 lines added or modified outside tests
  • 21 lines added or modified in tests
  • 19 lines removed in total
  • 4 files, 2 commits
Uncategorized15 PRs, unranked
3 rejected
  • Kernel (libbitcoinkernel): not a member. Touches src/kernel/chainparams.cpp and src/kernel/signet.h only for a single-line mechanical rename of GetSignetMessageStart.
  • Tools and scripts: not a member. PR has the 'Scripts and tools' label and targets bitcoin-cli argument parsing, but the category explicitly excludes the RPC client.
  • Validation: not a member. Touches src/chainparamsbase.cpp and src/chainparams.cpp without altering block or transaction validity rules.
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Extracts `-signetchallenge` parsing and hex validation into a shared `ReadSignetChallengeArg()` helper in `chainparamsbase`. Calls this helper from both `GetSignetDataDir()` and `ReadSigNetArgs()`, ensuring `bitcoin-cli` immediately rejects invalid hex or duplicate challenge arguments instead of failing later during RPC authentication.

Problem: When invalid hex or duplicate values are supplied to `-signetchallenge` in `bitcoin-cli`, the CLI tool does not validate the argument during data directory resolution. It silently falls back to the default signet datadir and eventually fails with an unhelpful authorization or missing RPC cookie error rather than reporting the malformed argument.

Evidence of importance:

  • PR description demonstrates the error discrepancy: `bitcoind -signet -signetchallenge=z` reports `Error: -signetchallenge must be hex, not 'z'`, while `bitcoin-cli` only fails later with `error: Authorization failed... Is your -signetchallenge correct for custom signets?`
  • Addressed review feedback from merged PR #34566: 'Addresses https://github.com/bitcoin/bitcoin/pull/34566#discussion_r3668123074'

Risk of a mistake: low (tooling, tests). A bug in argument parsing would prevent custom signet nodes or CLI commands from reading their challenge argument or finding the correct datadir.

Changed since the previous assessment: Dropped the helper function casing rename to `SigNet`, retaining `ReadSignetChallengeArg` with standard `Signet` casing.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: re-review after author updated commits and dropped rename 2026-10-30

Agreement

Positive

support without stated reasons, no open objection (Sjors)

Thread notes: Added Concept ACK from Sjors and their suggestion to drop the rename commit, which was fixed when the author dropped it.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ValeraFinebits (none)suggestion, correctnessAdd a test setting signetchallenge=51 in the [signet] config section and passing -signetchallenge=51 on the command line to assert the multiple-values errorfixed · fix pushed · author implemented the suggested test in commit cb7bb77a10answeredyes2026-10-29: '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.'
Status from: 2026-10-29: ekzyis: 'I replaced the CLI test which passes the same invalid value twice (`-signetchallenge=abc`) with a test that uses `signet'
Sjors (member)suggestion, scopeDrop the rename commit to avoid bike-sheddingfixed · fix pushed · author dropped the rename commit in an updateansweredyes2026-10-30: 'I suggest dropping the rename in 38998ad4104b1dca4db4dd039b0ddb5e7ea766fe. It's going to invite bike-shedding (and personally I prefer Signet over SigNet).'
Status from: 2026-10-30: ekzyis: 'Dropped the rename.'

Support:

  • Sjors (member) (Concept ACK, 2026-10-30) [read]: (no reason given) [not substantive]

Participants:

  • ValeraFinebits (none, neutral, 1 statements 2026-10-29 to 2026-10-29): suggested an optional test case covering config and CLI multiple-values handling
  • Sjors (member, support, 1 statements 2026-10-30 to 2026-10-30): Concept ACK; suggested dropping the rename commit and splitting the refactor

Files

21 lines under test/bench/ci.

  • src/chainparamsbase.cpp +22/-8
  • test/functional/feature_signet.py +21/-0
  • src/chainparams.cpp +1/-11
  • src/chainparamsbase.h +9/-0

Card

The short text the ranking pass compares across PRs.

Unifies -signetchallenge argument validation between bitcoind and bitcoin-cli by moving hex checking into a shared helper function. Currently, passing invalid hex or multiple challenge values to bitcoin-cli fails late with an ambiguous RPC authorization error instead of pinpointing the bad input. With this change, bitcoin-cli throws an immediate error on malformed or duplicate challenge arguments matching the error output of bitcoind.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:16, confidence high, input hash 0afebf72eb96da42
thread read through 2026-10-30 (5 statements), head 54fe0ec5d8
labels: Scripts and tools