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

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.

<details> <summary>What's changed in ArgsManager that allows the above.</summary>

• 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"
    }

</details>

<details><summary>Before and after outputs running RPC commands from <code>CLI</code>/ <code>bitcoin-cli</code>.</summary><ul> <li><details><summary>wallet commands using <code>CLI</code>:</summary><ul> <li><details><summary><code>getaddressinfo</code></summary><ul> <li>before/ <code>master</code>

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

</li> <li>after

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

</li> </ul></details></li> <li><details><summary><code>listtransactions</code></summary><ul> <li>before/ <code>master</code>

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

</li> <li>after

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

</li> </ul></details></li> <li><details><summary>any command that needs an option to be specifed, it can be added at the end directly instead of before the command</summary><ul> <li>before/ <code>master</code>

./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/btc listtransactions
error code: -19
error message:
Multiple 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).

./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/btc listtransactions -rpcwallet=""
error code: -19
error message:
Multiple 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).

</li> <li>after

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

</li> </ul></details></li> </ul></details></li> <li><details><summary>other binaries could benefit from it, e.g. <code>bitcoin-wallet:</code></summary><ul> <li><details><summary><code>create</code> command</summary><ul> <li>before/ <code>master</code>

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

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

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

</li> <li>after

./build/bin/bitcoin-wallet -regtest -datadir=/tmp/btc create -wallet=newWallet
Topping up keypool...
Wallet info
===========
Name: newWallet
Format: sqlite
Descriptors: yes
Encrypted: no
HD (hd seed available): yes
Keypool Size: 8000
Transactions: 0
Address Book: 0

</li> </ul></details></li> </ul></details></li> </ul> <!-- End --> </details>

-<ins>Notes</ins>:

• <ins>Backwards compatibility</ins>: 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).

• Refactored both ArgsManager::ProcessOptionKey (new function added in this PR) and ArgsManager::ParseParameters in separated commits making the code clearer and the PR much easier to follow and review.

• On Windows, command-specific options using the '/' prefix are now handled correctly when specified after a command.

• Updated bitcoin-tx to consume the command arguments already parsed by ArgsManager, avoiding re-processing raw argv in CommandLineRawTx().

Additionally, this PR includes release notes documenting the GNU-style option parsing behaviour and the distinction between command-line options and RPC named parameters.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33540.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

LLM Linter (✨ experimental)

Possible typos and grammar issues:

• command-options can be passsed before or after commands -> passed [misspelled word]
• Test options before and after using an RPC parameter startig with '-' -> starting [misspelled word]

^{2026-06-07 18:38:02}

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

Concept ACK

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.

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.

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.

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:

bitcoin-cli createwallet mywallet --load_on_startup=1

as an equivalent to:

bitcoin-cli -named createwallet mywallet load_on_startup=1

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

* 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:

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

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

./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc createwallet -xyz
Error 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:

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

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

-<ins>Updates</ins>:

• 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").

-<ins>Updates</ins>:

• Fixed previous Tidy CI failure ("recursive call chain"), but still working on a refactor of ArgsManager::ProcessOptionKey.

This command works on master but not in this PR: ./build/bin/bitcoin-cli --datadir=/tmp/btc1 --signet getblockhash 1000

Could it be related to GetCommandArgs()?

Restructuring ProcessOptionKey as below seems to solve the problem of ./build/bin/bitcoin-cli --datadir=/tmp/btc1 --signet getblockhash 1000.

That seems to fix the Method not found regression while preserving the GNU-style options after the command and the new double-dash-named-params feature.

