argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)

pablomartin4btc commented at 10:43 pm on October 4, 2025: member

This PR simplifies the argument parsing logic and resolves practical issues for bitcoin-cli and similar binaries (e.g. bitcoin-wallet) making their usage more consistent and predictable, enabling valid patterns such as mixing commands with wallet- or RPC-specific options—without relying on argument ordering quirks.

Previously, ArgsManager stopped interpreting options once a non-option argument was encountered (non-GNU parsing). For example:
```
  bitcoind -a -b param -c
```
would ignore -c because it followed the non-option argument param. In some cases ‘-c’ is interpreted as an argument for ‘param’, producing unexpected behavior (eg empty arrays for listtransactions command).
This PR changes the behavior so that options following non-option argument are recognized and processed (GNU parsing). This is needed for workflows for tools like CLI where command arguments may be followed by wallet- or RPC-specific options, e.g.:
```
  bitcoin-cli -rpcwallet="" listtransactions -rpcwallet=mywallet

  bitcoin-cli getaddressinfo <address> -rpcwallet=mywallet
```
(other binaries could also benefit from this change, e.g. bitcoin-wallet)
```
  bitcoin-wallet create -wallet=newname
```
Invalid options passed after non-options will be treated as RPC arguments that begin with a - character (so no behavioural change against master), e.g.:
```
  bitcoin-cli -regtest -datadir=/tmp/btc createwallet -mywallet
  {
      "name": "-mywallet"
  }
```

0./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/btc getaddressinfo bcrt1qrnz5xlcwz5d85n3fag6v0vc4lryjc90jhqtpsh -rpcwallet=nonExistent
1error message:
2getaddressinfo "address"
3...

0./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getaddressinfo bcrt1qrnz5xlcwz5d85n3fag6v0vc4lryjc90jhqtpsh -rpcwallet=nonExistent
1error code: -18
2error message:
3Requested wallet does not exist or is not loaded

0./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/btc listtransactions -rpcwallet=nonExistent
1[
2]

0./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc listtransactions -rpcwallet=nonExistent
1error code: -18
2error message:
3Requested wallet does not exist or is not loaded

0./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/btc listtransactions
1error code: -19
2error message:
3Multiple wallets are loaded. Please select which wallet to use by requesting the RPC through the /wallet/<walletname> URI path. Or for the CLI, specify the "-rpcwallet=<walletname>" option before the command (run "bitcoin-cli -h" for help or "bitcoin-cli listwallets" to see which wallets are currently loaded).

0./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/btc listtransactions -rpcwallet=""
1error code: -19
2error message:
3Multiple wallets are loaded. Please select which wallet to use by requesting the RPC through the /wallet/<walletname> URI path. Or for the CLI, specify the "-rpcwallet=<walletname>" option before the command (run "bitcoin-cli -h" for help or "bitcoin-cli listwallets" to see which wallets are currently loaded).

 0/build/bin/bitcoin-cli -regtest -datadir=/tmp/btc listtransactions -rpcwallet=""
 1[
 2  {
 3    "address": "bcrt1q5pqkrlfrp9rvsg43sxxagrees7r7ul6uyskhs4",
 4    "parent_descs": [
 5      "wpkh([76a11ab0/84h/1h/0h]tpubDCDCuyC1Rtm1weZ5kfBYtdchcmqa2t8db7M68wsPHbWhUrYnzFTrHu7ufVVRydt3FdWQy8iNmKbZYbgpWWcEKPAj896x53UbS6i2xRD1qct/0/*)#kuff3m9t"
 6    ],
 7    "category": "immature",
 8    "amount": 50.00000000,
 9    "label": "",
10...

0./build_master/bin/bitcoin-wallet -regtest -datadir=/tmp/btc create
1Wallet name must be provided when creating a new wallet.

(-wallet option has to be passed before the create command in order to work)

0./build_master/bin/bitcoin-wallet -regtest -datadir=/tmp/btc create -wallet=newWalletName
1Error: Additional arguments provided (-wallet=newWalletName). Methods do not take arguments. Please refer to `-help`.

 0./build/bin/bitcoin-wallet -regtest -datadir=/tmp/btc create -wallet=newWallet
 1Topping up keypool...
 2Wallet info
 3===========
 4Name: newWallet
 5Format: sqlite
 6Descriptors: yes
 7Encrypted: no
 8HD (hd seed available): yes
 9Keypool Size: 8000
10Transactions: 0
11Address Book: 0

-Notes:

Backwards compatibility: This PR is backward compatible in the sense that it allows new valid invocations that were previously rejected (failed or somehow ignored). In master, any valid option following a command is treated as a positional argument. This can lead to unexpected RPC errors or help messages due to mismatched argument types. With this PR, those cases are parsed as proper options when applicable.
bitcoin-cli continues to accept RPC arguments that begin with a - character as currently in master (check createwallet example in “What’s change in ArgsManager” section above).
bitcoin-cli could distinguish between options that begin with single and double dashes, and treat options after the RPC method name that begin with double dashes as RPC named parameters (check example in 4th commit message body).

argsman, refactor: Factor out ProcessOptionKey from ParseParameters

- Centralizes normalization of option keys in `ProcessOptionKey`.
  This will help in next commit to reuse this function avoiding code
  duplication.
- Improves readability and makes behavior easier to reason about.

Functional and unit tests confirm no behavior change.

15039d6c64

DrahtBot commented at 10:43 pm on October 4, 2025: 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/33540.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	w0xlt, ryanofsky

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:

#33192 (refactor: unify container presence checks by l0rinc)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
#17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

“)are” -> “) are” [missing space between words makes the phrase harder to read]
“startig” -> “starting” [spelling error]