bool ArgsManager::ProcessOptionKey(std::string& key, std::optional<std::string>& val, std::string& error, const bool found_after_non_option) {
    bool double_dash{false};
    const std::optional<std::string> original_val{val};
    std::string original_input{key};
    if (val) original_input += "=" + *val;

    // Normalize leading dashes
    if (key.length() > 1 && key[1] == '-') {
        key.erase(0, 1);   // `--foo` -> `-foo`
        double_dash = true;
    }
    key.erase(0, 1);       // `-foo`  -> `foo`

    KeyInfo keyinfo = InterpretKey(key);
    std::optional<unsigned int> flags = GetArgFlags('-' + keyinfo.name);
    const bool known_option = flags.has_value() && keyinfo.section.empty();

    // Handle the special "named RPC" case early ---
    if (double_dash && found_after_non_option && !known_option) {
        // Try to map this to `-named`, if supported by the binary.
        val.reset();
        KeyInfo named_keyinfo = InterpretKey("named");
        std::optional<unsigned int> named_flags = GetArgFlags('-' + named_keyinfo.name);

        if (named_flags && named_keyinfo.section.empty()) {
            // Binary supports -named: enable it and append the stripped parameter.
            std::optional<common::SettingsValue> named_value =
                InterpretValue(named_keyinfo, /* val */ nullptr, *named_flags, error);
            if (!named_value) return false;

            {
                LOCK(cs_args);
                m_settings.command_line_options[named_keyinfo.name].push_back(*named_value);
                // Strip the leading "--" before passing to the command as a named param
                m_command.emplace_back(original_input.substr(2));
            }
            return true;
        }

        // Binary doesn’t support -named: fall back and treat `--foo` as a
        // normal option (or unknown option handled below).
        keyinfo = InterpretKey(key);
        flags = GetArgFlags('-' + keyinfo.name);
    }

    // --- Normal option handling ---
    if (!flags || !keyinfo.section.empty()) {
        if (!found_after_non_option) {
            // Unknown global option: keep legacy behaviour (error)
            error = strprintf("Invalid parameter %s", original_input);
            return false;
        }

        // Unknown option after command: pass through to the command as an arg
        LOCK(cs_args);
        m_command.emplace_back(original_input);
        return true;
    }

    // Known option (including `--datadir`, `--signet`, etc.)
    std::optional<common::SettingsValue> value =
        InterpretValue(keyinfo, val ? &*val : nullptr, *flags, error);
    if (!value) return false;

    LOCK(cs_args);
    m_settings.command_line_options[keyinfo.name].push_back(*value);
    return true;
}

This command works on master but not in this PR: ./build/bin/bitcoin-cli --datadir=/tmp/btc1 --signet getblockhash 1000 <detail> <summarys>Thanks for testing it (not sure if that's valid, no test failed, perhaps we need to add one?), anyways just putting the output's diff here.</summarys><br>

master:

/build_master/bin/bitcoin-cli --datadir=/tmp/btc --signet getblockhash 1000
error code: -8
error message:
Block height out of range

this branch:

./build/bin/bitcoin-cli --datadir=/tmp/btc --signet getblockhash 1000
error: timeout on transient error: Could not connect to the server 127.0.0.1:8332

Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
Use "bitcoin-cli -help" for more info.

</details>

Restructuring ProcessOptionKey as below seems to solve the problem...

Checking it... Thanks for the suggestion!

Restructuring ProcessOptionKey as below seems to solve the problem...

Checking it... Thanks for the suggestion!

Ok, all tests pass...

First checked only the affected ones by my changes:

./build/test/functional/feature_config_args.py
./build/test/functional/tool_wallet.py
./build/test/functional/wallet_multiwallet.py
./build/test/functional/wallet_transactiontime_rescan.py

./build/bin/test_bitcoin --log_level=all --run_test=argsman_tests

Then I ran them all including the functional --extended.

Thanks again for working on this! I'm seeing if I can make the code a bit clearer otherwise I'll include your suggestion.

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20032344935/job/57444563462} _{LLM reason (✨ experimental): Python linting failed: ruff detected an f-string without placeholders in test code, causing the lint step to fail and CI to exit with error.}

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly 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.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

-<ins>Updates</ins>:

• Added @w0xlt's fix on long (double dash --) options that worked on master, adding a new test for it (the test would pass in master but fail without @w0xlt's changes).
• Refactored both ArgsManager::ProcessOptionKey (new function added in this PR) and ArgsManager::ParseParameters in separated commits making the code clearer and the PR much easier to follow and review.

Rebased.

On Windows the following commands are legal:

> bitcoin-cli.exe -regtest /rpcclienttimeout=1 getblockcount
> bitcoin-cli.exe -regtest -RPCCLIENTTIMEOUT=1 getblockcount

With this PR, it would be expected that the following command works:

> bitcoin-cli.exe -regtest getblockcount /rpcclienttimeout=1
> bitcoin-cli.exe -regtest getblockcount -RPCCLIENTTIMEOUT=1

But it is not happening since the HandleCommand only routes tokens beginning with '-' into option parsing and HandleCommandOption forwards raw option text directly to ProcessOptionKey and skips the WIN32 ToLower normalization used for pre-command options.

The patch below fixes this.