drahtbot_id_5_m

katesalazar commented at 9:10 am on October 5, 2025: contributor

Such good news. Hard to believe this PR doesn’t reference an existing user request.

w0xlt commented at 1:27 pm on October 5, 2025: contributor

Concept ACK

pinheadmz commented at 2:08 pm on October 5, 2025: member

Such good news. Hard to believe this PR doesn’t reference an existing user request.

@katesalazar

This comment does not contribute to the technical discussion. Please put in more effort in the future or you may be banned.

ryanofsky commented at 1:02 pm on October 6, 2025: contributor

Haven’t looked at code yet, so forgive if these are dumb questions, but could be nice if PR description addressed these:

Is there a way to pass bitcoin-cli RPC arguments that begin with a - character? Can you escape these arguments with -- for example?
What’s the PR’s goal for backwards compatibility? Is the PR only allowing command lines that were disallowed before, and therefore fully backwards compatible? Or are there some command lines which used to be accepted which will now be interpreted differently?
What are “numeric command arguments” referenced in the PR description? Maybe give an example. I wasn’t aware that numbers were treated differently than other characters for separating options arguments, and I would expect interpretation of numeric values to happen at a later phase than the phase distinguishing options from non-option arguments.

ryanofsky commented at 1:09 pm on October 6, 2025: contributor

Concept ACK on the idea. I think it probably makes sense to allow options arguments to follow non-option arguments even this change is not strictly backwards compatible, as long as there is some way to escape non-option arguments beginning with -, such as with a -- separator.

ryanofsky commented at 2:39 pm on October 6, 2025: contributor

Assuming we are ok with losing some backwards compatibility by starting to treating arguments that begin with - as options instead of non-options when they follow arguments that don’t begin with -, there could be other interesting ways to extend this PR.

For example, bitcoin-cli could distinguish between options that begin with single and double dashes, and treat options after the RPC method name that begin with double dashes as RPC named parameters. In other words, accept:

0bitcoin-cli createwallet mywallet --load_on_startup=1

as an equivalent to:

0bitcoin-cli -named createwallet mywallet load_on_startup=1

This seems like it could provide a more convenient way of using named RPC parameters.

pablomartin4btc commented at 5:11 pm on October 12, 2025: member

* Is there a way to pass `bitcoin-cli` RPC arguments that begin with a `-` character?

At the moment, please correct me if I’m wrong, there’s no or I haven’t seen an RPC that would receive a valid argument that starts with - (but I understand it could be in the future), and there’s no test for it otherwise I think the PR would have caught it.

Testing the scenario you mentioned in master would work like for example in:

0./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/btc createwallet -xyz
1{
2  "name": "-xyz"
3}

On this PR would fail because it would be an invalid option (from the ArgsMan):

0./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc createwallet -xyz
1Error parsing command line arguments: Invalid parameter -xyz

I think perhaps the scenario you mention is a valid one since any a string argument for an RPC could start with -, so only the “invalid” options (like -xyz in the previous example) would be leaved or passed to the RPC, which makes sense.

The change would look like this, and we can drop the 2nd commit (bringing back alive ParsePrechecks() and ParseDouble()), which won’t make sense anymore (I explain why I brought them back in your last question on this thread since you’ve asked as well). The change already passes the CIs - CentOS CI failure is unrelated.

So even the following case would be successful:

0./build_ryan/bin/bitcoin-cli createwallet -regtest -zyx -datadir=/tmp/btc
1{
2  "name": "-zyx"
3}