diff --git a/src/common/args.cpp b/src/common/args.cpp
--- a/src/common/args.cpp
+++ b/src/common/args.cpp
@@ -253,6 +253,11 @@ bool ArgsManager::ProcessOptionKey(std::string& key,
     std::string original_input{key};
     if (val) original_input += "=" + *val;
 
+#ifdef WIN32
+    key = ToLower(key);
+    if (!key.empty() && key[0] == '/') key[0] = '-';
+#endif
+
     NormalizeKey(key, double_dash);
 
     // ---- 1. special named-RPC handling ----
@@ -335,8 +340,8 @@ bool ArgsManager::HandleCommand(const char* const argv[], int& i, int argc, std:
     while (++i < argc) {
         const char* arg = argv[i];
 
-        if (arg[0] == '-') {
-            // dash → might be a command option
+        if (IsSwitchChar(arg[0])) {
+            // switch char → might be a command option
             if (!HandleCommandOption(arg, error)) {
                 return false;
             }
diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp
index 27691ce0b7..a210cfa2db 100644
--- a/src/test/argsman_tests.cpp
+++ b/src/test/argsman_tests.cpp
@@ -245,6 +245,13 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters)
     BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].front().get_str() == "argument");
     BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].back().get_str() == "multiple");
     BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2);
+
+#ifdef WIN32
+    const char* argv_windows_test[] = {"-ignored", "f", "/D=windows"};
+    BOOST_CHECK(testArgs.ParseParameters(3, argv_windows_test, error));
+    BOOST_CHECK(testArgs.IsArgSet("-d"));
+    BOOST_CHECK_EQUAL(testArgs.GetArg("-d", ""), "windows");
+#endif
 }
 
 BOOST_AUTO_TEST_CASE(util_ParseInvalidParameters)

On Windows the following commands are legal:

> bitcoin-cli.exe -regtest /rpcclienttimeout=1 getblockcount
> bitcoin-cli.exe -regtest -RPCCLIENTTIMEOUT=1 getblockcount

With this PR, it would be expected that the following command works:

> bitcoin-cli.exe -regtest getblockcount /rpcclienttimeout=1
> bitcoin-cli.exe -regtest getblockcount -RPCCLIENTTIMEOUT=1

The patch below fixes this.

Thanks @w0xlt for finding the issue and working on a solution. I'll check and will let you know.

@pablomartin4btc are you still working on this?

@pablomartin4btc are you still working on this?

Yeah, working on a fix for the windows bug detected by @w0xlt. I'll update it soon.

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Task Windows native, fuzz, VS: https://github.com/bitcoin/bitcoin/actions/runs/26322635035/job/77494454011} _{LLM reason (✨ experimental): CI failed because the fuzz test addr_info_deserialize crashed (exit code 3221226505) during the fuzz corpus run.}

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly 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.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

Could turn into draft while CI is red?

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Task lint: https://github.com/bitcoin/bitcoin/actions/runs/26967659993/job/79574492854} _{LLM reason (✨ experimental): CI failed because the trailing_newline lint check reported a missing trailing newline in doc/release-notes-33540.md.}

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly 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.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

-<ins>Updates</ins>:

• Rebased - Resolved conflicts with #34745 and incorporated the changes around guarding cs_args and the related helper refactors. - Resolved conflicts in argsman_tests.cpp with #28802 (92df785). Some tests relied on command-specific options appearing before the command; with the GNU-style parsing introduced in this PR, option ordering is no longer significant, so the affected tests were updated and additional cases were added.
• Added a fix for a Windows-specific bug where command-specific options using the / prefix were not handled correctly when specified after the command, together with regression test coverage. Thanks to @w0xlt for identifying the issue and proposing a fix (included as co-author).
• Added release notes.

ArgsManager now accepts options after non-option arguments, but bitcoin-tx still walked raw argv afterward.

That means any option placed after the tx input could be parsed as an option and then incorrectly processed again as a transaction mutation command.

Suggested changes:

<details> <summary>diff</summary>

diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
index f21db40f3c..7f94598d39 100644
--- a/src/bitcoin-tx.cpp
+++ b/src/bitcoin-tx.cpp
@@ -797,35 +797,42 @@ static int CommandLineRawTx(int argc, char* argv[])
     std::string strPrint;
     int nRet = 0;
     try {
-        // Skip switches; Permit common stdin convention "-"
-        while (argc > 1 && IsSwitchChar(argv[1][0]) &&
-               (argv[1][1] != 0)) {
-            argc--;
-            argv++;
+        std::vector<std::string> args;
+        if (const auto command{gArgs.GetCommand()}) {
+            args = command->args;
+        } else {
+            // Skip switches; Permit common stdin convention "-"
+            while (argc > 1 && IsSwitchChar(argv[1][0]) &&
+                   (argv[1][1] != 0)) {
+                argc--;
+                argv++;
+            }
+            args.assign(argv + 1, argv + argc);
         }
 
         CMutableTransaction tx;
-        int startArg;
+        size_t startArg;
 
         if (!fCreateBlank) {
             // require at least one param
-            if (argc < 2)
+            if (args.empty()) {
                 throw std::runtime_error("too few parameters");
+            }
 
             // param: hex-encoded bitcoin transaction
-            std::string strHexTx(argv[1]);
+            std::string strHexTx(args[0]);
             if (strHexTx == "-")                 // "-" implies standard input
                 strHexTx = readStdin();
 
             if (!DecodeHexTx(tx, strHexTx, true))
                 throw std::runtime_error("invalid transaction encoding");
 
-            startArg = 2;
-        } else
             startArg = 1;
+        } else
+            startArg = 0;
 
-        for (int i = startArg; i < argc; i++) {
-            std::string arg = argv[i];
+        for (size_t i = startArg; i < args.size(); i++) {
+            std::string arg = args[i];
             std::string key, value;
             size_t eqpos = arg.find('=');
             if (eqpos == std::string::npos)
diff --git a/test/functional/data/util/bitcoin-util-test.json b/test/functional/data/util/bitcoin-util-test.json
index 896decc4e3..03606149bf 100644
--- a/test/functional/data/util/bitcoin-util-test.json
+++ b/test/functional/data/util/bitcoin-util-test.json
@@ -56,6 +56,11 @@
     "output_cmp": "blanktxv2.json",
     "description": "Creates a blank transaction when nothing is piped into bitcoin-tx (output in json)"
   },
+  { "exec": "./bitcoin-tx",
+    "args": ["01000000000000000000", "-json"],
+    "output_cmp": "blanktxv1.json",
+    "description": "Parses an option after a transaction hex"
+  },
   { "exec": "./bitcoin-tx",
     "args": ["-create", "nversion=+1"],
     "return_code": 1,

</details>

The test can be improved by parsing --datadir=... with a fresh ArgsManager instead of using ForceSetArg(), so it exercises the real command-line path.

<details> <summary>diff</summary>

diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp
index cab21ad4aa..1b5cf4d862 100644
--- a/src/test/argsman_tests.cpp
+++ b/src/test/argsman_tests.cpp
@@ -36,9 +36,16 @@ BOOST_AUTO_TEST_CASE(util_datadir)
     args.ClearPathCache();
     BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase());
 
-    args.ForceSetArg("--datadir", fs::PathToString(dd_norm) + "/");
-    args.ClearPathCache();
-    BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase());
+    {
+        ArgsManager parsed_args;
+        parsed_args.AddArg("-datadir=<dir>", "", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS);
+        const std::string datadir_arg{"--datadir=" + fs::PathToString(dd_norm) + "/"};
+        const char* argv_datadir[] = {"ignored", datadir_arg.c_str()};
+        std::string error;
+        BOOST_CHECK(parsed_args.ParseParameters(2, argv_datadir, error));
+        BOOST_CHECK_EQUAL(error, "");
+        BOOST_CHECK_EQUAL(dd_norm, parsed_args.GetDataDirBase());
+    }
 
     args.ForceSetArg("-datadir", fs::PathToString(dd_norm) + "/.");
     args.ClearPathCache();

</details>

... bitcoin-tx still walked raw argv afterward.

Suggested changes...

Thanks, it makes sense. I agree bitcoin-tx should consume the already parsed command args instead of walking raw argv again, otherwise options accepted after non-options can be processed twice. I’ll update it and add the regression test as well.

The test can be improved by parsing --datadir=... with a fresh ArgsManager instead of using ForceSetArg(), so it exercises the real command-line path.

I agree, better to have it parsed, I'll update the test as well. Thanks!

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/27029132763/job/79776756106} _{LLM reason (✨ experimental): CI failed because clang-tidy reported (and treated as error) a performance-unnecessary-copy-initialization issue in bitcoin-tx.cpp (std::string arg = args[i];).}

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly 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.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

-<ins>Updates</ins>:

• Addressed @w0xlt feedback and incorporated the suggested changes:
  • (1) Updated bitcoin-tx to consume the command arguments already parsed by ArgsManager, avoiding re-processing raw argv.
    • Added regression test coverage (bitcoin-util-test.json) which fails if the bitcoin-tx changes are reverted (eg ./build/test/functional/tool_utils.py --loglevel=DEBUG).
  • (2) Updated the --datadir=... test in argsman_tests.cpp to use ArgsManager::ParseParameters() instead of ForceSetArg().

-<ins>Updates</ins>:

• Updated bitcoin-cli help text to clarify that options can be specified before or after commands (no behavioural change).