Can you escape these arguments with -- for example?

Not on this PR. I see you think we could extend it with the usage of -- in a later comment. I wanted to make a minimum change to allow this feature of GNU-style parsing options so I’d prefer to add that on a separate PR if that’s necessary.

* What's the PR's goal for backwards compatibility? Is the PR only allowing command lines that were disallowed before, and therefore fully backwards compatible? Or are there some command lines which used to be accepted which will now be interpreted differently?

The difference now will be that valid options starting with - will be considered or accepted while before or in master these scenarios will be considered that an extra argument is passed to the command and sometimes would be treated as a valid string (when the arg is of such type for the command) returning something empty if it had to match certain condition or would fail because (1) it’s a different type like a JSON error, (2) the RPC had received extra arguments resulting in the command help or (3) too many args for CLI commands. So in terms of backwards compatibility, it would consider valid scenarios that were invalid before, and if someone expected an error, if the option is valid it will be taken into consideration and the execution will be successful instead.

* What are "numeric command arguments" referenced in the PR description? Maybe give an example. I wasn't aware that numbers were treated differently than other characters for separating options arguments, and I would expect interpretation of numeric values to happen at a later phase than the phase distinguishing options from non-option arguments.

Yeah the interpretation of numeric values come later on the command execution, not during the parsing in the Argsman, but when I made the change, there was a a test that was failing:

https://github.com/bitcoin/bitcoin/blob/d2987102dd13f965c7a0bf1d5b8ee15bcf025398/test/functional/wallet_transactiontime_rescan.py#L182

So then I checked that there is another RPC case (getnetworkhashps) which has a default RPCArg::Type::NUM with RPCArg::Default{-1}:

https://github.com/bitcoin/bitcoin/blob/d2987102dd13f965c7a0bf1d5b8ee15bcf025398/src/rpc/mining.cpp#L120

That’s the reason of the 2nd commit, reintroducing doubles parsing, but since your first question on this thread, I’m considering this change to be pushed (added a test scenario where commands could receive args starting with -) so that 2nd commit can be dropped as it’s no longer needed (that’s done in the change mentioned in the previous sentence).

argsman, test: Allow options after non-option arguments

Previously, ArgsManager stopped interpreting options once a
non-option argument was encountered (non-GNU parsing).

For example:

    bitcoind -a -b param -c

would ignore `-c` because it followed the non-option argument
`param`. In cases where some 'param' command receives parameters,
'-c' is interpreted as one of them producing unexpected behavior
(e.g. empty arrays for listtransactions command).

This commit changes the behavior so that options following
non-option argument are recognized and processed (GNU parsing).
This is needed for workflows for tools like CLI where command
arguments may be followed by wallet- or RPC-specific options, e.g.:

    bitcoin-cli listtransactions -rpcwallet=mywallet

    bitcoin-cli getaddressinfo <address> -rpcwallet=mywallet

    bitcoin-wallet create -wallet=newname

Also, invalid options passed after non-options will be treated
as RPC arguments that begin with a '-' (dash) character (so no
behavioural change against master), e.g.:

    bitcoin-cli -regtest -datadir=/tmp/btc createwallet -mywallet
    {
        "name": "-mywallet"
    }

02ee492a38

cli, refactor, test: Use parsed command args from ArgsManager

Refactor bitcoin-cli to use command arguments provided by ArgsManager
instead of manually slicing argv.

- Introduce GetCommandArgs() helper.
- Remove local argv/argc skipping logic.
- Simplify CommandLineRPC argument collection.

This improves consistency in bitcoin-cli, avoiding duplicating parsing
logic.

Added a new functional test passing an option after a non-option
argument (GNU-style).

No behavior change intended.

3a4d83106e

pablomartin4btc force-pushed on Oct 13, 2025

pablomartin4btc commented at 3:37 am on October 13, 2025: member

-Updates:

Addressed @ryanofsky’s feedback:
- bitcoin-cli accepts RPC arguments that begin with a - character;
- added a note in description about backwards compatibility;
- bitcoin-cli treat options after the RPC method name that begin with double dashes as RPC named parameters;
Dropped “Reintroduce ParsePrechecks and ParseDouble” commit;
Updated description accordingly;
CentOS CI failure is unrelated (“no space left on device” -> it seems #33293);
Pending to fix Tidy CI failure (“recursive call chain”).

argsman, test: Double dashes as RPC named params

Treat options that begin wih double dashes as RPC named
params:

bitcoin-cli createwallet mywallet --load_on_startup=true

as an equivalent to:

bitcoin-cli -named createwallet mywallet load_on_startup=true

a101bbb61f